[diffshub] Add Shiki theme switcher#731
Open
necolas wants to merge 20 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
necolas
force-pushed
the
nicolas/diffshub-theme-switcher
branch
from
6ede954 to
6cd1a75
Compare
amadeus
reviewed
May 21, 2026
| // corresponding row, picks a theme, and is returned to the main view. The | ||
| // menu auto-resets to the main view whenever it closes so the next open | ||
| // always starts from the top. | ||
| function ThemeDropdown({ |
Member
There was a problem hiding this comment.
should we wrap this lad in a memo? I don't fully trust react compiler
| {isLight ? 'Light theme' : 'Dark theme'} | ||
| </span> | ||
| </DropdownMenuItem> | ||
| <div className="mt-1 max-h-[320px] overflow-y-auto"> |
Member
There was a problem hiding this comment.
Can we use cv-mini-scrollbar class here to clean up the scrollbars in this menu? https://github.com/pierrecomputer/pierre/blob/main/apps/docs/app/globals.css#L305
Adds a theme picker next to the gear icon in the diffshub header. Light and dark theme menus plus an Auto/Light/Dark color-mode toggle. Theme changes are pushed to the WorkerPool so the diff actually re-tokenizes with the selected themes.
After WorkerPoolManager.setRenderOptions changed the active theme, two caches kept the previous theme's :host CSS variables (--diffs-light, --diffs-dark, addition/deletion/modified colors) alive: 1. DiffHunksRenderer/FileRenderer's per-instance renderCache.result kept the themeStyles string from the previous tokenization. Collapsed files never re-tokenize (highlightDiffAST is gated on totalLines > 0), so their headers, line numbers, and indicator colors stayed on the old theme until the user toggled the file open. Add WorkerPoolManager.getCurrentThemeStyles() and have processDiffResult and processFileResult pull themeStyles + baseThemeType from it on every render, so the diff chrome refreshes immediately even when the tokens are still being recomputed. 2. FileDiff/File.applyThemeState trusted any data-theme-css <style> node adopted from the element pool (hasAdoptedThemeCSS=true) and skipped writing fresh CSS. When the pool's previous occupant was on a different theme, the recycled container kept showing that theme. Drop the short-circuit and always upsertHostThemeStyle once on adoption. The textContent write is cheap and only fires per pool reuse; the equality check above it still short-circuits steady-state renders.
The file tree had a hardcoded pierre-light-soft/pierre-dark-soft palette and the sidebar wrapper painted itself with the diffshub neutral chrome (--diffshub-sidebar-bg). Resolve the selected light/dark Shiki themes through resolveTheme/themeToTreeStyles in a small shared hook (useResolvedTreeThemeStyles), with a module-level cache so re-selecting a theme is synchronous. The hook feeds two consumers: - CodeViewFileTree applies the resolved TreeThemeStyles directly, so file names, git decorations, hover/selection chrome, etc. follow the theme's editor palette. - CodeViewSidebar inline-styles its wrapper with the theme's sideBar.background (falling back to editor.background) so the tabs row, diff stats panel, and footer share the same surface as the tree. The Tailwind chrome bg is kept as a fallback for the first render while the resolver returns. The diffshub-specific density and padding overrides stay; the color overrides that previously locked the sidebar to the neutral chrome were removed so the resolved theme can actually drive the surface.
The wrapper background already followed the Shiki theme but the sidebar chrome (Diff Stats labels, tab icons, "Powered by" footer) still used the global neutral --muted-foreground / --foreground from the app's ThemeProvider, so the text and icons looked unrelated to the diff and tree palette next to them. Extend useResolvedTreeTheme (formerly useResolvedTreeThemeStyles) to return the resolved Shiki theme's editor.foreground and sideBar.foreground / descriptionForeground alongside the TreeThemeStyles bundle, then override --color-foreground, --color-muted-foreground, --color-border, and --color-border-opaque on the sidebar wrapper: - Primary text uses editor.foreground — the highest-contrast color the theme defines, the same one the diff viewer next door uses for code. - Muted text (Diff Stats labels, file counts, "Powered by" footer) uses the theme's own sideBar.foreground / descriptionForeground shade, so it stays in the palette instead of being a fade-to-transparent of the primary text (which gets unreadable on themes whose editor.foreground sits close to the background). - Borders mix the primary fg at ~20% with the theme background so panel dividers stay subtle on both light and dark themes. The raw --foreground / --muted-foreground / --border variables are mirrored too in case any custom CSS reads them directly. The TreeThemeStyles-only export is preserved as a back-compat alias for callers that don't need the raw colors. Tailwind v4 bakes its @theme aliases out to concrete values at root level, so overriding the original variables alone wasn't enough — the --color-* names had to be overridden too for utilities like text-muted-foreground / border-border to pick up the theme.
Add a small usePersistedState hook that mirrors a string-valued useState to localStorage and rehydrates from it on mount (in an effect, not the useState initializer, so the SSR markup stays identical to the first client render and hydration doesn't mismatch). The rehydration only accepts values present in a `validValues` allowlist, so a stale or hand-edited key from a previous theme set can't push the picker into an unknown name. A hydrated-ref guard skips the first write so the initial commit's default value can't clobber the stored value before the read effect has run. Use it in ReviewUI for both LIGHT_THEME_STORAGE_KEY and DARK_THEME_STORAGE_KEY, replacing the inline read/write effects.
Themes like slack-ochin use opposite palettes for the editor and sidebar surfaces (white editor, dark navy sidebar). Picking `editor.foreground` for the sidebar's primary text and icons ended up writing #000 into --color-foreground on a #2D3E4C surface — making the file/comment tab icons, the Diff Stats ± icon, and the "Files"/"Lines" count values nearly invisible. Switch primaryFg to sideBar.foreground (with editor.foreground / theme.fg as fallbacks); muted text prefers descriptionForeground, otherwise the consumer fades primaryFg at 55%. Drop sideBarSectionHeader.foreground from the muted chain — slack-ochin defines it as #FFF, which is brighter than primary text and inverts the hierarchy.
Some themes ship a list.hoverBackground designed for their editor surface, whose palette can be the inverse of the sidebar's. Slack-ochin is the worst case I've seen: editor.background is white, but sideBar.background is dark navy with light gray text, and list.hoverBackground is #d5e1ea — a near-white wash. Applied to the sidebar that overlay lands on the same luminance band as sideBar.foreground and the hovered row's text disappears. Add a luminance-based heuristic to themeToTreeStyles: parse the hover bg, sidebar bg, and sidebar fg as hex (the dominant format in @shikijs/themes), compute relative luminance, and drop the hover bg when it sits closer in luminance to the fg than to the bg. Non-hex formats fall through unchanged so themes that use modern color syntax keep their intended hover. Also pick the rgba fallback's tint from the sidebar's actual luminance rather than `theme.type`. Otherwise slack-ochin (declared `light` for the editor) would land on `rgba(0,0,0,0.06)` over a dark navy surface — invisible. Reading the bg directly picks `rgba(255,255,255,0.08)` for the dark sidebar so users still get hover feedback. Cover the new behavior with focused unit tests for the slack-ochin shape, a typical light theme, the equal-color short-circuit, and a non-hex hover bg.
Extract the chrome CSS-variable logic that lived inline in CodeViewSidebar into a shared `useThemeChromeStyle` hook in `useResolvedTreeThemeStyles.ts`, and apply it to the header too. The header now lives on the same Shiki theme as the sidebar — bg, primary text/icons, muted text, and borders all follow the active theme's sideBar tokens instead of the global light/dark palette. Falls back to `--diffshub-sidebar-bg` during the first render before the theme resolves. Add three surface tokens to the chrome style for "cards" that sit on top of the sidebar bg (the comments list rows): `--diffshub-card-bg`, `--diffshub-card-hover-bg`, `--diffshub-card-border`, each a color-mix of primaryFg into the sidebar bg. Replaces the hardcoded `bg-card` / `dark:bg-neutral-800` / fixed-RGBA borders that broke on mixed light/dark themes like slack-ochin (classified "light" but renders a dark navy sidebar — the sidebar's foreground override then collided with a near-white card surface and the body text disappeared). Also derive `--diffshub-comment-add-fg` and `--diffshub-comment-del-fg` by parsing the surface's relative luminance: emerald-700/rose-700 on light surfaces, emerald-400/rose-400 on dark ones. CodeViewCommentsList reads these vars instead of using Tailwind's `dark:` variant for the +Line/-Line label tints, so additions and deletions stay legible on every theme — including slack-ochin's dark navy card where the global `dark:` variant would otherwise leave us with low-contrast 700 shades.
…ers" This reverts commit 4945fc0.
Basically with my fixes this stuff won't be required
The light/dark theme picker inside the header dropdown was using a plain `overflow-y-auto` container, so its scrollbar landed on the wide platform-default style instead of the slim thumb-on-hover that the comments list (and the rest of diffshub's secondary scrollables) uses. Reuse `cv-mini-scrollbar` from globals.css and add `overscroll-contain` so the wheel doesn't kick into the underlying CodeView when the list hits its top/bottom.
The diffshub WorkerPool is a long-lived singleton that survives across
client-side route changes. Until now ReviewUI fired its
`workerPool.setRenderOptions({ theme })` effect during the very first
render — when usePersistedState still returns DEFAULT_LIGHT_THEME /
DEFAULT_DARK_THEME because the localStorage rehydration effect hasn't
run yet. That was harmless on the initial load (the pool also started
on the defaults), but on a second visit (e.g. logo → home → another
PR) it would: (1) yank the pool back to the defaults, clearing its
caches and notifying every theme subscriber, (2) start tokenizing the
incoming files against the wrong theme, then (3) rehydrate and swap
back to the user's pick on the next render — leaving the diff visibly
unthemed until a full reload re-ran the chain in a different order.
Have usePersistedState expose an `isHydrated` flag (and use it as the
write-guard ref) so ReviewUI can compute `themesHydrated` from both
picks. The WorkerPool effect now bails out until that flag is true, and
the `viewerAvailable` gate withholds the CodeView mount until the same
condition holds — so the viewer is never constructed with a stale
theme prop and never asks the pool to tokenize against the wrong
palette. The hot path (initial load with no stored value) is
unaffected: the pool stays on its built-in defaults until hydration
completes, then a single setRenderOptions call applies the user's
choice.
The chrome and file tree both pulled text color straight from `sideBar.foreground`. Some themes ship a value that fails on the surface they're paired with (material-theme-ocean's #525975 on #0F111A is ~2.6:1; aurora-x doesn't set it at all). Add a contrast-aware picker that walks `sideBar.foreground` → `editor.foreground` → `theme.fg` in design-intent order, keeps the first candidate that clears 3:1 against the sidebar bg, and falls back to the highest-contrast option when none do. Apply the same gate to `descriptionForeground` so dim muted tokens (aurora-x's #576daf79, ~1.8:1) fall through to the primary fg instead of fading every label and icon into the surface. Mirror the resolved primary fg into the tree's tokens — `color`, `--trees-theme-sidebar-fg`, and the section-header / active-selection / focus-ring tokens when the theme doesn't set them explicitly — so the file rows track the chrome end-to-end.
When the header's light / dark theme picker opens, the currently selected row was wherever its alphabetical position placed it — often off-screen on a list of 40+ themes. Ref the scroll container and the selected row, and on view change scroll so the row sits one row below the top of the viewport (computed via bounding rects since the container isn't a positioned ancestor). The selected row lands right under the cursor and the row above it makes the prior theme a single arrow-key away, which helps sequential browsing.
The badge was hardcoded to `bg-neutral-200/700` + `text-neutral-700/200`, so on dark/themed sidebars it lit up as a neutral gray pill out of step with the rest of the chrome. Switch to a `color-mix(currentColor 18%, transparent)` background and drop the explicit text color so the badge inherits the tab button's color — it now tints with the active Shiki theme in both the unselected ghost state and the selected outline state.
A new button in the System Monitor row, left of the autoscroll toggle, drives a sweep through every light theme then every dark theme (and back), anchored on whichever theme is currently active so the visible state isn't yanked when the cycle starts. Plain click toggles cycling on/off. Shift-click bumps the per-step duration. Cycling state lives in a new `useThemeCycle` hook in ReviewUI and is prop-drilled through CodeViewSidebar to WorkerPoolStatus.
…containers
Three load-bearing pieces working together make code-view theme switching
reliable. Each addresses a distinct way theme state can fall out of sync,
and they are not interchangeable.
1. Keep `theme: { dark, light }` on the CodeView options.
`CodeView.setOptions` clears its element pool via `shouldClearPool`,
which compares `previousOptions.theme` to `nextOptions.theme`. Without
`theme` on the option object both sides resolve to `undefined`,
`areThemesEqual` returns true, and the pool is never cleared on theme
swaps. The pool then hands out containers carrying the previous
theme's shadow-root CSS. The option is also what flips
`areDiffRenderOptionsEqual` in `getRenderOptions`, which is how the
renderer detects a theme change and sets `forceHighlight=true`.
2. Refresh `themeStyles` and `baseThemeType` from the worker pool in
`processDiffResult` / `processFileResult` via
`WorkerPoolManager.getCurrentThemeStyles()`.
`setRenderOptions` clears the worker-side caches, but each renderer's
per-instance `renderCache.result` still holds the old themeStyles
string. Collapsed files never re-tokenize (the async highlight path
is the only thing that would refresh the cached result), so without
this override their headers, line numbers, and addition/deletion
colors stay on the previous theme until the user toggles the file
open. Pulling themeStyles from the worker manager on every render
refreshes only the chrome — the cached syntax tokens are left alone
and get replaced when `highlightDiffAST` completes.
3. Always write fresh CSS on element-pool adoption in `applyThemeState`
(do not trust the adopted shadow-root contents).
Theme changes clear the pool synchronously, but the new CSS for the
currently-visible containers is written by `CodeView.render()`, which
is queued via `queueRender` rather than running synchronously. A fast
scroll during a theme swap can release a visible container back into
the pool before that queued render writes the new CSS, so the pool
ends up holding containers whose shadow root still carries the
previous theme. When the next file adopts one of those containers and
`applyThemeState` shortcuts on `hasAdoptedThemeCSS`, the bookkeeping
updates but the stale `<style>` textContent never gets overwritten —
the wrong theme is then permanently locked in by the equality
fast-path. Falling through to `upsertHostThemeStyle` on first
adoption is a single cheap textContent write per pool reuse; the
equality check above it still short-circuits steady-state renders.
The contrast threshold for `descriptionForeground` was 3:1 (WCAG AA large text), inherited from the primary-fg picker where we deliberately honour the theme designer's dim sidebar values. That's the wrong bar for the sidebar's muted labels — "Diff Stats", "System Monitor", "Comments", file/addition/deletion counts, empty-state copy — which all render at normal body-text size and need 4.5:1 to stay legible. Ayu-dark's `descriptionForeground` is `#5a6378` on `#0d1017` (~3.27:1). It cleared the 3:1 floor and so was silently kept, but the labels read as dim shadow against the near-black sidebar. The previous fallback, when the threshold *did* fail, was the theme's primaryFg — flattening muted up to primary, which loses the visual hierarchy entirely. Split the muted threshold from the primary one (`MIN_MUTED_RATIO = 4.5`) and add `deriveMutedFg`: when the theme's value fails the bar, walk a 60% → 90% mix of primaryFg into bg and keep the first blend that clears 4.5:1, falling back to primaryFg only as a last resort. Ayu-dark now gets `#8a8986` on `#0d1017` (~5.8:1) — distinctly softer than its `#bfbdb6` primary but firmly readable. Other themes whose `descriptionForeground` was already legible keep their original value.
A theme change now flips header, file tree, comments, footer, and diff viewer together in the same paint frame. Previously the chrome trailed the diff (and on cold themes briefly flashed the pierre-soft fallback), and the comments cards / footer links visibly interpolated through the old palette for ~150ms on every swap. Four pieces, each fixing a distinct desync source: 1. `WorkerPoolManager.setRenderOptions` (packages/diffs): hoist the local-state update + cache clear + `themeSubscribers.rerender()` above the `await this.setRenderOptionsOnWorkers(...)`. The per-worker `postMessage` calls fire synchronously inside that Promise constructor, so each worker has the `set-render-options` task queued before this function returns — any highlight tasks the rerender submits are processed after the theme swap, preserving ordering. The await stays at the end so the public Promise still resolves on full completion. Effect: the diff's chrome (header/gutter/colors via `getCurrentThemeStyles()`) updates in the same synchronous run as the rerender, instead of waiting tens of ms for every worker to ACK. 2. `ReviewUI` worker-pool effect: `useEffect` → `useLayoutEffect`. The sidebar's inline-style update lands during React's commit phase, so running the worker-pool update in a paint-time effect kept the diff one frame behind the chrome on every swap. 3. `useResolvedThemeByName`: return the module cache directly during render instead of routing it through useState. `useState`'s initializer only runs on mount, so for cycled-through themes the hook was returning the *previous* render's value and waiting for the post-commit `useEffect` to call `setResolved` — one full React commit of chrome lag for every swap. Also keep the previously resolved theme visible while a cold theme loads, matching the diff side's behaviour (the diff stays on the previous theme during its own await), so chrome doesn't flash the pierre-soft fallback in the intermediate render. 4. Drop `transition-colors` from comment cards and `color, text-decoration-color` from `.inline-link`. Both are Tailwind hover- smoothness niceties, but they ALSO interpolate when the underlying CSS variables flip on a theme swap — the comment cards stepped through ~18 intermediate RGB values over 150ms before landing, visibly trailing the header / tree / diff which had no color transitions and snapped instantly. There's no CSS-level way to distinguish "user hovered" from "CSS variable changed", and a JS-driven `transition: none` class can't catch transitions that start during commit before any effect fires. Removing the transitions at the source costs only the smooth hover bg fade, which is far cheaper than a visibly desynchronised theme swap. Verified live by sampling `--color-foreground`, `--diffshub-card-bg`, the footer link's computed color, and a diff container's `--diffs-dark-bg` at 3ms intervals through a 7-second cycle: every transition lands in a single polling tick across all four surfaces, the footer no longer interpolates, and the diff is within 16ms of the chrome (one paint frame).
necolas
force-pushed
the
nicolas/diffshub-theme-switcher
branch
from
98af5bb to
d2dd0cc
Compare
Apply resolved Shiki chrome tokens to the theme picker, display settings menu, comment sidebar cards, and inline diff comments. Scope inline annotation variables so the diff viewport keeps its own border and scrollbar colors.
necolas
force-pushed
the
nicolas/diffshub-theme-switcher
branch
from
f53b8d6 to
5981882
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
2 participants
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.
Summary
Adds a light/dark Shiki theme picker to the diffshub header and wires it through
the diff viewer, the file tree, and the surrounding sidebar chrome so a single
selection re-themes everything in view.
Choices persist to localStorage via a small
usePersistedStatehook with anallowlist guard so stale keys can't push the picker into an unknown theme.
treessidebar palette (file rows, gitdecorations, hover/selection chrome) and the diffshub sidebar wrapper
(tabs row, Diff Stats panel, "Powered by" footer) — all via a shared
useResolvedTreeThemehook with a module-level cache so re-selecting issynchronous.
sideBar.foreground(falling back toeditor.foreground); muted text prefersdescriptionForeground. Avoids theslack-ochin case where the editor surface is white but the sidebar is dark
navy, which previously wrote
#000over#2D3E4C.themeToTreeStylesnow rejectslist.hoverBackgroundwhen its luminancesits closer to the sidebar fg than the sidebar bg (again, slack-ochin: a
near-white hover wash over a dark navy sidebar erased row text). The rgba
hover fallback is also picked from the sidebar's actual luminance instead
of
theme.type. Covered by focused unit tests.diffscaches were keeping the previous theme's:hostvariables aliveacross theme swaps:
DiffHunksRenderer/FileRenderer's per-instancerenderCache.result(collapsed files never re-tokenize, so their headersand indicator colors stayed stale) and
FileDiff.applyThemeState'shasAdoptedThemeCSSshort-circuit on pool reuse. Both now pull freshthemeStylesfromWorkerPoolManager.getCurrentThemeStyles()/always
upsertHostThemeStyleonce on adoption.Test plan
light theme) and confirm the diff viewer, file tree, and sidebar
chrome all re-theme together
without expanding the file
then swap themes, confirm recycled containers pick up the new theme