Skip to content

Add missing fields to ThemeVarField; add missing local-types package to base - #5982

Merged
burieberry merged 10 commits into
mainfrom
cs-12721-add-theme-var-fields
Sep 4, 2026
Merged

Add missing fields to ThemeVarField; add missing local-types package to base#5982
burieberry merged 10 commits into
mainfrom
cs-12721-add-theme-var-fields

Conversation

@burieberry

@burieberry burieberry commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Closes CS-12721

theme.css declares 54 non---boxel-* properties; ThemeVarField covered 47. This adds the 7 missing fields and settles the status-token fallback.

Changes

  • ThemeVarField: new warning color 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; warning joins the Form & Feedback swatch group and shadowColor the Box Shadows group.
  • theme.css themed-card boundary reset: --success and --warning fall back to the fixed Boxel status palette (--boxel-success, --boxel-warning) instead of initial, 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.
  • Number components (progress bar, progress circle, gauge, score, stat) use plain var(--success) / var(--warning); the hex fallbacks were dead once the tokens are always defined.
  • packages/base loads @cardstack/local-types like 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

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Preview deployments

Host Test Results

    1 files  ±    0      1 suites  ±0   2h 30m 49s ⏱️ + 2h 23m 5s
4 616 tests +4 318  4 602 ✅ +4 306  14 💤 +12  0 ❌ ±0 
4 631 runs  +4 333  4 617 ✅ +4 321  14 💤 +12  0 ❌ ±0 

Results for commit 865e02b. ± Comparison against earlier commit 1ed5000.

Realm Server Test Results

    1 files  ±0    190 suites  ±0   1h 7m 7s ⏱️ - 3m 52s
2 525 tests +7  2 525 ✅ +7  0 💤 ±0  0 ❌ ±0 
2 564 runs  +7  2 564 ✅ +7  0 💤 ±0  0 ❌ ±0 

Results for commit 865e02b. ± Comparison against earlier commit 1ed5000.

@burieberry burieberry changed the title Add warning and shadow primitive fields to ThemeVarField Add missing fields to ThemeVarField Sep 2, 2026
@burieberry burieberry changed the title Add missing fields to ThemeVarField Add missing fields to ThemeVarField; add missing local-types package to base Sep 2, 2026
@burieberry
burieberry force-pushed the cs-12721-add-theme-var-fields branch from 321cba5 to b934c97 Compare September 2, 2026 20:52
@burieberry
burieberry marked this pull request as ready for review September 2, 2026 21:00
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-02T21:03:59.559987Z b934c97 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@burieberry
burieberry requested a review from a team September 2, 2026 21:41

@habdelra habdelra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] This review chased the cascade consequences of swapping the initial sentinel for a real fallback, the tweakcn round-trip path (setCssapplyCssRulesToField → 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:

  1. Decide whether BoxelSwitch losing the theme's primary is intended — see the theme.css thread. That's the one substantive behavior question; the two lines it's anchored on are also a no-op as written.
  2. Correct the success / warning field descriptions — they promise fallbacks to primary/destructive that nothing implements, and they reach the assistant's card schema. See the thread on structured-theme-variables.gts.
  3. Pin the round-trip with assertions in the existing setCss test — the fixture already carries all six primitives.
  4. Distinguish the inert shadow primitives from the working scale in the edit UI; FieldContainer never surfaces a field description.
  5. --accent as the progress 50–75% band renders #e8e8e8 under the default theme; the color-name comments no longer describe the tokens.
  6. Finish or drop the partial hex-fallback sweep — it currently leaves two spellings inside one getter.
  7. 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

Comment thread packages/boxel-ui/src/styles/theme.css Outdated
Comment thread packages/base/structured-theme-variables.gts Outdated
Comment thread packages/base/structured-theme-variables.gts
Comment thread packages/base/structured-theme-variables.gts Outdated
Comment thread packages/base/number/components/progress-bar.gts Outdated
Comment thread packages/base/number/components/stat.gts
Comment thread packages/base/package.json Outdated
@burieberry
burieberry force-pushed the cs-12721-add-theme-var-fields branch from 1ed5000 to 865e02b Compare September 4, 2026 14:57
@burieberry
burieberry merged commit 3832190 into main Sep 4, 2026
82 of 83 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants