Skip to content
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

Merged
merged 8 commits into from
Sep 3, 2024

Conversation

dipikabh
Copy link
Contributor

Description

This PR focuses on the theme_color member page as part of the effort to improve web/manifest documentation.

Key changes:

  • Addition of "Syntax", "Description", and "See also" sections
  • "Description" section includes:
    • Importance of theme_color
    • Methods to override theme_color
    • Support for prefers-color-scheme

Motivation

To clarify the usage and relevance of theme_color and align the documentation more closely with the specification

Additional 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

@dipikabh dipikabh requested a review from a team as a code owner August 30, 2024 01:12
@dipikabh dipikabh requested review from hamishwillee and removed request for a team August 30, 2024 01:12
@github-actions github-actions bot added Content:Manifest size/m [PR only] 51-500 LoC changed labels Aug 30, 2024
Copy link
Contributor

github-actions bot commented Aug 30, 2024

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.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps

Suggested change
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.

Copy link
Contributor Author

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"

Copy link
Collaborator

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.

@hamishwillee
Copy link
Collaborator

A few comments. Much better than before. I like the split of lines on sentence :-)

@dipikabh
Copy link
Contributor Author

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.

Copy link
Collaborator

@hamishwillee hamishwillee left a 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.

@dipikabh
Copy link
Contributor Author

dipikabh commented Sep 3, 2024

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.

@dipikabh dipikabh merged commit c3c36c4 into mdn:main Sep 3, 2024
8 checks passed
@dipikabh dipikabh deleted the manifest-themecol branch September 17, 2024 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:Manifest size/m [PR only] 51-500 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants