Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Nabla swap design #483

Merged
merged 4 commits into from
Jun 4, 2024

Conversation

Sharqiewicz
Copy link
Collaborator

@Sharqiewicz Sharqiewicz commented May 31, 2024

What:

  • Remove "Your" from "Your Balance" from the UI inputs fields
  • Change to "Balance" in the destination input field
  • From "Your Balance: 22 X0", remove the token name i.e XO

✅ Remove NumberInput and replace any use of it with NumericInput
✅ Change texts

Closes: #473

@Sharqiewicz Sharqiewicz linked an issue May 31, 2024 that may be closed by this pull request
Copy link

netlify bot commented May 31, 2024

Deploy Preview for rococo-souffle-a625f5 ready!

Name Link
🔨 Latest commit 51e6a75
🔍 Latest deploy log https://app.netlify.com/sites/rococo-souffle-a625f5/deploys/665e22923d13fd0009b303a6
😎 Deploy Preview https://deploy-preview-483--rococo-souffle-a625f5.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just one comment

src/components/nabla/common/AmountSelector.tsx Outdated Show resolved Hide resolved
Copy link
Collaborator

@prayagd prayagd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
Not related by strange thing i observed was after opening the deploy preview, i could not see any button on the swap ui. see screenshot. It appeared after i selected the tokens from the dropdown
Screenshot 2024-06-03 at 4 49 25 PM

@Sharqiewicz
Copy link
Collaborator Author

@ebma I applied the changes you mentioned

Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice thanks 👍 something is off though. I noticed that on Foucoco, no matter which asset I'm using in the swap form, it's limited to 12 decimals.

If you check the foucoco squid with the following query, you'll see that the mock tokens have varying decimals though and not always just 12.

query MyQuery {
  nablaTokens {
    decimals
    name
    id
    symbol
  }
}

Did you log and check the values that maxBalance?.decimals gets? Maybe it's always undefined for some reason.

@Sharqiewicz
Copy link
Collaborator Author

Nice thanks 👍 something is off though. I noticed that on Foucoco, no matter which asset I'm using in the swap form, it's limited to 12 decimals.

If you check the foucoco squid with the following query, you'll see that the mock tokens have varying decimals though and not always just 12.

query MyQuery {
  nablaTokens {
    decimals
    name
    id
    symbol
  }
}

Did you log and check the values that maxBalance?.decimals gets? Maybe it's always undefined for some reason.

@ebma It works correctly for me. I receive decimals: 18 for BRL and in the input I can type max of 18 decimals. For AMPE - 12 decimals, EURC - 15 decimals, USDC - 7 decimals and USDT - 10 decimals

@ebma
Copy link
Member

ebma commented Jun 4, 2024

Okay, I notice the difference now. The decimals are only queried when an account is connected. If no wallet account is connected, it will not check for the decimals. It then works as expected. Since the user eventually needs to connect to an account to be able to submit anything, the validation will trigger and it works fine.
image

Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me then

@Sharqiewicz Sharqiewicz merged commit f921569 into main Jun 4, 2024
5 checks passed
@Sharqiewicz Sharqiewicz deleted the 473-text-changes-on-the-nabla-swap-screen branch June 4, 2024 09:41
@vadaynujra vadaynujra changed the title improve nabla swap design Improve Nabla swap design Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Text changes on the Nabla swap screen
3 participants