feat(configurator): unify text & display typographic scale controls - #631
Merged
Merged
Conversation
The sf-text and sf-display-text scales were edited with different inputs, adjustments and previews despite being the same fluid clamp() scale. Make them consistent and simpler to use: - Add a "Scale multiplier" slider to both scales (--sf-text-scale and --sf-text-display-scale) — the display multiplier was consumed by the preview engine but had no control; the text one likewise wasn't editable. - Add the missing Display S line-height; display line-heights are now s/m/l. - Note on the display scale that its modular ratio is shared with the text scale (matching the framework, which reuses --sf-text-ratio-*). - Give the display scale a per-step ramp preview alongside the text ramp in Scale preview (previously only text had one). - Extract a shared weight-grid snippet, removing four duplicated markup blocks. Configurator-only; no framework tokens changed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013BRVgKfEYocPCHz9KU96FN
…panels - Colors: add a "Copy to dark" action to the brand sources (primary, secondary, tertiary, action). It snapshots the light source's oklch value into the dark source and switches the row to manual — same effect as pasting the same colour into both. Not live-linked. Base/neutral/status are unchanged. - Colors: replace the lone raw <input type=range> gradient-angle control with RangeWithNumber, matching every other angle/hue control. - Macros: route the scrim, prose-marker and surface-bg colour fields through ColorInput (swatch + native picker) instead of plain text inputs. - Effects: remove the duplicated Scrim and Scroll-shadow sections — both edit .sf-scrim / .sf-scroll-shadow macro tokens whose richer editors (with previews and text-shadow) already live in the Macros panel. Configurator-only; no framework tokens changed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013BRVgKfEYocPCHz9KU96FN
…; z-index sliders Extract three reusable inputs to remove duplicated markup and unify controls that did the same thing differently across panels: - Toggle: the pill switch, previously hand-rolled in Shadows (glow), Motion (disable motion) and Misc (state flags). Two sizes. - AspectRatioInput: one preset grid for "pick a raw aspect ratio", used by Macros --sf-aspect (with preview) and Layout --sf-frame-ratio. - RawTokenRow: the "free-text token value + inline reset" row, used by Layout named ratios, Macros scrim text-shadow and the surface-bg image/overlay/ animation fields. Also give the Misc z-index tokens real per-step sliders (seeded from the framework defaults) instead of a read-only list that punted to the All-tokens tab, matching how every other numeric ladder is edited. Drops the phantom --sf-z-popover entry (no such framework token). Configurator-only; no framework tokens changed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013BRVgKfEYocPCHz9KU96FN
…nel headers
Every panel hand-copied the same button[aria-expanded] + chevron + {#if}
header block (~65 instances). Replace them with a single <Section> component
(bindable `open`, optional `variant="advanced"` and `spacing`), so section
headers are guaranteed consistent and defined once.
Rendered output is unchanged — verified across all 11 panels (sections expand/
collapse, zero runtime errors). Nested sub-section toggles that use <div>
wrappers are left as-is.
Configurator-only; no framework tokens changed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013BRVgKfEYocPCHz9KU96FN
The solid/dashed/dotted line-style enum was a button-grid in Borders and Misc but a <select> in Layout's equal-column rule. Switch it to the same button grid so the same enum is edited the same way everywhere. Configurator-only; no framework tokens changed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013BRVgKfEYocPCHz9KU96FN
The scale-preview "Aa" ramp clamped every step to 2.5rem, so all three display sizes (2.70/3.49/4.50rem) rendered identically. Render each step at its real midpoint rem size instead; the box scrolls if the largest steps are tall. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013BRVgKfEYocPCHz9KU96FN
…llapsing caps Several sidebar mini-previews scaled a value by a constant then clamped to a fixed pixel cap. Once several values in a set exceeded the cap they all rendered at the same width — different sizes looking identical (the same defect as the typography scale preview). Normalise each bar set against its own largest value so the biggest bar fills the track and every other stays proportional and distinct: spacing scale bars, motion duration bars, stagger delay bars, and the Misc component-size scale. (Absolute dimensions remain in the main preview pane.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013BRVgKfEYocPCHz9KU96FN
…ng token Panel controls already default to the scale dropdown (via SliderRow's variableOptions), but scale-referencing tokens without a dedicated control — editable only in the All-tokens / generic list — fell back to a plain text box, presenting a raw value instead of the variable. Add scaleForValue() to map a token's default (e.g. var(--sf-space-m)) to its sibling scale, and teach TokenRow to render that scale as a dropdown by default, with the raw text box behind "Custom…". Only a pure single-token reference qualifies, so compound values (e.g. a border shorthand) keep the text editor. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013BRVgKfEYocPCHz9KU96FN
|
Warning Review limit reached
Next review available in: 10 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 selected for processing (3)
📝 WalkthroughWalkthroughAdds reusable Svelte controls for sections, toggles, aspect ratios, and raw tokens. Refactors configurator panels to use them, adds scale-aware token selection, and updates color, typography, motion, spacing, and preview controls. ChangesShared configurator controls
Panel integrations
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 unifies the text and display typographic scale controls in the configurator, making both scales expose the same set of adjustments and previews. It also ships a broad refactoring of shared UI patterns across all panels.
Confidence Score: 5/5All changes are confined to the configurator UI; no framework tokens or CSS bundles are modified. The feature additions are straightforward and the token defaults used in the configurator (e.g. The Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User adjusts typography control] --> B{Which scale?}
B -->|Text scale slider| C[Modular Scale section]
B -->|Display scale slider| D[Display Type section]
B -->|Display S line-height| E[Display line-height grid 3-col]
B -->|Weight button grid| F{weightGrid snippet}
C --> G[Scale Preview Text ramp]
D --> H[Scale Preview Display ramp]
F --> I{clicked weight == default?}
I -->|Yes + override exists| J[onReset]
I -->|No| K[onSet]
G --> L[max-h-96 scrollable container]
H --> L
M[scaleForValue] --> N{pure var ref?}
N -->|Yes| O[VarOption array]
N -->|No| P[null]
O --> Q[TokenRow dropdown]
P --> Q
%%{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[User adjusts typography control] --> B{Which scale?}
B -->|Text scale slider| C[Modular Scale section]
B -->|Display scale slider| D[Display Type section]
B -->|Display S line-height| E[Display line-height grid 3-col]
B -->|Weight button grid| F{weightGrid snippet}
C --> G[Scale Preview Text ramp]
D --> H[Scale Preview Display ramp]
F --> I{clicked weight == default?}
I -->|Yes + override exists| J[onReset]
I -->|No| K[onSet]
G --> L[max-h-96 scrollable container]
H --> L
M[scaleForValue] --> N{pure var ref?}
N -->|Yes| O[VarOption array]
N -->|No| P[null]
O --> Q[TokenRow dropdown]
P --> Q
Reviews (1): Last reviewed commit: "feat(configurator): variable-first dropd..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| <div class="flex gap-1"> | ||
| {#each WEIGHT_OPTIONS as w (w)} | ||
| <button | ||
| onclick={() => w === defaultVal ? onReset(tokenName) : onSet(tokenName, w)} |
There was a problem hiding this comment.
weightGrid snippet simplifies the onclick handler compared to the original inline versions, but drops a deliberate no-op guard. The original handler did nothing when you clicked the already-selected default weight with no override (w === defaultVal && !isOverridden ? undefined); the snippet now always calls onReset(tokenName) in that case. If onReset has any side effects beyond removing the CSS override (e.g. marking a token as "dirty", triggering export diffing), clicking the default button on a fresh/unmodified state would now fire that path unnecessarily.
Suggested change
| onclick={() => w === defaultVal ? onReset(tokenName) : onSet(tokenName, w)} | |
| onclick={() => w === defaultVal ? (tokenName in overrides ? onReset(tokenName) : undefined) : onSet(tokenName, w)} |
![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
🧹 Nitpick comments (1)
configurator/src/components/inputs/Section.svelte (1)
21-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd
type="button"to shared button controls.Several new shared input components omit
type="button"on their<button>elements. By default, buttons act as submit buttons. While this doesn't break anything in the current client-only architecture if they aren't wrapped in a<form>, it's a best practice to explicitly settype="button"on interactive UI toggles and resets to prevent unexpected form submissions in the future.
configurator/src/components/inputs/Section.svelte#L21-L27: addtype="button"to the accordion toggle button.configurator/src/components/inputs/AspectRatioInput.svelte#L31-L38: addtype="button"to the preset option button.configurator/src/components/inputs/RawTokenRow.svelte#L31-L33: addtype="button"to the reset button.🤖 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/components/inputs/Section.svelte` around lines 21 - 27, Add type="button" to the interactive button elements in Section.svelte (accordion toggle), AspectRatioInput.svelte (preset option), and RawTokenRow.svelte (reset control), preserving their existing handlers and behavior.
🤖 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/ColorsPanel.svelte`:
- Around line 435-441: Update copyLightToDark so it resolves the light source to
its current concrete color value before passing it to onSet, rather than storing
the potentially dynamic result of sourceValue(light). Preserve the autoDarkSet
removal and ensure the stored dark value is a one-shot snapshot unaffected by
later light-source changes.
In `@configurator/src/components/panels/MotionPanel.svelte`:
- Line 278: Update the stagger bar width in the MotionPanel markup to normalize
each bar’s delayMs against a maxStaggerDelay, replacing the fixed n / 5
calculation. Compute maxStaggerDelay from the same effective token values used
when deriving each delayMs, matching the duration preview’s normalization
behavior.
---
Nitpick comments:
In `@configurator/src/components/inputs/Section.svelte`:
- Around line 21-27: Add type="button" to the interactive button elements in
Section.svelte (accordion toggle), AspectRatioInput.svelte (preset option), and
RawTokenRow.svelte (reset control), preserving their existing handlers and
behavior.
🪄 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: 82502df6-d214-452c-87cc-e57f92b113c5
📒 Files selected for processing (17)
configurator/src/components/inputs/AspectRatioInput.svelteconfigurator/src/components/inputs/RawTokenRow.svelteconfigurator/src/components/inputs/Section.svelteconfigurator/src/components/inputs/Toggle.svelteconfigurator/src/components/inputs/TokenRow.svelteconfigurator/src/components/panels/BordersPanel.svelteconfigurator/src/components/panels/ColorsPanel.svelteconfigurator/src/components/panels/ComponentsPanel.svelteconfigurator/src/components/panels/EffectsPanel.svelteconfigurator/src/components/panels/LayoutPanel.svelteconfigurator/src/components/panels/MacrosPanel.svelteconfigurator/src/components/panels/MiscPanel.svelteconfigurator/src/components/panels/MotionPanel.svelteconfigurator/src/components/panels/ShadowsPanel.svelteconfigurator/src/components/panels/SpacingPanel.svelteconfigurator/src/components/panels/TypographyPanel.svelteconfigurator/src/lib/variableScales.ts
- weightGrid: restore the no-op guard so clicking the already-selected default weight on an unmodified token no longer calls onReset unnecessarily. - copyLightToDark: freeze a concrete snapshot — copy a plain oklch/hex/rgb literal as-is (keeps OKLCH authoring), but resolve a var()-referencing light value so the copied dark value can't keep tracking its dependency. - stagger preview: size the delay bars by delayMs / max stagger delay, matching the duration preview's normalisation instead of a fixed n/5 staircase. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013BRVgKfEYocPCHz9KU96FN
13 tasks
Merged
13 tasks
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.
The sf-text and sf-display-text scales were edited with different inputs,
adjustments and previews despite being the same fluid clamp() scale. Make
them consistent and simpler to use:
--sf-text-display-scale) — the display multiplier was consumed by the
preview engine but had no control; the text one likewise wasn't editable.
scale (matching the framework, which reuses --sf-text-ratio-*).
Scale preview (previously only text had one).
Configurator-only; no framework tokens changed.
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_013BRVgKfEYocPCHz9KU96FN
Summary by CodeRabbit