feat(configurator): Merge vs Replace import chooser with report preview (follow-up) - #704
Conversation
|
Warning Review limit reached
Next review available in: 31 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 (3)
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 stacked configurator update introduces the Merge/Replace import preview and substantially reorganizes token navigation, editing, history, domain classification, and change review.
Confidence Score: 3/5The PR should not merge until the new value editor reliably cancels Escape edits and refreshes its mode after external override changes. The unified editor currently commits the edited DOM value when Escape triggers blur, and its persistent manual mode can misrepresent values restored or changed elsewhere. Files Needing Attention: configurator/src/components/inputs/ValueField.svelte
|
| Filename | Overview |
|---|---|
| configurator/src/App.svelte | Integrates import preview/application, undo coalescing, mobile navigation, deep linking, and contextual preview layout. |
| configurator/src/components/inputs/ValueField.svelte | Adds the unified mode-based value editor, but Escape commits cancelled edits and manual mode becomes stale after external value changes. |
| configurator/src/lib/importOverrides.ts | Adds the shared CSS/JSON sanitization, migration, and reporting pipeline used by the chooser. |
| configurator/src/lib/tokenModel.ts | Introduces token roles, dependency relationships, scale-shadow detection, validation, and change summaries. |
| configurator/src/lib/domains.ts | Replaces overlapping substring classification with deterministic namespace and exception mapping. |
| configurator/src/data/domain-map.json | Defines the shared authoritative namespace-to-panel classification consumed at runtime and during curation checks. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
F[Select CSS or JSON file] --> P[Parse and sanitize]
P --> M[Migrate renamed and removed tokens]
M --> R[Preview import report]
R -->|Merge| G[Overlay imported overrides]
R -->|Replace| X[Discard current set and apply import]
R -->|Cancel or Escape| C[Leave overrides unchanged]
Reviews (1): Last reviewed commit: "feat(configurator): Merge vs Replace cho..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| 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 synchronously blurs the input while its DOM value still contains the draft, so onblur commits the value the user attempted to discard.
| 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(); } | |
| }} | |
| 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).value = draft; | |
| (e.currentTarget as HTMLInputElement).blur(); | |
| } | |
| }} |
| let manualMode = $state<ValueMode | null>(null); | ||
| let mode = $derived<ValueMode>(manualMode ?? detectMode(overrideValue)); |
There was a problem hiding this comment.
When an override changes through undo, reset, or another control after the user selects Value or Expression, manualMode continues to override detection, causing the editor to display a mode that no longer matches the current value.
| let manualMode = $state<ValueMode | null>(null); | |
| let mode = $derived<ValueMode>(manualMode ?? detectMode(overrideValue)); | |
| let manualMode = $state<ValueMode | null>(null); | |
| let mode = $derived<ValueMode>(manualMode ?? detectMode(overrideValue)); | |
| $effect(() => { | |
| overrideValue; | |
| manualMode = null; | |
| }); |
ae985db to
0062648
Compare
Import applied silently and always merged. Now selecting a file opens a chooser that previews the parsed report before anything changes: - Shows the filename and counts: tokens to import, migrated (renamed), dropped (removed by the framework), unknown in this build, skipped (invalid). - Explains the two actions and lets the user pick: Merge (keep current overrides and add on top) or Replace (discard the current set first); Cancel/Escape aborts. - A malformed or empty file skips the dialog and just reports (nothing to apply). Builds on the existing parseImport/summarizeImport (no parser changes). check, lint, 286 unit tests and shell e2e pass; screenshot + e2e verified the dialog and that Merge keeps existing overrides while Replace discards them.
575ee27 to
5b8378b
Compare
2 participants
Follow-up #8 — explicit Merge/Replace import chooser
Import applied silently and always merged. Now selecting a file opens a chooser that previews the parsed report before anything changes.
Change
Built on the existing
parseImport/summarizeImport— no parser changes.Verification
npm run check0 errors ·npm run lintcleannpm run test: 286/286 · shell e2e greenThis completes the follow-up series
All eight deferred content follow-ups from the UX redesign are now shipped as stacked PRs #698–#704: Depth panel, Accessibility panel, Components preview fence, contextual tool preview, scale-detach notices, System panel, mobile drawer, and this import chooser.