-
Notifications
You must be signed in to change notification settings - Fork 499
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
Fix #3782 : Made the project use Themes instead of Styles [Dark Mode Implementation]. #3851
Conversation
@rt4914 PTAL If this approach is acceptable. |
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.
@ayush0402 Nit changes suggested. @BenHenning Can you confirm if this approach is correct.
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.
Took a first pass, but I'd like to take a closer look tomorrow. I think this generally looks right, though.
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 @ayush0402. Took a closer look. PR generally LGTM, but had a couple of comments--PTAL.
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
@BenHenning PTAL. |
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 @ayush0402. This LGTM!
Explanation
Fixes #3782
Most part of the existing
styles.xml
already followed suggested practices forthemes.xml
according to https://medium.com/androiddevelopers/android-styling-themes-vs-styles-ebe05f917578 with semantic naming for color resources.Separated certain styles suited more for
themes.xml
fromstyles.xml
.Possible solution for further PRs :
Adding
themes.xml
andcolors.xml
undervalues-night
and changing theme's parent toDayNight
variant with changes undervalues-night
for proper implementation of Dark Mode.Essential Checklist