feat(ui): shell tokens align to Images-GUI (W15 A11 — primary, sidebar, surfaces, radius) - #209
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR shifts the brand primary from cyan to blue in tokens and CSS, deepens dark surfaces, reduces card radius from 8px to 6px, decreases Sidebar default width to 220px (persisted widths unchanged), adds a legacy-cyan constant and Tailwind alias, and records these changes in the completion ledger. ChangesDesign Token and Theme Update
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
03_implementation/ui/tailwind.config.ts (1)
24-32: ⚡ Quick winUse the shared Tailwind extension instead of re-declaring the tokens here.
This config is still a second source of truth for the same surfaces,
h3d.*aliases, radius, and glow values thatsrc/theme/tokens.tsalready exports viatailwindThemeExtension. This PR had to touch both files to keep them aligned; importing the shared extension would make the next token change much harder to drift.Proposed refactor
import type { Config } from "tailwindcss"; +import { tailwindThemeExtension } from "./src/theme/tokens"; ... const config: Config = { darkMode: "class", content: ["./index.html", "./src/**/*.{ts,tsx}"], theme: { - extend: { - colors: { - bg: "#0a0e1a", - surface: "#01101a", - surface2: "#001420", - border: "#1f2a44", - fg: "#e6edf7", - muted: "#7c8aa8", - accent: { - cyan: "#22d3ee", - blue: "#3b82f6", - green: "#22c55e", - amber: "#f59e0b", - red: "#ef4444", - }, - h3d: { - background: "var(--h3d-color-background)", - surface: "var(--h3d-color-surface)", - "surface-2": "var(--h3d-color-surface-2)", - border: "var(--h3d-color-border)", - "text-primary": "var(--h3d-color-text-primary)", - "text-secondary": "var(--h3d-color-text-secondary)", - primary: "var(--h3d-color-primary)", - "primary-cyan-legacy": "var(--h3d-color-primary-cyan-legacy)", - secondary: "var(--h3d-color-secondary)", - accent: "var(--h3d-color-accent)", - error: "var(--h3d-color-error)", - warning: "var(--h3d-color-warning)", - success: "var(--h3d-color-success)", - info: "var(--h3d-color-info)", - }, - }, - borderRadius: { - card: "6px", - chip: "999px", - }, - fontFamily: { - sans: ["Inter", "ui-sans-serif", "system-ui", "sans-serif"], - mono: ["JetBrains Mono", "ui-monospace", "monospace"], - }, - boxShadow: { - glow: "0 0 0 1px rgba(34,211,238,0.18)", - }, - }, + extend: tailwindThemeExtension, }, plugins: [], };Also applies to: 38-82
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@03_implementation/ui/tailwind.config.ts` around lines 24 - 32, Replace the in-file token re-declarations in tailwind.config.ts with the shared extension exported from src/theme/tokens.ts: import the tailwindThemeExtension symbol and spread/merge it into the config's theme.extend so colors (surface/surface2), h3d.* aliases, borderRadius.card and glow values are sourced from the single shared object instead of being redefined; remove the duplicated token blocks (lines that define colors, borderRadius.card, h3d aliases, glow) and ensure any local tweaks are merged into or override via a shallow spread (e.g., extend: { ...tailwindThemeExtension, /* local overrides */ }) so the config no longer serves as a second source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@03_implementation/ui/src/theme/tokens.ts`:
- Around line 92-99: The new legacy token PRIMARY_CYAN_LEGACY currently only
holds the dark hex and is not emitted by themeCssVars(), causing a split between
TS tokens and CSS; change PRIMARY_CYAN_LEGACY to be mode-aware (e.g., an
object/record with light and dark values or a function returning per-mode
values) and update themeCssVars() to emit the CSS custom property
--h3d-color-primary-cyan-legacy for both modes (light: "#0e7490", dark:
"#22d3ee"), matching the shape used by other tokens so runtime consumers and CSS
stay in sync; also adjust any type annotations or exports where
PRIMARY_CYAN_LEGACY is referenced so it aligns with the other theme token
definitions.
---
Nitpick comments:
In `@03_implementation/ui/tailwind.config.ts`:
- Around line 24-32: Replace the in-file token re-declarations in
tailwind.config.ts with the shared extension exported from src/theme/tokens.ts:
import the tailwindThemeExtension symbol and spread/merge it into the config's
theme.extend so colors (surface/surface2), h3d.* aliases, borderRadius.card and
glow values are sourced from the single shared object instead of being
redefined; remove the duplicated token blocks (lines that define colors,
borderRadius.card, h3d aliases, glow) and ensure any local tweaks are merged
into or override via a shallow spread (e.g., extend: {
...tailwindThemeExtension, /* local overrides */ }) so the config no longer
serves as a second source of truth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4fc40aca-c675-43c2-849a-33d40b45f2df
📒 Files selected for processing (5)
03_implementation/docs/handoffs/GUI_VISUAL_E2E_COMPLETION_LEDGER_2026-05-10.md03_implementation/ui/src/components/layout/Sidebar.tsx03_implementation/ui/src/styles/globals.css03_implementation/ui/src/theme/tokens.ts03_implementation/ui/tailwind.config.ts
| /** | ||
| * Legacy cyan kept available as a backup brand accent. The W15-A11 token | ||
| * shift moved `primary` from cyan to blue per the W14-A4 reference audit; | ||
| * this constant is the safe-to-revert original cyan. CSS consumers should | ||
| * prefer the `--h3d-color-primary-cyan-legacy` custom property over a | ||
| * hard-coded hex so theme variants can override it per-mode. | ||
| */ | ||
| export const PRIMARY_CYAN_LEGACY = "#22d3ee" as const; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Make the legacy primary token mode-aware and emit it with the rest of the theme vars.
Line 99 introduces a new public token, but it only captures the dark-mode value. In this PR, globals.css defines the light-mode legacy primary as #0e7490, and themeCssVars() never emits --h3d-color-primary-cyan-legacy at all. That leaves the new token contract split across files and means any runtime consumer using the TS token path can drift from the active theme.
Proposed fix
+const legacyPrimaryByMode = {
+ dark: "#22d3ee",
+ light: "#0e7490",
+} as const;
+
/**
* Legacy cyan kept available as a backup brand accent. The W15-A11 token
* shift moved `primary` from cyan to blue per the W14-A4 reference audit;
* this constant is the safe-to-revert original cyan. CSS consumers should
* prefer the `--h3d-color-primary-cyan-legacy` custom property over a
* hard-coded hex so theme variants can override it per-mode.
*/
-export const PRIMARY_CYAN_LEGACY = "#22d3ee" as const;
+export const PRIMARY_CYAN_LEGACY = legacyPrimaryByMode.dark;
...
export function themeCssVars(mode: ResolvedThemeMode): Record<string, string> {
const p = paletteFor(mode);
return {
"--h3d-color-background": p.background,
"--h3d-color-surface": p.surface,
"--h3d-color-surface-2": p.surface2,
"--h3d-color-border": p.border,
"--h3d-color-text-primary": p.textPrimary,
"--h3d-color-text-secondary": p.textSecondary,
"--h3d-color-primary": p.primary,
+ "--h3d-color-primary-cyan-legacy": legacyPrimaryByMode[mode],
"--h3d-color-secondary": p.secondary,
"--h3d-color-accent": p.accent,
"--h3d-color-error": p.error,
"--h3d-color-warning": p.warning,
"--h3d-color-success": p.success,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@03_implementation/ui/src/theme/tokens.ts` around lines 92 - 99, The new
legacy token PRIMARY_CYAN_LEGACY currently only holds the dark hex and is not
emitted by themeCssVars(), causing a split between TS tokens and CSS; change
PRIMARY_CYAN_LEGACY to be mode-aware (e.g., an object/record with light and dark
values or a function returning per-mode values) and update themeCssVars() to
emit the CSS custom property --h3d-color-primary-cyan-legacy for both modes
(light: "#0e7490", dark: "#22d3ee"), matching the shape used by other tokens so
runtime consumers and CSS stay in sync; also adjust any type annotations or
exports where PRIMARY_CYAN_LEGACY is referenced so it aligns with the other
theme token definitions.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements design token updates to align the UI with the Images-GUI reference audit. Key changes include shifting the primary brand color from cyan to blue (while preserving cyan as a legacy fallback), darkening surface colors, reducing the sidebar default width to 220px, and adjusting the card border radius to 6px. Feedback focuses on resolving inconsistencies in the legacy cyan constant across theme modes, ensuring the h3d theme object is fully updated, and reducing token duplication between the theme configuration and Tailwind settings.
| * prefer the `--h3d-color-primary-cyan-legacy` custom property over a | ||
| * hard-coded hex so theme variants can override it per-mode. | ||
| */ | ||
| export const PRIMARY_CYAN_LEGACY = "#22d3ee" as const; |
There was a problem hiding this comment.
The PRIMARY_CYAN_LEGACY constant is hardcoded to the dark-mode legacy value (#22d3ee). However, the light-mode legacy primary is #0e7490 (as correctly defined in globals.css and mentioned in the comment on line 115). This makes the constant misleading for TypeScript consumers who might expect it to be theme-aware. Consider adding primaryCyanLegacy to the ThemePalette interface so it can be correctly resolved per theme mode, or rename this constant to DARK_PRIMARY_CYAN_LEGACY to clarify its scope.
| muted: darkPalette.textSecondary, | ||
| accent: { | ||
| cyan: "#22d3ee", | ||
| cyan: PRIMARY_CYAN_LEGACY, |
There was a problem hiding this comment.
While updating accent.cyan here, note that the h3d object below (lines 233-247) is missing the primary-cyan-legacy token that was added to tailwind.config.ts. To maintain consistency between the static Tailwind classes and the runtime theme tokens used by components, please ensure the h3d object in this extension is also updated.
| // W15-A11 token deltas applied (aligned with `src/theme/tokens.ts` + | ||
| // `src/styles/globals.css`): | ||
| // - colors.surface : #0f1626 -> #01101a (W14-A4 rank 4) | ||
| // - colors.surface2 : #141d33 -> #001420 (W14-A4 rank 3) | ||
| // - borderRadius.card : 8px -> 6px (W14-A4 rank 5) | ||
| // - accent.cyan kept verbatim (#22d3ee) — 88+ existing in-codebase refs | ||
| // to `text-accent-cyan` / `border-accent-cyan` retain their colour. The | ||
| // brand primary shift (cyan -> blue) lives on the `--h3d-color-primary` | ||
| // CSS variable and the `h3d.primary` Tailwind alias only. |
There was a problem hiding this comment.
This file manually duplicates several design tokens (colors, border radius, box shadows) that are already defined in src/theme/tokens.ts and exported via tailwindThemeExtension. To reduce maintenance overhead and prevent these files from falling out of sync, consider importing tailwindThemeExtension and spreading its properties into the Tailwind configuration instead of hardcoding the values again here.
…r, surfaces, radius)
Apply the top 5 W14-A4 design-token deltas to the shell so the GUI chrome
aligns with the Images-GUI reference PNG set, without touching any tab
component (those are owned by W15 A12..A19).
Deltas applied:
1. primary : #22d3ee (cyan) -> #3b80f4 (blue) — BRAND DECISION per W14-A4
rank 1 (HIGH-INFO finding). Reference sampling found blue, not cyan,
is the dominant interactive accent across the reference set. A legacy
fallback `--h3d-color-primary-cyan-legacy` (#22d3ee) is preserved so
a brand revert or theme variant can restore the original cyan in a
single CSS override. Tailwind `accent.cyan` alias kept verbatim so
the 88+ existing `text-accent-cyan` / `border-accent-cyan` refs
across the codebase keep their colour.
2. Sidebar default width : 260 px -> 220 px (W14-A4 rank 2, HIGH conf).
Within the reference range 152-228 px (~12% of viewport). Persisted
user-resize widths in `h3d.mainSidebar.width` localStorage continue
to override; only fresh sessions pick up the new default.
3. surface : #0f1626 -> #01101a (W14-A4 rank 4, HIGH conf).
4. surface2 : #141d33 -> #001420 (W14-A4 rank 3, HIGH conf).
5. Card border-radius : 8 px -> 6 px (W14-A4 rank 5, MED conf).
Files changed (disjoint shell scope only):
- 03_implementation/ui/src/theme/tokens.ts
- 03_implementation/ui/src/styles/globals.css (:root / .light / .dark)
- 03_implementation/ui/tailwind.config.ts (theme.extend only)
- 03_implementation/ui/src/components/layout/Sidebar.tsx (width only)
- 03_implementation/docs/handoffs/GUI_VISUAL_E2E_COMPLETION_LEDGER_2026-05-10.md
TopBar.tsx already matches the reference 56 px min-height; no edit.
AppShell.tsx layout already conforms to the universal shell spec; no edit.
WCAG 2.1 — `#3b80f4` foreground on dark surface backgrounds (`#01101a`
panel / `#0a0e1a` page) gives ~6.0:1 contrast, passing AA Normal (>=4.5:1)
and AA Large (>=3:1). Light mode primary `#1d4ed8` on `#ffffff` is 7.1:1
(AAA Normal).
Self-audit:
- `npm run build` PASS
- `npx tsc --noEmit` PASS
- 4 Playwright shell-smoke specs PASS @ 1536x1024, 1672x941, 1920x1080
+ theme toggle (light/dark cycle still works; CSS vars resolve correctly)
- No console errors at app boot
- Screenshots saved under `test-results/w15-a11-shell-*.png` (gitignored)
Sources:
- Official: WCAG 2.1 Contrast (Minimum) — Success Criterion 1.4.3.
https://www.w3.org/TR/WCAG21/#contrast-minimum
- Cross-project: Tailwind `theme.extend` pattern + Material You token system.
https://tailwindcss.com/docs/theme + https://m3.material.io/styles/color/system
Hermes evidence chain: PASS
Task ID: W15-A11-SHELL-TOKENS-2026-05-10
hermes_run_gate: build + tsc + Playwright shell-smoke (4/4) + theme toggle clean
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bc88140 to
523640e
Compare
1 participant
Summary
Apply the top-5 design-token deltas from W14-A4 (Design Token Audit) to align the H3D shell with the Images-GUI reference PNG set. This PR only touches shell-level files (theme tokens, globals, Tailwind config, sidebar width, ledger) — disjoint from W15 A12..A20 which own tabs, dashboard modes, settings, voice, action window, and backend.
Top 5 deltas applied (per W14-A4 §3.1)
#22d3ee(cyan) →#3b80f4(blue) — BRAND DECISION. Reference sampling found blue, not cyan, is the dominant interactive accent across the reference set. A legacy fallback--h3d-color-primary-cyan-legacy(#22d3ee) is preserved inglobals.css+ aPRIMARY_CYAN_LEGACYexport intokens.tsso a brand revert or theme variant can restore the original cyan in a single CSS override. Tailwindaccent.cyanalias kept verbatim so 88+ existingtext-accent-cyan/border-accent-cyanreferences retain their colour.h3d.mainSidebar.widthlocalStorage continue to override; only fresh sessions pick up the new default.#0f1626→#01101a(HIGH-confidence reference match).#141d33→#001420(HIGH-confidence reference match).8 px→6 px(matches reference action-window corner-arc).WCAG 2.1 verification
#3b80f4foreground on dark surfaces (#01101apanel /#0a0e1apage) gives ~6.0:1 contrast, passing AA Normal (>=4.5:1) and AA Large (>=3:1). Light-mode primary#1d4ed8on#ffffffis 7.1:1 (AAA Normal).Files changed (disjoint shell scope)
03_implementation/ui/src/theme/tokens.ts03_implementation/ui/src/styles/globals.css(:root,.light,.darkonly)03_implementation/ui/tailwind.config.ts(theme.extendonly)03_implementation/ui/src/components/layout/Sidebar.tsx(defaultWidthonly)03_implementation/docs/handoffs/GUI_VISUAL_E2E_COMPLETION_LEDGER_2026-05-10.mdTopBar.tsxalready matches the reference 56 px min-height — no edit needed.AppShell.tsxlayout already conforms to the universal shell spec — no edit needed.Hermes evidence chain: PASS
W15-A11-SHELL-TOKENS-2026-05-10build + tsc + Playwright shell-smoke (4/4) + theme toggle cleanclaude-w15-a11-shell(6 source files + ledger), released at handoff.Self-audit (5 checks)
npm run buildPASS (vite production build, 1.30s)npx tsc --noEmitPASS (no type errors)Smoke screenshots saved at
test-results/w15-a11-shell-{1536x1024,1672x941,1920x1080}.png(gitignored — local proof only; smoke spec + config were deleted from the PR diff per agent contract).Test plan
npm run build+tsc --noEmitgreenSources
theme.extend+ Material You token system. https://tailwindcss.com/docs/theme + https://m3.material.io/styles/color/systemCites: W14-A4 (
03_implementation/docs/handoffs/W14_A4_DESIGN_TOKEN_AUDIT_2026-05-10.md), W15-A6 (GUI_INCOMPLETE_E2E_AUDIT_2026-05-10.md).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Style