Skip to content

feat(ui): dark theme, profile menu and topbar theme switcher - #65

Open
SomiVista wants to merge 14 commits into
devfrom
new-design
Open

feat(ui): dark theme, profile menu and topbar theme switcher#65
SomiVista wants to merge 14 commits into
devfrom
new-design

Conversation

@SomiVista

Copy link
Copy Markdown
Collaborator

Batch promotion of new-design. Adds the light/dark theme layer, the topbar profile menu, and the topbar theme switcher.

⚠️ Read before merging to main

The dark palette in this PR is not the design system's. ui/src/styles/theme-tokens.css carries the values from the earlier profile-menu handoff; the design system's own subturtle-theme.css supersedes it wholesale and was not reachable while this was built (the design MCP needs an interactive /design-login). The two genuinely disagree:

this PR subturtle-theme.css
ramp inversion step-for-step around the mid-step, --rose-500 / --jade-500 keeping exact brand values
--paper #161219 #15111a
--surface-card #211b28 #1e1826
--white not re-pointed #1e1826 — "the card neutral"

The file carries a banner saying not to reconcile them by hand. Swapping it is a one-file change, and everything else here is already written for it — see below.

main stays frozen regardless (CLAUDE.md), so this lands on dev only.

What's in it

  • Theme layer. Applied as data-theme="light" | "dark" on <html>; both Tailwind builds use darkMode: ['selector', '[data-theme="dark"]']. @nuxtjs/color-mode owns the preference (subturtle:theme) and injects the pre-paint script.
  • StThemeSwitcher. One round 40px button cycling Light → Dark → System, in the shared dashboard topbar, on /auth/login, and in the practice session bar.
  • StProfileMenu. Identity header over four rows, teleported to <body>. Replaces the pilotui ThemeSwitcher + ProfileButton pair.
  • Audit fixes across ui/ so existing components theme without edits.

Decisions a reviewer should check

  • Both data-theme and the light/dark class are written. Deliberate. pilotui's compiled CSS and every un-migrated screen's dark: utility were built against .dark; dropping it would take their dark mode with it. plugins/theme.client.ts mirrors the preference into pilotui's store so the two never disagree — including on an OS-level flip while on system, which never moves preference.
  • --white no longer means white. The incoming layer makes it the card neutral, so everything meaning "ink on a rose CTA" moved to a new on-brand colour (a hard #fff). This audit is correct under both palettes and survives the swap.
  • The -600 ramp steps were flipped too. The handoff flips -700 for the "text on a soft tint" role; this codebase uses -600 that way as well and the audit didn't enumerate them. Left alone, #e30b4b on --color-primary-soft measured 2.8:1, under the 3:1 the handoff requires for icons. Values reused from what the handoff already supplies, never invented.
  • Un-migrated pilotui screens are not reconciled. They keep their own dark: behaviour until each migrates, per the migration plan.

Known gaps

  • No ClickUp task id on any commit subject — none was available. Worth fixing before merge if you want git log greppable.
  • --red-600 on --color-danger-soft measures 4.4:1, marginally under AA for the 11px badge label. Flagged rather than substituted; picking a different red is a design call. --text-faint (2.9:1) and white-on-rose (3.6:1) are also sub-AA, but those are the handoff's own values and fail identically in light today — pre-existing, not regressions.
  • The topbar plan pill was removed on review feedback, so the switcher sits where the design places it "between the plan pill and the avatar" with no pill present.
  • The Google button outline (audit item: "keep it white, add a --border-subtle outline in dark") is not applied — this screen's CTA is the rose primary button with a small white plate behind the mark, so there is no white-on-white surface to outline.
  • layouts/default.vue and stores/profile.ts are left Prettier-unformatted; both already failed format:check before this branch touched them.

Verified

Driven in a real browser against a live server and a standard freemium session:

  • data-theme="dark" present at document-commit on hard reload with system + OS dark — no flash.
  • system follows prefers-color-scheme live, both directions, no reload; data-theme and the pilotui body class stay in lockstep.
  • Switcher: cycle order, persistence, aria-label and tooltip in all three states ("System · dark").
  • Menu: panel is a direct child of <body> at position: fixed (the topbar's backdrop-filter no longer bleeds over it), tracks the trigger on resize and inner-<main> scroll, arrow-key roving with wrap, Escape restores focus, outside mousedown / route change close.
  • Geometry measured, not eyeballed: 40×40 in all three placements, login at 22/26, review-bar gap exactly 10px.
  • ui builds clean; 43/43 frontend tests pass.

Two bugs found by testing, worth knowing about

  • vue-i18n blanks patterns. Passing t('theme.aria') through as a pattern looks right and silently isn't — it interpolates {current}/{next} on the way out, so the aria-label rendered "Theme: . Switch to .". The label API now takes formatters.
  • st-relative beat absolute. subturtle-ui/style.css loads after the app's Tailwind, so a fall-through absolute on the switcher lost the specificity tie and the button landed mid-page. It's positioned by a wrapper now — the st- prefix can't help when both rules set position on the same element.

Store-side fix included

The Google profile picture wasn't rendering. downloadAndCachePicture() draws to a canvas and calls toDataURL() — the only reason it needs crossOrigin — so its usual failure is a CORS one, and the catch treated that as "URL is broken", marking it failed for 24h and stripping gPicture. A plain <img src> needs no CORS at all. The mark stays (it stops re-encoding every load); gPicture is no longer stripped, and StAvatar now falls back to initials on the <img> error event instead.

🤖 Generated with Claude Code

SomiVista and others added 7 commits September 1, 2026 10:45
Opening the topbar avatar menu rendered it behind the page: the "Install
extension" button and the stat cards drew on top of the open dropdown.

<main> is position: relative while <header> was static, and CSS paints
positioned elements above static ones whatever the DOM order — so the
content column and its whole subtree covered the header, and with it any
popover opened from the #header-right slot. The header's backdrop-blur
does not rescue this: the stacking context it creates still paints in the
non-positioned layer.

Adds st-relative st-z-sticky to the header. 100 sits above <main>
(z-auto) and below the mobile drawer and its scrim (z-overlay, 200),
which must keep covering the header below md.

Fixed in the shell rather than in ProfileButton so the next popover in
that slot — and the extension, which shares this library — gets it too.

Also drops three template comments in this file.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds @nuxtjs/color-mode with `classSuffix: ''`, so a bare `light`/`dark` class
lands on <html> — the hook subturtle-ui's token override will use, and the class
both Tailwind builds already resolve `darkMode: 'class'` against.

Three states, with `system` following `prefers-color-scheme` live: the module
registers its own matchMedia listener, so the composable stays a thin accessor
rather than double-registering one.

No flash on a hard reload, including `system`: the module inlines a blocking
pre-paint script into the SPA shell's <head> that reads the stored preference,
resolves `system` from the media query, and stamps the class before <body>.
Verified at document-commit, not just by eye.

plugins/theme.client.ts carries the two things the module does not do:

  - The cross-fade guard. Nearly every `st-` surface transitions its colours, so
    a palette swap would otherwise fade dozens of properties independently on
    different frames. An `html.theme-switching` class kills transitions for the
    one frame in which the swap lands; the rule ships in subturtle-ui's
    stylesheet, un-namespaced so it reaches app markup too. The module's own
    equivalent (`disableTransition`, an injected anonymous <style>) is turned off
    so there is exactly one mechanism, and it is inspectable in devtools. The
    watcher is `flush: 'sync'` because it has to have added the class before the
    module's own watcher swaps the one on <html>.

  - The pilotui mirror. Un-migrated screens keep their own `dark:` styling; this
    only keeps pilotui's store in step so the two never disagree. It watches the
    resolved value as well as the preference — pilotui resolves `system` against
    the media query at the moment toggleTheme() is called, so an OS-level flip,
    which never moves `preference`, has to re-poke it.

Also adds the seven profile-menu icons to the generated allowlist.

The dark palette itself (ui/src/styles/theme-tokens.css) is not in this commit —
it is transcribed verbatim from the design handoff, which is not yet available.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ships the design handoff's theme-tokens.css: the same token names re-pointed
under `html.dark`, loaded straight after tokens.css. Anything already styled
with var(--surface-card) / var(--text-body) / var(--ink-100) themes itself, so
no component grew a `dark:` variant.

ONE DELIBERATE DIFFERENCE from the handoff file, and it is not optional: the
handoff writes hex, tokens.css stores space-separated RGB channels so Tailwind
can compose `rgb(var(--token) / <alpha-value>)`. Shipping the hex verbatim would
emit `rgb(#211b28 / 1)` and silently break every st- colour utility in dark. The
values are the handoff's, converted channel-for-channel. Its translucent tints
cannot survive that conversion (a channel token has no room for its own alpha),
so they are pre-composited against --surface-card with the original rgba kept in
a comment beside each.

The two flips the handoff calls out are in, and are what the audit fixes below
lean on: the --ink-* ramp inverts (ink-50 darkest), and --rose-700 / --jade-700
become light tints because components use them as "text on a soft brand tint".
--white is NOT re-pointed; it means ink on a rose CTA, never a surface.

Audit list:

  - Avatar's --white ring and online dot -> --surface-card (new `border-card`).
  - Topbar backdrop -> --surface-topbar, a finished token, because dark needs a
    different base AND a different alpha (translucent --surface-card) than the
    old bg-page/80 could express.
  - Ambient blobs -> --blob-alpha (5% light, 3% dark); the login screen's
    stronger radials scale off the same token.
  - Skeletons -> base --ink-100, highlight --ink-150, animating background-color
    instead of a single fill's opacity, per the handoff's stated pair.
  - Scrims and the bundle-cover pill -> `bg-overlay`, NOT `bg-ink-950`. That ramp
    inverts, so a scrim written against it turns into a white veil in dark.
  - Solid `neutral` on Button/IconButton/Badge -> `bg-inverse` + `text-page`, for
    the same reason in the other direction: an ink-900 fill with white text would
    become a near-white fill with white text.

Two findings worth review, both measured with WCAG contrast in the browser
rather than eyeballed:

  - The handoff flips the -700 steps for the "text on soft tint" role, but this
    codebase uses the -600 steps that way too (StIconButton soft, StBadge soft,
    the Progress lock tile, LoginBoardPreview) and the audit did not enumerate
    them. Left alone they kept light values: #e30b4b on --color-primary-soft
    measured 2.8:1, under the 3:1 the handoff requires for icons. Flipped using
    values the handoff already supplies for the adjacent step or the status
    colour, never invented. See the comment in theme-tokens.css.
  - --red-600 on --color-danger-soft measures 4.4:1, marginally under AA for the
    11px badge label. FLAGGED, not substituted — picking a different red is a
    design call. --text-faint (2.9:1) and white-on-rose (3.6:1) are likewise
    below AA, but they are the handoff's own values and fail identically in the
    light theme today, so they are pre-existing rather than regressions.

The Google plate on /auth/login is left as-is. The audit's item assumes a white
Google button; this screen's CTA is the rose primary button with a small white
plate behind the mark, which is already "ink on a rose CTA" — there is no
white-on-white surface to outline.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replaces the pilotui ThemeSwitcher + ProfileButton pair in StAppShell's
#header-right with a plan pill and one avatar-triggered account menu. Changed
once in the shared layout, so every migrated screen picks it up.

StProfileMenu lives in the library rather than in a page because the menu is
identical on every screen and the browser extension needs the same one. It has
no router, store or i18n coupling: `items` is a plain array carrying the caller's
own handlers (translating ProfileMenu.d.ts's contract), the theme is a controlled
prop plus `update:theme`, and every visible string arrives through `labels`.

Two implementation details that are requirements, not preferences:

  - The panel is TELEPORTED to <body> and positioned `fixed` against the trigger
    rect, re-placed on resize and on CAPTURE-PHASE scroll. The topbar sets
    backdrop-filter, which bleeds its blur under an absolutely-positioned
    descendant and washes the panel out; the design prototype hit exactly this
    and fixes it the same way. Capture phase matters because the shell scrolls an
    inner <main> — a bubbling window listener never sees it.
  - Because of that teleport, keydown is bound to BOTH the wrapper and the panel.
    Teleported DOM events do not bubble through the wrapper the way React's
    synthetic events do in the prototype.

Appearance is a non-closing row: three icon-only segments in a role="radiogroup",
hand-rolled because SegmentedControl is not ported yet and the library may not
depend on pilotui. Choosing a theme leaves the menu open so the page repaints
behind it.

The Subscription row's meta comes from the live stores — voice minutes on a paid
plan, allowed_save_words on Free — and is omitted entirely while the
subscription is still fetching or a field is missing, rather than rendering a
placeholder that would read as a real number.

New account.* / appearance.* strings are sentence case per the handoff. The
existing `preferences.nav` and `sign-out` keys are Title Case and still used by
un-migrated pilotui screens, so they are left alone rather than restyled out from
under them; `profile.profile` and the sign-out confirmation keys are reused.

Verified in the browser against a real freemium session: panel is a direct child
of body at position fixed, 296px, z-index 200; rows render "9 / 200 saves" from
the store; arrow keys rove and wrap; Escape closes and restores focus to the
trigger; outside mousedown, route change and row activation close; the
Appearance row does not, and repaints the page behind it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three changes from the annotated screenshot.

1. Drop the topbar plan badge. StPlanPill goes with it — nothing else used it,
   and the git history has it if the extension's streak-pill variant ever wants
   it back. The plan pill inside the menu's identity header stays.

2. Show the Google profile picture. The avatar was falling back to initials
   because the store was deleting a perfectly good URL.

   downloadAndCachePicture() draws the avatar to a canvas and calls toDataURL(),
   which is the ONLY reason it sets crossOrigin — so its usual failure mode is a
   CORS one. The catch treated that as "this URL is broken", marked it failed for
   24h and stripped gPicture from local state. But a plain <img src> needs no
   CORS at all; the browser renders that URL fine. So one CORS-blocked cache
   write hid the avatar for a day. The old ProfileButton masked this by falling
   back to a placeholder PNG, which is why it surfaced only now that the design's
   initials fallback replaced it.

   The failure mark stays (it stops us re-attempting the encode every load), but
   gPicture is no longer stripped, on that path or on the knownFailed path. A
   genuinely dead URL is now caught where it belongs: StAvatar falls back to
   initials on the <img> error event, and resets that when `src` changes so a
   later working URL is not suppressed by an earlier failure.

3. Move the theme switch out of the menu and into the topbar, beside the avatar,
   where the previous version had it. The three-segment control is extracted to
   StThemeSwitch so both placements render the same component: the dashboard uses
   the topbar one, and StProfileMenu's `themeSwitch` row — which the handoff
   specifies and the extension may still want — now renders it too rather than
   duplicating the markup.

Verified: topbar badge gone, switch sits before the avatar and drives the theme,
Appearance row gone from the menu, and the avatar renders a working URL, falls
back to initials on a dead one, and recovers when a working one replaces it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ith --white

Two mechanism changes the design system's own theme layer requires, landed ahead
of that layer so the palette swap is a one-file change.

1. data-theme, not a class. @nuxtjs/color-mode gets `dataValue: 'theme'`, so it
   stamps data-theme="light|dark" on <html> — in the pre-paint script too, which
   is what keeps a hard reload flash-free, `system` included. Both Tailwind
   builds move to darkMode: ['selector', '[data-theme="dark"]'].

   The `light`/`dark` CLASS is deliberately kept alongside it. pilotui's compiled
   CSS and every un-migrated screen's `dark:` utility were built against `.dark`;
   dropping it would take those screens' dark mode with them. The two always
   agree. Storage key moves to `subturtle:theme`.

2. --white no longer means white. The incoming layer redeclares it as the CARD
   NEUTRAL (#1e1826), because that is how the design system uses it. So every
   place that meant "ink on a rose CTA" had to stop saying it: those now use a
   new `on-brand` colour, a hard #fff no theme can reach. Audited both
   directions — StBadge/StButton/StIconButton solids, StBundleCard's cover pill
   and scrim, StSidebarNav's active item. Surfaces keep using `bg-card`, which
   was already correct. The dark scope's --color-on-primary / --color-on-accent /
   --text-on-dark are now explicit literals rather than var(--white), since an
   alias declared at :root has already resolved by the time the dark scope runs.

theme-tokens.css is retargeted to the new selector and carries a banner: it still
holds the EARLIER profile-menu handoff's palette, and subturtle-theme.css
supersedes it wholesale. The two genuinely disagree — ramps invert step-for-step
here vs around their mid-step there, different --paper and --surface-card, and
the --white reversal above — so the banner says not to reconcile them by hand.
The literal-white audit is correct under both and survives the swap.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
One round icon button cycling Light -> Dark -> System, replacing the segmented
control from the earlier profile-menu handoff. That handoff's Appearance row is
superseded and removed: the design system ships a dedicated topbar control, so
StProfileMenu keeps its four rows and loses `themeSwitch` entirely.

40px `md` + `soft` everywhere, per the design's "no per-screen sizing": the
shared dashboard topbar (so every migrated screen inherits it), /auth/login
top-right at 22/26, and the practice session bar 10px left of End session. That
last one lives in PracticeToolScaffold, which every practice tool shares, so
Smart Review gets it along with the rest rather than only the one screen.

The glyph carries the state as much as the shape — sun in --amber-500, moon in
--rose-600, monitor in --text-muted — and is keyed on the mode so each press
remounts it and replays the 180deg spin. `system` resolves through a live
matchMedia listener, so the tooltip reads "System · dark" and follows the OS at
sunset without a reload.

Two things worth knowing, both found by testing rather than by reading:

  - `labels.aria` and `labels.resolved` are FORMATTERS, not patterns. Passing
    t('theme.aria') straight through looks right and silently is not: vue-i18n
    interpolates {current}/{next} on the way out, so the aria-label arrived as
    "Theme: . Switch to ." The consumer now passes (current, next) => t(...).
  - The switcher is positioned by a WRAPPER element, never by handing layout
    classes to the component. Its root carries `st-relative`, and
    subturtle-ui/style.css loads after the app's Tailwind, so a fall-through
    `absolute` loses the specificity tie and the button lands mid-page. The `st-`
    prefix cannot help here — both rules set `position` on the same element.

In the dashboard the control runs CONTROLLED, with persist-key="" and
:apply="false", because @nuxtjs/color-mode already owns the attribute and the
storage key; standalone persist/apply stays for the DS specimens and the
extension. Strings move from appearance.* to theme.*.

Verified: cycle order and persistence, aria and tooltip in all three states,
data-theme and the pilotui body class in lockstep, live OS follow on `system`,
data-theme="dark" already present at document-commit on reload, and the measured
geometry on both hand-placed screens (40x40; login 22/26; review gap exactly 10).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@SomiVista
SomiVista requested a review from navidshad September 1, 2026 16:03
@navidshad navidshad added the review Claude Routine will take this and review the PR label Sep 1, 2026

Copy link
Copy Markdown
Contributor

Automated PR Review

Primary Task: No ClickUp task ID found — see "Known gaps" note in the PR body.
Related tasks from commits: None identified (no CU- or #taskId in any commit subject or body).


Task alignment

This is a batch promotion of the new-design branch covering three shipped milestones: the dark/light/system theme layer, StThemeSwitcher, and StProfileMenu. No ClickUp task was linked (acknowledged in the PR body), so alignment is evaluated against the PR's own stated scope:

  • Theme layer@nuxtjs/color-mode wired, data-theme stamping, subturtle:theme storage key, pre-paint flash guard, both Tailwind builds updated to ['selector', '[data-theme="dark"]'].
  • StThemeSwitcher — new ui/src/elements/StThemeSwitcher.vue, placed on the shared dashboard topbar, /auth/login, and PracticeToolScaffold (covers all practice tools).
  • StProfileMenu — new ui/src/shell/StProfileMenu.vue, replaces old PartialProfileButton.vue (deleted), adapter in frontend/components/partial/ProfileMenu.vue.
  • Audit fixes--whiteon-brand migration in StBadge, StButton, StIconButton, StBundleCard, StSidebarNav; skeleton pulse corrected; avatar ring and blob alpha corrected.
  • Store fixdownloadAndCachePicture CORS failure no longer strips gPicture; StAvatar falls back to initials on <img> error and resets when src changes.
  • CLAUDE.md updated — dark-mode status section updated to reflect current state.

Scope is exactly what the PR claims — no unexplained extras, nothing obviously missing for this milestone.


Commit messages

# Commit Assessment
1 fix(ui): lift the app shell topbar above the content column fix is correct — a real rendering bug (z-index). Clear, specific.
2 feat: wire the three-state theme switch and its no-flash guard ⚠️ Missing scope: feat(ui): would be consistent with the rest of the series. The message body is thorough but the subject alone doesn't say where this lands. Minor.
3 feat(ui): make dark real on the st- design system ✅ Correct type and scope. Detailed body.
4 feat(ui): add the topbar profile menu with the theme switch ✅ Correct.
5 fix(ui): act on the profile-menu review ✅ Correct — three concrete changes from review feedback.
6 refactor(ui): apply the theme by data-theme, and stop meaning white with --white ⚠️ This is a mechanism change that required auditing and fixing components — it is closer to feat(ui): than refactor. refactor means no user-facing change; moving from darkMode: 'class' to data-theme selector and introducing on-brand / overlay / card / page Tailwind aliases is a capability addition. This will suppress a semver bump it arguably earns. Not a blocker on dev, but worth noting before the branch reaches main.
7 feat(ui): add StThemeSwitcher and place it on all three surfaces ✅ Correct. Detailed and accurate.

Cross-cutting gap (acknowledged in PR): None of the seven commits carry a ClickUp task ID (CU-… or #taskId). Per CLAUDE.md § Commits & versioning: "when the work has a task id, append it as #<taskId> to the commit subject". The PR body flags this ("No ClickUp task id on any commit subject — none was available"). If a task ID exists or is assigned later, the PR title at least should carry it, as squash-merge uses the PR title as the commit message.


Prior review follow-up

No prior automated or manual reviews found. Nothing to check.


Convention check

✅ Architecture / module structure — No violations. New StThemeSwitcher lives in ui/src/elements/ and StProfileMenu in ui/src/shell/, consistent with the existing layout. Dashboard adapter lives in frontend/components/partial/ as expected. useAppTheme composable in frontend/composables/, plugins/theme.client.ts in frontend/plugins/. All correct.

✅ Pilotui boundaryStProfileMenu ships with no router, store, or i18n coupling, consistent with the "library ships to the extension too" constraint documented in CLAUDE.md. Consumer adapter (ProfileMenu.vue) holds all app-specific wiring.

✅ SSR offplugins/theme.client.ts is correctly .client.ts so it does not run server-side. useColorMode and useAppStore are only called there. No Nuxt server-only feature used.

⚠️ StProfileMenu.vue — stale prop in withDefaults (ui/src/shell/StProfileMenu.vue, line ~1719 in the diff):

{
    name: '',
    align: 'right',
    width: 284,
    theme: 'system',    // ← `theme` is not a declared prop
    ...
}

theme does not appear in the defineProps generic — StProfileMenu carries no theme prop (theme is owned by StThemeSwitcher / useAppTheme). This stale default is harmless at runtime (TypeScript should catch it if strict mode is enforced), but it is dead code and may generate a TS warning.

⚠️ theme-tokens.css selector mismatch — The file is loaded under html[data-theme='dark'] (correct for the new mechanism), but ui/src/styles/index.css imports it with the comment "MUST come after tokens.css — it re-points the same names under html.dark". The comment says html.dark; the actual selector in the file is html[data-theme='dark']. The behavior is correct; only the comment in index.css is wrong. Low-priority.

⚠️ disableTransition: false in nuxt.config.ts — the comment says "turns OFF the module's own cross-fade guard", but false is the default and also happens to be the value that enables transitions, meaning the guard is not active either way when set to false. What disables the module's built-in transition suppression is setting disableTransition: true. The current config (false) means the module does nothing about transitions — which is the intended outcome since plugins/theme.client.ts handles it. The value is correct for the intended effect, but the comment ("turns OFF the module's own cross-fade guard") could mislead: false here means "do not disable transitions" (the guard stays off), which is exactly what you want so the plugin doesn't interfere. Suggest rewording to: "disableTransition: false — the default; the module leaves transitions alone, so our html.theme-switching class in plugins/theme.client.ts is the only mechanism."

✅ Tailwind config — Both frontend/tailwind.config.cjs and ui/tailwind.config.cjs updated to ['selector', '[data-theme="dark"]']. Consistent.

✅ i18n strings — New keys (account.*, theme.*) added to frontend/locales/en.json. Sentence case per handoff. Existing preferences.nav and sign-out keys untouched (correct — they're still used by un-migrated screens).

✅ Prettier — The PR body acknowledges layouts/default.vue and stores/profile.ts were already failing format:check before this branch; the diff confirms this branch did not introduce new violations.

StAvatar fixbroken ref reset on src change is correct and handles the "later working URL suppressed by earlier failure" case properly.

✅ Teleport + capture-phase scroll — The StProfileMenu implementation correctly uses { capture: true } (true as the third argument to addEventListener) and removes with the matching true so the listener is actually removed. ✅

ℹ️ Known design-system gap (acknowledged)theme-tokens.css carries the earlier handoff palette, not subturtle-theme.css. The PR body explains this and documents the swap as a one-file change. Acceptable to merge to dev; must not reach main until the palette is swapped.


Verdict

NEUTRAL (with one item to confirm)

The implementation is thorough, well-reasoned, and self-consistent. The known palette gap is clearly documented and the dev-only target is appropriate. Three minor issues worth addressing before merge or as a follow-up:

  1. Stale theme default in StProfileMenu.withDefaults — remove it (it's not a declared prop).
  2. Comment in ui/src/styles/index.csshtml.darkhtml[data-theme='dark'].
  3. disableTransition comment in nuxt.config.ts — rephrase to avoid "turns OFF" confusion.

None of these are blockers for dev. The missing ClickUp task ID on commits is acknowledged and also not blocking.


Generated by Claude Code

@navidshad navidshad removed the review Claude Routine will take this and review the PR label Sep 1, 2026 — with Claude
SomiVista and others added 7 commits September 1, 2026 19:35
Automated review on #65, the three findings that were real:

  - StProfileMenu's withDefaults still carried `theme: 'system'`. The `theme`
    prop went with the Appearance row the design system superseded, so this was
    a default for a prop that no longer exists. vue-tsc did not catch it —
    withDefaults does not reject keys absent from the props generic.
  - index.css said theme-tokens.css re-points names under `html.dark`. It is
    `html[data-theme='dark']` since the mechanism change.
  - The disableTransition note read as though `false` were switching something
    off. It is the module's own default; the value is right, the explanation
    was not. Reworded to say what each value does and why the explicit setting
    is worth keeping.

No behaviour change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fourth screen in the pilotui -> subturtle-ui migration, after the app shell,
Progress and Login. Page markup moves to the `st-` Tailwind namespace; all
script logic (both stores, the parallel fetchBoard/fetchPool under `loading`,
consumeActivity -> /practice/review, the page meta) is unchanged.

The design's "Due today" row is three fixed cards; the board only ever returns
the activities the server actually raised, so the grid is one card per activity.
`leitner_review` gets the amber icon tile, the "Due" badge and the items_due
line; any other type falls back to a generic card. The double-spinner becomes
StSkeleton blocks and the caught-up state an StEmptyState inside an StCard.

Three design elements are deliberately not shipped, because no data stands
behind them:

- The level pips under Smart Review. Every BoardService.refreshActivity call
  site writes meta as { dueCount, isActive } — there is no level distribution
  on the activity to draw them from.
- "Resting". No board data behind it at all.
- Two of the three optional-practice tiles. Match game has no route; Text chat
  needs a dispatcher-resolved ?session= and lands in errorMode without one.
  Flashcards -> /practice/bundle-review is kept (it falls through to /bundles
  when no review is staged, which is the "pick a bundle" behaviour that tile
  wants).

The grids use auto-fill rather than the design's auto-fit: with real data a row
often holds a single card, and auto-fit stretches it the full width.

PoolCard stays pilotui and unrestyled — it gets its own PR. Its height is why
the Smart Review card stretches; that resolves when it migrates.

board.no_activities and the activities.* group lose their only consumer here and
are dropped. solar:card-2-bold-duotone replaces the handoff's
solar:cards-bold-duotone, which does not exist in @iconify/json.

Verified against a standard (freemium) user under both data-theme values: the
populated board, the caught-up empty state, and Start review -> /practice/review.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two fidelity defects against the design screen:

- Cards stretched to PoolCard's height. PoolCard is still the pilotui card and
  is roughly twice a design card's height, so the grid's implicit stretch
  inflated the Smart Review card to ~570px against the design's ~280px, leaving
  a large void above the button. The grid is items-start until PoolCard
  migrates, at which point stretch is worth revisiting.
- "Optional practice" rendered in the caught-up state. The design gates it and
  "Due today" on the same `listShow`, so it belongs inside the populated branch,
  not beside the empty-state card.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The board's encode-queue card was the last pilotui surface on the screen, and at
roughly twice a design card's height it was what stopped the row reading as the
design. It now uses the same board-card pattern as its Smart Review sibling:
54px tinted icon tile, text-md/800 title, text-sm/600 body, mt-auto footer.

The handoff has no pool card of its own — the design's "Due today" row is Smart
Review, Live session and Resting — so it inherits the shape that row already
defines rather than inventing one. Jade keeps it distinct from Smart Review's
amber and the Flashcards tile's sky.

All logic is unchanged: the poolCount > 0 guard, chunkSize, estMinutes, the
four-range cardCopy, start() and doNext().

With both cards the same height, the grid goes back to the design's stretch so
the footers share a baseline. pool.module_label loses its only consumer (the
design's cards carry no eyebrow) and is dropped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Second screen in the pilotui -> subturtle-ui migration batch. Adds StInput,
StTextarea and StModal to the library — each lands with its first consumer.

The page keeps the dataProvider.list query shape, the page meta and
GenerativeCard (already on StBundleCard). The header, filter, grid, footer and
the New bundle modal move to the `st-` namespace.

Behaviour changes, both deliberate:

- The filter actually filters. The controller was built once at setup, which
  captured `filter.value` as '' for the lifetime of the page, so typing in
  "Filter bundles" never re-queried. It is now rebuilt per fetch, debounced
  300ms, always back to page 1.
- The footer is the design's "Showing X of Y" + Load more, appending pages,
  replacing the Pagination control.

Wiring the filter gives the empty state two meanings, and the design only covers
one. A non-matching filter now gets its own variant ("No bundles match …" with
Clear filter); the install/New-bundle empty state is kept for a genuinely empty
library.

Three bugs found while verifying, all pre-existing in the code being replaced:

- A successful create never navigated. analytic.track() throws when Mixpanel has
  no token (CI, e2e, local dev), which fell into the .catch, where destructuring
  `{ error }` off a TypeError gave `undefined.includes`. The track call is now
  guarded the way the subscription page's pricing-page_viewed is, and the catch
  no longer assumes the rejection shape.
- The list kept stale rows when a query returned nothing: the controller's
  `onFetched` hook does not fire for an empty result, so a non-matching filter
  showed the previous page under "Showing 4 of 0". Rows now come from
  fetchPage's return value.
- pilotui's toaster renders nothing app-wide — `toastError` asks it for a
  `TairoToaster` component this app never registers (10+ call sites affected).
  Out of scope to fix here, but it means the duplicate-title toast is invisible,
  so that case is also surfaced inline on the Name field.

The subtitle counts the whole library, not the filtered result, and is
pluralised. The design's phrase total is left out: only the loaded page carries
phrase arrays, so the number would be wrong on every page but the last.

The old `[role='dialog']` z-index workaround is NOT carried over: it existed
because pilotui's modal outranked the toast, and StModal's --z-modal (300) is
already below pilotui's toast (1060).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Third screen in the pilotui -> subturtle-ui migration batch. Adds StSwitch to
the library and reuses StInput / StModal from the Bundles screen.

The page becomes the design's single view: three stat tiles, then a left column
("The journey" + "Start over") and a right column ("The chain" — a row per level
with the number chip, tag, count bar, interval and cards-per-session inputs, and
Manage). The header carries the status dot, Discard and Save preferences.

All the data logic is unchanged: the get-stats fetch with its onSaved/onReset
refetch, localSettings, adjustArrays, getItemCount, performSave/performReset and
their emits, isDirty/settingsDirty, and the picker's loadPickerData,
fetchPhrases, addPhrase/removePhrase and emits.

- LeitnerPhrasePicker stops being a modal and becomes the inline panel under the
  level row, one open at a time, with activeBox following the open row. Only its
  shell changed.
- Discard restores the FETCHED settings, not the component's hardcoded defaults.
- The status count compares against the fetched settings, and per-level arrays
  only over their overlapping range: adjustArrays resizes both when the level
  count changes, which otherwise reported "3 unsaved changes" for one edit.
- The timezone line reads the profile's timeZone, falling back to the browser's
  rather than a hardcoded UTC.

The free-plan lock is NOT shipped — no dimmed levels, no "Learner" pills, no
dark upsell card. featureCapFor(_, "smart_review") returns null with the comment
"unlimited on every tier (Council 004)" (server/src/modules/subscription/
tiers.ts:125), and both test suites assert it, so there is no tier that caps
Smart Review levels and the design's lock would be invented.

The Cloze tag at level 3 IS shipped: FlashCard.vue switches to the fill-in-the-
blank card at `leitnerLevel >= 3`, so it describes real behaviour.

The Smart Review / Pool tab row stays as one route; PoolSettings keeps its
pilotui styling for its own PR. A save failure now also raises the design's
error banner — pilotui's toaster renders nothing in this app, so toastError
alone left a failed save with no visible outcome.

The hour field is a 0-23 number with ":00" beside it rather than the design's
free-text time, because the stored value is an integer hour.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Brings Today's board and the migrated PoolCard onto new-design alongside
Phrase bundles and Review settings, so all three migrated screens live on the
one branch.

Both conflicts were the shared icon allowlist, where each side had appended its
own names. Resolved as the union — solar:check-circle-bold is the reset modal's
bullet and solar:check-circle-bold-duotone is the board's caught-up state, so
both are needed. icons.generated.ts was regenerated rather than hand-merged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

2 participants