Refactor persistence: explicit save with live preview injection

[jackgranatowski]

Summary

Refactor the persistence layer to separate live CSS preview (injected on every change) from explicit persistence (triggered only on user save). This gives users immediate visual feedback while maintaining control over when changes are actually saved to storage or the REST API.

Key Changes

• Split persistence concerns: persistOverrides() → injectLivePreview() (every change) + saveOverrides() (explicit save only)

  • injectLivePreview() is now called in a reactive effect on every overrides change
  • saveOverrides() is an async function called only when user clicks Save or presses Ctrl+S
  • Removed debouncing logic; WP saves now happen immediately on explicit save rather than debounced on every change

• Add save state tracking in App.svelte:

  • hasPendingChanges: tracks whether there are unsaved modifications
  • saveState: tracks UI state ('idle' | 'saving' | 'saved')
  • Keyboard shortcut (Ctrl+S / Cmd+S) now triggers handleSave()

• Update StudioHeader with new Save button:

  • Shows "Save" when changes are pending
  • Shows "Saving…" with spinner during async save
  • Shows "Saved" with checkmark for 2 seconds after successful save
  • Button disabled when no pending changes or save is in progress
  • Tooltip indicates Ctrl+S shortcut

• Simplify persistence.ts:

  • Remove cancelPendingWpSave() and debounce timer logic
  • Remove persistOverrides() function
  • Export injectLivePreview() for reactive injection
  • Export saveOverrides() as the explicit persistence entry point
  • Update module documentation to reflect new behavior

Implementation Details

• Live preview injection remains synchronous and side-effect free (safe for reactive effects)
• Save state transitions: pending → saving → saved (auto-resets to idle after 2s)
• Error handling in handleSave() logs warnings and resets to idle state
• Works in both standalone (localStorage + URL hash) and WordPress (REST API) modes

https://claude.ai/code/session_01MA1xz4tzW2uFeday7suPaG

[coderabbitai]

Warning

Review limit reached

@jackgranatowski, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 23 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 777df3af-40f0-4d73-ae22-e564ad3ca6c0

📥 Commits

Reviewing files that changed from the base of the PR and between 69b2335 and 78d079d.

📒 Files selected for processing (3)

• configurator/src/App.svelte
• configurator/src/components/shell/StudioHeader.svelte
• configurator/src/lib/persistence.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/session-58k028

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[qodo-code-review]

PR Summary by Qodo

Refactor overrides persistence: live preview injection + explicit Save flow
✨ Enhancement 🕐 40+ Minutes

Description

• Split live CSS injection from persistence to avoid saving on every input change.
• Add explicit Save UX (button + Ctrl/Cmd+S) with pending/saving/saved states.
• Simplify persistence layer by removing WordPress debounce/cancel logic.

Diagram

graph TD
  A["App (overrides)"] --> B["injectLivePreview()"] --> C["DOM <style> update"]
  A --> D["StudioHeader Save"] --> E["handleSave()"] --> F["saveOverrides()"]
  F --> G["Standalone: localStorage + hash"]
  F --> H["WP: REST /tokens/overrides"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Keep debounced autosave (WP) + separate live preview

• ➕ No additional user action required to persist changes
• ➕ Maintains protection against REST API flooding during rapid edits
• ➖ Still persists unintended intermediate states
• ➖ More complex (timers/cancellation) and harder to reason about correctness/ordering

2. Hybrid: explicit Save + periodic background autosave snapshot

• ➕ Reduces risk of data loss if users forget to save
• ➕ Still provides a clear 'committed' Save action
• ➖ Reintroduces implicit persistence semantics
• ➖ Requires additional UI/logic to explain autosaved vs saved states

3. Server-side draft model (WP) with explicit publish/commit

• ➕ Clear separation between draft and published configuration
• ➕ Can support history/versioning in WP
• ➖ Significantly larger scope: REST API + storage model changes
• ➖ More UI complexity (draft status, revert, conflict handling)

Recommendation: The PR’s explicit-save approach is the best fit for the stated goal (immediate preview, user-controlled persistence) and materially simplifies the persistence seam by removing debounce/cancel complexity. The main follow-up to consider is whether a lightweight safeguard against unsaved changes (e.g., beforeunload prompt) is desired, but the core split (injectLivePreview vs saveOverrides) is sound.

Files changed (3) +71 / -34

Enhancement (2) +62 / -9

App.svelteAdd dirty tracking + explicit save handler; inject preview on change +30/-7

Add dirty tracking + explicit save handler; inject preview on change

• Replaces the prior effect that persisted on every overrides change with a reactive live-preview injection effect. Introduces hasPendingChanges and saveState, wires an async handleSave() that calls saveOverrides(), and adds Ctrl/Cmd+S to trigger saves. Passes new props and onSave callback into StudioHeader.

configurator/src/App.svelte

StudioHeader.svelteAdd Save button with saving/saved UI states and shortcut hint +32/-2

Add Save button with saving/saved UI states and shortcut hint

• Extends header props to receive hasPendingChanges/saveState and an onSave callback. Adds a Save button that disables appropriately, shows Saving… with a spinner while saving, and Saved state styling after success.

configurator/src/components/shell/StudioHeader.svelte

Refactor (1) +9 / -25

persistence.tsSplit persistence into injectLivePreview() and async saveOverrides() +9/-25

Split persistence into injectLivePreview() and async saveOverrides()

• Renames the style injection helper into exported injectLivePreview() and removes persistOverrides(), debounce timer, and cancelPendingWpSave(). Introduces saveOverrides() as the explicit persistence entry point, saving immediately to WP REST when embedded or to localStorage+hash in standalone mode.

configurator/src/lib/persistence.ts

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 6 rules

1. Undo/redo breaks dirty state ✓ Resolved 🐞 Bug ≡ Correctness

Description

hasPendingChanges is only updated inside setOverrides() and after handleSave(), but
handleUndo()/handleRedo() mutate overrides directly, so dirty state can become incorrect
(e.g., undo after a save keeps Save disabled even though overrides changed). This can prevent users
from saving a state reached via undo/redo and can also leave the UI showing pending changes when the
state matches the last saved snapshot.

Code

configurator/src/App.svelte[R58-68]

  function setOverrides(updater: ((prev: Record<string, string>) => Record<string, string>) | Record<string, string>) {
    const prev = overrides;
    const next = typeof updater === "function" ? updater(prev) : updater;
    if (JSON.stringify(prev) !== JSON.stringify(next)) {
      past = [...past.slice(-49), prev];
      future = [];
+      hasPendingChanges = true;
+      if (saveState === 'saved') saveState = 'idle';
    }
    overrides = next;
  }

Relevance

⭐⭐⭐ High

Team accepted prior fixes preventing UI state desync when overrides change via import/undo/redo (PR
#428).

PR-#428

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new dirty flag is set only in setOverrides(), while undo/redo modify overrides directly and
never touch hasPendingChanges, so dirty state can drift from the actual overrides value.

configurator/src/App.svelte[51-68]
configurator/src/App.svelte[119-135]
configurator/src/App.svelte[70-82]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`hasPendingChanges` is maintained only via `setOverrides()` and `handleSave()`, but undo/redo bypasses this logic by directly assigning to `overrides`. This makes the Save button/state incorrect.

### Issue Context
With explicit persistence, the UI must accurately represent whether the current `overrides` differs from the last persisted snapshot, regardless of *how* the state changed (direct edit, undo, redo, reset, import, theme apply).

### Fix Focus Areas
- configurator/src/App.svelte[51-68]
- configurator/src/App.svelte[70-135]

### Suggested fix
- Introduce a `lastSavedSnapshot` (e.g., serialized JSON string or a shallow-cloned object + stable hash) initialized from `loadInitialOverrides()`.
- Derive `hasPendingChanges` from `overrides !== lastSavedSnapshot` (or JSON comparison) in a reactive effect so undo/redo updates it automatically.
- Update `lastSavedSnapshot` only after a confirmed successful save of the same snapshot.
- Alternatively (less robust), route undo/redo through a helper that updates `hasPendingChanges` consistently, but this still won’t handle “undo back to saved state” correctly without a saved baseline.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Save race clears dirty ✓ Resolved 🐞 Bug ≡ Correctness

Description

handleSave() unconditionally sets hasPendingChanges = false after awaiting
saveOverrides(overrides), so if the user edits overrides while the save request is in-flight, the
later completion will incorrectly clear the dirty flag and show “Saved” even though there are
unsaved changes. This can lead to data loss (user thinks changes are saved when they are not).

Code

configurator/src/App.svelte[R70-81]

+  async function handleSave() {
+    if (!hasPendingChanges || saveState === 'saving') return;
+    saveState = 'saving';
+    try {
+      await saveOverrides(overrides);
+      hasPendingChanges = false;
+      saveState = 'saved';
+      setTimeout(() => { if (saveState === 'saved') saveState = 'idle'; }, 2000);
+    } catch (err) {
+      console.warn('slashed: save failed', err);
+      saveState = 'idle';
+    }

Relevance

⭐⭐ Medium

No direct historical evidence for guarding async save completion against mid-flight edits; closest
is persistence snapshotting in PR #443.

PR-#443

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

handleSave() clears the dirty flag after await saveOverrides(overrides) with no snapshot/version
check, and there is no guard in setOverrides() to prevent edits while a save is in flight.

configurator/src/App.svelte[58-68]
configurator/src/App.svelte[70-82]
configurator/src/components/shell/StudioHeader.svelte[113-125]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`handleSave()` clears `hasPendingChanges` and sets `saveState='saved'` after the awaited save completes, but it does not verify that the currently displayed `overrides` still matches what was saved. If edits happen during the async save, the completion path overwrites the newer dirty state.

### Issue Context
Edits can occur while `saveState === 'saving'` because the UI only disables the Save button; `setOverrides()` does not block updates during a save.

### Fix Focus Areas
- configurator/src/App.svelte[58-68]
- configurator/src/App.svelte[70-82]

### Suggested fix
- Capture a stable snapshot at save start (e.g., `const snapshot = overrides; const snapshotJson = JSON.stringify(snapshot);`).
- Await `saveOverrides(snapshot)` (or pass a clone) so what you save is deterministic.
- After await, only mark saved/clear dirty if the current overrides still match the snapshot (or if a revision counter hasn’t changed). Otherwise, keep `hasPendingChanges=true` and return `saveState` to `'idle'` (or show a distinct “Saved previous changes” state).
- Optionally, maintain an `overridesRevision` incremented on every change; store `savingRevision` at save start and compare on completion.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Save timer not cleaned ✓ Resolved 🐞 Bug ☼ Reliability

Description

After a successful save, handleSave() schedules a setTimeout to reset saveState, but the timer
is never cleared on component teardown, so it may fire after unmount and attempt to update state.
This creates avoidable lifecycle leaks and can cause post-destroy state writes.

Code

configurator/src/App.svelte[77]

+      setTimeout(() => { if (saveState === 'saved') saveState = 'idle'; }, 2000);

Relevance

⭐⭐⭐ High

They previously accepted clearing pending timers to avoid post-unmount effects (cancelPendingWpSave
cleanup in PR #443).

PR-#443

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

handleSave() schedules a timeout to mutate saveState, but the only teardown logic present
removes the keydown listener and does not clear that timer.

configurator/src/App.svelte[70-82]
configurator/src/App.svelte[179-200]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
A `setTimeout` created in `handleSave()` is not cleared when the component unmounts.

### Issue Context
The component already sets up a global keydown handler in `onMount()` and returns a cleanup function; a similar cleanup should clear any pending save-state timer.

### Fix Focus Areas
- configurator/src/App.svelte[70-82]
- configurator/src/App.svelte[179-200]

### Suggested fix
- Store the timeout id in a module/component variable (e.g., `let saveStateTimer: ReturnType<typeof setTimeout> | null = null`).
- Clear any existing timer before creating a new one.
- Clear the timer in the component teardown path (`onDestroy(...)` or the existing `onMount` cleanup) to prevent late callbacks after unmount.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---