Skip to content

fix(configurator): live preview now reacts to panel changes - #424

Merged
jackgranatowski merged 3 commits into
mainfrom
claude/configurator-live-preview-fix-1ydq71
Jun 26, 2026
Merged

fix(configurator): live preview now reacts to panel changes#424
jackgranatowski merged 3 commits into
mainfrom
claude/configurator-live-preview-fix-1ydq71

Conversation

@jackgranatowski

@jackgranatowski jackgranatowski commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Two root-cause bugs:

  1. Double-wrapped :root{} in slashed-overrides style element — fa() with
    mode:"root" already returns a complete :root{...} block, but both
    buildIframeHTML and the patch effects wrapped it in a second :root{}.
    With CSS nesting in modern browsers the inner :root is interpreted as
    :root :root (descendant selector) which matches nothing, so no token
    ever applied.

  2. html/$derived included overrides, so every token change triggered a
    full srcdoc reload that raced with the patch effect and discarded its
    work. Now srcdoc only encodes template/motion/theme (structural
    changes), and the effects own all override + theme patching.

Also replace the loaded boolean with loadCount counters so the patch
effect correctly re-runs after any iframe reload (template/motion/theme
changes), not just after the first load.

Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01ExUMnARDWD8897H8MzctkF

Summary by CodeRabbit

  • New Features
    • Improved configurator preview behavior, with override styles updating more reliably across live refreshes and when opening the preview in a new tab.
  • Bug Fixes
    • Updated how override CSS is injected into the page and preview iframes, avoiding wrapper-related inconsistencies.
    • Ensured iframe reloads apply the latest overrides and that refresh fully resets preview state to prevent stale visuals.

@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 17e4d8f9-cfe0-4b6b-976e-81dc0e5efca8

📥 Commits

Reviewing files that changed from the base of the PR and between 194b2de and d41b213.

📒 Files selected for processing (4)
  • .gitignore
  • badges/badge-optimal.json
  • configurator/src/App.svelte
  • configurator/src/components/shell/PreviewPanel.svelte
✅ Files skipped from review due to trivial changes (2)
  • .gitignore
  • badges/badge-optimal.json
🚧 Files skipped from review as they are similar to previous changes (2)
  • configurator/src/App.svelte
  • configurator/src/components/shell/PreviewPanel.svelte

📝 Walkthrough

Walkthrough

The PR changes how override CSS is embedded and how preview iframes reload. Preview panes now use per-frame load counters to gate style updates, and the new-tab action rebuilds iframe HTML from the current settings.

Changes

Preview override CSS flow

Layer / File(s) Summary
Direct override CSS output
configurator/src/App.svelte, configurator/src/components/shell/PreviewPanel.svelte
Override CSS is written directly into the parent and iframe style elements without a :root wrapper.
Iframe load counters
configurator/src/components/shell/PreviewPanel.svelte
Preview iframe effects use per-frame load counters to control override updates, and refresh and load handlers update those counters.

Sequence Diagram(s)

sequenceDiagram
  participant App as App.svelte
  participant Panel as PreviewPanel.svelte
  participant Frame as iframe document
  App->>App: assign fa(...) output to style#sf-parent-overrides
  Panel->>Panel: buildIframeHTML(overrides, previewTheme, previewMotion, previewTemplate, frameworkCSSStatic)
  Panel->>Frame: load preview HTML with style#slashed-overrides
  Frame-->>Panel: onload increments loadCount or split load counts
  Panel->>Frame: set style#slashed-overrides textContent
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

codex

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main configurator fix: making the live preview react to panel changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/configurator-live-preview-fix-1ydq71

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

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

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix configurator live preview updates after panel changes
🐞 Bug fix ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

Description

• Fix override CSS injection by removing accidental double :root{} wrapping.
• Prevent token edits from forcing iframe srcdoc reloads; patch overrides/theme via effects.
• Track iframe reloads with load counters so patches re-apply after template/motion/theme changes.
Diagram

graph TD
  A["Configurator panels"] --> B["Overrides/theme state"] --> C["PreviewPanel.svelte"]
  C --> D["buildIframeHTML()"] --> E["iframe srcdoc (structural)"]
  C --> F["$effect patchers"] --> G["iframe DOM (style + class)"]
  F --> H["fa(mode=root) CSS"] --> G
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. postMessage-based override syncing
  • ➕ Avoids DOM querying/mutation from the parent component
  • ➕ Can batch updates and acknowledge apply completion inside the iframe
  • ➖ Requires adding a script runtime inside the iframe and message protocol
  • ➖ More code/complexity for a small preview tool
2. Inline CSS variables on instead of a style tag
  • ➕ Simplifies updates (setProperty per token) and avoids CSS parsing
  • ➕ Potentially clearer separation of generated tokens vs authored CSS
  • ➖ Requires mapping all overrides to CSS custom properties
  • ➖ Less flexible if overrides include non-variable CSS rules

Recommendation: Keep the PR’s approach: restricting srcdoc to structural changes and applying overrides/theme via post-load patch effects is the smallest, most direct fix for the observed reload race. The load counters are a pragmatic improvement over a one-time loaded boolean, ensuring patches re-apply after any iframe reload.

Files changed (3) +26 / -25

Bug fix (2) +25 / -24
App.svelteStop double-wrapping :root when patching parent overrides CSS +1/-1

Stop double-wrapping :root when patching parent overrides CSS

• Removes an extra ':root { ... }' wrapper when writing the overrides style element. This aligns with 'fa(..., mode:"root")' already returning a complete ':root{}' block and prevents nested ':root :root' selectors that match nothing.

configurator/src/App.svelte

PreviewPanel.svelteDecouple iframe srcdoc from overrides; re-apply patches after every reload +24/-23

Decouple iframe srcdoc from overrides; re-apply patches after every reload

• Stops embedding overrides in iframe 'srcdoc' derivations so token edits no longer force full iframe reloads that can race with patching effects. Replaces loaded booleans with load counters so override/theme patch effects rerun after any iframe reload, and fixes overrides CSS injection to avoid double ':root{}' wrapping. The “open in new tab” action now builds a full HTML document including overrides for the exported preview.

configurator/src/components/shell/PreviewPanel.svelte

Other (1) +1 / -1
badge-optimal.jsonRegenerate bundle size badge output +1/-1

Regenerate bundle size badge output

• Updates the prebuilt badge JSON (gzip size message) as a side-effect of rebuilding during verification.

dist/badge-optimal.json

@coderabbitai coderabbitai Bot added the codex label Jun 26, 2026
@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

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

Context used
✅ Compliance rules (platform): 5 rules

Grey Divider


Remediation recommended

1. Blob URL not revoked 🐞 Bug ☼ Reliability
Description
PreviewPanel's “Open in new tab” creates a Blob URL but never revokes it, so repeated uses retain
the underlying Blob in memory for the lifetime of the page. This can accumulate because the
generated HTML includes the full framework CSS.
Code

configurator/src/components/shell/PreviewPanel.svelte[R378-380]

+        const blob = new Blob([buildIframeHTML(overrides, previewTheme, previewMotion, previewTemplate, frameworkCSSStatic)], { type: "text/html" });
        const url = URL.createObjectURL(blob);
        window.open(url, "_blank");
Relevance

⭐⭐ Medium

No prior accepted/rejected history for URL.revokeObjectURL usage in configurator code.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PreviewPanel creates an object URL and opens it without revocation; other parts of the configurator
explicitly revoke object URLs after triggering a download, showing the intended pattern in this
codebase.

configurator/src/components/shell/PreviewPanel.svelte[375-381]
configurator/src/App.svelte[183-192]
configurator/src/components/panels/ExportPanel.svelte[33-41]

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

### Issue description
The “Open in new tab” handler creates a blob URL via `URL.createObjectURL` and opens it, but never calls `URL.revokeObjectURL`, which can leak memory across repeated clicks.

### Issue Context
Other download/export flows in this repo revoke object URLs after use. For blob URLs opened in a new tab, you generally need to revoke after a short delay (or immediately if `window.open` fails), so the new tab has time to load the resource.

### Fix Focus Areas
- configurator/src/components/shell/PreviewPanel.svelte[375-381]

### Suggested change (example)
- Create the URL
- Call `window.open`
- If it fails, revoke immediately
- Otherwise, revoke after a short timeout, e.g.:

```ts
const url = URL.createObjectURL(blob);
const w = window.open(url, "_blank");
if (!w) {
 URL.revokeObjectURL(url);
} else {
 setTimeout(() => URL.revokeObjectURL(url), 60_000);
}
```

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



Informational

2. Misleading srcdoc comment 🐞 Bug ⚙ Maintainability
Description
A comment says srcdoc only encodes layout/motion/template, but html is still derived from
buildIframeHTML(..., previewTheme, ...), so theme changes can still reload the iframe. This
mismatch can mislead future changes around preview reload behavior.
Code

configurator/src/components/shell/PreviewPanel.svelte[R230-234]

+  // srcdoc only encodes layout/motion/template — overrides and theme are applied
+  // by the effects below, avoiding a full iframe reload on every token change.
+  let html = $derived(buildIframeHTML({}, previewTheme, previewMotion, previewTemplate, frameworkCSSStatic));
+  let htmlLight = $derived(buildIframeHTML({}, "light", previewMotion, previewTemplate, frameworkCSSStatic));
+  let htmlDark = $derived(buildIframeHTML({}, "dark", previewMotion, previewTemplate, frameworkCSSStatic));
Relevance

⭐⭐ Medium

Description fixes sometimes accepted (PR339) but similar misleading description fix was rejected
(PR299).

PR-#339
PR-#299

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The comment explicitly excludes theme, but the derived html includes previewTheme in the
buildIframeHTML call, which will change srcdoc when theme changes.

configurator/src/components/shell/PreviewPanel.svelte[230-234]

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

### Issue description
The comment above `html/htmlLight/htmlDark` says srcdoc only encodes layout/motion/template, but the current code also includes `previewTheme` in the derived `html`.

### Issue Context
This PR’s behavior may be intentional (theme changes are treated as structural and may reload), but the comment is currently out of sync with what triggers a reload.

### Fix Focus Areas
- configurator/src/components/shell/PreviewPanel.svelte[230-234]

### Suggested fix
Update the comment to match reality (e.g., “srcdoc only encodes template/motion/theme; overrides are applied by effects”), or if the intent is truly to avoid theme-driven reloads, remove `previewTheme` from the derived HTML and rely solely on the patch effect for theme class toggling.

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


Grey Divider

Qodo Logo

claude added 2 commits June 26, 2026 17:57
Two root-cause bugs:

1. Double-wrapped :root{} in slashed-overrides style element — fa() with
   mode:"root" already returns a complete :root{...} block, but both
   buildIframeHTML and the patch effects wrapped it in a second :root{}.
   With CSS nesting in modern browsers the inner :root is interpreted as
   :root :root (descendant selector) which matches nothing, so no token
   ever applied.

2. html/$derived included `overrides`, so every token change triggered a
   full srcdoc reload that raced with the patch effect and discarded its
   work. Now srcdoc only encodes template/motion/theme (structural
   changes), and the effects own all override + theme patching.

Also replace the loaded boolean with loadCount counters so the patch
effect correctly re-runs after any iframe reload (template/motion/theme
changes), not just after the first load.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUMnARDWD8897H8MzctkF
Bundle size badge regenerated as a side-effect of running npm run build
to verify the configurator fix.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUMnARDWD8897H8MzctkF
@jackgranatowski
jackgranatowski force-pushed the claude/configurator-live-preview-fix-1ydq71 branch from 194b2de to d01f442 Compare June 26, 2026 17:58
The build script still emits CSS to dist/ locally even after the
badges/ rename (PR #425). Add dist/ to .gitignore so stale build
artifacts don't show up as untracked files.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUMnARDWD8897H8MzctkF
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants