-
Notifications
You must be signed in to change notification settings - Fork 22.5k
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
docs(manifest): Improve theme_color
member page
#35643
Conversation
Preview URLs (comment last updated: 2024-09-03 20:46:43) |
These override methods provide you the flexibility to adapt your app's appearance for specific pages or user preferences, improving the overall user experience. | ||
|
||
Browsers may also adjust the applied theme color based on user preferences. | ||
If a user has set a preference for light or dark mode, browsers may override the manifest `theme_color` value to support any [`prefers-color-scheme`](/en-US/docs/Web/CSS/@media/prefers-color-scheme) media query defined in your app's CSS. |
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.
Perhaps
If a user has set a preference for light or dark mode, browsers may override the manifest `theme_color` value to support any [`prefers-color-scheme`](/en-US/docs/Web/CSS/@media/prefers-color-scheme) media query defined in your app's CSS. | |
If a user has set a preference for light or dark mode, browsers may override the manifest `theme_color` value to better match any [`prefers-color-scheme`](/en-US/docs/Web/CSS/@media/prefers-color-scheme) media query defined in your app's CSS. |
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.
In this case, I think it might be better to stick to what spec has, which is "support"
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.
Support is definitely "sub optimal", but perhaps "comply with" would be better. Not worth arguing about.
A few comments. Much better than before. I like the split of lines on sentence :-) |
Thanks a lot, Hamish, for the review! I've accepted your suggestions or left a comment. The revised version is ready for you to take another look. |
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.
@dipikabh Couple of nits on the syntax/values. Approving so you are not blocked.
Thanks, Hamish! I'll merge this for now. I've fixed your last two comments slightly differently. If you have follow-up comments, feel free to leave them here. I can take them up along with my other manifest work. |
Description
This PR focuses on the
theme_color
member page as part of the effort to improve web/manifest documentation.Key changes:
theme_color
theme_color
prefers-color-scheme
Motivation
To clarify the usage and relevance of
theme_color
and align the documentation more closely with the specificationAdditional details
Spec: https://w3c.github.io/manifest/#theme_color-member
Related issues and pull requests
Tracking issue: mdn/mdn#560
PR to standardize the format across manifest member: #35557