Skip to content

ethicalads: follow Material/Zensical dark mode - #716

Open
ericholscher wants to merge 1 commit into
mainfrom
ethicalads-material-dark-mode
Open

ethicalads: follow Material/Zensical dark mode#716
ericholscher wants to merge 1 commit into
mainfrom
ethicalads-material-dark-mode

Conversation

@ericholscher

Copy link
Copy Markdown
Member

Ads on Material for MkDocs and Zensical sites always render in light mode, even when the page itself is dark — reported on https://goss.readthedocs.io/en/stable/ (Zensical with an auto light/dark palette) with a dark system preference.

Root cause: we add the adaptive-css class to the placement, but the EthicalAds client's adaptive mode only reacts to markers it knows on html/body — the dark/auto classes, data-theme, or data-bs-theme. Material-family themes (Material for MkDocs, Zensical, sphinx-immaterial) signal dark mode with data-md-color-scheme="slate" instead, so no selector ever matches and the ad keeps the light default.

Rather than teaching the ad client about a theme-specific attribute, this handles it on the addons side, where the per-theme knowledge already lives: sync the client's documented dark class with data-md-color-scheme, observing attribute changes so the ad also follows the palette toggle and OS preference switches live. Themes without the attribute are unaffected.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Y4ChPUE9B1c2E2nyww8fqv

The EthicalAds client's adaptive-css mode doesn't recognize the
data-md-color-scheme attribute, so ads stayed light on dark pages.
Toggle the client's dark class to match, and observe changes.

Claude-Session: https://claude.ai/code/session_01Y4ChPUE9B1c2E2nyww8fqv
@ericholscher
ericholscher requested review from a team and humitos as code owners September 1, 2026 14:18
@humitos

humitos commented Sep 1, 2026

Copy link
Copy Markdown
Member

Root cause: we add the adaptive-css class to the placement, but the EthicalAds client's adaptive mode only reacts to markers it knows on html/body — the dark/auto classes, data-theme, or data-bs-theme. Material-family themes (Material for MkDocs, Zensical, sphinx-immaterial) signal dark mode with data-md-color-scheme="slate" instead, so no selector ever matches and the ad keeps the light default.

It seem this should be implemented in ethical ad client instead so the adaptive-css works out of the box, right?

@davidfischer

Copy link
Copy Markdown
Contributor

One possibility since there is an unlimited number of attributes where the light/dark mode could be set is to set the attribute on the ad placement (eg. data-ea-adaptive-attribute). Then the ad client could monitor that attribute for changes and trigger some sort of update code.

@ericholscher

ericholscher commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

Could be in either, we should just fix it somewhere because it currently looks pretty bad :(

I really wish there were standards here. :(

@davidfischer

davidfischer commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Unfortunately, there isn't a standard here.

Looking more closely at what Zensical is doing, they're setting data-md-color-scheme="slate" or data-md-color-scheme="default". Our normal light/dark selectors wouldn't even work. There's a few possibilities:

  1. We could just handle Zensical's settings. We would just add body[data-md-color-scheme="slate"] here. This is straight-forward but a one-off fix for them.
  2. We add an attribute like data-ea-dark-selector="body[data-md-color-scheme='slate']". The client sets up a MutationObserver to watch for changes to that selector. When it changes, we'd have logic to evaluate whether the ad should be dark mode or not and then apply the class manually. It's more work but it would allow this to work on any theme as long as that attribute is set. I added a not fully tested draft implementation here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants