From 2dcc2b4d216ad1f19597d261b54f84bf96783396 Mon Sep 17 00:00:00 2001
From: Josh Knapp <jknapp85@gmail.com>
Date: Tue, 14 Jul 2026 12:12:01 -0700
Subject: [PATCH] fix(site-builder): entrance-animation reveal script survives
 Preview + well-formed void-tag attrs + no-JS fallback

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 craft/src/panels/topbar/TopBar.tsx  |  16 +++-
 craft/src/utils/html-export.test.ts | 142 +++++++++++++++++++++++++++-
 craft/src/utils/html-export.ts      |  44 ++++++++-
 3 files changed, 195 insertions(+), 7 deletions(-)
diff --git a/craft/src/panels/topbar/TopBar.tsx b/craft/src/panels/topbar/TopBar.tsx
index 5a01082..ceac12f 100644
--- a/craft/src/panels/topbar/TopBar.tsx
+++ b/craft/src/panels/topbar/TopBar.tsx
@@ -133,7 +133,7 @@ export const TopBar: React.FC<TopBarProps> = ({ device, onDeviceChange, showGuid
   const handlePreview = useCallback(() => {
     try {
       const serialized = query.serialize();
-      import('../../utils/html-export').then(({ exportToHtml, exportBodyHtml }) => {
+      import('../../utils/html-export').then(({ exportToHtml, exportBodyHtml, buildAnimationScript }) => {
         // Get header HTML
         let headerHtml = '';
         try {
@@ -154,8 +154,18 @@ export const TopBar: React.FC<TopBarProps> = ({ device, onDeviceChange, showGuid
           }
         } catch (e) { console.warn('Footer export failed:', e); }
 
-        // Compose full page: header + body + footer
-        const composedBody = headerHtml + bodyHtml + footerHtml;
+        // Compose full page: header + body + footer. `handlePreview` below
+        // replaces the ENTIRE wrapped-doc `<body>` inner (including the
+        // in-body reveal `<script>` wrapInDocument already emitted) with
+        // this composed string, so the script would otherwise be clobbered
+        // and animated elements would stay hidden forever
+        // ([data-animation]{opacity:0} with no IntersectionObserver to ever
+        // add `.animated`). Re-append the reveal script here -- built from
+        // the SAME composed content it will end up living alongside -- so
+        // it survives the replacement below and fires exactly once.
+        const composedBody =
+          headerHtml + bodyHtml + footerHtml +
+          buildAnimationScript(headerHtml + bodyHtml + footerHtml);
 
         // PKG-H: fold the active page's own SEO overrides + the site-wide
         // design tokens/favicon into the Preview export so editor Preview
diff --git a/craft/src/utils/html-export.test.ts b/craft/src/utils/html-export.test.ts
index 006a97d..07294e2 100644
--- a/craft/src/utils/html-export.test.ts
+++ b/craft/src/utils/html-export.test.ts
@@ -1,5 +1,5 @@
 import { describe, test, expect } from 'vitest';
-import { exportBodyHtml, exportToHtml, ExportOptions } from './html-export';
+import { exportBodyHtml, exportToHtml, buildAnimationScript, ExportOptions } from './html-export';
 import { DEFAULT_SITE_DESIGN, SiteDesign } from '../state/SiteDesignContext';
 
 /**
@@ -335,3 +335,143 @@ describe('PKG-H: SEO/meta + favicon + design-token <head> emission', () => {
     });
   });
 });
+
+/**
+ * FIX: entrance-animation broken in Preview. Root causes (see
+ * .superpowers/sdd/fix-animation-contract.md):
+ *   1. injectAttrs inserted data-attrs before the tag's first `>`, so a void
+ *      tag (`<img ... />`) became malformed (`<img ... / data-animation="...">`,
+ *      attrs landing AFTER the self-close slash, outside the tag).
+ *   2. wrapInDocument's in-body reveal <script> was destroyed by TopBar's
+ *      handlePreview, which replaces the whole <body> inner with a
+ *      recomposed header+body+footer string that never carried the script.
+ */
+describe('injectAttrs well-formed void-tag attrs (animation fix)', () => {
+  // ImageBlock.toHtml renders `<img src="..." ... />` -- a real void-tag
+  // producer that goes through injectAttrs via renderNode.
+  const imageState = (props: Record<string, unknown>) =>
+    JSON.stringify({
+      ROOT: {
+        type: { resolvedName: 'ImageBlock' },
+        isCanvas: false,
+        props: { src: '/uploads/photo.jpg', style: {}, ...props },
+        displayName: 'ImageBlock',
+        custom: {},
+        hidden: false,
+        nodes: [],
+        linkedNodes: {},
+      },
+    });
+
+  test('void <img/> gets attrs INSIDE the tag, no " / " sequence before the final >', () => {
+    const { html } = exportBodyHtml(imageState({ animation: 'bounce' }));
+    expect(html).toContain('data-animation="bounce"');
+    // Well-formed: the attribute sits before the self-close slash.
+    expect(html).toMatch(/data-animation="bounce"\s*\/>/);
+    // Malformed shape from the bug: attrs landing after the slash.
+    expect(html).not.toMatch(/\/\s*data-animation="bounce"/);
+    expect(html).not.toContain('/ data-animation');
+  });
+
+  test('non-void tag (Container div) is unaffected -- attrs still inserted before its only >', () => {
+    const state = JSON.stringify({
+      ROOT: {
+        type: { resolvedName: 'Container' },
+        isCanvas: true,
+        props: { tag: 'div', style: {}, animation: 'fade-in' },
+        displayName: 'Container',
+        custom: {},
+        hidden: false,
+        nodes: [],
+        linkedNodes: {},
+      },
+    });
+    const { html } = exportBodyHtml(state);
+    expect(html).toMatch(/^<div[^>]*data-animation="fade-in"[^>]*>/);
+    expect(html).not.toContain('/>');
+  });
+});
+
+describe('buildAnimationScript (animation fix)', () => {
+  test('returns the IntersectionObserver reveal script when body contains data-animation', () => {
+    const body = '<div data-animation="fade-in">Hi</div>';
+    const script = buildAnimationScript(body);
+    expect(script).toContain('<script>');
+    expect(script).toContain('IntersectionObserver');
+    expect(script).toContain("querySelectorAll('[data-animation]')");
+  });
+
+  test('returns empty string when body has no data-animation', () => {
+    expect(buildAnimationScript('<div>Hi</div>')).toBe('');
+  });
+});
+
+describe('Preview body-replacement keeps exactly one reveal script (animation fix)', () => {
+  const animatedState = JSON.stringify({
+    ROOT: {
+      type: { resolvedName: 'Container' },
+      isCanvas: true,
+      props: { tag: 'div', style: {}, animation: 'fade-in' },
+      displayName: 'Container',
+      custom: {},
+      hidden: false,
+      nodes: [],
+      linkedNodes: {},
+    },
+  });
+
+  test('wrapped doc alone already contains exactly one script + the CSS + the noscript fallback', () => {
+    const { html } = exportToHtml(animatedState, { title: 'Page' });
+    const scriptCount = (html.match(/IntersectionObserver/g) || []).length;
+    expect(scriptCount).toBe(1);
+    expect(html).toContain('[data-animation]{opacity:0}');
+    expect(html).toContain('<noscript><style>[data-animation]{opacity:1}</style></noscript>');
+  });
+
+  test('simulated handlePreview body-replacement: composed body built WITH buildAnimationScript still yields exactly one reveal script and the head CSS survives', () => {
+    // Mirror TopBar.tsx handlePreview: exportToHtml gives the wrapped doc
+    // (head CSS/noscript + its own in-body script); a "composedBody" of
+    // header+body+footer (no script of its own) is what actually replaces
+    // the <body> inner. Without appending buildAnimationScript to
+    // composedBody, the wrapped doc's script would be clobbered and the
+    // element would never reveal.
+    const { html: wrapped } = exportToHtml(animatedState, { title: 'Page' });
+    const headerHtml = '';
+    const { html: bodyHtml } = exportBodyHtml(animatedState);
+    const footerHtml = '';
+    const composedBody =
+      headerHtml + bodyHtml + footerHtml +
+      buildAnimationScript(headerHtml + bodyHtml + footerHtml);
+
+    const bodyMatch = wrapped.match(/<body[^>]*>([\s\S]*)<\/body>/i);
+    expect(bodyMatch).toBeTruthy();
+    const finalHtml = wrapped.replace(bodyMatch![1], () => composedBody);
+
+    const scriptCount = (finalHtml.match(/IntersectionObserver/g) || []).length;
+    expect(scriptCount).toBe(1);
+    expect(finalHtml).toContain('[data-animation]{opacity:0}');
+    expect(finalHtml).toContain('data-animation="fade-in"');
+    // No malformed void-tag artifact should leak into the final assembly.
+    expect(finalHtml).not.toContain('/ data-animation');
+  });
+
+  test('non-animated body: no animation CSS, no noscript, no reveal script anywhere', () => {
+    const plainState = JSON.stringify({
+      ROOT: {
+        type: { resolvedName: 'Container' },
+        isCanvas: true,
+        props: { tag: 'div', style: {} },
+        displayName: 'Container',
+        custom: {},
+        hidden: false,
+        nodes: [],
+        linkedNodes: {},
+      },
+    });
+    const { html } = exportToHtml(plainState, { title: 'Page' });
+    expect(html).not.toContain('[data-animation]');
+    expect(html).not.toContain('<noscript>');
+    expect(html).not.toContain('IntersectionObserver');
+    expect(buildAnimationScript(exportBodyHtml(plainState).html)).toBe('');
+  });
+});
diff --git a/craft/src/utils/html-export.ts b/craft/src/utils/html-export.ts
index e0554a4..07df8ab 100644
--- a/craft/src/utils/html-export.ts
+++ b/craft/src/utils/html-export.ts
@@ -52,12 +52,29 @@ function buildDataAttrs(props: Record<string, any>): string {
 
 /**
  * Inject data attributes into the first HTML opening tag of a rendered string.
+ *
+ * For a void/self-closing tag (e.g. `<img src="x" />`) the first `>` is
+ * preceded by a `/` -- naively inserting before the `>` produces the
+ * malformed `<img ... / data-animation="...">` (attrs land AFTER the
+ * self-close slash, outside the tag). Detect that trailing `/` and insert
+ * the attrs before it instead, yielding well-formed `<img ... data-animation="..."/>`.
+ * Non-void tags (no trailing `/`) are unaffected.
  */
 function injectAttrs(html: string, attrs: string): string {
   if (!attrs) return html;
   // Find the first > of the opening tag and inject before it
   const idx = html.indexOf('>');
   if (idx === -1) return html;
+  if (idx > 0 && html[idx - 1] === '/') {
+    // Void/self-closing tag (`<img ... />`): inserting before `>` would land
+    // the attrs after the `/`, outside the tag (`<img ... / data-x="y">`).
+    // Insert before the `/` instead -- also trim any whitespace directly
+    // preceding it so we don't end up with a double space, since `attrs`
+    // already carries its own leading space(s).
+    let contentEnd = idx - 1;
+    while (contentEnd > 0 && /\s/.test(html[contentEnd - 1])) contentEnd--;
+    return html.slice(0, contentEnd) + attrs + html.slice(idx - 1);
+  }
   return html.slice(0, idx) + attrs + html.slice(idx);
 }
 
@@ -344,6 +361,22 @@ document.querySelectorAll('[data-animation]').forEach(function(el) {
 });
 </script>`;
 
+// No-JS safety net (contract "No-JS safety"): un-hides animated elements
+// when JS is disabled, so `[data-animation]{opacity:0}` never permanently
+// hides content that the reveal script would otherwise never run for.
+const ANIMATION_NOSCRIPT = `<noscript><style>[data-animation]{opacity:1}</style></noscript>`;
+
+/**
+ * Returns the reveal `<script>` (byte-identical to the shared contract, and
+ * to the backend's `generateCompiledHTML` emission) when `bodyHtml` contains
+ * an animated element, else `''`. Single source of the script string so
+ * every caller (wrapInDocument's in-body emission, and TopBar's Preview
+ * body-replacement) stays in sync.
+ */
+export function buildAnimationScript(bodyHtml: string): string {
+  return bodyHtml.includes('data-animation') ? ANIMATION_SCRIPT : '';
+}
+
 function wrapInDocument(bodyHtml: string, options: ExportOptions): string {
   const title = options.title || 'Untitled Page';
   const minify = options.minifyCss !== false;
@@ -361,10 +394,15 @@ function wrapInDocument(bodyHtml: string, options: ExportOptions): string {
   const seoMeta = buildSeoMeta(options, title);
   const tokenCss = buildTokenCss(design);
 
-  // Only include animation CSS + script if body contains data-animation
+  // Only include animation CSS + noscript fallback + script if body contains
+  // data-animation (contract gate). `buildAnimationScript` is the single
+  // source of the reveal-script string -- TopBar's Preview body-replacement
+  // uses the same helper so the two emissions never drift apart.
   const hasAnimations = bodyHtml.includes('data-animation');
   const animationBlock = hasAnimations ? animation : '';
-  const animationScript = hasAnimations ? `\n${ANIMATION_SCRIPT}` : '';
+  const animationNoscript = hasAnimations ? `\n    ${ANIMATION_NOSCRIPT}` : '';
+  const revealScript = buildAnimationScript(bodyHtml);
+  const animationScript = revealScript ? `\n${revealScript}` : '';
 
   return `<!DOCTYPE html>
 <html lang="en">
@@ -372,7 +410,7 @@ function wrapInDocument(bodyHtml: string, options: ExportOptions): string {
     <meta charset="UTF-8">
     <meta name="viewport" content="width=device-width, initial-scale=1.0">
     ${seoMeta}${fonts}
-    <style>${reset}${responsive}${visibility}${animationBlock}${tokenCss}</style>${headCode}
+    <style>${reset}${responsive}${visibility}${animationBlock}${tokenCss}</style>${animationNoscript}${headCode}
 </head>
 <body>
 ${bodyHtml}${animationScript}
-- 
2.52.0

