Extend the theme contract with kit tokens and add sample UI to the theme dashboards - #5989
Conversation
Preview deploymentsHost Test Results 1 files ±0 1 suites ±0 2h 22m 7s ⏱️ - 8m 17s Results for commit 17a5649. ± Comparison against earlier commit 09d80f0. Realm Server Test Results 1 files ± 0 200 suites +1 1h 8m 6s ⏱️ - 4m 56s Results for commit 17a5649. ± Comparison against earlier commit 09d80f0. |
a4b7c67 to
d99de33
Compare
d99de33 to
c63db45
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e720a3e92b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
habdelra
left a comment
There was a problem hiding this comment.
[Claude Code 🤖] This review went after how the new tokens land on code that already exists: name collisions with --boxel-* variables boxel-ui components already read, whether the theme.css defaults hold under the light/dark and themed-card-scope rules, and whether the inherited-swatch resolution does what it is described as doing. It does not judge the naming or visual design of the token set itself, and the acceptance module was not run locally.
The token set and its theme.css defaults hold up; two things block. The new --boxel-label-* slot takes over a variable family Label and FieldContainer already read, silently resizing every card's field labels, and the inherited-swatch resolution does not follow the dark toggle in Style Reference.
On the open thread about dark defaults at themed-card boundaries: the reply there is correct and I verified the mechanism independently. .dark, [data-theme='dark'] matches only an element that itself carries the class or attribute, while :where([data-boxel-theme-scope]) matches the card scope element, so the child declaration wins for every contract token — --background, --card, --border included — not only the ones added here. These tokens follow the existing rule rather than introducing the behavior.
Recommendations, in order:
- Rename the new UI-label typography slot, or accept the label resize deliberately and review the Percy diffs for it — see the comment on the
ui labelblock incard-container/index.gts. - Make the inherited-swatch resolution re-run on a color-mode flip, and cover it with a test — see the comment on the
resolvemodifier instructured-theme-variables.gts. The description's light/dark claim only holds for the theme card today. - Decide whether
Switchmoves off--input, or soften the--inputdescription — comment on that field. - Fix the Style Reference header measure subtraction, or reword the comment above it.
- Drop the unrelated
package.jsonreorder.
Adjacent, out of scope: InheritedSwatch appends a probe span, reads getComputedStyle, and removes it once per unset token, so a blank theme's grid pays a style recalc per swatch on every render and every mode flip. Only worth hoisting to one shared probe in FieldGrid if the dashboard starts feeling slow — not asked of this PR.
Generated by Claude Code
b559415 to
47af0bd
Compare
47af0bd to
09d80f0
Compare
… contract Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…e theme dashboards
…ds from specimens
09d80f0 to
17a5649
Compare
Extends the theme contract with the generic tokens a component kit needs beyond shadcn's set, so a kit's cards and the boxel-ui components nested in them read from one contract, and gives the theme dashboards realistic specimens to judge a theme on. Builds on #5982 (merged).
Linear: CS-12723 (parent CS-12711).
Naming decisions to review
Names are Boxel's, not copied from the kit. The rules applied:
--success/--warningprecedent for anything theme.css declares.--boxel-sp-*,--boxel-border-radius-*,--boxel-font-size-*), gets no new field.--x/--x-foregroundcover ink on a hue's fill; nothing named the hue used as ink on a neutral surface. Added--x-inkfor all eight hues (primary, secondary, accent, destructive, success, warning, info, attention). The default iscolor-mix(in oklch, var(--x) 60%, var(--foreground)), so a theme that only sets the hue still gets an ink that darkens on light and lightens on dark. The formula is re-declared in.darkbecause avar()inside a custom property resolves where it is declared, so a dark subtree would otherwise inherit the light-resolved color.New tokens (30
ThemeVarFieldfields)--canvas,--inset,--field(input background;--inputis the border),--hover(translucent),--stripe,--selected,--tooltip+--tooltip-foreground--subtle-foreground(third ink step),--border-strong--info+-foreground,--attention+-foreground,--success-foreground,--warning-foreground,--overlay--primary-ink,--secondary-ink,--accent-ink,--destructive-ink,--success-ink,--warning-ink,--info-ink,--attention-ink--chart-6,--chart-7--control-height(wired into--boxel-form-control-heightby CardContainer),--shadow-insettheme.css declares light and dark defaults for every one, in the same block as the existing contract, so they reset at every themed-card boundary too.
Typography maps onto roles, not new tokens
The kit's flat type ladder maps onto the existing
ThemeTypographyFieldslots. The residue became:label(UI/control text) andeyebrow(the tracked-out kicker above a title), emitted as--theme-label-*/--theme-eyebrow-*and derived by CardContainer as--boxel-ui-label-*/--boxel-eyebrow-*with the same fallback shape as the existing slots. The--boxel-label-*family is left alone: it is the Label component's own contract and keeps falling back to the body slot;letterSpacingonTypographyField, so every slot emits--theme-<slot>-letter-spacingand CardContainer derives--boxel-<slot>-letter-spacing. Body, caption, and label follow the theme's base tracking. Nothing applies these yet; that is the components' call.Space, radius, shadow composites, and control chrome all map onto tokens the contract already has, so no fields were added for them.
Dark defaults at themed-card boundaries
theme.css re-declares the whole light contract on every
[data-boxel-theme-scope]so token values can't leak into a themed card. That reset also applied inside dark subtrees, so a themed card in dark chrome rendered every token its theme omitted with the light default. A new@container style(--boxel-color-scheme: dark)block applies the dark contract at those boundaries, keyed off the inherited scheme signal, the same mechanism a card's own.darkvariables use. The--boxel-switch/buttonchrome knobs are deliberately left out of that block so themed cards keep deriving them from their own tokens in both schemes.Theme dashboards
/* not set */: a swatch painted with the resolved default, the value text, and an "inherited" tag whose tooltip shows the declared expression. The value is read at the swatch's position inside the theme scope, so it follows the light/dark toggle.--dsr-content-max-width, 72rem). Style Reference dropped its whole-card 50rem cap to match.:rootexcerpt of the tokens that define the look, following the previewed scheme, with a copy button.CSSFieldaccepts--css-field-max-height; the dashboards cap it at 40vh with an edge fade driven by a scroll timeline, shown only at an edge with more content beyond it.Select
BoxelSelectderives its dropdown colors from contract tokens instead of the fixed palette: selected text from--primary-foreground, hover from--muted, highlight-hover mixed from the highlight color. A--boxel-dropdown-hover-text-coloroverride is added and synced into the portaled dropdown.Tests
structured-theme-test: the extended tokens round-trip throughsetCssandcssVariables; label/eyebrow slots emit theme variables including letter-spacing; unset variables render as inherited with a resolved literal color and follow the dark toggle.theme-card-test: an omitted token resolves to the dark default in the dark preview.theme-dark-boundary-test(boxel-ui): the two dark blocks declare the same tokens with the same values; dark defaults hold inside a dark subtree; a light-forced subtree stays light.Follow-ups
BoxelInputshould read--fieldand--subtle-foreground. The Surfaces specimen remaps those onto the input's wrapper until then.🤖 Generated with Claude Code