Skip to content

Library: the night sheet reads everywhere it is written on - #220

Merged
manager merged 2 commits into
devfrom
library-shared-memory
Sep 10, 2026
Merged

Library: the night sheet reads everywhere it is written on#220
manager merged 2 commits into
devfrom
library-shared-memory

Conversation

@manager

@manager manager commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Wolf's dark-mode contrast list, and the ones he did not list.

The defects were all in the night sheet, not in the components. Every label, counter, hint and placeholder in the Library is ink that is black at a fraction by day (--black-transparent-300, 14 places: the tag form's labels, the rating box, the character counters, the notes editor, the shelf header, pagination). On night paper that ink sat at 1.11:1. The signal colours were tuned for paper too: --red-600 3.06, --green-100 3.34, --red-500 3.70, --gray-medium 4.18, and --purple-100 (the AI shelf's own heading colour) 2.11.

The night sheet now carries light ink at a fraction and lifted signals. Measured against all six paper surfaces in the dark palette, every token the Library uses as text is now 4.83:1 or better (worst: --gray-300), and ink on a coloured fill (buttons, chips, the step circle, the rune seal) runs 5.85 to 8.22.

Also in:

  • The dim behind a dialog was a fifth of black, which is a veil over a light page and nothing over a dark one. It is 66% at night.
  • One error message pointed at var(--red, #d33), a token this project never defined; it now takes --red-500. Another rule used var(--color-text), also undefined.
  • The Libraries page kept its daylight paper: the sheet is a canvas tile handed to the element as an inline background, with washes that multiply. At night the tile is put away and the flat night sheet carries the page.

Found and NOT changed: the daylight palette has its own pre-existing weak pairs (--white on --green-200 2.61, --gray-medium on paper 2.68, --gray 3.26, --white on --brown 4.26). Repainting the light theme Wolf already approved is his call, not a side effect of a dark-mode fix.

🤖 Generated with Claude Code

Every label, counter, hint and error in the Library is ink that was black at
a fraction, or a signal colour tuned for paper: on night paper they sat at
1.1:1 to 3.7:1, which is what the tag form, the rating box, the About editor
and the object forms all showed. The night sheet now carries light ink at a
fraction, lifted reds, green and purple, a stronger dim behind a dialog, and
one error colour that pointed at a token this project never had.

The Libraries page kept its daylight paper because the sheet is a canvas tile
handed to the element inline, with washes that multiply: at night the tile is
put away and the flat night sheet carries the page.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

Claude finished @manager's task in 3m 19s —— View job


Reviewing PR #220

  • Gather context (diff, changed files, CLAUDE.md/AGENTS.md rules)
  • Review themes.scss contrast/token changes
  • Review BrushPaper.module.scss dark mode handling
  • Review Object.module.scss / Shelf.module.scss changes
  • Post inline comments for issues found
  • Post summary

Summary

Scope is four SCSS files only — no React/TS/a11y-markup changes in this diff, so most of the standard checklist (hooks, hydration, TS types, App Router patterns) doesn't apply here.

Verified against the dark-mode contrast passport: I recomputed WCAG contrast ratios independently for a sample of the retuned tokens (--gray-300 on the lightest dark surface --white-100 lands at 4.84:1, matching the PR's own claim of "worst: --gray-300, 4.83:1"). The numbers in the PR description check out.

Checked for a subtler regression class: since themes.scss inverts --white to a dark ink value in body.darkTheme, several places that pair a lifted signal color as a background with color: var(--white) (e.g. Button.module.scss .warning/hover, RecommendedShelf.module.scss .regenerate, RecommendedBookCard.module.scss .statusLocked/.action:hover) still resolve to dark-ink-on-light-fill in dark mode, not white-on-light. I confirmed contrast there stays well above 4.5:1 (≈6.9–7.6:1) — no regression, the token-inversion pattern holds up correctly across all the background-fill usages of --red-500, --red-700, and --purple-100 I could find in the Library tree.

One real issue found (inline comment on src/styles/library/themes.scss:78): --gray-medium is declared twice inside the same body.darkTheme .library selector — once pre-existing at line 37 (#8f877e) and again by this PR at line 78 (#a89f95). The second silently wins, so it works today, but it leaves a dead declaration that will mislead the next person who edits line 37 expecting it to have effect. Worth consolidating into one declaration.

Minor, non-blocking note: the new --black-transparent-100 and --black-transparent-200 dark-mode overrides don't appear to be consumed anywhere in src/components/library or src/styles/library currently (only --black-transparent-300 and --black-100 are referenced). Harmless as written, just flagging in case they were meant to land alongside a consumer that didn't make it into this diff.

Everything else checks out:

  • Object.module.scss / Shelf.module.scss: both replace genuinely undefined tokens (var(--color-text), var(--red, #d33)) with real tokens from the design system, per the PR description — correct fixes, no fallback values invented.
  • BrushPaper.module.scss: the !important on background-image: none is justified — it's overriding an inline style.backgroundImage set by the canvas tile from JS, which can only be beaten by !important in the stylesheet. Selector nesting (:global(body.darkTheme) &) scopes correctly to the component's own class.
  • No Tailwind/CSS-in-JS/inline-style violations, no hardcoded values outside the token-definition file itself (which is the correct place for raw hex in this system).

Comment thread src/styles/library/themes.scss Outdated
--red-700: #ff8f7a;
--green-100: #6fd39a;
--purple-100: #b49bff;
--gray-medium: #a89f95;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--gray-medium is already declared at line 37 (#8f877e, part of the pre-existing "ink turns to light" block) and is redeclared here with a different value (#a89f95). Since both live in the same body.darkTheme .library selector, the second declaration silently wins and the first becomes dead — confusing for anyone who edits line 37 expecting it to take effect. Worth folding into the single existing declaration instead of adding a second one.

Fix this →

Answers the review: --gray-medium was set twice in the night sheet, so the
first line was dead weight.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@manager

manager commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

Fixed: one --gray-medium in the night sheet now, carrying #a89f95. Verified the token table again after the fold, and against the production build: the emitted stylesheet carries --black-transparent-300:hsla(35,37%,90%,.72) inside body.darkTheme .library, and the background-image:none rule that puts the daylight paper away is in the shipped sheets.

@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

Claude finished @manager's task in 49s —— View job


Reviewing PR #220 (new commits pushed)

  • Gather context (diff since last review, changed files)
  • Check whether the previously flagged --gray-medium duplicate was addressed
  • Review the new commit for contrast/token/SCSS issues
  • Post summary

Summary

New commit 9f270e9 ("one declaration for the lifted mid grey") answers the one issue flagged in the previous pass: --gray-medium in src/styles/library/themes.scss was declared twice inside body.darkTheme .library (line 37 pre-existing at #8f877e, and a second one added by this PR at #a89f95). The fix removes the dead first declaration and keeps a single one at line 37 with the intended contrast-fixed value #a89f95 — correct resolution, no leftover duplicate.

Confirmed --gray-medium is only declared once now in the dark theme block, and is consumed consistently as a color across Sidebar, ShareSelectionPanel, ObjectOverviewModal (×3), and RatingBox — all text usages, so the lifted value is the right one to keep.

No other files changed in this push (BrushPaper.module.scss, Object.module.scss, Shelf.module.scss are untouched since the prior review, which already passed them). Nothing new to flag — this PR is clean.

@manager
manager merged commit 8b3b499 into dev Sep 10, 2026
2 checks passed
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