fix(vscode): stop theme sync overriding the user's chosen theme (#1053) - #1357
Merged
Merged
Conversation
The theme bridge wrote VS Code's colors as inline custom properties on <html>, the same element ThemeProvider stamps `theme-<palette>` and `light` on, and it forced the `light` class to the IDE's theme kind. An inline property outranks every `.theme-*` rule, so picking Light in a dark IDE produced dark VS Code tokens sitting under a `.light` class, and any palette chosen in Plannotator's settings was painted over. The bridge now reconciles instead of applying once on arrival: VS Code colors are only painted while the user is on the default palette and the app is already rendering the IDE's light/dark side, anything it painted is removed the moment that stops holding, and it no longer writes the `light` class except to map System onto the IDE's theme kind. Panels that have never stored a mode seed System, so a first-time user in a light IDE still gets a light panel now that the bridge does not force the mode. Reported by @it-sha.
backnotprop
added a commit
that referenced
this pull request
Aug 21, 2026
…em (#1362) #1357 made the panel defer to the app's stored theme mode and seeded System only when no mode was stored. That helped first-time panels and nobody else: every panel opened before it already stored `dark`, written by ThemeProvider on its first mount rather than chosen by anyone, so the seed never fired and the panel stayed dark in a light IDE. That is issue #1053 exactly, still broken for the users who reported it. There is no provenance in the store to read: it is one flat cookie string in globalState with no timestamps and no per-cookie metadata, and the app writes the same `plannotator-theme=dark` whether the user picked Dark or never opened the theme settings. The migration leans on the three signals that do exist. `light` and `system` are values the auto-seed cannot produce, so they are choices and are never touched. A new `plannotator-vscode-seed` marker, written on every load, makes the re-seed run at most once per store, so a Dark picked afterwards is permanent. And a mode the user actually picked is recorded server-side in ~/.plannotator/config.json by configStore.set, which configStore.init applies over the cookie and writes back, so a real choice outranks the seed and re-asserts itself in the same page load. What remains is a Dark that exists only as a cookie with nothing in config.json behind it. That is indistinguishable from the auto-seed and is reset once: invisible in a dark IDE, and in a light IDE one re-pick makes it stick for good. Also fixes the type error #1357 shipped in applyPanelCookieDefaults and adds the extension's own tsc to CI, which had never run there. Co-authored-by: Michael Ramos <backnotprop@gmail.com>
1 task
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
1 participant
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.
TLDR: Choosing Plannotator's light theme inside a dark VS Code did nothing, because the extension's theme bridge painted the IDE's colors as inline CSS custom properties on
<html>and forced thelightclass to the IDE's theme kind. Inline properties outrank every.theme-*rule, so the user's own choice was unreachable. The bridge now steps aside whenever the user has expressed a preference of their own. Reported by @it-sha in #1053.Root cause
ThemeProvider.applyThemeClasses(packages/ui/components/ThemeProvider.tsx:65) owns the app's appearance by stampingtheme-<palette>andlighton<html>, and the palettes are class scoped (.theme-plannotator/.theme-plannotator.light).The bridge wrote to that same element:
apps/vscode-extension/src/vscode-theme.ts:91(old) set 16 mapped VS Code colors withroot.style.setProperty(...). An inline custom property beats any class selector, so whatever palette the user picked was painted over and could never win.apps/vscode-extension/src/vscode-theme.ts:99-102(old) then ranroot.classList.remove("light")and re-added it only for a light IDE, stripping the class the user's Light choice had just produced.The result for the reported case is the worst of both: dark VS Code tokens under a
.lightclass. It applied once per message and never reconsidered, so a theme change made from Plannotator's settings mid session left the injected colors in place.Precedence contract
The app owns its own appearance. The bridge only fills in for a user who has expressed no conflicting preference:
<html>carries any palette other than the default, the bridge paints nothing and removes anything it painted earlier.MutationObserveron the app's class list re-decides wheneverThemeProviderrewrites it, so switching themes in Settings mid session removes the injected colors immediately, and a theme message that arrives before the app has mounted no longer paints an empty class list.Failure direction is deliberate: if the default palette id ever drifts from
themeRegistry.DEFAULT_COLOR_THEME, the bridge stops syncing and the app renders its own theme, which is safe, rather than painting over it.Why the cookie seed is part of the fix
Plannotator's own default mode is Dark. With the bridge no longer forcing the mode, a first-time user in a light IDE would have been left with a dark panel, which would have traded one bug for another. So a panel that has never stored a mode now seeds System (
applyPanelCookieDefaultsincookie-proxy.ts), which is what "adapts to your VS Code color theme" means here. It is a seed and not a write: an already stored mode is never touched, so it stops applying the moment the user picks a mode. This is the one piece that is a judgement call rather than a straight bug fix, so it is easy to drop if you would rather keep Dark as the in-editor default.Tests
New
apps/vscode-extension/src/vscode-theme.test.ts(8 tests) evaluates the injected listener against an isolated DOM, passingwindow,documentandMutationObserverin as parameters so each test gets its own root element, message listeners and cookie jar, and so the harness can only offer the two DOM surfaces the bridge is allowed to read.vscode-theme.tspreviously had no test coverage at all.Covered: the reported case, sync still working when the modes agree, a chosen palette never being stood in for, overrides being removed when the user switches to Light mid session, System mapping onto the IDE, a pinned Dark surviving a light IDE, and the pre-mount race.
Non-vacuity check: reverting
buildThemeListenerScriptto its old body and re-running fails 5 to 6 of the 8, including the headlineleaves the app's light theme alone in a dark IDE (issue #1053).Three more tests in
cookie-proxy.test.tscover the seed: absent means System, an existing mode is never overwritten, stored cookies and the auto-close flag survive.bun test apps/vscode-extension/src: 26 pass, 8 skip, 0 failDOM_TESTS=1 bun test apps/vscode-extension/src/vscode-theme.test.ts: 8 passbun run typecheck: clean.bun run build:vscode: clean..github/workflows/test.ymlalongside the otherDOM_TESTS=1suites.On the full suite:
bun teston this branch reports a few failures beyond the 12 stable baseline ones, all in unrelated server tests (api-404-guard, and a favicon assertion that reads the real~/.plannotator). This is not from this change. Dropping an empty placeholder test file at the same path on a clean tree reproduces the same class of extra failures, so it is shared process ordering interacting with suite tests that touch the real data directory.Not done here
No opt-out setting was added. With the precedence fixed, turning sync off is just picking a theme in Plannotator's settings. Say the word if you want an explicit
plannotatorWebview.syncThemetoggle as well.Closes #1053.
AI-assisted (Claude) under maintainer direction.