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

feat: Wallet account view 4 #678

Merged
merged 74 commits into from
Jul 23, 2024
Merged

feat: Wallet account view 4 #678

merged 74 commits into from
Jul 23, 2024

Conversation

janmichek
Copy link
Collaborator

Description

fixes #536

There are 2 minor UX issues I will try to resolve in the following (Portfolio) issue, so I don't block the review

  1. After successful wallet connection there is no loader when redirecting to connected account detail
  2. When clicking on Try again, the whole page gets reloaded, which is not necessary. Only scanning again should work, but for some reason it's not working Wallet re-connection #572

Demo

firefox_BZzKh8r8cp.mp4

Checklist:

Copy link

Copy link
Collaborator

@michele-franchi michele-franchi left a comment

Choose a reason for hiding this comment

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

Generally it looks good but I would adjust the following:

  1. The "Connect Wallet" button should have the same height of the network switcher:
image

padding: var(--space-1); on the button solves it.

  1. The "Exit Wallet" dropdown should be visually similar to the menu dropdowns.
image

Example:

image

Without the min-width of the other menu items.

  1. When you connect the wallet and you refresh the page the connection is lost.

.env.example Outdated Show resolved Hide resolved
nuxt.config.ts Show resolved Hide resolved
src/components/TheWalletAccountControls.vue Outdated Show resolved Hide resolved
src/components/WalletConnectionPanel.vue Outdated Show resolved Hide resolved
src/utils/hints/walletHints.js Outdated Show resolved Hide resolved
@janmichek
Copy link
Collaborator Author

Generally it looks good but I would adjust the following:

1. The "Connect Wallet" button should have the same height of the network switcher:
image

padding: var(--space-1); on the button solves it.

2. The "Exit Wallet" dropdown should be visually similar to the menu dropdowns.
image

Example:
image

Without the min-width of the other menu items.

3. When you connect the wallet and you refresh the page the connection is lost.

I have been discussing this with Denis. Currently is not possible to achieve this seamlessly. The app can remember last used account, but we cannot go over the user prompt to connect to the wallet again like on superhero.com
Implementation-wise, the SDK can do something to simplify it, but fundamentally will stay the same.

For now, I suggest not to implement it, wait for SDK completion, and the implement the persistency of last logged address @michele-franchi

@janmichek janmichek requested a review from Liubov-crypto July 18, 2024 12:07
@janmichek janmichek mentioned this pull request Jul 18, 2024
3 tasks
@janmichek janmichek removed the request for review from michele-franchi July 18, 2024 12:09
Copy link
Collaborator

@Liubov-crypto Liubov-crypto left a comment

Choose a reason for hiding this comment

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

  1. As I understood from the comments the button should have the same width:

but

  1. There is no account icon:
    icon

  2. After I connect the wallet and refresh the page the connection is lost. As I understood currently it's not possible to fix it. Correct?

@janmichek
Copy link
Collaborator Author

  1. As I understood from the comments the button should have the same width:

but

2. There is no account icon:
   ![icon](https://private-user-images.githubusercontent.com/69896204/350337352-6312902c-8279-4117-9296-63ef97a57e4c.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjE3MjUwMzgsIm5iZiI6MTcyMTcyNDczOCwicGF0aCI6Ii82OTg5NjIwNC8zNTAzMzczNTItNjMxMjkwMmMtODI3OS00MTE3LTkyOTYtNjNlZjk3YTU3ZTRjLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDA3MjMlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwNzIzVDA4NTIxOFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTVhNjYxNjFiNGY1YzFlNmIyNmZmODQ4YmE1NjNhODkzMmQ3MWU2MDQxOTg0MmE3ODgxMThjZWRkNDYxNDQwNmUmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.yn2sCLGbQiCh7wpGljRsW_CKPpOi5I0c1oKQkp0QeeY)

3. After I connect the wallet and refresh the page the connection is lost. As I understood currently it's not possible to fix it. Correct?
  1. yep, fixed
  2. Interesting, I haven't see this issue anywhere. Might be some VPN filtering out? Could you retry it ?
  3. yea for now it is like that

@janmichek janmichek requested a review from Liubov-crypto July 23, 2024 08:54
@Liubov-crypto
Copy link
Collaborator

  1. Fixed.

I found the reason behind the images blocking:

icn
img

@janmichek
Copy link
Collaborator Author

  1. Fixed.

I found the reason behind the images blocking:

icn img

Thanks, found the issue and fixed it.

@janmichek janmichek merged commit f211ebc into develop Jul 23, 2024
4 checks passed
@janmichek janmichek deleted the Wallet-assets-listing branch July 23, 2024 13:34
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.

Wallet account view
3 participants