-
Notifications
You must be signed in to change notification settings - Fork 46
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
Top-level setting page UI v2 update #1035
Top-level setting page UI v2 update #1035
Conversation
- replace toast notice with show modal when change pass, enable/disable startup password
4f260be
to
f445768
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.
The max width of the content area should be reduced
vs
This PR doesn't seem to solve #994 as stated
This PR links to a PR that closes #993, but the issue is also not resolved here, there's also a new mockup for said issue here https://www.figma.com/file/e0cYaAzkXyXXPY00v23cQY/GoDCR-%26-DCRMobile?node-id=4659%3A182253
@dreacot |
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.
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 PR looks good but a few things are out of place:
- The moon icon should be the dark shade and the sunshine icon should be the light shade. I think you might need to extract the black moon icon and the light sunshine icon on figma.
- On the about page, the license row blends into the background when hovered, maybe adding a shadow could help (this only happens in light mode)
I think we should change all hover color, because all button in app occur and it is other issue |
I agree, an issue for created for this and it can be treated separately |
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.
LGTM
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.
Move this line
godcr/ui/page/settings_page.go
Line 101 in 3fafcdd
pg.updateSettingOptions() |
From the Layout function to
godcr/ui/page/settings_page.go
Line 93 in 3fafcdd
func (pg *SettingsPage) OnNavigatedTo() { |
The layout function is called each time there is a layout change or an action is fired so performing all that computation each time an event occurs will lag the UI.
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.
LGTM
Resolve #1031 #993 #994
Fix #1012
Fix #1013
Subtask:
Screenshots