-
-
Notifications
You must be signed in to change notification settings - Fork 2.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
Improve alert dialog style #16549
Improve alert dialog style #16549
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
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.
While the videos are helpful it would be better if you could supply several images for dialogs where we have a before and after side by side so it's easy to see the differences.
049efdd
to
72bce15
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
72bce15
to
4db6682
Compare
The code changes themselves are fine, but it needs a manual check of the other dialogs. The app has many of them and some are relatively complex or not follow some of the best practices for styling. An automated screenshot tool would really help here. |
4db6682
to
35311e1
Compare
not necessary
it was purple in dark mode tested by showing the delete note dialog in the reviewer
it was purple in dark mode tested by showing the delete note dialog in the reviewer
@lukstbit @BrayanDSO please take a look or tell me what to do do |
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.
I think this looks good, with (as far as I can tell, and I was trying...) all comments addressed with reasonable support
This statement may be due for review and perhaps a follow-on now that GSoC is merged (I think - most are merged, hopefully the ones that were interfering with this are merged):
I was going to remove AlertDialogStyle as well but someone else is doing something with it
I believe comments were addressed
Purpose / Description
The dialogs didn't look much like material dialogs, so I applied the overlay theme to get animations, round corners and material style
Fixes
Approach
I used Material Dialog Theme Overlay and did a fix with the text color
I was going to remove AlertDialogStyle as well but someone else is doing something with it
How Has This Been Tested?
dialog.webm
light.webm
API.25.webm
Checklist
Please, go through these checks before submitting the PR.