Add light mode following the OS color scheme, with --theme to force#56
Merged
Conversation
The theme is known at server start but was riding on /api/diff, so a forced scheme applied only after the slowest request resolved (OS-scheme flash for the whole diff run) and never applied at all when the fetch failed. Carry it as ?theme= on the URL the CLI opens and read it at module load, before first paint. Deletes the server and DiffResponse plumbing; THEME_MODES in shared/types.ts now drives the CLI choices and the client's param validation from one list. A forced theme now also passes a single CodeView theme instead of the dual object: Shiki tokenizes once instead of once per theme (the dual path runs a full grammar pass per theme and ships both themes' token styles even though a pinned color-scheme can only ever render one). Also reorder the error guard above the loading guard: a failed fetch settles with data undefined, so the loading-first check showed 'Loading...' forever and the error branch was unreachable.
Both were left fixed at their dark values in the light-dark() conversion. In light mode the primary button hover lightened under white text (contrast dropping to about 3.6:1, opposite hover direction from --blue-bright) and the stale viewed label rendered about 3.6:1 on the control surface. Darken both light arms to 50% L, in line with the other accent text variants.
Vite's default cssTarget predates light-dark(), so esbuild lowered every token pair to a prefers-color-scheme polyfill that follows the OS and ignores the color-scheme --theme pins. Dev serves the raw CSS, which is why the bug only showed in the built package: forced light rendered the diff bodies light (the library generates its CSS at runtime) but left the page chrome dark. Raise cssTarget to the first versions with light-dark() support; not a new floor in practice, since the diff library already emits light-dark() at runtime.
Collaborator
|
Looks good, thanks. Will take a closer look tomorrow. |
Collaborator
Collaborator
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
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.



Made this for me (light diffs on a dark desktop) and it might help others. Closes #55.
styles.cssbecomes alight-dark()OKLCH pair resolved bycolor-schemeon:root. The dark side is byte-identical to the current palette.--theme light|dark|systemrides on the URL the CLI opens (?theme=), read before first paint, so there's no wrong-scheme flash and a forced theme still applies when the diff fetch fails. A hand-typed bare URL falls back to the OS scheme.cssTargetraised to the firstlight-dark()versions. Otherwise esbuild lowers the tokens to aprefers-color-schemepolyfill that ignores the forced scheme; dev serves raw CSS so it only shows in the built package. Not a new floor in practice,@pierre/diffsalready emitslight-dark()at runtime.DiffView. A failed fetch settles withdataundefined, so the error screen was unreachable and it showed Loading... forever. Pre-existing; found it testing the theme on the error path.Verified against the built package on both OS schemes with all three
--themevalues.npm run cigreen.