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

How to deal with custom styles in dark mode? #4687

Open
xfq opened this issue Apr 12, 2024 · 18 comments
Open

How to deal with custom styles in dark mode? #4687

xfq opened this issue Apr 12, 2024 · 18 comments
Labels

Comments

@xfq
Copy link
Contributor

xfq commented Apr 12, 2024

Description of problem

URL to affected spec or repo:

What happened (e.g., it crashed)?:

  1. Open https://deploy-preview-130--bp-i18n-specdev.netlify.app/#lang_misc when the system is in light mode.
  2. Select 'dark' in the lower left corner of the spec.
  3. The colour is incorrect.

If the OS is in dark mode, the colour is correct.

Expected behavior (e.g., it shouldn't crash):

Colours should follow the @media (prefers-color-scheme: dark) media query.

@marcoscaceres
Copy link
Contributor

marcoscaceres commented Apr 12, 2024

I think the problem is with .links being applied to the aside (i.e., this is problem with that spec... not ReSpec, as ReSpec doesn't set the style for that)

So that local.css should define

@media (prefers-color-scheme: dark) { .links { /**/} }

@sidvishnoi
Copy link
Member

sidvishnoi commented Apr 12, 2024

Using the media query won't work with forced dark-mode via the toggle in sidebar (that comes from fixup.js). See w3c@031abec#diff-cefc979f9d55dee149c3b7b5b410cf2d5db0c13d16bf3b379780a1a7ccaab5e2R24-R30 for example.

So, the best strategy would be to use one of the variables from https://www.w3.org/StyleSheets/TR/2021/dark.css (and https://www.w3.org/StyleSheets/TR/2021/base.css) to support consistent dark mode styles (including custom styles). Even with ReSpec's dark mode support, I avoided media query as much as possible and used variables from W3C stylesheet.

@sidvishnoi
Copy link
Member

sidvishnoi commented Apr 12, 2024

Opinion: I'm not in favor of existence of toggles. The prefers-color-scheme is already the preference set by user that we're after. Adding site-wise color scheme should be something that either the browsers or extensions should do.

@sidvishnoi
Copy link
Member

Alternative would be to watch whether dark.css applies using JS (mutation observer on that link element), and then add a class to html and use that instead of (only) media query. But yeah, mostly the pain of having a toggle.

@marcoscaceres
Copy link
Contributor

I like/use the toggle myself... because I don't like the specs on the dark background, even when the rest of the OS is in dark mode... so I get it.

@sidvishnoi
Copy link
Member

So maybe fixup.js should add a .dark class when it's using dark mode. Then we can adapt the styles on our end - using media query along with :not(.dark).
Can you file an issue there (tr-design)?

@xfq
Copy link
Contributor Author

xfq commented Apr 15, 2024

I see. Thanks.

Filed w3c/tr-design#349

@sidvishnoi
Copy link
Member

Someday in future we can use style queries to simplify that. Someday...

@xfq
Copy link
Contributor Author

xfq commented Apr 23, 2024

fixup.js now adds a .darkmode class when it's using dark mode.

@sidvishnoi
Copy link
Member

sidvishnoi commented Apr 23, 2024

I actually cannot find a way to make it work in case JS is disabled, while avoiding dark style duplication. With duplication, I'm hoping something like following:

.foo {
  /** light mode styles */
}

/** prefers dark mode, and not forced light mode */
@media (prefers-color-scheme: dark) {
  body:not(.lightmode) .foo {
    /** dark mode styles */
  }
}

/** forced JS darkmode */
body.darkmode .foo {
  /** dark mode styles again */
}

@tabatkins any guidance 🙏🏽?

@tabatkins
Copy link

The better route, rather than add a class to the body, is to add (or manipulate an existing) <meta name=color-scheme> to have content="light" or content="dark", which'll force the page into light/dark mode and make the MQ match appropriately.

@tabatkins
Copy link

(And ideally, generate the spec with content="light dark" so it'll respect the OS setting by default.)

@marcoscaceres
Copy link
Contributor

Hmmm... so, we do generate content="light dark", but we should be checking if the author has included a <meta name=color-scheme> in the document first.

I think we should:

  • Check if meta name=color-scheme is present. If no, add it with "light dark".
  • Deprecate and remove conf.darkMode

That would solve for this.

@marcoscaceres
Copy link
Contributor

@sidvishnoi
Copy link
Member

@tabatkins Not sure I followed right, but shouldn't following result in forced dark mode?

image

(Chrome 124 on Mac OS 14)

@tabatkins
Copy link

Sigh, I thought it did, but I guess I just tested its effect on the used color scheme, not the MQ. It looks like it does indeed not affect the MQ. I'm gonna raise this in the CSSWG.

@xfq
Copy link
Contributor Author

xfq commented Apr 26, 2024

Tracked in w3c/csswg-drafts#10249

@xfq
Copy link
Contributor Author

xfq commented Aug 30, 2024

Any progress on this issue? I assume we are able to solve this in a way that does not depend on changes to CSS specs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants