From 7973ee9ba8ec60e0bd3458a2e16d6b3f51da7d48 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 12 Jul 2026 13:26:46 -0700 Subject: [PATCH] fix(builder): await TemplateModal's load sequence before closing (D5) The template-load sequence (clear canvas -> add components -> add pages -> apply header/footer) ran as a chain of setTimeout callbacks, but onClose() fired synchronously right after kicking it off. The modal disappeared immediately, so "Loading..." never had a chance to show, and the staged mutations kept running against a dialog the caller had already dismissed. Convert the staged steps to async/await (a promise-based `wait()` replaces the setTimeout chaining) and only call onClose() in a finally block once the whole sequence has settled. Add a mountedRef guard so the trailing setLoading(false) is skipped if the component happens to unmount mid-sequence. End result (pages/header/footer/ design applied) is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- craft/src/panels/topbar/TemplateModal.tsx | 187 ++++++++++------------ 1 file changed, 83 insertions(+), 104 deletions(-) diff --git a/craft/src/panels/topbar/TemplateModal.tsx b/craft/src/panels/topbar/TemplateModal.tsx index 746c4e4..53c3e52 100644 --- a/craft/src/panels/topbar/TemplateModal.tsx +++ b/craft/src/panels/topbar/TemplateModal.tsx @@ -1,4 +1,4 @@ -import React, { useState, useCallback, useMemo, useEffect } from 'react'; +import React, { useState, useCallback, useMemo, useEffect, useRef } from 'react'; import { useEditor } from '@craftjs/core'; import { usePages } from '../../state/PageContext'; import { useSiteDesign } from '../../state/SiteDesignContext'; @@ -50,6 +50,15 @@ export const TemplateModal: React.FC = ({ open, onClose }) = const [applyDesign, setApplyDesign] = useState(true); const [loading, setLoading] = useState(false); + // Guards state updates after the component has unmounted mid-sequence. + const mountedRef = useRef(true); + useEffect(() => { + mountedRef.current = true; + return () => { + mountedRef.current = false; + }; + }, []); + // Close on Escape key useEffect(() => { if (!open) return; @@ -112,73 +121,63 @@ export const TemplateModal: React.FC = ({ open, onClose }) = } }, [query, actions]); + /** Promise-based `setTimeout` — lets the staged apply steps below be awaited + * in sequence instead of chained via nested callbacks. */ + const wait = useCallback((ms: number) => new Promise((resolve) => setTimeout(resolve, ms)), []); + + /** + * Switch to a zone (header/footer), clear it, and add the template's + * components, giving Craft.js a tick to settle between each step. + */ + const applyZone = useCallback( + async (switchFn: () => void, components: TemplateComponent[]) => { + switchFn(); + await wait(30); + try { + clearCanvas(); + await wait(20); + try { + addTemplateComponents(components); + } catch (e) { + console.warn('Failed to add zone components:', e); + } + } catch (e) { + console.warn('Failed to clear zone:', e); + } + await wait(30); + }, + [wait, clearCanvas, addTemplateComponents], + ); + /** * After all pages are loaded, apply header and footer template content. * Switches to header/footer editing mode, clears, adds components, then returns to firstPageId. */ const applyHeaderFooter = useCallback( - (tpl: TemplateDefinition, firstPageId: string) => { + async (tpl: TemplateDefinition, firstPageId: string) => { const hasHeader = tpl.header?.components?.length > 0; const hasFooter = tpl.footer?.components?.length > 0; - if (!hasHeader && !hasFooter) { - setLoading(false); - return; - } + if (hasHeader) await applyZone(editHeader, tpl.header.components); + if (hasFooter) await applyZone(editFooter, tpl.footer.components); - const applyZone = ( - switchFn: () => void, - components: TemplateComponent[], - next: () => void, - ) => { - switchFn(); - setTimeout(() => { - try { - clearCanvas(); - setTimeout(() => { - try { - addTemplateComponents(components); - } catch (e) { - console.warn('Failed to add zone components:', e); - } - setTimeout(next, 30); - }, 20); - } catch (e) { - console.warn('Failed to clear zone:', e); - setTimeout(next, 30); - } - }, 30); - }; - - const finish = () => { - // Switch back to the first page - switchPage(firstPageId); - setTimeout(() => setLoading(false), 30); - }; - - const doFooter = () => { - if (hasFooter) { - applyZone(editFooter, tpl.footer.components, finish); - } else { - finish(); - } - }; - - if (hasHeader) { - applyZone(editHeader, tpl.header.components, doFooter); - } else { - doFooter(); - } + // Switch back to the first page + switchPage(firstPageId); + await wait(30); }, - [clearCanvas, addTemplateComponents, switchPage, editHeader, editFooter], + [applyZone, switchPage, editHeader, editFooter, wait], ); - // Load the selected template - const handleLoad = useCallback(() => { + // Load the selected template. Awaits the full staged sequence (clear, add + // components, add pages, apply header/footer) before closing the modal, so + // "Loading…" stays visible for the duration and onClose() never fires + // while the sequence is still mutating a canvas the dialog just tore down. + const handleLoad = useCallback(async () => { if (!confirmTemplate) return; setLoading(true); const tpl = confirmTemplate; + setConfirmTemplate(null); try { // 1. Optionally apply design tokens @@ -195,66 +194,46 @@ export const TemplateModal: React.FC = ({ open, onClose }) = // 3. Clear the current canvas and add the first template page's components clearCanvas(); + // Short delay to let the clear settle before adding new nodes + await wait(20); - // Use a short delay to let the clear settle before adding new nodes - setTimeout(() => { - try { - const firstPage = tpl.pages[0]; - addTemplateComponents(firstPage.content.components); + const firstPage = tpl.pages[0]; + addTemplateComponents(firstPage.content.components); - // 4. Add remaining pages (if multi-page template) - const finishPages = () => { - // 5. Apply header and footer template content - if (keepId) { - applyHeaderFooter(tpl, keepId); - } else { - setLoading(false); - } - }; + // 4. Add remaining pages (if multi-page template) + if (tpl.pages.length > 1) { + await wait(30); + for (let pageIndex = 1; pageIndex < tpl.pages.length; pageIndex++) { + const pageDef = tpl.pages[pageIndex]; + addPage(pageDef.name, pageDef.slug); - if (tpl.pages.length > 1) { - let pageIndex = 1; - const addNextPage = () => { - if (pageIndex >= tpl.pages.length) { - // Switch back to first page, then apply header/footer - if (keepId) switchPage(keepId); - setTimeout(finishPages, 30); - return; - } - - const pageDef = tpl.pages[pageIndex]; - addPage(pageDef.name, pageDef.slug); - - // After addPage, the new page is active with an empty canvas. - // Give a tick for Craft.js to settle, then add components. - setTimeout(() => { - try { - addTemplateComponents(pageDef.content.components); - } catch (e) { - console.warn('Failed to add components for page', pageDef.name, e); - } - pageIndex++; - setTimeout(addNextPage, 30); - }, 30); - }; - - setTimeout(addNextPage, 30); - } else { - finishPages(); + // After addPage, the new page is active with an empty canvas. + // Give a tick for Craft.js to settle, then add components. + await wait(30); + try { + addTemplateComponents(pageDef.content.components); + } catch (e) { + console.warn('Failed to add components for page', pageDef.name, e); } - } catch (e) { - console.error('Failed to load template:', e); - setLoading(false); + await wait(30); } - }, 20); + + // Switch back to the first page before applying header/footer + if (keepId) switchPage(keepId); + await wait(30); + } + + // 5. Apply header and footer template content + if (keepId) { + await applyHeaderFooter(tpl, keepId); + } } catch (e) { console.error('Failed to load template:', e); - setLoading(false); + } finally { + if (mountedRef.current) setLoading(false); + onClose(); } - - setConfirmTemplate(null); - onClose(); - }, [confirmTemplate, applyDesign, query, actions, pages, addPage, switchPage, deletePage, updateDesign, addTemplateComponents, clearCanvas, applyHeaderFooter, onClose]); + }, [confirmTemplate, applyDesign, pages, addPage, switchPage, deletePage, updateDesign, addTemplateComponents, clearCanvas, applyHeaderFooter, wait, onClose]); // Close on backdrop click const handleBackdropClick = useCallback(