Repository navigation
refactor(shell): express panel hierarchy with surfaces and add the design-token guard - #46
Merged
Merged
Conversation
…dows The three workspace panels (source / ask / notes), the settings dialog and the sidebar now sit on the ladder: sunken window chrome, base canvas between panels, raised content panels with a hairline, overlay only for floating layers. The panel trio of rounded-xl + border-0 + shadow-md is gone, as are the shadow-sm on the tab bar and the settings content panel. Settings gains a real hierarchy: the dialog is sunken, the nav rail is transparent on it, and the content panel is raised. Previously the rail and the panel were the same surface-base value, so the nav had no visual container. List rows (documents, notes, generated items) share one selected expression, bg-surface-selected with hover:bg-surface-hover, and their meta lines move to the tertiary text level. Row padding drops to px-2 py-2, the delete affordance also reveals on focus-visible, and timestamp/meta text stops competing with titles. Two latent bugs fixed on the way: - the sidebar menu button's outline variant wrapped an oklch() token in hsl(), which is invalid CSS, so the 1px outline never rendered. - TopNavigationBar cast a KeyboardEvent through 'as unknown as MouseEvent' to satisfy a handler that only calls stopPropagation(); the parameter is now typed SyntheticEvent and the cast is gone. Also makes bg-muted a recessed fill in dark mode (0.275, below surface-raised at 0.325), so nested containers read as inset in both colour schemes instead of flipping direction between them.
Library, home, reader, chat, quiz, flashcards, mind map and settings pages now use the surface ladder, the three text levels and the two elevation tokens. This is the last of the legacy usage: bg-card/bg-popover/bg-accent, shadow-sm/md/lg, rounded-sm/xl/2xl and the raw blue/green/red/purple palette are gone from src/renderer. Notable decisions: - the chat user bubble is neutral (bg-muted) instead of a solid accent fill; accent is now limited to the primary action, focus, selection, links and progress fills, as DESIGN.md states. - quiz correctness uses a new --success token plus --destructive, with icons and labels, instead of blue/green/red literals that only worked in one theme. - the flashcard type chips became outline badges with a chart-coloured dot: --chart-* sits at one lightness in both themes, so a text-bearing fill had no readable text colour. - the home hero drops from 48px/bold to 20px/medium, and the quiz score from 72px to 36px: both were marketing-page scale inside a desktop app. - markdown/editor inline code loses its accent colour and gains the control radius (it was a fourth radius value in raw CSS). Two latent bugs fixed on the way: - markdown.css used 'color: var(--primary) / 80', which is not valid CSS, so the link hover colour never applied. - quiz questionsData was typed through 'as any as QuizQuestion[]' in three places. The schema column now carries .$type<QuizQuestion[]>(), which removes all three casts; the neighbouring metadata columns move from Record<string, any> to Record<string, unknown>.
scripts/check-design-tokens.mjs reads the renderer sources and fails on a radius utility outside md/lg/full, an elevation utility other than shadow-elevation/shadow-control/shadow-none, a raw Tailwind-palette or arbitrary colour, alpha stacked on a text level, and a raw CSS border-radius outside the scale. It needs no build and no dependencies, so Verify runs it before the packaging matrix. 'npm run check:design -- --list' prints the rules. The three violations it found on the first run are fixed here: a bare 'rounded' on the close-tab affordance, the sidebar's floating variant using a bare 'shadow', and a comment that contained an example of the removed arbitrary shadow value. Modal scrims move from bg-black/80 to a --scrim token, which was the last raw colour in the tree. DESIGN.md's enforcement section now states exactly what the guard covers and what it deliberately leaves to review.
--foreground-dark was never defined in theme.css, so the class generated nothing. The base text-foreground already covers both themes.
mrsibe
force-pushed
the
design/semantic-tokens-and-shell
branch
from
September 24, 2026 07:41
9755e35 to
4632643
Compare
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.
What does this PR do?
Moves the app shell (three-panel workspace, sidebar, settings dialog, list rows)
and every remaining screen onto the token layer introduced in the base PR, and
adds
npm run check:design, a guard that mechanically enforces the part ofDESIGN.mdthat can be checked from source text.Stacked on #45. Merge that one first; this PR only contains the shell, page,
guard and CI work.
Why?
#45 established the vocabulary (surface ladder, three radius values, two
elevation tokens, three text levels) but deliberately stopped at the base
primitives. Until the shell and the pages use it, the ladder does not actually
express anything: panels still floated on
shadow-md, list selection still usedbg-primary/10, and the settings dialog put its nav rail and its content panelon the same surface so the nav had no container.
Fixes #43 (part 2).
What changed?
Shell —
SourcePanel/NotePanel/ProcessPaneldrop therounded-xl border-0 shadow-mdtrio forrounded-lg border border-border bg-surface-raised; the settings dialog becomes a sunken dialog with atransparent nav rail and a raised content panel;
PanelHeaderis 44px; the tabbar and drag handles lose their shadows and accent hovers; document / note /
generated-item rows share one selected expression (
bg-surface-selected,hover:bg-surface-hover) and their meta lines move to the tertiary text level.Pages — library, home, reader, chat, quiz, flashcards, mind map and settings:
bg-muted) instead of a solid accentfill; accent is now limited to the primary action, focus, selection, links and
progress fills;
--successtoken plus--destructive, with iconsand labels, instead of
blue/green/redliterals that only worked in one theme;--chart-*sits at one lightness in both themes and a text-bearing fill had noreadable text colour;
to 36px — both were marketing-page scale inside a desktop app;
bg-black/80to a--scrimtoken, the last raw colour;radius (it was a fourth radius value in raw CSS).
At this point
bg-card/bg-popover/bg-accent/border-input,shadow-sm|md|lg|xl|2xl,rounded-sm|xl|2xl|[inherit],roundedbare and theraw Tailwind palette are all gone from
src/renderer.Guard —
scripts/check-design-tokens.mjs(npm run check:design) reads therenderer sources with no build and no dependencies and fails on: a radius
utility outside md/lg/full, an elevation utility other than
shadow-elevation/shadow-control/shadow-none, a palette or arbitrarycolour, alpha stacked on a text level, and a raw CSS
border-radiusoutside thescale.
--listprints the rules and allowlists. It runs inVerifybefore thepackaging matrix.
Latent bugs fixed on the way
oklch()token inhsl()— invalid CSS, so the 1px outline never rendered;markdown.cssusedcolor: var(--primary) / 80, also invalid, so the linkhover colour never applied;
TopNavigationBarcast aKeyboardEventthroughas unknown as React.MouseEventto satisfy a handler that only callsstopPropagation(); the parameter is now typedSyntheticEventand the castis gone;
quiz.questionsDatawas reached throughas any as QuizQuestion[]in threeplaces; the schema column now carries
.$type<QuizQuestion[]>(), which removesall three casts (its
metadataneighbours move fromRecord<string, any>toRecord<string, unknown>, matching the shared DTOs);--foreground-dark,--background-dark, andtext-h1(undefined, so two settings headings silently rendered at body size).
Related issue
Fixes #43
How was this tested?
npm run check:design— no violations. Verified negatively too: injectingrounded-xl,shadow-md,bg-slate-100,text-gray-500andtext-muted-foreground/70produces all five rule IDs and exit code 1.npm run lint— 0 errors; warnings 118 → 112 (the reductions come from theremoved casts and dead tokens).
npm run typecheck— passes (node + web + test configs).npm test— 11 pass, 0 fail.npm run build(electron-vite) — passes; confirmed.bg-scrimand.text-successare generated from the new tokens.Not verified: the visual result. My environment has no display (no Xvfb), so I
could not launch Electron or capture screenshots.
npm run smoke:packagedalsoneeds a display and was not run locally; CI covers it with
xvfb-run.Screenshots / recordings
Could not be captured (no display in the environment this was built in). The
changes a reviewer should look at:
raised content) and its selected nav item is neutral, not a solid blue chip;
Pre-existing issues found, not touched
Both are on
mainand unrelated to this change; I left them alone rather thanwidening the diff.
src/main/db/index.ts:275—getSqlite()has an inferred return type thatreferences
BetterSqlite3.Database, so TypeScript cannot name it in adeclaration emit (
TS4058). It does not appear innpm run typecheck(nodeclaration emit there). The file is byte-identical to
main, so this predatesthe branch; the fix is a one-line explicit return type if you want it.
.github/workflows/verify.yml—actions/checkout@v4andactions/setup-node@v4are not pinned to SHAs (zizmor: unpinned-uses). ThisPR adds a
run:step only and does not touch those lines.Checklist
npm run typecheckpasses.npm run buildpasses.Desktop / build changes
verify.yml;npm run build:unpackand
npm run smoke:packagedwere not run locally (no display), CI covers both.