feat(components,configurator): button label-size scale multiplier + per-size knobs - #590
Conversation
…er-size knobs
Framework:
- Add --sf-btn-font-scale (multiply every button's label by one factor,
keeping the xs…xl ladder intact) and per-size label knobs
--sf-btn-{xs,s,m,l,xl}-font-size (retune a single rung; parity with the
per-heading size knobs).
- font-size now reads calc(<hard-override | per-size tier> * font-scale).
The flatten-all --sf-btn-font-size override still works but is demoted
in the configurator UI.
Configurator:
- ComponentsPanel: primary label-size control is now the scale multiplier;
add a per-size editor; demote the flatten-all knobs (font-size override,
padding-block/inline, min-height) behind an Advanced disclosure.
- AllTokensTab: add tier group dividers (Public → Advanced → Internal) so
advanced knobs are visually separated from everyday ones in each domain.
Regenerate token registry / docs / audit / demo artifacts; update the
token-api snapshot, empty-token coverage list, and CHANGELOG.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GnwqYHgHNAWPbsUJWUeY1w
|
Warning Review limit reached
Next review available in: 9 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
📝 WalkthroughWalkthroughAdds global and per-size ChangesButton label sizing
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Configurator
participant ButtonTokens
participant ButtonCSS
Configurator->>ButtonTokens: Set global scale or per-size override
ButtonTokens->>ButtonCSS: Provide resolved label size
ButtonCSS->>ButtonCSS: Apply scaled font-size to .sf-btn
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Greptile SummaryThis PR introduces
Confidence Score: 4/5The CSS and configurator changes are well-structured and the new token cascade is correctly wired end-to-end; the two findings are confined to the All-Tokens tab divider edge case and a misleading demo comment. The core CSS logic (calc with the scale multiplier, per-size knob fallback chain) is correctly implemented and verified against the scenarios table in the PR description. All generated artifacts (registry, LLM guide, snapshot, test spec) are consistent. The two issues found are both in peripheral areas: the tier divider can render orphaned at the top of the list when filtering leaves no PUBLIC tokens visible, and the demo overrides file silently shadows the per-size font knobs with the flatten-all token it also sets. configurator/src/components/panels/AllTokensTab.svelte (tier divider edge case) and demos/generate.mjs (per-size font knobs shadowed in ultimate-override demo) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["font-size: calc(A * B)"] --> B["A = var(--sf-btn-font-size, --sf-btn-font-size--size)"]
A --> C["B = var(--sf-btn-font-scale, 1)"]
B --> D{"--sf-btn-font-size set?"}
D -- Yes --> E["Use flatten-all value\n(overrides every size)"]
D -- No --> F["--sf-btn-font-size--size"]
F --> G{"Per-size knob set?\n--sf-btn-{size}-font-size"}
G -- Yes --> H["Use per-rung value\n(single rung only)"]
G -- No --> I["--sf-text-{size}\n(scale default)"]
C --> J{"--sf-btn-font-scale set?"}
J -- Yes --> K["Multiply by scale factor\n(ladder preserved)"]
J -- No --> L["x1 (no-op)"]
%%{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["font-size: calc(A * B)"] --> B["A = var(--sf-btn-font-size, --sf-btn-font-size--size)"]
A --> C["B = var(--sf-btn-font-scale, 1)"]
B --> D{"--sf-btn-font-size set?"}
D -- Yes --> E["Use flatten-all value\n(overrides every size)"]
D -- No --> F["--sf-btn-font-size--size"]
F --> G{"Per-size knob set?\n--sf-btn-{size}-font-size"}
G -- Yes --> H["Use per-rung value\n(single rung only)"]
G -- No --> I["--sf-text-{size}\n(scale default)"]
C --> J{"--sf-btn-font-scale set?"}
J -- Yes --> K["Multiply by scale factor\n(ladder preserved)"]
J -- No --> L["x1 (no-op)"]
Reviews (1): Last reviewed commit: "feat(components,configurator): button la..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
configurator/src/data/api-index.generated.json (1)
1285-1304: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winEmpty
notefield on the 6 new button typography tokens.Every sibling BUTTON TOKENS row (
--sf-btn-font-size,--sf-btn-font-weight,--sf-btn-min-height,--sf-btn-radius, etc.) has a populated per-tokennotedescribing its specific role, but--sf-btn-font-scale,--sf-btn-l-font-size,--sf-btn-m-font-size,--sf-btn-s-font-size,--sf-btn-xl-font-size, and--sf-btn-xs-font-sizeall ship with"note": "". Since these tokens are the new public surface this PR is meant to introduce, the missing notes reduce clarity in the configurator UI and any docs/LLM reference generated from this field.✏️ Proposed fix
{ "name": "--sf-btn-font-scale", ... - "note": "", + "note": "Proportional multiplier applied to all .sf-btn label font sizes while preserving the xs–xl size ladder.", ... },(repeat with a size-specific note for each of the five per-size font-size tokens)
Also applies to: 1365-1404, 1485-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 1285 - 1304, Populate the empty note fields for the six new BUTTON TOKENS entries: --sf-btn-font-scale, --sf-btn-l-font-size, --sf-btn-m-font-size, --sf-btn-s-font-size, --sf-btn-xl-font-size, and --sf-btn-xs-font-size. Add clear, token-specific notes describing each token’s typography role, using size-specific wording for the five per-size font-size tokens and matching the style of existing button token notes.tests/tokens.spec.js (1)
32-36: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd focused coverage for per-size button overrides.
These tokens are excluded from the generic coverage check because their default is intentionally
initial. Add a browser assertion that setting each token on the root changes the matching button size’s computed font size; otherwise regressions in the fallback or consumption chain can pass unnoticed.🤖 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 `@tests/tokens.spec.js` around lines 32 - 36, Add focused browser coverage in the token tests for the per-size button font tokens listed in the diff. Set each token on the root element, then assert the corresponding button size’s computed font size changes accordingly, covering the fallback and token-consumption chain for every size.
🤖 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 `@demos/full-api-demo-with-overrides.html`:
- Around line 1332-1333: Fix the token visualization generator for
--sf-btn-font-scale so unitless numeric values are not compared with 3px in the
generated inline-size max() expression. Omit the bar for this token or map
numeric tokens to a CSS-compatible visualization, then regenerate both demo
files and verify the output contains no incompatible CSS types.
---
Nitpick comments:
In `@configurator/src/data/api-index.generated.json`:
- Around line 1285-1304: Populate the empty note fields for the six new BUTTON
TOKENS entries: --sf-btn-font-scale, --sf-btn-l-font-size, --sf-btn-m-font-size,
--sf-btn-s-font-size, --sf-btn-xl-font-size, and --sf-btn-xs-font-size. Add
clear, token-specific notes describing each token’s typography role, using
size-specific wording for the five per-size font-size tokens and matching the
style of existing button token notes.
In `@tests/tokens.spec.js`:
- Around line 32-36: Add focused browser coverage in the token tests for the
per-size button font tokens listed in the diff. Set each token on the root
element, then assert the corresponding button size’s computed font size changes
accordingly, covering the fallback and token-consumption chain for every size.
🪄 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: e11f59da-ffc7-4bd1-8284-160ebe0194a3
⛔ Files ignored due to path filters (1)
dist/css-custom-data.jsonis excluded by!**/dist/**
📒 Files selected for processing (23)
CHANGELOG.mdconfigurator/src/components/panels/AllTokensTab.svelteconfigurator/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-index.jsondocs/token-index.mddocs/tokens.mdoptional/components.cssoptional/tokens.components.cssreports/full-api-audit/results/tokens-report.jsontests/token-api.snapshot.jsontests/tokens.spec.jstoken-registry.json
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| {#if t.tier === "PUBLIC-ADVANCED" && (i === 0 || list[i - 1].tier !== "PUBLIC-ADVANCED")} | ||
| <div class="flex items-center gap-2 px-1.5 pt-3 pb-1"> | ||
| <span class="text-[8px] font-bold uppercase tracking-widest text-amber-600/80 dark:text-amber-400/80">Advanced</span> | ||
| <div class="h-px flex-1 bg-amber-600/25"></div> | ||
| </div> | ||
| {/if} | ||
| {#if t.tier === "INTERNAL" && (i === 0 || list[i - 1].tier !== "INTERNAL")} | ||
| <div class="flex items-center gap-2 px-1.5 pt-3 pb-1"> | ||
| <span class="text-[8px] font-bold uppercase tracking-widest text-rose-700/80 dark:text-rose-400/80">Internal</span> | ||
| <div class="h-px flex-1 bg-rose-800/25"></div> | ||
| </div> | ||
| {/if} |
There was a problem hiding this comment.
When a search query or the "only modified" filter produces results where the first item is a PUBLIC-ADVANCED or INTERNAL token (no PUBLIC entries in view), the condition i === 0 fires and the divider is rendered at position 0 — with nothing above it to separate from. The label "Advanced" or "Internal" floats at the top of an otherwise empty public section, losing its visual meaning. Adding i > 0 to the guard (i.e. i > 0 && list[i - 1].tier !== "PUBLIC-ADVANCED") prevents the orphaned header.
| '--sf-btn-padding-inline': '2rem', | ||
| '--sf-btn-min-height': '4rem', | ||
| // Uniform label-size multiplier (unitless) + per-size label knobs (also | ||
| // `initial`) — explicit lengths exercise each rung's per-size override. | ||
| '--sf-btn-font-scale': '1.6', | ||
| '--sf-btn-xs-font-size': '0.9rem', | ||
| '--sf-btn-s-font-size': '1.1rem', | ||
| '--sf-btn-m-font-size': '1.4rem', |
There was a problem hiding this comment.
The CURATED object also contains '--sf-btn-font-size': '1.5rem' (the flatten-all), which shadows the five --sf-btn-{xs,s,m,l,xl}-font-size values added here. The inline comment "explicit lengths exercise each rung's per-size override" is therefore not accurate — the per-size overrides are inert whenever the flatten-all is active. The ultimate-override demo cannot independently verify each per-size knob is wired correctly when the two are set simultaneously.
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!
- AllTokensTab: guard tier-group divider with `i > 0` so the "Advanced"/
"Internal" header isn't rendered orphaned at the top of a filtered list
with no Public tokens above it.
- token-annotations.json: add notes for the 6 new button typography tokens
(--sf-btn-font-scale + per-size --sf-btn-{xs,s,m,l,xl}-font-size) and
clarify --sf-btn-font-size as the flatten-all override; these flow into
the configurator note field and the api-index description.
- demos/generate.mjs: skip the flatten-all --sf-btn-font-size in the
ultimate-override demo so it no longer shadows the per-size knobs (which
now visibly exercise each rung); omit the visual bar for unitless numeric
tokens (--sf-btn-font-scale) so the generated inline-size no longer does
an invalid max(<number>, 3px).
- tests/tokens.spec.js: add a browser test proving each per-size knob
retunes only its rung and --sf-btn-font-scale multiplies the whole ladder.
Regenerate demo/docs/configurator artifacts.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GnwqYHgHNAWPbsUJWUeY1w
2 participants
Why
The button label-size story was incoherent: the only exposed control was the global
--sf-btn-font-size, which flattens every size to one value (destroying the.sf-btn--xs…xlladder) — a foot-gun the UI had to warn about, while the real per-size source (the scale) wasn't editable at all. This reworks it into two coherent, non-destructive controls (plus optional per-rung parity with headings), and demotes the flatten-all override.Framework
--sf-btn-font-scale(new PUBLIC knob) — a uniform label-size multiplier.font-sizenow readscalc(<hard-override | per-size tier> * var(--sf-btn-font-scale, 1)), so it scales every button proportionally while keeping the xs…xl ladder intact. This is the everyday "bigger/smaller button text" knob.--sf-btn-{xs,s,m,l,xl}-font-size(5 new PUBLIC knobs) — per-rung label size, the button counterpart of the per-heading--sf-h{1..6}-sizeknobs. Each isinitial→ falls through to its--sf-text-{size}default; set one to retune a single rung without touching the others.--sf-btn-font-sizeoverride still works unchanged (composes with the multiplier), just demoted in the UI.Verified in Chromium (built dist)
--sf-btn-font-scale:1.5--sf-btn-xs-font-size:3rem--sf-btn-font-size:1remConfigurator
Artifacts & gates
Regenerated token registry / docs / audit / demo artifacts; updated the token-api snapshot, empty-token coverage list, and CHANGELOG.
All green: 9 CSS gates (registry, llm-guide, macros, dead-knobs, mirrors, bundle-defs, annotations, version, audit), 109 node unit tests, 164 vitest + 25 component tests (configurator),
svelte-check0 errors,vite buildOK,check-artifacts/demos OK.Notes
## Unreleased; the release process handles the bump).SLASHED-Pluginsis intentionally untouched — itsadmin-app/srcis vendored from this repo and will inherit these changes viaupdate-framework/sync on the next framework release.🤖 Generated with Claude Code
Generated by Claude Code
Summary by CodeRabbit
New Features
Documentation