[diffshub] Add Shiki theme switcher#731
Closed
necolas wants to merge 22 commits into
Closed
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
necolas
force-pushed
the
nicolas/diffshub-theme-switcher
branch
from
98af5bb to
d2dd0cc
Compare
necolas
force-pushed
the
nicolas/diffshub-theme-switcher
branch
from
f53b8d6 to
5981882
Compare
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. Three pieces, each fixing a distinct desync source: 1. `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. 2. `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. 3. 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
ef12ac7 to
af91e91
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
af91e91 to
85c361f
Compare
Undo diffs related changes
4 tasks
Contributor
Author
|
See #745 for a clean history version of this PR now that we've simplified some of the changes |
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