-
Notifications
You must be signed in to change notification settings - Fork 274
UI4: Transaction Details Scene #4586
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
Conversation
4b5d08c to
82c3d63
Compare
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.
I found a few small things, but overall I am really happy with how this cleaned up!
If the UI4 work is going to side on the sidelines for a few weeks, would it make sense to cherry-pick the function-style component upgrade into its own mini-PR? That way we can get the work in right away.
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.
Add != null
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.
Still not fixed, but not a big deal.
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.
Oops, I'll get this in before merging
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 is a good cleanup! The only reason TransactionFiatTiles existed was so we could use hooks, and now that the parent is finally converted, all this stuff can finally move over. 🎉
13b05c3 to
38381b8
Compare
5f3f31b to
48510a7
Compare
48510a7 to
bbf2671
Compare
6e4c5c3 to
f7977a0
Compare
Adhere to a new convention of not arbitrarily grouping elements together that are not really a visual grouping
f7977a0 to
d411062
Compare
Using the new "Row" naming since this is exclusively used in a UI4 scene now
Padding wasn't being properly passed. When all scenes are updated to UI4, all components will share a split 0.5/0.5 rem margin/padding by default.
- Move My Receive Address into the card - Move Transaction ID out of the card
Also add more bottom margin. This allows the user to bring the button closer the middle of the screen. The middle of the screen has a higher tap accuracy than the bottom edge of the screen.
Previously the string was completely right justified, inconsistent with the other rows and also looking like a tappable element when the percent change was positive and in the same area as the right tappable icons. - Make the percent string more visually similar to TickerText - Make spacing for + signs consistent in the amount field. - Only show the percent change if there is a visual difference in fiat price
d411062 to
53afac3
Compare
Uh oh!
There was an error while loading. Please reload this page.
Implement UI4 changes for Transaction Details Scene
image
image
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
#4593
Requirements
If you have made any visual changes to the GUI. Make sure you have: