Add missing fields to ThemeVarField; add missing local-types package to base - #5982
Conversation
Preview deploymentsHost Test Results 1 files ± 0 1 suites ±0 2h 30m 49s ⏱️ + 2h 23m 5s Results for commit 865e02b. ± Comparison against earlier commit 1ed5000. Realm Server Test Results 1 files ±0 190 suites ±0 1h 7m 7s ⏱️ - 3m 52s Results for commit 865e02b. ± Comparison against earlier commit 1ed5000. |
321cba5 to
b934c97
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. |
habdelra
left a comment
There was a problem hiding this comment.
[Claude Code 🤖] This review chased the cascade consequences of swapping the initial sentinel for a real fallback, the tweakcn round-trip path (setCss → applyCssRulesToField → the cssVariableName lookup) that the new fields feed, and the accuracy of the new field descriptions and comments. It did not evaluate the visual design of the edit-template layout, and it did not exercise the app.
The field additions and the --boxel-*/tsconfig plumbing are sound — the token count lines up at 54 and the round-trip mechanism does what the comment says — but the theme.css boundary change reaches a consumer the description doesn't mention, and two of the new field descriptions state fallbacks that no code implements.
Recommendations, in order:
- Decide whether
BoxelSwitchlosing the theme's primary is intended — see thetheme.cssthread. That's the one substantive behavior question; the two lines it's anchored on are also a no-op as written. - Correct the
success/warningfield descriptions — they promise fallbacks to primary/destructive that nothing implements, and they reach the assistant's card schema. See the thread onstructured-theme-variables.gts. - Pin the round-trip with assertions in the existing
setCsstest — the fixture already carries all six primitives. - Distinguish the inert shadow primitives from the working scale in the edit UI;
FieldContainernever surfaces a field description. --accentas the progress 50–75% band renders#e8e8e8under the default theme; the color-name comments no longer describe the tokens.- Finish or drop the partial hex-fallback sweep — it currently leaves two spellings inside one getter.
- Dep ordering nit in
packages/base/package.json.
CI is green across all 83 checks; nothing to act on there.
Adjacent, out of scope: boxel-cli/plugin/skills/boxel/references/theme-design-system.md enumerates the canonical token contract and lists neither --success nor --warning, so an assistant authoring card CSS has no reason to know they exist. Pre-existing for --success; this PR widens it by making five base components read bare var(--warning). Worth a line in that reference next time it's touched.
Generated by Claude Code
b934c97 to
1b237bb
Compare
689ccf3 to
1ed5000
Compare
Resolve ts-plugin lint warnings
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… band Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1ed5000 to
865e02b
Compare
Closes CS-12721
theme.cssdeclares 54 non---boxel-*properties;ThemeVarFieldcovered 47. This adds the 7 missing fields and settles the status-token fallback.Changes
ThemeVarField: newwarningcolor field and the six tweakcn shadow primitives (shadowX,shadowY,shadowBlur,shadowSpread,shadowOpacity,shadowColor). The primitives are stored so tweakcn themes round-trip losslessly; the composed shadow scale does not derive from them. Edit template gets rows for each;warningjoins the Form & Feedback swatch group andshadowColorthe Box Shadows group.theme.cssthemed-card boundary reset:--successand--warningfall back to the fixed Boxel status palette (--boxel-success,--boxel-warning) instead ofinitial, so a themed card that omits them still renders green/amber rather than leaking the ambient theme's value. A theme that sets them still wins.var(--success)/var(--warning); the hex fallbacks were dead once the tokens are always defined.packages/baseloads@cardstack/local-typeslike every other package, so the<style scoped>augmentation applies under base's own tsconfig.Purely additive for existing themes: unset fields emit nothing.
🤖 Generated with Claude Code