feat: implement .sf-button, .sf-card, scroll anchoring, shade ramp, and font specimen - #375
feat: implement .sf-button, .sf-card, scroll anchoring, shade ramp, and font specimen#375jackgranatowski wants to merge 2 commits into
Conversation
|
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 (17)
✅ Files skipped from review due to trivial changes (7)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughThis PR activates full CSS implementations for ChangesButton & Card Component Activation
Scroll Offset Layout Utilities
Configurator UI: ShadeRamp and Specimen Preview
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@configurator/src/components/ShadeRamp.svelte`:
- Around line 79-82: The style:background attribute on the sr__swatch element is
using the background shorthand, which resets the background-image property and
removes the checkerboard placeholder texture on empty swatches. Change
style:background to style:background-color in the sr__swatch element to set only
the background color without clearing the background-image property that
provides the placeholder texture.
In `@configurator/src/data/api-index.generated.json`:
- Around line 1230-1313: The api-index.generated.json file contains duplicate
generic descriptions across multiple button and card tokens (such as
--sf-button-border-width, --sf-button-gap, --sf-button-font-weight, and
--sf-button-min-height), which causes downstream documentation to mislabel these
tokens. Locate the source generator that creates this api-index.generated.json
file and update it to assign token-specific descriptions that accurately reflect
each token's purpose rather than reusing the same generic description about
padding and radius. After updating the source generator with unique descriptions
for each token, regenerate the api-index.generated.json file to reflect these
changes.
In `@docs/api-index.md`:
- Line 13: The header count in classes.md at line 6 currently reports "208
.sf-classes" but needs to be updated to "210 .sf-classes" to reflect the two new
utility classes (.sf-scroll-target and .sf-section--offset-header) that have
been added to the documentation. This will correct the discrepancy between the
total element count in api-index.md (which now shows 257 classes) and ensure the
documentation headers are consistent and accurate.
In `@docs/classes.md`:
- Line 6: The class count in the header on line 6 of classes.md is out of sync
with the actual number of .sf-classes documented. Currently it states "208
.sf-classes" but should be updated to "210 .sf-classes" to reflect the two new
layout utilities (.sf-scroll-target and .sf-section--offset-header) that were
added to the list. Update the number "208" in the header "**208 .sf-classes, 40
.is-classes.**" to "210" to match the actual count of layout utilities in the
document.
In `@optional/components.css`:
- Around line 147-152: The `.sf-button[aria-disabled="true"]` CSS selector only
visually disables the button through pointer-events and opacity, but does not
prevent keyboard activation for link elements or other interactive controls. To
properly disable buttons with aria-disabled, ensure the CSS is paired with
behavioral controls: for link-based buttons, remove or nullify the href
attribute, add tabindex="-1" to remove keyboard focus, and implement JavaScript
logic to prevent activation (click, Enter, Space) when the disabled state is
set. Update the component documentation to specify that developers must apply
these additional constraints when using aria-disabled on interactive elements to
prevent inconsistent disabled-looking controls that can still be activated.
🪄 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: 9c4cd647-6397-45f0-8169-9dac582e8ae6
⛔ 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 (17)
configurator/src/components/DomainPanel.svelteconfigurator/src/components/DomainPreview.svelteconfigurator/src/components/ShadeRamp.svelteconfigurator/src/data/api-index.generated.jsonconfigurator/src/data/token-registry.generated.jsonconfigurator/src/lib/domainPreviews.jscore/layout.cssdocs/api-index.jsondocs/api-index.mddocs/classes.mddocs/registry.jsondocs/token-index.jsondocs/token-index.mddocs/tokens.mdoptional/components.cssoptional/tokens.components.csstoken-registry.json
| { | ||
| "name": "--sf-button-border-width", | ||
| "tier": "PUBLIC", | ||
| "role": "consumption", | ||
| "namespace": "button", | ||
| "category": "Component tokens", | ||
| "group": "BUTTON TOKENS", | ||
| "description": "Padding and radius tokens that control button geometry. Override these to give buttons a distinct visual language from form fields.", | ||
| "note": "", | ||
| "value": "var(--sf-border-width-1)", | ||
| "aliasOf": "--sf-border-width-1", | ||
| "registered": false, | ||
| "syntax": null, | ||
| "fallbackOnly": false, | ||
| "optional": true, | ||
| "layer": "slashed.tokens", | ||
| "bundles": [ | ||
| "full", | ||
| "optimal-components" | ||
| ] | ||
| }, | ||
| { | ||
| "name": "--sf-button-font-weight", | ||
| "tier": "PUBLIC", | ||
| "role": "consumption", | ||
| "namespace": "button", | ||
| "category": "Component tokens", | ||
| "group": "BUTTON TOKENS", | ||
| "description": "Padding and radius tokens that control button geometry. Override these to give buttons a distinct visual language from form fields.", | ||
| "note": "", | ||
| "value": "var(--sf-font-weight-interactive)", | ||
| "aliasOf": "--sf-font-weight-interactive", | ||
| "registered": false, | ||
| "syntax": null, | ||
| "fallbackOnly": false, | ||
| "optional": true, | ||
| "layer": "slashed.tokens", | ||
| "bundles": [ | ||
| "full", | ||
| "optimal-components" | ||
| ] | ||
| }, | ||
| { | ||
| "name": "--sf-button-gap", | ||
| "tier": "PUBLIC", | ||
| "role": "consumption", | ||
| "namespace": "button", | ||
| "category": "Component tokens", | ||
| "group": "BUTTON TOKENS", | ||
| "description": "Padding and radius tokens that control button geometry. Override these to give buttons a distinct visual language from form fields.", | ||
| "note": "", | ||
| "value": "var(--sf-space-2xs)", | ||
| "aliasOf": "--sf-space-2xs", | ||
| "registered": false, | ||
| "syntax": null, | ||
| "fallbackOnly": false, | ||
| "optional": true, | ||
| "layer": "slashed.tokens", | ||
| "bundles": [ | ||
| "full", | ||
| "optimal-components" | ||
| ] | ||
| }, | ||
| { | ||
| "name": "--sf-button-min-height", | ||
| "tier": "PUBLIC", | ||
| "role": "consumption", | ||
| "namespace": "button", | ||
| "category": "Component tokens", | ||
| "group": "BUTTON TOKENS", | ||
| "description": "Padding and radius tokens that control button geometry. Override these to give buttons a distinct visual language from form fields.", | ||
| "note": "", | ||
| "value": "var(--sf-touch-target)", | ||
| "aliasOf": "--sf-touch-target", | ||
| "registered": false, | ||
| "syntax": null, | ||
| "fallbackOnly": false, | ||
| "optional": true, | ||
| "layer": "slashed.tokens", | ||
| "bundles": [ | ||
| "full", | ||
| "optimal-components" | ||
| ] | ||
| }, |
There was a problem hiding this comment.
Use token-specific descriptions here.
These new button/card entries all reuse the same generic metadata, so downstream docs will mislabel --sf-button-border-width vs --sf-button-gap vs the card radius/shadow tokens. Please move the descriptions/note text to token-specific values in the source generator before regenerating.
Also applies to: 1377-1544
🤖 Prompt for 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.
In `@configurator/src/data/api-index.generated.json` around lines 1230 - 1313, The
api-index.generated.json file contains duplicate generic descriptions across
multiple button and card tokens (such as --sf-button-border-width,
--sf-button-gap, --sf-button-font-weight, and --sf-button-min-height), which
causes downstream documentation to mislabel these tokens. Locate the source
generator that creates this api-index.generated.json file and update it to
assign token-specific descriptions that accurately reflect each token's purpose
rather than reusing the same generic description about padding and radius. After
updating the source generator with unique descriptions for each token,
regenerate the api-index.generated.json file to reflect these changes.
| > run `npm run docs:classes` to refresh. Do not edit by hand. | ||
|
|
||
| **183 .sf-classes, 40 .is-classes.** Every `.sf-*` layout/macro class and `.is-*` state class. | ||
| **208 .sf-classes, 40 .is-classes.** Every `.sf-*` layout/macro class and `.is-*` state class. |
There was a problem hiding this comment.
Update stale class count header in classes.md.
Line 6 still reports "208 .sf-classes" but should reflect the 2 new layout utilities added (.sf-scroll-target, .sf-section--offset-header), making it 210 .sf-classes. The header is out of sync with the actual class list below and contradicts the 257 total reported in api-index.md.
This appears to be a regeneration artifact—the layout primitives list was updated (line 11: "128 classes") but the total header was not.
🔧 Proposed fix
-**208 .sf-classes, 40 .is-classes.** Every `.sf-*` layout/macro class and `.is-*` state class.
+**210 .sf-classes, 40 .is-classes.** Every `.sf-*` layout/macro class and `.is-*` state class.Also applies to: 6-6
🤖 Prompt for 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.
In `@docs/classes.md` at line 6, The class count in the header on line 6 of
classes.md is out of sync with the actual number of .sf-classes documented.
Currently it states "208 .sf-classes" but should be updated to "210 .sf-classes"
to reflect the two new layout utilities (.sf-scroll-target and
.sf-section--offset-header) that were added to the list. Update the number "208"
in the header "**208 .sf-classes, 40 .is-classes.**" to "210" to match the
actual count of layout utilities in the document.
| .sf-button:disabled, | ||
| .sf-button[aria-disabled="true"] { | ||
| opacity: var(--sf-opacity-disabled); | ||
| cursor: not-allowed; | ||
| pointer-events: none; | ||
| } |
There was a problem hiding this comment.
aria-disabled here is only visual and can still allow keyboard activation on links.
For .sf-button[aria-disabled="true"], this CSS blocks pointer events but does not disable keyboard-triggered navigation/activation on <a> (or custom interactive elements). That creates a disabled-looking control that can still act enabled. Please pair this with a documented behavioral contract (e.g., remove href, set tabindex="-1", and prevent activation in JS when disabled).
🤖 Prompt for 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.
In `@optional/components.css` around lines 147 - 152, The
`.sf-button[aria-disabled="true"]` CSS selector only visually disables the
button through pointer-events and opacity, but does not prevent keyboard
activation for link elements or other interactive controls. To properly disable
buttons with aria-disabled, ensure the CSS is paired with behavioral controls:
for link-based buttons, remove or nullify the href attribute, add tabindex="-1"
to remove keyboard focus, and implement JavaScript logic to prevent activation
(click, Enter, Space) when the disabled state is set. Update the component
documentation to specify that developers must apply these additional constraints
when using aria-disabled on interactive elements to prevent inconsistent
disabled-looking controls that can still be activated.
…nd font specimen Components (optional/components.css + optional/tokens.components.css): - .sf-button: full bulletproof implementation — primary/secondary/ghost/danger/neutral variants; s/m/l sizes; icon-only, pill, full-width shape modifiers; disabled (:disabled + aria-disabled) and loading (.is-loading) integration; all 7 button tokens activated (gap, font-weight, min-height, border-width). - .sf-card: full bulletproof implementation — bordered/elevated/interactive/glow/flat style variants; horizontal/no-pad layout variants; divided modifier for visible header/footer separators; overflow:hidden for edge-to-edge image cards; concentric radius via --sf-card-radius-outer; all 8 card tokens activated. Layout anchoring (core/layout.css): - .sf-section--offset-header: section modifier that extends padding-block-start by --sf-sticky-offset so content clears a sticky/fixed header. - .sf-scroll-target: sets scroll-margin-block-start: var(--sf-sticky-offset) on anchor targets so hash navigation doesn't hide content under a sticky header. Configurator — shade ramp preview (H2): - New ShadeRamp.svelte: resolves and displays the 7-step shade ramp (superlight → superdark) for all 6 brand color families via the probe host; live-updates on override or theme change. - Integrated into DomainPanel.svelte as a collapsible "Shade ramp" card in the Colors domain, alongside the existing Semantic roles panel. Configurator — font pairing specimen (H3): - Added 'Specimen' group to the typography domain in domainPreviews.js. - DomainPreview.svelte renders a live heading + body paragraph pairing using --sf-font-heading, --sf-h2-size, --sf-font-body, --sf-leading-normal. All 9 framework API tests + all 465 configurator tests pass. 12 new component tokens registered; 19 new .sf- classes in the public surface. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LDcZ7i7rtRbRqXJ9HgnaNN
088945e to
51e7ccb
Compare
There was a problem hiding this comment.
3 issues found and verified against the latest diff
You’re at about 98% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Fix all with cubic | Re-trigger cubic
|
I've fixed all 3 issues:
✅ All tests passing (build + Playwright e2e + version check) Pushed commits to |
- ShadeRamp: use style:background-color instead of style:background to preserve checkerboard pattern on unresolved swatches - sf-card: remove persistent will-change:transform from --interactive variant to reduce compositing cost on large lists - sf-card: restore section wrappers to flex in --horizontal mode so body content stays grouped (display:contents broke containment)
Components (optional/components.css + optional/tokens.components.css):
variants; s/m/l sizes; icon-only, pill, full-width shape modifiers; disabled
(:disabled + aria-disabled) and loading (.is-loading) integration; all 7 button
tokens activated (gap, font-weight, min-height, border-width).
style variants; horizontal/no-pad layout variants; divided modifier for visible
header/footer separators; overflow:hidden for edge-to-edge image cards;
concentric radius via --sf-card-radius-outer; all 8 card tokens activated.
Layout anchoring (core/layout.css):
by --sf-sticky-offset so content clears a sticky/fixed header.
anchor targets so hash navigation doesn't hide content under a sticky header.
Configurator — shade ramp preview (H2):
(superlight → superdark) for all 6 brand color families via the probe host;
live-updates on override or theme change.
Colors domain, alongside the existing Semantic roles panel.
Configurator — font pairing specimen (H3):
--sf-font-heading, --sf-h2-size, --sf-font-body, --sf-leading-normal.
All 9 framework API tests + all 465 configurator tests pass.
12 new component tokens registered; 19 new .sf- classes in the public surface.
Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01LDcZ7i7rtRbRqXJ9HgnaNN
Summary by cubic
Implements production-ready
.sf-buttonand.sf-cardcomponents, sticky-header-safe scroll anchoring, and new configurator previews for 7-step shade ramps and font pairings. Adds 12 component tokens with full docs/registry updates and addresses review feedback for ShadeRamp rendering and.sf-cardperformance.New Features
.sf-button(primary/secondary/ghost/danger/neutral; s/l; icon-only, pill, full-width; disabled and loading; activates gap, font-weight, min-height, border-width tokens)..sf-card(bordered/elevated/interactive/glow/flat; horizontal/no-pad; divided header/footer; overflow clipping; concentric radius via--sf-card-radius-outer; activates 8 card tokens)..sf-section--offset-headerand.sf-scroll-targetkeep content visible under sticky headers, including hash links.Bug Fixes
.sf-card--interactive: remove persistent will-change to reduce compositing cost..sf-card--horizontal: restore section wrappers to flex so body content stays grouped.Written for commit 6c9dbf4. Summary will update on new commits.
Summary by CodeRabbit