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

Don't show empty balances when account not connected #477

Merged

Conversation

ebma
Copy link
Member

@ebma ebma commented May 29, 2024

I replaced all related loaders I could find with just 'nothing' expect for the Backstop Pool screen where 'My pool balance' now shows 0 if no account is connected.

Closes #469.

@ebma ebma linked an issue May 29, 2024 that may be closed by this pull request
@ebma ebma requested review from Sharqiewicz and prayagd May 29, 2024 16:03
Copy link

netlify bot commented May 29, 2024

Deploy Preview for rococo-souffle-a625f5 ready!

Name Link
🔨 Latest commit 000204f
🔍 Latest deploy log https://app.netlify.com/sites/rococo-souffle-a625f5/deploys/665d88f59dc706000894c9f0
😎 Deploy Preview https://deploy-preview-477--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
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.

All good @ebma with the swap UI, can we remove the "0" also from the backstop pool UI? and then go ahead and merge.

@ebma
Copy link
Member Author

ebma commented May 30, 2024

I left the 'My pool balance: 0' at the backstop pool because I thought it might look weird because it's so empty otherwise. But I can remove that too, no problem.

@ebma
Copy link
Member Author

ebma commented May 30, 2024

Please check again @prayagd

@prayagd
Copy link
Collaborator

prayagd commented May 30, 2024

@ebma the text "My pool balance" also went away, no worries lets merge. This is for the testnet campaign anyway

@ebma
Copy link
Member Author

ebma commented May 30, 2024

Wait, I thought that's how you wanted it to look like? 😅 At least that's exactly what I changed it to. Did you expect something else? I can also change it again.

Copy link
Collaborator

@Sharqiewicz Sharqiewicz left a comment

Choose a reason for hiding this comment

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

@ebma perfect ✅

@prayagd
Copy link
Collaborator

prayagd commented May 30, 2024

"My pool balance"

@ebma oh sorry for the confusion. just show this text back, that's it

@ebma
Copy link
Member Author

ebma commented May 30, 2024

Out of the three options we have:

  1. Show 'My pool balance: 0'
  2. Don't show 'My pool balance' at all
  3. Only show 'My pool balance' but without an actual number after it

Isn't option 3) the most confusing? 😅

@vadaynujra
Copy link

If there is nothing the user can do on that page, without connecting their wallet, then that's where we should be sending them (and so should be what they see).

References from other DEXes

HydraDX's Wallet Page:

image

Pablo's portfolio view

image

Both dApps:

  • fill up the screen to tell the user they should connect their wallet - on the lines of @TorstenStueber's suggestion
  • while still keeping placeholders of what the user should expect to see, once they do connect their wallet - I think this is what @prayagd wanted to cater to with the suggestion of keeping the My Pool Balance

Specifically on the options:

  • Show 'My pool balance: 0' - this would be incorrect, as 0 is also a number and may or may not be true
  • Don't show 'My pool balance' at all - user wouldn't know that they can see their balance on this screen once they connect
  • Only show 'My pool balance' but without an actual number after it - this is totally fine as long as it is clear to the user that this is a placeholder, which will have a true value one they connect their wallet.

Since my suggested solution might require some more work, can the immediate solution just be that the animation is made static, everything else remaining the same? See HydraDX's wallet view without connecting wallet for reference.

@TorstenStueber
Copy link
Member

I suggest: remove "My pool balance" as @ebma did and just add a "Connect Wallet" button instead. Don't we have a component for this and adding such a button is a trivial change?

@ebma
Copy link
Member Author

ebma commented May 31, 2024

Thanks for providing some references. Personally, I think it's fine the way it is now with not showing anything related to balances as long as no wallet is connected. I can add the change @TorstenStueber suggested and instead make the button for depositing/swapping prompt to connect a wallet account instead. The same is already done when you go to the Spacewalk issue/redeem dialog without being connected to a wallet.

@vadaynujra
Copy link

Sounds good, so both buttons become Connect Wallet, when the user connects wallet, the My Pool Balance appears with the correct amount.

@ebma ebma requested review from Sharqiewicz and prayagd June 3, 2024 09:14
@ebma
Copy link
Member Author

ebma commented Jun 3, 2024

I changed it so that all Nabla primary action buttons are replaced with 'Connect to Wallet'. And I added the change to make the wallet account selection scrollable.

@prayagd
Copy link
Collaborator

prayagd commented Jun 3, 2024

@ebma looks good we can merge, checked all the buttons and all the connect wallet buttons work and open the pop-up

@ebma ebma merged commit 8a9cae4 into main Jun 3, 2024
5 checks passed
@ebma ebma deleted the 469-show-bsp-and-swap-balance-0-when-wallet-not-connected branch June 3, 2024 14:05
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.

Show BSP and swap balance 0 when wallet not connected
5 participants