feat(components,configurator): per-size .sf-btn padding + min-height knobs; move flatten-all to Advanced - #591
Merged
jackgranatowski merged 2 commits intoJul 11, 2026
Conversation
…knobs; move flatten-all to Advanced
Extends the per-size button model (added for label font-size in v0.7.14) to
padding and height, and reworks the configurator button panel around it.
Framework:
- Add 15 PUBLIC per-size knobs — --sf-btn-{xs,s,m,l,xl}-{padding-block,
padding-inline,min-height} — each `initial`, wired as the first arg of the
matching --sf-btn-{prop}--size tier so an unset knob falls through to that
rung's scale default. Retune one rung's font/padding/height independently
without flattening the --xs…xl ladder.
- Re-tier the flatten-all overrides (--sf-btn-font-size, --sf-btn-padding-block,
--sf-btn-padding-inline, --sf-btn-min-height) to PUBLIC-ADVANCED — same SemVer
guarantee, surfaced as advanced (per-size knobs are the everyday path).
Configurator:
- ComponentsPanel: the per-size editor now tunes font-size / padding-block /
padding-inline / min-height for the selected rung, and selecting a rung drives
the live preview (fixes "preview doesn't react" to per-rung edits). The
flatten-all "Advanced" disclosure is removed from the panel — those knobs now
live only in the All-tokens tab, where the tier dividers group them under
Advanced.
Regenerate registry / docs / annotations / demos / audit artifacts; extend the
token-api snapshot, empty-token coverage list, per-size browser test (now also
asserts padding + min-height per rung), and CHANGELOG.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GnwqYHgHNAWPbsUJWUeY1w
|
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 selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughButton sizing now exposes per-rung padding and minimum-height tokens alongside font-size controls. The configurator edits each rung with live preview, flatten-all overrides move to advanced tiering, and generated registries, demos, documentation, audits, and tests are updated. ChangesButton sizing
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Author as Configurator
participant Tokens as Button token overrides
participant CSS as .sf-btn styles
participant Preview as Live preview
Author->>Tokens: set rung font-size, padding, and min-height
Tokens->>CSS: resolve per-rung custom properties
CSS->>Preview: render updated button geometry
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![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 `@configurator/src/components/panels/ComponentsPanel.svelte`:
- Around line 340-369: Add aria-pressed to each button in the BTN_SIZE_RUNGS
group and the TEXT_STEPS group, binding it to the same selection checks used for
visual state: activeBtnRung === rung and cur === step respectively. Keep the
existing click handlers and styling unchanged.
In `@configurator/src/data/api-index.generated.json`:
- Around line 1385-1444: The generated button-token entries use an overly narrow
“Padding and radius tokens” description that excludes font-size and min-height.
Update the canonical button-token metadata to use a broader button-geometry
description, then regenerate the API index so all affected entries, including
those identified by names such as --sf-btn-l-min-height and
--sf-btn-l-padding-block, contain the corrected description.
🪄 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: d1bd4d86-cf54-4f25-9039-625d8926a0da
⛔ Files ignored due to path filters (1)
dist/css-custom-data.jsonis excluded by!**/dist/**
📒 Files selected for processing (24)
CHANGELOG.mdconfigurator/src/components/panels/ComponentsPanel.svelteconfigurator/src/data/api-index.generated.jsonconfigurator/src/data/token-registry.generated.jsondemos/full-api-demo-with-overrides.htmldemos/full-api-demo.htmldemos/generate.mjsdemos/ultimate-override.cssdocs/api-index.jsondocs/api-index.mddocs/llm-guide.mddocs/registry.jsondocs/test-coverage-6-token-reference.htmldocs/token-annotations.jsondocs/token-index.jsondocs/token-index.mddocs/tokens.mdoptional/components.cssoptional/tokens.components.cssreports/full-api-audit/results/tokens-report.jsonscripts/token-tiers.jstests/token-api.snapshot.jsontests/tokens.spec.jstoken-registry.json
Greptile SummaryThis PR extends the per-size
Confidence Score: 4/5Safe to merge — the CSS fallback chain is structurally correct, all five rungs are wired symmetrically, and the configurator's bidirectional rung-preview sync is a genuine fix. The only gap is cosmetic: the llm-guide lists the 15 new tokens without their The CSS and token changes are well-structured and match what the configurator expects. The RUNG_SIZING numeric defaults align with the CSS fallback values, and the Playwright test correctly asserts the two new box-metric knobs. The one issue worth fixing is the llm-guide format: the new tokens are listed as space-separated names without values, breaking the established per-token documentation pattern that CLAUDE.md explicitly requires for new PUBLIC entries. docs/llm-guide.md — the 15 new per-size padding/min-height tokens need one-per-line entries with their Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[":root override\n--sf-btn-xs-padding-inline: 5rem"] -->|set| B["var(--sf-btn-xs-padding-inline, …)"]
C["unset / initial"] -->|falls through| B
B --> D["--sf-btn-padding-inline--size\n(resolved in .sf-btn--xs)"]
D --> E["--sf-btn-padding-inline\n(PUBLIC-ADVANCED flatten-all)"]
E -->|set| F["Flat value — overrides ALL sizes"]
E -->|initial / unset| G["--sf-btn-padding-inline--size\n(per-size tier wins)"]
G --> H["Applied padding-inline\n(only xs rung affected)"]
F --> H
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[":root override\n--sf-btn-xs-padding-inline: 5rem"] -->|set| B["var(--sf-btn-xs-padding-inline, …)"]
C["unset / initial"] -->|falls through| B
B --> D["--sf-btn-padding-inline--size\n(resolved in .sf-btn--xs)"]
D --> E["--sf-btn-padding-inline\n(PUBLIC-ADVANCED flatten-all)"]
E -->|set| F["Flat value — overrides ALL sizes"]
E -->|initial / unset| G["--sf-btn-padding-inline--size\n(per-size tier wins)"]
G --> H["Applied padding-inline\n(only xs rung affected)"]
F --> H
Reviews (1): Last reviewed commit: "feat(components,configurator): per-size ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+1118
to
+1120
| --sf-btn-xs-padding-block --sf-btn-s-padding-block --sf-btn-m-padding-block --sf-btn-l-padding-block --sf-btn-xl-padding-block | ||
| --sf-btn-xs-padding-inline --sf-btn-s-padding-inline --sf-btn-m-padding-inline --sf-btn-l-padding-inline --sf-btn-xl-padding-inline | ||
| --sf-btn-xs-min-height --sf-btn-s-min-height --sf-btn-m-min-height --sf-btn-l-min-height --sf-btn-xl-min-height |
There was a problem hiding this comment.
: initial values, breaking the one-token-per-line-with-default pattern established by the font-size entries directly above. CLAUDE.md requires the "qualitative layer" update for new PUBLIC tokens beyond what the CI gate enforces — an LLM reading this block currently has no signal that these tokens default to initial (falls through to the rung's scale default), whereas the font-size entries each show their resolved default explicitly.
Suggested change
| --sf-btn-xs-padding-block --sf-btn-s-padding-block --sf-btn-m-padding-block --sf-btn-l-padding-block --sf-btn-xl-padding-block | |
| --sf-btn-xs-padding-inline --sf-btn-s-padding-inline --sf-btn-m-padding-inline --sf-btn-l-padding-inline --sf-btn-xl-padding-inline | |
| --sf-btn-xs-min-height --sf-btn-s-min-height --sf-btn-m-min-height --sf-btn-l-min-height --sf-btn-xl-min-height | |
| --sf-btn-xs-padding-block: initial /* unset → 0.125rem (rung default) */ | |
| --sf-btn-s-padding-block: initial /* unset → var(--sf-space-2xs) */ | |
| --sf-btn-m-padding-block: initial /* unset → var(--sf-space-xs) */ | |
| --sf-btn-l-padding-block: initial /* unset → var(--sf-space-s) */ | |
| --sf-btn-xl-padding-block: initial /* unset → var(--sf-space-m) */ | |
| --sf-btn-xs-padding-inline: initial /* unset → var(--sf-space-xs) */ | |
| --sf-btn-s-padding-inline: initial /* unset → var(--sf-space-s) */ | |
| --sf-btn-m-padding-inline: initial /* unset → var(--sf-space-m) */ | |
| --sf-btn-l-padding-inline: initial /* unset → var(--sf-space-l) */ | |
| --sf-btn-xl-padding-inline: initial /* unset → var(--sf-space-xl) */ | |
| --sf-btn-xs-min-height: initial /* unset → var(--sf-size-xs) */ | |
| --sf-btn-s-min-height: initial /* unset → var(--sf-size-s) */ | |
| --sf-btn-m-min-height: initial /* unset → var(--sf-size-m) */ | |
| --sf-btn-l-min-height: initial /* unset → var(--sf-size-l) */ | |
| --sf-btn-xl-min-height: initial /* unset → var(--sf-size-xl) */ |
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Comment on lines
+337
to
+340
| // Per-size padding + min-height knobs retune the xs rung (5rem = 80px, 7rem = 112px). | ||
| expect(boxBefore.pi).not.toBeCloseTo(80, 0); | ||
| expect(boxAfter.pi).toBeCloseTo(80, 0); | ||
| expect(boxAfter.mh).toBeCloseTo(112, 0); |
There was a problem hiding this comment.
not.toBeCloseTo assertion, but the min-height baseline is only asserted after the knob is set. Adding the baseline check for mh mirrors the pattern used for pi and would catch a future regression where the xs default accidentally landed at 112 px.
Suggested change
| // Per-size padding + min-height knobs retune the xs rung (5rem = 80px, 7rem = 112px). | |
| expect(boxBefore.pi).not.toBeCloseTo(80, 0); | |
| expect(boxAfter.pi).toBeCloseTo(80, 0); | |
| expect(boxAfter.mh).toBeCloseTo(112, 0); | |
| // Per-size padding + min-height knobs retune the xs rung (5rem = 80px, 7rem = 112px). | |
| expect(boxBefore.pi).not.toBeCloseTo(80, 0); | |
| expect(boxAfter.pi).toBeCloseTo(80, 0); | |
| expect(boxBefore.mh).not.toBeCloseTo(112, 0); | |
| expect(boxAfter.mh).toBeCloseTo(112, 0); |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
…utton group desc, symmetric test baseline Address CodeRabbit + Greptile review on PR #591: - ComponentsPanel: add aria-pressed to the per-size rung tabs and label-size step buttons so the selected option is exposed to assistive tech. - llm-guide: list the 15 per-size padding/min-height knobs one-per-line with their `initial` default + per-rung scale fallback, matching the font-size entries (CLAUDE.md qualitative-layer requirement). - token-annotations: broaden the BUTTON TOKENS group description beyond "padding and radius" to cover the per-size font-size/min-height knobs; regenerated api-index/docs. - tests: add the symmetric min-height baseline assertion (not.toBeCloseTo 112) mirroring the padding-inline check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GnwqYHgHNAWPbsUJWUeY1w
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.
Why
Follow-up to the v0.7.14 button label-size work. That release added per-size font-size knobs, but padding and min-height stayed global-only — an inconsistency: the per-size editor could tune one rung's label but not its box. It also surfaced the flatten-all overrides in the control panel, where they read like ordinary settings but silently collapse the
.sf-btn--xs…xlladder. This completes the per-size model and moves the flatten-all knobs to where advanced knobs belong.Framework
--sf-btn-{xs,s,m,l,xl}-{padding-block,padding-inline,min-height}— joining the per-sizefont-sizeknobs. Each isinitial, wired as the first arg of the matching--sf-btn-{prop}--sizetier, so an unset knob falls through to that rung's own scale default. Retune one rung's font / padding / height independently, ladder intact.--sf-btn-font-size,--sf-btn-padding-block,--sf-btn-padding-inline,--sf-btn-min-height) to PUBLIC-ADVANCED — same SemVer guarantee, surfaced as advanced (the per-size knobs are the everyday path). Not a breaking change (tier is a documentation/SemVer signal, not a rename).Verified in Chromium (built dist)
--sf-btn-xs-padding-inline: 5rem--sf-btn-xs-min-height: 7rem--sf-btn-font-scalestill multiplies proportionallyConfigurator
Artifacts & gates
Regenerated registry / docs / annotations / demos / audit artifacts; extended the token-api snapshot (756 tokens), empty-token coverage list, and the per-size browser test (now also asserts padding + min-height per rung).
All green: 10 CSS gates (registry 792 ids, llm-guide 524 refs / 0 undocumented, audit 756 tokens, macros, mirrors, dead-knobs, annotations, bundle-defs, lint, version 0.7.14), 109 node unit tests, 164 vitest + 25 component tests (configurator),
svelte-check/tsc0 errors,vite buildOK.Notes
## Unreleased; the release process handles it).main(v0.7.14) since the prior PR feat(components,configurator): button label-size scale multiplier + per-size knobs #590 merged, so this is a fresh change.SLASHED-Pluginsis untouched — it vendors this repo and inherits viaupdate-frameworkon the next framework release.🤖 Generated with Claude Code
Generated by Claude Code
Summary by CodeRabbit