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

Make system-color a proper data type page #21207

Merged
merged 2 commits into from
Oct 5, 2022

Conversation

wbamberg
Copy link
Collaborator

@wbamberg wbamberg commented Sep 30, 2022

This PR makes "System color kewords" into a proper reference page for the <system-color> type, as discussed in #15540 (comment) and following comments.

This will also fix the <system-color> link in the formal syntax for the <color> data type: https://developer.mozilla.org/en-US/docs/Web/CSS/color_value#formal_syntax.

This PR depends on mdn/browser-compat-data#17924.

@wbamberg wbamberg requested review from a team as code owners September 30, 2022 19:08
@wbamberg wbamberg requested review from dipikabh and removed request for a team September 30, 2022 19:08
@github-actions github-actions bot added the Content:CSS Cascading Style Sheets docs label Sep 30, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Sep 30, 2022

Preview URLs

Flaws (2)

URL: /en-US/docs/Web/CSS/named-color
Title: <named-color>
Flaw count: 1

  • bad_bcd_queries:
    • No BCD data for query: css.types.color.named-color

URL: /en-US/docs/Web/CSS/system-color
Title: <system-color>
Flaw count: 1

  • bad_bcd_queries:
    • No BCD data for query: css.types.color.system-color
External URLs (1)

URL: /en-US/docs/Web/CSS/system-color
Title: <system-color>

(this comment was updated 2022-10-03 19:42:02.341463)


#### Result

{{EmbedLiveSample("Using system colors")}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is all good except it would be good to provide links/hints on how to set forced color mode - even better (if possible) would be to make pressing the button force the mode.

Otherwise not much point this being a live example.

I'm going to merge anyway, because this is excellent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I copied the example from the forced-colors page: https://developer.mozilla.org/en-US/docs/Web/CSS/@media/forced-colors#examples.

I think you can't make pressing the button enable it, it's an OS setting that's not available to Web pages. The only concrete example I know of is Windows high contrast mode: https://blogs.windows.com/msedgedev/2020/09/17/styling-for-windows-high-contrast-with-new-standards-for-forced-colors/ . I just added a reference to that, it's helpful I think.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Works for me/don't have any better ideas :-)

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.

Good for me, modulo comment. Did you want to wait on the BCD to go in first before merging @wbamberg ?

@wbamberg
Copy link
Collaborator Author

wbamberg commented Oct 3, 2022

Good for me, modulo comment. Did you want to wait on the BCD to go in first before merging @wbamberg ?

Thanks Hamish. The BCD PR just merged but I'm going to wait for a couple of days for it to percolate through to the page, and hope noone makes any merge conflicts in the meantime: 🤞 .

Copy link
Contributor

@dipikabh dipikabh left a comment

Choose a reason for hiding this comment

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

Just tiny nits


### Using system colors

In this example we have a button that normally gets its contrast using the {{cssxref("box-shadow")}} property. In forced colors mode, `box-shadow` is forced to `none`, so the example uses the `forced-colors` media feature to ensure there is a border of the appropriate color (`ButtonBorder` in this case).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
In this example we have a button that normally gets its contrast using the {{cssxref("box-shadow")}} property. In forced colors mode, `box-shadow` is forced to `none`, so the example uses the `forced-colors` media feature to ensure there is a border of the appropriate color (`ButtonBorder` in this case).
The button in this example gets its contrast using the {{cssxref("box-shadow")}} property. In the `forced-colors` mode, `box-shadow` is forced to `none`. The example uses the `forced-colors` media feature to ensure that there is a border of the appropriate color (`ButtonBorder` in this case).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

forced-colors (with code styling and a hyphen) is the name of the media feature, but "forced colors mode" is the name of the abstract feature. So I think "In forced colors mode" is correct here.

Copy link
Contributor

@dipikabh dipikabh left a comment

Choose a reason for hiding this comment

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

Thank you! 👍

@dipikabh dipikabh merged commit 7c13b9a into mdn:main Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:CSS Cascading Style Sheets docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants