Skip to content

feat(paper): reachable in-app Appearance theme toggle (off/paper/paper-night/auto)#1221

Open
Chris0Jeky wants to merge 12 commits into
mainfrom
feat/paper-appearance-settings-toggle
Open

feat(paper): reachable in-app Appearance theme toggle (off/paper/paper-night/auto)#1221
Chris0Jeky wants to merge 12 commits into
mainfrom
feat/paper-appearance-settings-toggle

Conversation

@Chris0Jeky

Copy link
Copy Markdown
Owner

Summary

Adds a real, reachable in-app theme toggle at /workspace/settings/appearance — a new Settings page with a 4-mode control (Off (Legacy / Obsidian) · Paper (Light) · Paper Night (Dark) · Auto (match system)) bound to the existing paperThemeStore.

This is the Wave-2 archive-pivot straggler that un-gates the later default-flip: today the only way to turn Paper on is the unlinked /styleguide/paper route, so a default-off user has no reachable switch. The new Appearance page lives in the Legacy shell (the current default) and is also linked from the Paper sidebar, so the theme is reachable from either shell. It is also the Legacy escape hatch ADR-0038 requires before the canonical default can flip to paper.

What changed

  • New views/AppearanceSettingsView.vue — segmented control; Off is labelled "Off (Legacy / Obsidian)" to make clear it swaps the whole shell, not just colours. Pure UI on the store (setMode / mode); a live hint line describes the active mode.
  • Route /workspace/settings/appearance (workspace-settings-appearance, requiresShell, breadcrumb "Appearance"), slotted beside the existing /workspace/settings/* cluster.
  • Nav links from ShellSidebar (Legacy) and PaperSidebar meta nav.
  • Tests tests/views/AppearanceSettingsView.spec.ts — renders 4 options, reflects current mode via aria-pressed, click → setMode + localStorage + body class, Auto stays Auto, Off restores Legacy (no paper body class).

Explicitly OUT of scope (deferred to Wave 3)

  • No default-theme flippaperThemeStore fallback stays 'off'.
  • No storage-key v2 / migration — key stays 'td.paper.mode'; ADR-0038's td.paper.mode.v2 is not introduced here.
  • No store changes — this is pure UI on the existing setMode/mode API.
  • No existing-test/default changespaperThemeStore.spec, AppShell.spec, and the .td-*-coupled E2E specs are untouched because the default is unchanged.

Design was reviewed maintainer-first (location chosen: dedicated Appearance view). Top-bar gear shortcut deferred to a follow-up.

Verification

  • npm run typecheck — clean
  • npx vitest --run --maxWorkers=2 on the new spec + PaperSidebar + ShellSidebar + paperThemeStore70 passed
  • npx eslint on all changed files — clean

Refs ADR-0038 (Paper UI canonical / Legacy frozen); archive-pivot Wave 2 (theme-toggle activation prerequisite).

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new Appearance settings view allowing users to choose between four theme options (Legacy, Light, Dark, and Auto), along with corresponding sidebar navigation items, routing, and unit tests. The reviewer recommended using a unique glyph/icon (such as 'U') for the navigation item to avoid a conflict with the existing 'Today' item, utilizing CSS Grid instead of flexbox wrapping for the theme segment buttons to ensure a consistent layout on smaller screens, and adding custom focus styles to the buttons to improve keyboard accessibility.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread frontend/taskdeck-web/src/components/paper/PaperSidebar.vue Outdated
Comment thread frontend/taskdeck-web/src/components/shell/ShellSidebar.vue Outdated
Comment thread frontend/taskdeck-web/src/views/AppearanceSettingsView.vue Outdated
Comment thread frontend/taskdeck-web/src/views/AppearanceSettingsView.vue

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 45245374a0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread frontend/taskdeck-web/src/router/index.ts
Comment thread frontend/taskdeck-web/src/components/shell/ShellSidebar.vue
@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Self-review (2 independent adversarial lenses + verification) — all findings fixed

Ran two independent adversarial reviews (Lens A: correctness / a11y / Vue-TS idiom / CSS-token validity; Lens B: integration / route-guard parity / conventions / scope / test quality), then an adversarial verification pass that re-checked every finding against the actual codebase. The highest-value check passed: all 12 --td-* tokens used in the view are confirmed defined; route-guard parity with sibling settings routes is correct; both sidebar specs and the command-palette specs were re-run green; scope boundaries (no default-flip, no store change, no migration, no existing-test edits) confirmed.

Findings (by severity) → fixes

Severity Finding Verdict Fix
LOW Panel uses Legacy --td-* tokens → renders Obsidian-dark inside the Paper shell (Paper never redefines --td-*; data-theme is never set) Real — but it's the convention for all /workspace/settings/* views, not a PR-specific defect; readable, just not Paper-skinned Added a code comment documenting the intentional Legacy-token use + that Paper-tokenizing settings surfaces is a later wave (cd43ba69)
LOW aria-pressed on a single-select group rather than role=radiogroup/aria-checked Real — but matches the universal codebase convention (zero radiogroup anywhere; aria-pressed used in PaperStyleGuideView, Today/Review rails); buttons are keyboard-operable Kept aria-pressed for consistency (a partial radiogroup without roving arrow-key focus would be a net regression); improved labelling via aria-labelledby → the visible "Theme" heading, replacing the duplicate aria-label (cd43ba69)
LOW Nav glyph/icon T collides with Today in the icon-only rails Real (cosmetic) Changed Appearance glyph/icon TE in both PaperSidebar (c425d11e) and ShellSidebar (2e6775ec)
LOW segmentByLabel test helper used substring matching (latent brittleness as Paper/Paper Night share a prefix) Real (latent) Added a stable :data-mode hook to each button; test now selects by [data-mode] (cd43ba69 view, 4a1d06bf spec)
Curly apostrophe U+2019 "only occurrence under src/views" INVALID — factually wrong; verifier found U+2019 also in PaperTodayView.vue. No change.

Verification after fixes

  • npm run typecheck — clean
  • npx vitest --run --maxWorkers=2 (new view spec + PaperSidebar + ShellSidebar) — 53 passed
  • npx eslint on all changed files — clean

No out-of-scope findings to seed. Awaiting CI + bot reviews (Gemini/Codex/Copilot); will address any.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Bot review round — all 6 findings addressed

Read and addressed every Gemini + Codex thread (replied in-thread + resolved):

Bot Finding Disposition
Gemini (MED) Glyph T collides with Today (PaperSidebar) Fixed → E (c425d11e) — also caught by self-review
Gemini (MED) Icon T collides with Today (ShellSidebar) Fixed → E (2e6775ec)
Gemini (MED) flex-wrap causes uneven segment wrapping Fixed → CSS Grid repeat(auto-fit, minmax(140px,1fr)) (33a08eca)
Gemini (MED) Buttons lack :focus-visible ring Fixed → explicit box-shadow: var(--td-focus-ring) (33a08eca)
Codex (P2) Appearance not discoverable in Legacy shell (catalog entry is Ctrl+K-only) Fixed → visible footer link in ShellSidebar, beside Settings, ungated by newAuth; Settings active-match scoped to /profile; +2 tests (91338efe, 0460582a)
Codex (P2) STATUS docs still say the toggle doesn't exist Tracked for post-merge docs-sweep — STATUS reflects merged reality and is concurrently being rewritten on PR #1220; updating it here (off main, pre-merge) would be inaccurate + conflict. Recorded in the continuity doc.

The Codex discoverability catch was the most valuable — the original PR would have left the theme reachable only via command-palette search for a default-off user, undercutting the whole point. There's now a visible Appearance entry in the Legacy sidebar footer.

Verification (post-fixes)

  • npm run typecheck clean · npx eslint clean · npx vitest --maxWorkers=2 on the view + both sidebar specs → green (2 new ShellSidebar tests for the footer link).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Pending

Development

Successfully merging this pull request may close these issues.

1 participant