-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[5.3] [IMPROVEMENT] Update SelectQRAccounts UI #4070
Conversation
Why do we have specific formatting for the iphone X in app/components/Views/ConnectQRHardware/SelectQRAccounts/index.tsx and app/components/Views/ConnectQRHardware/index.tsx, @soralit? |
Just following up on my comment above:
I now see that it is the Eth balance appearing below the public address (not the address wrapping to a new line). Should we preserve the unit "ETH" to make this more clear, or was that removed intentionally? |
It was a bug, its already fixed in the last commit
I added the text "QR Hardware" as its showed in the Wallet View. What do you think @holantonela and @plasmacorral? In the following screenshots you can see the evidence, |
app/components/Views/ConnectQRHardware/SelectQRAccounts/index.tsx
Outdated
Show resolved
Hide resolved
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.
Nice work. Only thing I would say is we should try and move withQRHardwareAwareness
to a hook but i'd suggest we do this in another PR to reduce the scope. I didn't run the app so this is just a code review!
Candidate for 5.3 |
Description
This PR updates the style of the
SelectQRAccounts
component and refactors the HOCwithQRHardwareAwareness
Use Case
GIVEN I'm importing my accounts from my Keystone to MMM
WHEN I select an account or more
THEN the account(s) should be imported to MMM
Checklist
Issue
Progresses #3952