Skip to content

Conversation

@gordonwoodhull
Copy link
Contributor

@gordonwoodhull gordonwoodhull commented Apr 5, 2025

This is a case of "it was easier to fix this than to document it the way it is".

We can watch the media query prefers-color-scheme in exactly the same way we watch the visual toggle.

This does not introduce any new FOUC. However we still have a FOUC on some browsers when toggling dark mode dynamically #12309. This turns out to be a quarto preview issue.

I am hesitant to merge this, but it would make documenting respect-user-color-scheme much easier if we don't have to say it's only on page load and not dynamic.

First let's see if tests pass. I have added a bunch of them in this PR.

@gordonwoodhull gordonwoodhull added html Issues with HTML and related web technology (html/css/scss/js) themes Related to HTML theming or any other style related issue (like highlight-style) brand `_brand.yml` labels Apr 5, 2025
@gordonwoodhull gordonwoodhull force-pushed the feature/dynamic-prefers-color-scheme branch 2 times, most recently from e0ee0bb to 1ff7663 Compare April 5, 2025 23:39
gordonwoodhull added a commit to quarto-dev/quarto-web that referenced this pull request Apr 6, 2025
update ref
remove language about it not being dynamic, due to
quarto-dev/quarto-cli#12465
@cscheid
Copy link
Collaborator

cscheid commented Apr 7, 2025

I think this is good to merge!

gordonwoodhull added a commit to quarto-dev/quarto-web that referenced this pull request Apr 7, 2025
update ref
remove language about it not being dynamic, due to
quarto-dev/quarto-cli#12465
@gordonwoodhull
Copy link
Contributor Author

gordonwoodhull commented Apr 7, 2025

This has an annoying interaction with the visual toggle. Right now they both "win" in the sense that changing the OS setting always changes the appearance when respect-user-color-scheme is set, but the local storage setting from the toggle is still saved and takes precedence on reload.

It's like a 1-bit CRDT problem.

I see three ways to make them consistent:

  1. the OS setting should have no effect when there is a local storage setting
  2. option 1, but the local storage setting should get removed when it matches the OS setting
  3. the local storage setting should get removed when the OS setting changes

1 is simplest, but you can't ever get out once you've clicked the toggle, without going into dev tools and deleting the local storage key.

I think 2 is my favorite but it's slightly complicated to implement and document.

3 is simple but kind of weird and hard to explain.

@cscheid cscheid added this to the v1.7 milestone Apr 7, 2025
@cscheid
Copy link
Collaborator

cscheid commented Apr 7, 2025

Huh, yeah, this is all a bit weird. I wonder what other SSGs that have dark-light mode do in this case...

@gordonwoodhull
Copy link
Contributor Author

gordonwoodhull commented Apr 7, 2025

According to The Complete Guide to the Dark Mode Toggle

If the user has set an OS-level preference for a color scheme and has not set a preference on our site during an earlier visit, then we should respect the OS-level preference and render the site accordingly in light or dark mode.

This is option 1 and the simplest. It's consistent with what currently happens on page load.

I think I'll do this, and we can consider if we want to be more respectful of the user's OS setting in the future.

it's the same as the visual toggle except we don't set local storage
we set quarto-light or quarto-dark at the end of this script,
with the correct setting. so there is no need to set it here.
also, exhaustively test visual toggle
@gordonwoodhull
Copy link
Contributor Author

gordonwoodhull commented Apr 7, 2025

From a code review standpoint, this PR adds two more entries to window (in order to share functions between scripts to update the visual toggle after the page has loaded).

Is this okay?

also, the localAlternateSentinel is used dynamically
so it needs to be updated when we detect an OS change
so that the toggle works on the first click
@gordonwoodhull gordonwoodhull force-pushed the feature/dynamic-prefers-color-scheme branch from 1ff7663 to 71d9248 Compare April 7, 2025 20:27
@gordonwoodhull gordonwoodhull marked this pull request as ready for review April 7, 2025 20:28
@gordonwoodhull
Copy link
Contributor Author

I'll go ahead merge this, and ask my code review question on Thursday.

Glad to go back and clean this up later.

@gordonwoodhull gordonwoodhull merged commit b5af774 into main Apr 7, 2025
24 checks passed
@gordonwoodhull gordonwoodhull deleted the feature/dynamic-prefers-color-scheme branch April 7, 2025 21:27
gordonwoodhull added a commit to quarto-dev/quarto-web that referenced this pull request Apr 8, 2025
update ref
remove language about it not being dynamic, due to
quarto-dev/quarto-cli#12465
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

brand `_brand.yml` html Issues with HTML and related web technology (html/css/scss/js) themes Related to HTML theming or any other style related issue (like highlight-style)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants