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 · 17 comments
Open

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

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

Comments

@xfq
Copy link
Member

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
Member

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 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
Member

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
Member 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
Member 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
Member

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
Member

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

@marcoscaceres
Copy link
Member

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
Member

Sent #4700

@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
Member

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
Member Author

xfq commented Apr 26, 2024

Tracked in w3c/csswg-drafts#10249

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