Skip to content

fix: select forced theme palettes by a data attribute - #60

Merged
alekswebnet merged 1 commit into
masterfrom
fix/theme-followups
Oct 19, 2026
Merged

alekswebnet merged 1 commit into
masterfrom
fix/theme-followups

Conversation

@alekswebnet

@alekswebnet alekswebnet commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Follow-ups from the review of #58 (keep an explicit LIGHT theme light on a dark OS).

  • Forced palettes are selected by a data attribute, not by inline-style text. The theme matched the forced DARK palette with [style*="color-scheme"][style*="dark"] and, since Respect explicit light theme #58, skipped the system dark rules with [style*="color-scheme: light"]. Both are substring tests on the whole style attribute, so they depend on how the browser serializes the inline style and misfire on values such as color-scheme: light dark or an unrelated --x: lightgray. applyViewerTheme() now also sets html[data-color-scheme="light|dark"] (removed for AUTOMATIC), and all 19 guards in the theme match that attribute exactly. The inline color-scheme is still written, since PDF.js and the browser UI use it.
  • The forced palette is applied before PDF.js initializes, so the first paint already has it. Previously applyViewerTheme() only ran after initializedPromise.
  • LIGHT on a dark OS no longer gets PDF.js's dark hover and disabled styles. viewer.css switches --hover-filter and --button-disabled-opacity on the system scheme alone. The theme now sets both in its light base and its dark blocks, so they follow the chosen theme.
  • Parity: the neutral forced-dark .sidebar block gains the --sidebar-box-shadow its @media twin already had.
  • Comments explain the :where() / &:root pattern (no added specificity, so consumer overrides at :root level still win) and cross-reference the second palette.
  • README: the "Viewer CSS theme" section now says LIGHT and DARK override the system color scheme, and AUTOMATIC (the default) follows it.

Test

The theme test from #58 now also checks data-color-scheme and --hover-filter for every mode, drops a repeated loop iteration, and filters media rules by conditionText instead of a cross-realm instanceof. The dark-block count assertion is kept: it is the only assertion that touches the warm palette's @media block, since the neutral palette hides its colors.

Full suite 21/21, pnpm build succeeds, both attribute guards are present in dist/paper-and-ink.css.

Not included

Driving the theme from color-scheme with light-dark() would remove the duplicated dark palettes (a @media block plus a forced block per palette) and the guards altogether. That rewrites the theme file, so it belongs in its own PR.

🤖 Generated with Claude Code

Follow-ups from the review of #58:

- The theme matched forced palettes by substring tests on the inline
  style attribute, which depend on serialization and misfire on values
  like `color-scheme: light dark`. The element now sets
  html[data-color-scheme="light|dark"] and the theme matches it exactly.
- Apply the theme before PDF.js initializes, so a forced palette is in
  place at first paint.
- Keep --hover-filter and --button-disabled-opacity in step with the
  chosen theme instead of viewer.css's system-scheme switch.
- Add the missing --sidebar-box-shadow to the neutral forced-dark block.
- Document that LIGHT and DARK override the system color scheme.
- Test: check data-color-scheme and --hover-filter per mode.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@alekswebnet alekswebnet changed the title fix: match forced light theme like forced dark fix: select forced theme palettes by a data attribute Oct 19, 2026
@alekswebnet
alekswebnet merged commit b590349 into master Oct 19, 2026
6 checks passed
@alekswebnet alekswebnet mentioned this pull request Oct 19, 2026
5 tasks done
@alekswebnet
alekswebnet deleted the fix/theme-followups branch October 19, 2026 18:43
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.

1 participant