Update staking layout #244
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
itswisdomagain
requested changes
Nov 13, 2023
itswisdomagain
requested changes
Nov 14, 2023
|
the entire page should have a scroll bar. also does the reward seem correct? considering the total voted tickets(this is testnet)
even when i expand the page there isn't enough bottom margin
|
|
going to the staking menu for a wallet that hasn't been synced causes the app to crash |
I just update only layout, but I thinks it correct |
It done @dreacot |
Pleases give me more detail, I can't reproduce it |
Create a new wallet, don't sync it, click on staking menu |
fixed |
|
The codes LGTM. |
dreacot
reviewed
Nov 23, 2023
itswisdomagain
requested changes
Nov 23, 2023
|
the divider can be removed in watch only wallets
|
|
fix conflicts |
itswisdomagain
requested changes
Nov 28, 2023
itswisdomagain
approved these changes
Nov 28, 2023
There was a problem hiding this comment.
Approved, pending @Sirmorrison's comment on one of the changes here.
| return formatBalance(gtx, l, amount, mainTextSize, .85, l.Theme.Color.PageNavText, true, true) | ||
| return formatBalance(gtx, l, amount, mainTextSize, l.Theme.Color.PageNavText, true, true) |
There was a problem hiding this comment.
The default scale is .7, but this guy was using .85. I see that this LayoutBalanceWithUnitSizeBoldText was added by @Sirmorrison in #247. Let's get his thoughts on this change to use .7 instead of .85.
There was a problem hiding this comment.
Its used by the mixer layout to scale the size of the main text and subtext to the update UI.
There was a problem hiding this comment.
I think it's fine to use the same scale all through. However, I'm not sure the default scale should be .7. From what I see in many places on figma, the significant part of the amount text uses 18px while the insignificant part uses 14px, which is a scale of 0.78 (14/18). Please verify and adjust accordingly @JustinBeBoy.
| if pg.showMaterialLoader { | ||
| gtx.Constraints.Min.X = gtx.Constraints.Max.X | ||
| return layout.Center.Layout(gtx, pg.materialLoader.Layout) | ||
| } | ||
| return pg.scroll.List().Layout(gtx, 1, func(gtx C, i int) D { | ||
| gtx.Constraints.Max.Y = ticketHeight |
There was a problem hiding this comment.
Someone who has tickets should verify if this is necessary. What happens if it's removed?
I missed this in my last review. The tickets card isn't as wide as the other cards. Distorts the UI. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR resolve #212
This PR:
ScreenShot