feat(configurator): inline detached-scale notices for radius/border/motion (follow-up) - #701
Conversation
|
Warning Review limit reached
Next review available in: 39 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?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. 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 (4)
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 SummaryThe PR broadens the configurator redesign and adds inline detached-scale notices for radius, border width, and motion. It also centralizes token-domain classification, adds token relationship and change-management UI, improves import/history behavior, and restructures navigation.
Confidence Score: 4/5The PR should not merge until scale notices stop treating live expression overrides as fixed pinned values; the stale editor-mode display is non-blocking but should also be corrected. The new radius, border-width, and motion notices can misrepresent var() or scale-dependent calc() overrides and lead users to remove intentional relationships, while ValueField can display a mode that no longer matches the current override. Files Needing Attention: configurator/src/lib/tokenModel.ts, configurator/src/components/inputs/ValueField.svelte
|
| Filename | Overview |
|---|---|
| configurator/src/lib/tokenModel.ts | Adds the central token relationship and scale-shadow model, but key-presence-only shadow detection misclassifies expression-based step overrides. |
| configurator/src/components/inputs/ValueField.svelte | Adds explicit editing modes, though a manually selected mode remains stale after external override changes. |
| configurator/src/components/panels/BordersPanel.svelte | Surfaces radius and border-width shadow notices and moves focus-ring controls to Accessibility. |
| configurator/src/components/panels/MotionPanel.svelte | Adds the duration-scale shadow notice using the shared token model. |
| configurator/src/lib/domains.ts | Centralizes deterministic namespace-based token-domain classification with explicit exceptions. |
| configurator/src/App.svelte | Integrates coalesced history, validated imports, search deep links, and full-width tool screens. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
O[Active overrides] --> S[scaleShadows]
S --> B[Shape notices]
S --> M[Motion notice]
B --> C[Clear affected steps]
M --> C
O --> T[tokenState]
T --> P[Changes panel and token rows]
Reviews (1): Last reviewed commit: "feat(configurator): inline detached-scal..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| for (const family of SCALE_FAMILIES) { | ||
| const shadowedSteps = family.steps.filter((s) => s in overrides); | ||
| if (shadowedSteps.length === 0) continue; |
There was a problem hiding this comment.
When a radius, border-width, or duration step contains a var() relink or scale-dependent calc(), scaleShadows treats the override as a fixed shadow solely because its key exists. The notice then falsely labels a live relationship as pinned and offers to clear the user's intentional expression.
| let manualMode = $state<ValueMode | null>(null); | ||
| let mode = $derived<ValueMode>(manualMode ?? detectMode(overrideValue)); |
There was a problem hiding this comment.
After a user selects Value or Expression, an external reset, undo, import, or theme application can change overrideValue while the keyed component retains manualMode. The editor then displays Value or Expression as active for a token that is currently inherited, misleading the user about its state.
8a16aea to
45e6172
Compare
…width On the lifecycle/reference tools — Changes, Presets, Install & export, Reference — the live preview sat there showing an irrelevant Color gallery over half the screen while the operation (export CSS, browse classes, review overrides) got a cramped 360px column. These screens now hide the preview entirely and let the panel take the full width (the CSS output, class catalogue and change list all benefit). The mobile Controls/Preview fold bar is hidden there too, since there's nothing to fold to. Accessibility deliberately keeps its preview — the contrast checker reads the rendered colours from the iframe. check, lint, 285 unit tests and shell e2e pass; screenshot + e2e verified the full-width tool layout and that token/Accessibility screens still show the preview.
…width & motion The Type and Spacing panels already warn when concrete per-step overrides shadow their generated scale (making the scale knob inert). Radius, border-width and motion had the same failure mode with no warning — the audit flagged this gap. Reuse the existing ScaleShadowNotice, fed by tokenModel.scaleShadows(): - Shape: a notice above the Radius section (pinned --sf-radius-* steps) and in the Border widths section (pinned --sf-border-width-* steps). - Motion: a notice in the Durations section (absolute --sf-duration-* values that override the global --sf-motion-scale). Each offers "Clear them and use the scale" to restore the generated ladder. No new logic — the generalised detection already shipped in tokenModel (and the Changes panel); this surfaces it inline where the knobs live. check, lint, 285 unit tests and shell e2e pass; screenshot + e2e verified the radius and motion notices and that clearing restores the scale. Note: the broader migration of every panel SliderRow to the explicit Inherit/Value/Expression editor remains a separate follow-up — the generic All-tokens/Changes editor already uses ValueField, and SliderRow already supports a variable/raw escape hatch.
Plain domain navigation (sidebar, mobile nav, and the palette's no-token branch) left a previous search's focusRequest active, so returning to that domain later re-triggered a stale token focus/scroll. Clear focusRequest on those paths. Also gate the palette's "Go to" heading on navCount > 0 so a token-only result set no longer shows an empty "Go to" section.
45e6172 to
f2d2006
Compare
2 participants
Follow-up #5 — detached-scale notices for radius, border-width & motion
Type and Spacing already warn when concrete per-step overrides shadow their generated scale (making the source knob inert). Radius, border-width and motion had the same invisible failure mode — the audit flagged the gap.
Change
Reuse the existing
ScaleShadowNotice, fed bytokenModel.scaleShadows():--sf-radius-*steps) and in Border widths (pinned--sf-border-width-*).--sf-duration-*overriding the global--sf-motion-scale).Each offers "Clear them and use the scale" to restore the generated ladder. No new logic — the generalised detection already shipped in
tokenModel(and powers the Changes panel); this surfaces it inline where the knobs live.Verification
npm run check0 errors ·npm run lintcleannpm run test: 285/285 · shell e2e greenScope
The broader migration of every panel
SliderRowto the explicit Inherit/Value/Expression editor remains a separate follow-up: the generic All-tokens/Changes editor already usesValueField, andSliderRowalready offers a variable/raw escape hatch — so the highest-risk, audit-flagged part (invisible detachment) is what this lands.