-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Minor DAO ui fixes #2109
Minor DAO ui fixes #2109
Conversation
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.
NACK
Need to change all labels to upper case
@@ -58,7 +58,7 @@ public MenuItem(Navigation navigation, | |||
this.viewClass = viewClass; | |||
this.baseNavPath = baseNavPath; | |||
|
|||
setLabelText(title); | |||
setLabelText(title.toUpperCase()); |
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 doesn't handle the 'Vote on proposals'/'Browse open proposals' text that changes depending on phase
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.
Could be fixed in GovernanceView.java:90/92
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.
Thanks for pointing it out. Just fixed it by converting to uppercase also on label update.
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.
And I removed now an unnecessary uppercasing. Should have looked through the code more carefully 🙄
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.
Yes, better to only set it inside setLabelText as you do now
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.
ACK
Of course, much better to handle it there :)
No description provided.