-
Notifications
You must be signed in to change notification settings - Fork 20
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
Add accordion to settings page #361
Conversation
This is a nice improvement, especially on mobile. Just a couple of feedback points:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great!! I've a couple small changes that I'll push, like for the WalletConnect icon. I'll do it later today.
The links to setting > watched account need to be modified in the home page (and maybe some other places?) with the #panel-watched-accounts
. This should make the Cypress test pass, or get us closer to.
We need to add click on accordion for cypress test as before the inputs appeared right away. |
packages/ui/src/components/WalletConnect/WalletConnectSession.tsx
Outdated
Show resolved
Hide resolved
@Lykhoyda Those updates for the labels look good 👍
If you can update the link for the button below to use settings#watched-acccounts then it's going to be expanded when arriving on that screen and will be better for users and also no need to update the test |
I pushed some changes to
There are still a couple comments above that weren't taken into account yet, but it should be quick. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome, this looks a million time better. Thank you!
Closes #336
!
There is no Wallet connect design yet