refactor(website): declare every theme colour once via light-dark() - #1217
Conversation
components/ui/ is meant to stay empty here, left free for `webjs ui add` to own exactly as it is in a scaffolded app, with the gallery's preview copies living in the generated modules/ui/components/ mirror instead. Eleven byte-identical copies of that mirror were committed into it by accident and sat unimported ever since. Gitignore the directory rather than only emptying it, since .gitignore already covers the other two generated paths and missing this one is what let the copies land in the first place.
The site was the counter-example to the rule the framework teaches its own users. references/styling.md in the skill, and the palette webjs create generates, both say to write each colour token once as light-dark(LIGHT, DARK) and let color-scheme pick the side. Here the dark half was written twice instead, verbatim, once under the OS media query and once under the toggle's attribute selector, so an edit to either copy drifted the two paths apart with nothing to catch it. The kit palette scoped to .ui-preview said the same nineteen colours three times over. The prerequisite was already in place: :root declares color-scheme light dark, and the toggle's two attribute rules force one. So the three-way selector logic was redundant with a cascade that already existed, and the dark blocks collapse into the light-dark() calls with no behaviour change. Three tokens genuinely cannot ride light-dark(), which is colour-only, so they keep an explicit override pair. --glow-strength is a bare number. --cta-mix is the CTA fill's mix ratio, split out of --cta-surface because the two themes differ by the ratio rather than by a colour. --shadow-spread is the geometry half of --shadow, whose cast widens in dark mode; its two colours became their own light-dark() tokens, and --shadow-sm keeps identical geometry so its colour rides light-dark() inline. Everything derived through color-mix() now tracks both themes with no override at all. Verified colour-identical against main in a real browser: 282 resolved readings, every token in all six theme states (OS light and dark, each with the toggle unset, forced light, and forced dark), zero differences. Shadows are read back through box-shadow rather than as custom properties, since a custom property's computed value keeps light-dark() unsubstituted and comparing that compares source text, not paint.
The layout was documented as rendering an announcement strip above a sticky header. Neither is true. The strip is gone, and #610 made the header position: fixed on purpose, because a sticky one flickers on iOS WebKit during a client-router navigation. Describe what is actually there, including the --header-h offset that re-adding a strip would have to account for. Also type the search debounce handle, which was any. undefined rather than null for the unset state, since clearTimeout takes undefined as a no-op but rejects null.
The layout and .ui-preview were not the only places writing the dark half twice. The syntax-highlight classes in public/input.css repeated five colours across both theme selectors, and the home page's code-sample tokens repeated three. Same collapse onto light-dark(). Replace the per-file assertion with a sweep over every stylesheet and every page, layout, and component that writes CSS, asserting the rule rather than the two known sites: no colour may be declared under a per-theme selector, only the three non-colour tokens may. A per-file check is what let these two survive the first pass. Docs code samples are excluded, since a pre block teaching the reader is not this site's own styling. Re-verified against main in a real browser after the change: 360 resolved readings across the six theme states, now covering the highlight classes and the code tokens as well, zero differences.
|
Design rationale: why the shadow tokens split, and why the check became a sweep Two calls worth writing down. The shadow split.
Why the check is a sweep. My first version asserted the two palettes I had found by hand. That is exactly the check that would not have found the other two: the syntax-highlight classes in Two things the sweep taught me while writing it, both of which cost a counterfactual to find. Comments have to be stripped before matching, or prose that NAMES a selector to explain why the block does not use it reads as a live rule. And the block terminator cannot anchor to a newline: |
Two related passes over the site's own code. Types. Five modules each carried their own (globalThis as any).process cast to read SITE_URL, which is one missing type worked around five times, so lib/env.ts declares the shape once and returns a typed partial record. copy-cmd cast setTimeout's handle through unknown to a number and reached window.gtag through an unknown-shaped object literal; both now have real types, gtag via a declare global that matches the tag the root layout installs. The example registry in modules/ui typed its class helpers (...args: any[]), which waved through a direct call with invented arguments; never[] states what is actually true, since every call goes through the Function evaluator rather than a typed call site. lib/links.ts turned out to hold a dead env read: the docs and the gallery used to be sibling apps needing a configurable URL each, and both moved in-app, so every link there is now a literal. DOM access. copy-cmd found its command line with querySelector on every copy and preview-tabs built a selector from the mode string to focus a tab. Both elements come out of render(), so both now bind through the ref directive, which is the lit-style API the framework provides and cannot silently return null if the markup moves. preview-tabs defers the focus behind updateComplete so it lands after the roving tabindex is committed. The browser suite already covered the clipboard write and the tabindex roving, so both refs are load-bearing there (dropping either reds it). The FOCUS move was not covered, which is the half that changed shape, so add an activeElement assertion for it. Note the deferral itself is ordering only: focusing a tabindex="-1" element programmatically is legal, so a synchronous focus still passes and the test cannot tell the two apart. State needed no change: all eight pieces of component state were already signals, with no state:true prop anywhere in the site's own components.
The "Light DOM web components" card showed a static <like-button> tag
wrapped around a Tailwind-classed button. That demonstrated neither half
of the claim: not that light DOM is the DEFAULT, nor how to leave it. It
was also the only card in that grid using a markup snippet where its
neighbours (Server actions, Built-in essentials, Streaming, Progressive
enhancement) all use a schematic.
Replace it with two rows in the same idiom the Built-in essentials card
uses: the accent border marks light DOM as the default, and the muted row
carries the real one-line opt-in, static shadow = true. Checked in both
themes and down to 480px, where the opt-in still fits on one line.
The <like-button> component keeps its live instance further down the page
in "What the browser receives", which is where an interactive demo
belongs; only this card's static copy of the markup goes.
Also mark the prop({ state: true }) entry in the components docs as lit
parity with a signal preferred, so the options reference does not read as
an equal recommendation next to the page's own "signals are the default
state primitive".
Review of my own commit. Two problems, both in the new file. env() was exported with no consumer outside the module: only siteUrl() called it, and the NODE_ENV member of its type was never read. Removing dead code from lib/links.ts and adding some in the same commit is not a good trade, so inline the read and let siteUrl() be the whole surface. The docstring justified the globalThis guard by naming lib/links.ts as a browser-loading consumer, and that same commit deleted the links.ts consumer. All four remaining callers are server-only. The guard is still worth keeping, but for the reason that is actually true: it keeps the module importable from either side, and the origin is public information already in the rendered HTML, so nothing here wants the server boundary. Also record why modules/ is outside the theme-selector sweep, since its only subtree is the gitignored kit mirror rather than this site's code.
vivek7405
left a comment
vivek7405
left a comment
There was a problem hiding this comment.
Went back over this one properly. The palette work is the strong part: collapsing four duplicated dark blocks onto light-dark() is a clear net negative in lines, the three non-colour tokens that genuinely cannot ride it are carved out for the right reason rather than left behind by accident, and the 360-reading browser comparison against main is the kind of evidence I want on a refactor that touches every colour on the site. The sweep test is the piece I would keep longest: asserting the rule instead of the two known sites is what caught the highlight classes and the code tokens, which a per-file check had already walked past once.
Two things I got wrong, both in lib/env.ts, both mine from this branch.
I also checked two things I had half-suspected and cleared both. The ref() binding leaves no artifact in the SSR output (<copy-cmd data-wj-host><!--webjs-hydrate-->), so the byte-stability the conditional-GET path depends on is intact. And updateComplete.then() with no .catch() is safe here: component.js is explicit that the promise always resolves and never rejects, so there is no unhandled rejection to guard against.
Fixed in 6664b07.
1 participant
Closes #1216
Brings the marketing site in line with the styling and layout rules WebJs ships to its own users. It was the counter-example to its own guidance on three counts.
Each theme colour is declared once, via
light-dark()references/styling.mdin the skill, and the palettewebjs creategenerates, both teach one rule: write each colour token ONCE aslight-dark(LIGHT, DARK)and letcolor-schemepick the side. The site did the opposite everywhere it had a palette.Four places wrote it twice instead, verbatim, once under
@media (prefers-color-scheme: dark)and again under:root[data-theme='dark']:app/layout.ts, the site palette (16 colours).public/input.css, the.ui-previewkit palette (19 colours, said three times over).public/input.css, the syntax-highlight classes (5 colours).app/page.ts, the home page's code-sample tokens (3 colours).Every pair was already byte-equal, so any future edit had to land in both copies or the OS-dark and toggle-dark paths would drift apart silently.
The prerequisite was already there:
:rootdeclarescolor-scheme: light darkand the toggle's two attribute rules force one. So the three-way selector logic was redundant with a cascade that already existed.Three tokens genuinely cannot ride
light-dark(), which is colour-only, so they keep the explicit override pair the styling reference prescribes:--glow-strengthis a bare number.--cta-mixis the CTA fill's mix ratio, split out of--cta-surfacebecause the two themes differ by the ratio, not by a colour.--shadow-spreadis the geometry half of--shadow, whose cast widens in dark mode. Its two colours became their ownlight-dark()tokens, and--shadow-smkeeps identical geometry in both themes so its colour rideslight-dark()inline.Everything derived through
color-mix()(--accent-tint,--accent-surface,--accent-border,--shadow-glow) now tracks both themes with no override at all.components/ui/is empty againwebsite/AGENTS.mdsays the directory is intentionally empty, left free forwebjs ui addto own exactly as in a scaffolded app, with the gallery's preview copies living in the generatedmodules/ui/components/mirror. It was not empty: eleven files (766 lines) landed in it in #1213, every one byte-identical to its counterpart in that mirror and imported by nothing..gitignorecovered the other two generated paths but not this one, which is how they got in. Now ignored as well as removed.Doc drift
website/AGENTS.mddocumented an announcement banner above a "sticky" header. The banner is gone and #610 made the headerposition: fixeddeliberately, because a sticky one flickers on iOS WebKit during a client-router nav. Also typed the search debounce handle, which wasany.Test plan
mainbuild on a second port. Zero differences. Shadows are read back throughbox-shadowrather than as custom properties, since a custom property's computed value keepslight-dark()unsubstituted, so reading the property compares source text rather than what the browser paints. That distinction was load-bearing: the first probe pass compared raw strings and reported eighteen false differences.test/ssr/design-tokens.test.tsrewritten to parse thelight-dark()pairs (its old parser read the now-deleted dark block) and extended to pin the DRY rule itself: only the three non-colour tokens may carry a per-theme override, and all threecolor-schemedeclarations must be present. Newtest/ssr/kit-surfaces.test.tspins the emptycomponents/ui/, the.ui-previewpalette's shape, and sweeps every stylesheet, page, layout, and component asserting the rule itself: no colour under a per-theme selector, only the three non-colour tokens. That sweep replaced a per-file assertion, which is what let the highlight classes and code tokens survive the first pass.7642b099: reintroducing a colour override under a theme selector, reverting one token to a bare declaration, dropping the forced-darkcolor-scheme, committing a file intocomponents/ui/, re-adding a duplicated.ui-previewdark block, and the three shapes the sweep catches (a bare colour on a class under a dark selector, a custom-property colour under a media query, a colour token in a page's own style block). Each reds the suite; restoring greens it.webjs checkandwebjs typecheckclean./,/docs/getting-started,/ui,/ui/button,/brandin prod mode.Doc surfaces
website/AGENTS.md: updated (the styling rule, the header section, thecomponents/ui/note)..agents/skills/webjs/andpackages/cli/lib/create.js: N/A, both already teach thelight-dark()rule. This PR makes the site follow it, not the other way round.packages/*/src: N/A, no framework surface changed.Deliberately not done here
The
.ui-previewcomment claimed to mirrorpackages/ui/packages/registry/themes/index.cssbyte for byte. It never did (the registry keys its dark half off a.darkclass, this site off[data-theme]), so the comment is corrected to say which parts are shared and which differ. Converting the registry theme itself tolight-dark()is a scaffold surface change, needs thewebjs-scaffold-syncskill and a generate-boot-check pass, and belongs in its own issue.