-
Notifications
You must be signed in to change notification settings - Fork 9.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
Theme Color wording fixes #2466
Theme Color wording fixes #2466
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.
thanks @khempenius totally agree with this improvement!
aside: revisiting those docs I'm not sure we should require the manifest value to be set at all since we require meta theme-color...
https://developers.google.com/web/updates/2015/08/using-manifest-to-set-sitewide-theme-color |
Some internal discussion on whether the address bar theming should be required: https://goo.gl/kXzeO0 FWIW, it's currently not part of the Baseline PWA checklist. |
@kaycebasques if you have the time to take a look |
wasn't there still some difference between the two? I can never keep track. We should just match the checklist, though, which makes these decisions easier (i.e. somebody else's job to keep track :) |
hi @khempenius, sorry for the slow movement on this. Would you mind rebasing with current master? The main thing that changed since you opened the PR was the themed-omnibox I would resolve the merge conflict on your behalf, but I believe when I've done that before and then squashed, the commit author was switched to me, so that's no good :) |
@brendankenny No worries. This should be good to go now. |
Thanks! |
Updated description
Corrected reference to manifest theme color ("theme_color" vs. "theme-color")
Changes for clarity: