fix(a11y): hold sidebar small text to WCAG AA contrast - #986
Draft
h4yfans wants to merge 2 commits into
Draft
Conversation
Sidebar text that carries real information renders at 10-11px, which WCAG AA treats as small text and holds to 4.5:1. Several of those elements were well under the floor on the paper sidebar: SidebarSection collapsed count --sidebar-muted/60 1.43:1 Tag-group heading --muted-foreground/70 3.62:1 Tag list show-more control --sidebar-muted 1.87:1 Per-tag note count --muted-foreground/40 1.95:1 The section heading also swapped to `hover:text-sidebar-foreground`, which is 3.18:1 there, so it lost its guarantee exactly while being pointed at. All of them now take `--sidebar-section-heading`, the sidebar's de-emphasised small-text token, kept separate from `--sidebar-muted` because that one also colours chevrons and decorative icon buttons where the extra weight is too loud. Headings drop their hover colour — the chevron fading in, or the row's `bg-muted`, carries the affordance. The show-more control is a control rather than a label, so it keeps a hover colour, but one that raises the ratio instead of lowering it. Measured against each theme's own surface: 5.07 / 5.16 / 6.42 on `--sidebar`, and 5.07 / 5.06 / 6.01 on `--muted` for the elements whose row paints a hover background while the text is visible. jsdom has no cascade and no layout, so a render can never prove a ratio. The palette is literal hex in base.css, so tests/utils/contrast.ts reads the theme blocks straight from source and does the arithmetic. It fails on any state variant that repaints the text below the floor, or merely below where it rested, and understands Tailwind's `/NN` opacity modifier (`text-sidebar-muted/60` is not `--sidebar-muted`). Both components seed UI state from localStorage and nothing in the renderer setup clears it, so the tests pin it in `beforeEach`: a stored value would otherwise outrank `defaultExpanded`, collapse the tag category, or reorder the tags out from under a `maxVisible` slice, and which happened would depend on test order.
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sidebar text that carries real information renders at 10–11px, which WCAG AA treats as small text and holds to 4.5:1. Several of those elements were well under the floor on the paper sidebar.
SidebarSectioncollapsed count--sidebar-muted/60— 1.43:1--sidebar-section-heading--muted-foreground/70— 3.62:1--sidebar-section-heading--sidebar-muted— 1.87:1--sidebar-section-heading--muted-foreground/40— 1.95:1--sidebar-section-headingThe section heading also swapped to
hover:text-sidebar-foreground, which is 3.18:1 there — so it lost its guarantee exactly while being pointed at.Approach
--sidebar-section-headingis the sidebar's de-emphasised small-text token, kept separate from--sidebar-mutedbecause that one also colours chevrons and decorative icon buttons, where darkening is too loud.Headings drop their hover colour — the chevron fading in, or the row's
bg-muted, carries the affordance. The show-more control is a control rather than a label, so it keeps a hover colour, but one that raises the ratio (--sidebar-primary, 15.22:1 on paper) instead of lowering it. Elements whose row paintshover:bg-mutedwhile the text is visible are measured against--mutedtoo, not just--sidebar.How this is tested
jsdom has no cascade and no layout, so a render can never prove a ratio. The palette is literal hex in
base.css, sotests/utils/contrast.tsreads the theme blocks straight from source and does the arithmetic. It checks:root,.whiteand.dark, understands Tailwind's/NNopacity modifier (text-sidebar-muted/60is not--sidebar-muted), and fails on any state variant that repaints the text below the floor or merely below where it rested — however the variant is spelled (hover:,group-hover/section:,peer-hover:,[&:hover]:).Ten new assertions across the two components. Regressing any token or class in this PR turns them red.
One thing worth a reviewer's eye
Both components seed UI state from
localStorage, and neithertests/setup-dom.tsnortests/setup.tsclears it — jsdom is fresh per file but shared by every test inside it. A stored value silently outranksdefaultExpanded, collapses the tag category, or reorders tags out from under themaxVisibleslice, and which one bit you would depend on test order. Both test files now pin it inbeforeEach; the pin was mutation-tested by pre-seeding each key and confirming the target element disappears. Documented incontribute/testing.mdsince it applies to any renderer component that persists state.Verification
pnpm --filter @memry/desktop test:renderer— 565 files / 6318 pass, 0 failpnpm typecheck— 16/16 taskspnpm lint— 0 errors (14 pre-existing warnings, none in these files)pnpm docs:impact --base f4b2fa5cf --strict— covered ·pnpm docs:build— cleanScope
Branched off
mainand deliberately limited to the a11y work. The Excalidraw context-menu fix and the canvas-folder schema work that shared a worktree with this are not included and remain unpushed.🤖 Generated with Claude Code