feat: @property typed tokens, surface consolidation, macro/CQ tests, drift-check - #386
Merged
Merged
Conversation
…drift-check Issue #359 — @Property registrations for output tokens Register radius (13 tokens, <length>), space (22 tokens, <length>), and shadow (16 tokens, *) tokens via @Property in core/tokens.css. Radius and space gain CSS transitions (border-radius, gap, padding can now interpolate on hover/theme switch). Shadow tokens gain cascade isolation and typed DevTools inspection. initial-value uses bare 0 (unitless zero valid for <length>). Surface consolidation (roadmap) Convert .sf-surface--* named variants from standalone precomputed rules into thin aliases that set --sf-surface-color. The generic .sf-surface @supports block (extended with :is() to cover all 12 variants) handles auto-contrast derivation uniformly. Removes ~220 lines of duplicated contextual cascade. Element-scoped brand overrides now also re-derive contrast (improved behavior). Old engines (Chrome <119 / Safari <16.4) get background only; accepted trade-off. Issue #246 — macro catalog drift-check + comprehensive behavioral tests Add scripts/check-macro-catalog.js: parses core/macros.css + core/motion.css, compares against docs/macros.md, exits 1 on drift. Wired as npm run check:macros. Add 11 missing behavioral tests to tests/macros.spec.js covering every previously untested macro: .sf-prose, .sf-not-prose, .sf-scrim, .sf-text-protect, .sf-text-gradient, .sf-link-external, .sf-link--subtle (hover), .sf-link--reverse (hover), .sf-content-auto, .sf-tabular-nums, .sf-entrance--fade (including prefers-reduced-motion gating). Issue #205 — additional auto-color/link test cases Add 4 tests to tests/auto-color.spec.js: .sf-link--subtle hover reveals underline, .sf-link--reverse underline at rest, dark theme link underline token non-transparent, --sf-color-link--visited differs from --sf-color-link. Update element-scoped override test to reflect improved behavior. Container query coverage Add .sf-cq unnamed container test to tests/container-queries.spec.js, proving children resolve @container queries against the .sf-cq width (not the viewport). Token snapshot update Remove viewport-dependent clamp() space tokens from raw snapshot list; regenerate snapshot with updated --sf-radius-m value (8px via @Property). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds CSS ChangesToken
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Lines 287 to 290 in 7b8e1f1
Use on-color tokens for named surface foregrounds
In current browsers with sign() support, named variants now derive --sf-surface-contrast directly from --sf-surface-color, so an override like .sf-surface--primary { --sf-color-text--on-primary: oklch(0.12 0 0); } no longer changes color, --sf-color-text, borders, links, focus rings, etc. This breaks the documented per-surface override contract in docs/theming.md:200-226; only generic .sf-surface should need direct public-token pinning, while named variants should continue to seed their foreground from the matching --sf-color-text--on-* token.
ℹ️ 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".
Comment on lines
+258
to
+262
| .sf-surface--primary { --sf-surface-color: var(--sf-color-primary); background: var(--sf-color-primary); } | ||
| .sf-surface--secondary { --sf-surface-color: var(--sf-color-secondary); background: var(--sf-color-secondary); } | ||
| .sf-surface--tertiary { --sf-surface-color: var(--sf-color-tertiary); background: var(--sf-color-tertiary); } | ||
| .sf-surface--action { --sf-surface-color: var(--sf-color-action); background: var(--sf-color-action); } | ||
| .sf-surface--neutral { --sf-surface-color: var(--sf-color-neutral); background: var(--sf-color-neutral); } |
There was a problem hiding this comment.
For the supported floor browsers that have oklch(from …) but not sign() (the core/tokens.css comments call out Chrome 125–137 and Firefox 128–130), the contextual block below is skipped, so these named surface rules are the only ones that apply. Dropping color: var(--sf-color-text--on-*) means .sf-surface--primary/status surfaces render with inherited page text on colored backgrounds, regressing the previous accessible foreground fallback for those browsers.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/check-macro-catalog.js`:
- Around line 80-87: In the wildcard matching block where glob is true, after
the inner for loop that iterates through cssClasses and adds matches to
docsClasses, add validation to check if the wildcard pattern matched any CSS
classes. If no matches were found for the wildcard prefix (meaning no classes
were added for that specific pattern), report an error or warning to flag the
unmatched wildcard instead of silently ignoring it.
In `@tests/auto-color.spec.js`:
- Around line 293-308: The test in the dark theme link underline token check
only validates that textDecorationColor is not transparent, but does not verify
that an underline is actually rendered on the link element. Before asserting the
color value in the page.evaluate function, add the appropriate CSS class (such
as `.sf-link--reverse`) to the anchor element created in the test, and also
verify that the underline is actually present (by checking the textDecoration
property or similar) before asserting that the decoration color is
non-transparent. This ensures the test fails if no underline is rendered
regardless of the computed color value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5cba6e71-4d59-45f2-bab9-d970306461be
⛔ Files ignored due to path filters (2)
dist/badge-essential.jsonis excluded by!**/dist/**dist/badge-optimal.jsonis excluded by!**/dist/**
📒 Files selected for processing (11)
configurator/src/data/api-index.generated.jsoncore/macros.csscore/tokens.cssdocs/api-index.jsonpackage.jsonscripts/check-macro-catalog.jstests/auto-color.spec.jstests/container-queries.spec.jstests/macros.spec.jstests/token-semantic.snapshot.jsontests/token-semantic.spec.js
…hecks Address three issues flagged in the PR #386 automated review: - core/macros.css: add `color: var(--sf-color-text--on-*)` back to each named surface variant. Chrome 125–137 / Firefox 128–130 support oklch(from) but not sign(), so the @supports cascade block is skipped; without an explicit color declaration those browsers inherit page-body text onto coloured backgrounds. Restores accessible foreground for the intermediate browser tier while the auto-contrast cascade still takes over in fully modern engines. - scripts/check-macro-catalog.js: track wildcard patterns that expand to zero CSS classes and report them as errors. Previously a stale wildcard (e.g. .sf-entrance--* after a rename) would pass the drift-check silently. - tests/auto-color.spec.js: add .sf-link--reverse to the dark-theme underline test anchor and assert textDecorationLine === 'underline' before checking the decoration colour. Prevents the assertion from passing vacuously when no underline is rendered. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TJ5PentbTtNx4serLVMZcb
This was referenced Jun 28, 2026
This was referenced Jul 4, 2026
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
2 participants
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.
Issue #359 — @Property registrations for output tokens
Register radius (13 tokens, ), space (22 tokens, ), and
shadow (16 tokens, *) tokens via @Property in core/tokens.css. Radius and
space gain CSS transitions (border-radius, gap, padding can now interpolate
on hover/theme switch). Shadow tokens gain cascade isolation and typed
DevTools inspection. initial-value uses bare 0 (unitless zero valid for ).
Surface consolidation (roadmap)
Convert .sf-surface--* named variants from standalone precomputed rules into
thin aliases that set --sf-surface-color. The generic .sf-surface @supports
block (extended with :is() to cover all 12 variants) handles auto-contrast
derivation uniformly. Removes ~220 lines of duplicated contextual cascade.
Element-scoped brand overrides now also re-derive contrast (improved behavior).
Old engines (Chrome <119 / Safari <16.4) get background only; accepted trade-off.
Issue #246 — macro catalog drift-check + comprehensive behavioral tests
Add scripts/check-macro-catalog.js: parses core/macros.css + core/motion.css,
compares against docs/macros.md, exits 1 on drift. Wired as npm run check:macros.
Add 11 missing behavioral tests to tests/macros.spec.js covering every
previously untested macro: .sf-prose, .sf-not-prose, .sf-scrim, .sf-text-protect,
.sf-text-gradient, .sf-link-external, .sf-link--subtle (hover), .sf-link--reverse
(hover), .sf-content-auto, .sf-tabular-nums, .sf-entrance--fade (including
prefers-reduced-motion gating).
Issue #205 — additional auto-color/link test cases
Add 4 tests to tests/auto-color.spec.js: .sf-link--subtle hover reveals
underline, .sf-link--reverse underline at rest, dark theme link underline
token non-transparent, --sf-color-link--visited differs from --sf-color-link.
Update element-scoped override test to reflect improved behavior.
Container query coverage
Add .sf-cq unnamed container test to tests/container-queries.spec.js,
proving children resolve @container queries against the .sf-cq width
(not the viewport).
Token snapshot update
Remove viewport-dependent clamp() space tokens from raw snapshot list;
regenerate snapshot with updated --sf-radius-m value (8px via @Property).
Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
Summary by CodeRabbit
Bug Fixes
New Features
@propertyfor better typing/animation support (radii, spacing/gap, and shadow/shadow-glow/text-shadow).Tests
Chores