fix(configurator): correct token metadata drift & misleading help (#684–689) - #690
Conversation
…-689) Fixes a batch of display-only defects where the configurator/framework docs promised behaviour or values that no longer match reality. - #684: sync hardcoded --sf-size-l default in SIZE_TOKENS (2.75 → 3rem) to match the framework (core/tokens.css). - #685: drop the rawDefault="var(--sf-size-l)" + SIZE_SCALE picker on the Min touch size control; --sf-touch-target is a deliberately independent 2.75rem/44px WCAG floor, not an alias of the size scale. Help updated; remove now-unused SIZE_SCALE import. - #686: relabel the Z-index "Base offset" slider and rewrite its help — the framework rungs are independent literals, --sf-z-base is not added to them; it is only the base local-stacking rung consumed by the .sf-z-base utility. - #687: rework TokenRow.guessType to prefer generated syntax metadata (<color>/length/number) and narrow the name fallback so border shorthands, --sf-border-width-* lengths and background shorthands no longer get a color picker. - #688: fix Icon sizes help class names (.sf-icon-* → .sf-icon--*, .sf-icon-box → .sf-icon--boxed). - #689: trim optional/legacy.css header to the fallbacks it actually ships (drop has() and the duplicated dvh).
|
Warning Review limit reached
Next review available in: 47 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day 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 (1)
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 (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe changes make token type detection syntax-aware, update miscellaneous configurator defaults and control ranges, and correct icon utility and legacy fallback documentation. ChangesConfigurator updates
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: 🔵 Low · up to The configurator changes the valid value domains for touch-target and z-base settings, so previously saved overrides may need normalization or rejection to avoid confusing or invalid displayed values. This is mergeable with explicit owner awareness and follow-up. 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 corrects configurator metadata, labels, token editor classification, utility-class help, and the legacy module header.
Confidence Score: 3/5The PR should not merge until the touch-target and z-base controls preserve and accurately display existing overrides. Removing raw touch-target editing rewrites valid rem or variable values as px literals, while the narrowed z-base range misrenders and clamps values previously accepted by the configurator. Files Needing Attention: configurator/src/components/panels/MiscPanel.svelte
|
| Filename | Overview |
|---|---|
| configurator/src/components/inputs/TokenRow.svelte | Narrows token-name heuristics and prioritizes generated syntax metadata; no concrete affected-token regression was established. |
| configurator/src/components/panels/MiscPanel.svelte | Corrects several labels and defaults, but the narrowed touch-target and z-base controls mishandle existing valid overrides. |
| optional/legacy.css | Accurately trims a comment describing the unchanged legacy fallbacks. |
Reviews (1): Last reviewed commit: "fix(configurator): correct token metadat..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| <SliderRow | ||
| label="Min touch size" value={touchTarget} min={32} max={64} step={1} unit="px" | ||
| help="--sf-touch-target — minimum tappable area for interactive elements (WCAG 2.5.5)" | ||
| help="--sf-touch-target — minimum tappable area for interactive elements (WCAG 2.5.5). Independent literal (2.75rem / 44px) — deliberately NOT an alias of the --sf-size-* scale, so retuning sizes never drops below the accessibility floor." | ||
| overridden={"--sf-touch-target" in overrides} | ||
| onChange={(v) => onSet("--sf-touch-target", `${v}px`)} | ||
| onReset={() => onReset("--sf-touch-target")} |
There was a problem hiding this comment.
When a persisted or imported --sf-touch-target override uses 2.75rem or var(--sf-size-l), this px-only control displays an out-of-range or fallback value and replaces the original expression with a clamped px literal when edited.
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!
There was a problem hiding this comment.
Fixed in d33606b. Restored the raw-CSS escape hatch (currentRaw/onRawSet) with an honest independent literal default (2.75rem) instead of the misleading var(--sf-size-l) alias, and no scale picker. Persisted var()/calc()/clamp() overrides now surface as editable raw text via the </> toggle / expr auto-detect rather than being misread as an out-of-range px value and clobbered to px on edit. This still satisfies #685 (no --sf-size-* alias suggestion).
| <SliderRow | ||
| label="Base offset" value={zBaseOffset} min={0} max={1000} step={10} | ||
| help="--sf-z-base — added to all z-index tokens to avoid conflicts with existing stacking contexts" | ||
| label="Base" value={zBaseOffset} min={-1} max={10} step={1} | ||
| help="--sf-z-base — base local-stacking rung (default 0), consumed by the .sf-z-base utility. It is NOT added to the other rungs: every ladder step below is an independent literal." | ||
| overridden={"--sf-z-base" in overrides} | ||
| onChange={(v) => onSet("--sf-z-base", String(v))} | ||
| onReset={() => onReset("--sf-z-base")} |
There was a problem hiding this comment.
When a persisted theme contains a --sf-z-base value above 10, including values produced by the former control, the narrowed range renders it beyond the track and clamps it to 10 on the next interaction, overwriting the theme.
There was a problem hiding this comment.
Fixed in d33606b. Reverted the slider range back to the original 0–1000 (step 10), so themes with a --sf-z-base above 10 are no longer rendered off-track and clamped/overwritten on interaction. The range narrowing was incidental to my change; the actual #686 fix (relabel + accurate help clarifying that --sf-z-base is not added to the other rungs) is unchanged.
…(review) Address Greptile P1 backward-compat regressions on the MiscPanel controls: - Min touch size: restore the raw-CSS escape hatch (currentRaw/onRawSet) with an honest independent literal default (2.75rem, not var(--sf-size-l)) and no scale picker. Persisted var()/calc()/clamp() overrides are surfaced as editable raw text instead of being misread as an out-of-range px value and clobbered to a px literal on edit. Still satisfies #685 (no size-scale alias). - Z-index Base: revert the slider range to the original 0–1000 (step 10) so themes with a --sf-z-base above 10 are no longer rendered off-track and clamped/overwritten. The #686 fix (label + accurate help) is unchanged.
2 participants
Summary
Batch fix for six display-only defects where the configurator (and one framework module header) advertised behaviour or values that no longer match the framework. Grouped into a single PR since they all share the "metadata/docs drifted from reality" theme and touch overlapping files.
All fixes were verified against the real framework values (
core/tokens.css, generatedapi-index, source selectors).Fixes
--sf-size-ldefault inSIZE_TOKENS(2.75→3rem) to matchcore/tokens.css.rawDefault="var(--sf-size-l)"+SIZE_SCALEpicker on Min touch size.--sf-touch-targetis a deliberately independent2.75rem/44px WCAG floor, not an alias of the size scale. Help text clarified; unusedSIZE_SCALEimport removed.--sf-z-baseis not added to them — it is only the base local-stacking rung consumed by the.sf-z-baseutility. Slider range tightened to reflect a local rung (-1…10).TokenRow.guessTypeto prefer generatedsyntaxmetadata (<color>/ length / number) and narrow the name fallback so border shorthands,--sf-border-width-*lengths and background shorthands (--sf-surface-bg-*,--sf-bg-layer-*) no longer get a color picker. Genuine colors still resolve via the--sf-color-*namespace or a-bgsuffix..sf-icon-*→.sf-icon--*,.sf-icon-box→.sf-icon--boxed.optional/legacy.cssheader to the fallbacks it actually ships (drophas()and the duplicateddvh).Verification
tsc --noEmit— cleansvelte-check— 0 errors, 0 warningscheck:curation— OKvite build— successNotes
#687classification was validated against every-bg/-border/sourcetoken in the generatedapi-indexto confirm no genuine color token loses its picker and no shorthand/length keeps one.Closes #684, #685, #686, #687, #688, #689
Summary by CodeRabbit
Bug Fixes
Improvements