fix(ui): restore dark mode token values in globals.css - #3566
Open
westernCasa wants to merge 1 commit into
Open
Conversation
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.
What
The
.darkblock inpackages/ui/src/globals.cssis currently identical to:rootall 33 tokens carry their light values, so applying thedarkclass changes nothing that depends on a semantic token. This restores real dark palette.Why
apps/framework-editor/styles/globals.cssalready documents this and worked around it locally:Since
@trycompai/uiis published, consumers following the dark-mode setup in the package README get a theme that does nothing. Inside the monorepo the impact is narrower —apps/appandapps/portaltake their tokens from@trycompai/design-system— butframework-editoris carrying the override, and the published package is affected.What changed
The palette is FRAME-5's, upstreamed unchanged, with one exception.
--primary-foregroundwas0 0% 98%. Near-white on the dark-mode primary (165 70% 42%) measures 2.57:1, below WCAG AA for normal text and below AA-large. Setting it to0 0% 7%gives 7.29:1 while leaving the brand's teal untouched.Contrast for every pair in the block:
Deliberately out of scope
--chart-*tokens — still light values inside the dark block; several are low-contrast on a dark ground. Would love to follow up separately.border/backgroundis 1.38:1 — not text, so AA doesn't apply, but WCAG 1.4.11 suggests 3:1 for meaningful UI boundaries. Left as-is; your call whether it should move. My judgement is it should to mantain visual legibility and UI rythm.framework-editoroverride — now redundant, left in place rather than removed in the same PR.toast.tsx:106usestext-[#878787]where--muted-foregroundexists;toaster.tsx:30,32hardcode#0064D9and#FF3638;time-range-input.tsxrepeats#878787. These can't respond to any theme — same root cause seen from the component side. Not touched here.Note:
CONTRIBUTING.mdsays PRs should targetdev, but that branch no longer exists, so this targetsmain. Happy to open a separate PR fixing that line if useful.Summary by cubic
Restores the dark-mode palette in
packages/ui/src/globals.cssso thedarkclass now actually changes semantic token values instead of mirroring light values.--primary-foregroundis now0 0% 7%instead of0 0% 98%, raising contrast on the brand teal from 2.57:1 to 7.29:1 (passes WCAG AA).--chart-*tokens at light values,--bordercontrast at 1.38:1, the now-redundant override inapps/framework-editor/styles/globals.css, and hardcoded hexes intoast.tsx,toaster.tsx, andtime-range-input.tsxunchanged.Written for commit fc4ad75. Summary will update on new commits.