feat(configurator): unified validated import + mobile preview dropdown (UX redesign, phase 7) - #697
Conversation
|
Warning Review limit reached
Next review available in: 8 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 (5)
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 redesigns the configurator around metadata-driven token domains, token relationships, a Changes panel, unified value editing, validated imports, improved navigation, undo coalescing, and a mobile preview selector.
Confidence Score: 2/5The PR should not merge until Escape cancellation, interaction-aware undo grouping, and collision reporting for imported overrides are corrected. The new editor persists values when users press Escape, rapid but separate same-token edits lose an undo boundary, and migration collisions can discard an imported override without appearing in the promised status report. Files Needing Attention: configurator/src/components/inputs/ValueField.svelte, configurator/src/App.svelte, configurator/src/lib/importOverrides.ts
|
| Filename | Overview |
|---|---|
| configurator/src/App.svelte | Integrates import reporting, search deep-links, and undo coalescing; time-only grouping merges distinct rapid edits into one undo step. |
| configurator/src/components/inputs/ValueField.svelte | Adds explicit inherit, literal, and expression modes, but Escape triggers the blur commit it is intended to cancel. |
| configurator/src/lib/importOverrides.ts | Adds the unified validated import pipeline, but omits migration collisions from its user-visible report. |
| configurator/src/lib/tokenModel.ts | Centralizes token roles, dependency relationships, scale-shadow detection, validation, and change classification with extensive unit coverage. |
| configurator/src/lib/domains.ts | Replaces overlapping substring patterns with deterministic manifest namespace classification and explicit exceptions. |
| configurator/src/components/shell/PreviewPanel.svelte | Provides a compact mobile template selector while retaining desktop template tabs. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
F[CSS or JSON file] --> P[parseImport]
P --> S[Sanitize names and values]
S --> M[Migrate renamed and removed tokens]
M --> R[Import report]
M --> O[Validated overrides]
O --> G[Merge into current overrides]
G --> H[Undo and persistence state]
R --> B[Transient status banner]
Reviews (1): Last reviewed commit: "feat(configurator): unified validated im..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| onblur={(e) => { editing = false; commit((e.target as HTMLInputElement).value); }} | ||
| onkeydown={(e) => { | ||
| if (e.key === "Enter") (e.currentTarget as HTMLInputElement).blur(); | ||
| if (e.key === "Escape") { editing = false; draft = overrideValue ?? token.value ?? ""; (e.currentTarget as HTMLInputElement).blur(); } |
There was a problem hiding this comment.
When a user edits a value and presses Escape, the handler immediately calls blur(), whose unconditional blur handler commits the input's still-edited DOM value. The value the user attempted to discard therefore becomes an active override.
| // current single-token gesture. | ||
| if (!shouldCoalesce(coalesce, keys, now)) { | ||
| past = [...past.slice(-49), prev]; |
There was a problem hiding this comment.
When two separate edits to the same token complete within 600 milliseconds, shouldCoalesce suppresses the second history snapshot because grouping uses only the key and elapsed time. Undo then skips the intermediate value and reverses both interactions instead of only the latest edit.
| overrides: migrated.overrides, | ||
| report: { | ||
| format: isJson ? "json" : "css", | ||
| accepted: Object.keys(migrated.overrides).length, | ||
| renamed: migrated.renamed.length, | ||
| removed: migrated.removed.length, | ||
| unknown: migrated.unknown.length, | ||
| invalid, |
There was a problem hiding this comment.
When two legacy token names migrate to the same current token, migration drops one value and records a collision, but this report omits that collision. The import banner consequently reports success without telling the user that one override was discarded.
8f2c9f8 to
c952a7d
Compare
…e dropdown
Two phase-7 fixes.
1) Unified import (src/lib/importOverrides.ts). The header import had two silent,
inconsistent paths — a .json REPLACED the whole state with no validation, a
.css MERGED via a loose regex, and an unrecognisable file did nothing with no
feedback. Now one pipeline handles both: detect JSON (flat map or a
theme-file { tokens }) vs CSS, sanitise every value, drop keys that aren't a
real --sf-* name or are empty/unsafe after sanitising, migrate renamed/
removed tokens and flag unknown ones, and return a report. App.handleImport
MERGES the result (predictable, non-destructive for both formats) and shows a
transient status banner ("Imported N tokens · M migrated · K skipped"), so an
import can no longer fail silently. Header tooltip fixed to "Import overrides
(CSS or JSON)". parseImport/summarizeImport are unit-tested (10 tests).
2) Mobile preview template selector. The horizontal tab strip was clipped in the
narrow mobile viewport; it's now a compact <select> dropdown on mobile and the
full tab strip on sm+.
check + lint clean; 282 unit tests and the shell e2e suite pass; import merge +
banner and the mobile dropdown screenshot-verified.
Scope: lands the validated/merged import + mobile preview dropdown. A full
mobile category drawer and an explicit merge-vs-replace chooser modal remain
follow-ups (replace + rename migration is already available via the Install &
export panel's theme-file import).
c952a7d to
b482679
Compare
2 participants
Phase 7 of the configurator UX redesign — unified import & mobile preview
1. Unified, validated import
The header import had two silent, inconsistent paths: a
.jsonfile replaced the whole state with no validation, a.cssfile merged via a loose regex, and a file with nothing recognisable did nothing, with no feedback.src/lib/importOverrides.tsis now one pipeline for both formats:{ tokens }) vs CSS,--sf-*name or are empty/unsafe after sanitising,themeFile.migrateOverrides),App.handleImportnow merges the result (predictable and non-destructive for both formats) and shows a transient status banner — "Imported N tokens · M migrated · K skipped" — so an import can never fail silently again. Header tooltip corrected to "Import overrides (CSS or JSON)".2. Mobile preview template selector
The horizontal template tab strip was clipped in the narrow mobile viewport. It's now a compact
<select>dropdown on mobile and the full tab strip onsm+.Verification
npm run check: 0 errors ·npm run lint: clean · build OKnpm run test: 282/282 (importOverrides10) · shell e2e greenScope / follow-ups
Lands the validated/merged import + mobile preview dropdown. A full mobile category drawer and an explicit merge-vs-replace chooser modal remain follow-ups — replace + rename migration is already available through the Install & export panel's theme-file import.