From 2a4f8499406ca99ff736a4998d68eeb4532d739a Mon Sep 17 00:00:00 2001 From: Tanish Manku Date: Fri, 1 May 2026 21:55:23 -0700 Subject: [PATCH] Upload modals: stop remounting children on every parent re-render MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `UploaderModalContent` was defined inline in each upload modal's function body and rendered as ``. Each parent re-render created a fresh component identity, which React treats as a new component type — unmounting the old subtree and mounting a new one. PageScene's many zustand subscriptions fire often enough that this remount sometimes happens between mousedown and mouseup of a click on the file uploader's drag-drop zone. Result: original `` ("Upload a file" link) is destroyed mid-click, mouseup lands on the modal wrapper instead of ``, WebView2 refuses to synthesize `click`, and the library's native click listener that opens the OS file picker never fires. Other pages have lighter parent re-render frequency, so the remount usually doesn't land inside a click window. Render `UploaderModalContent` as a function call (`{UploaderModalContent()}`) instead of a component instance. The function reference still changes per render but React just sees the returned JSX inline — no component identity, no remount. Same fix in all five upload modals so the bug doesn't recur on any of them. Also strips the temporary [click-trace] debug logger from PageEditor.tsx (which identified the cause via mid-click DOM mutations). --- .../reusable/UploadModal3D/UploadModal3D.tsx | 2 +- .../UploadModalImage/UploadModalImage.tsx | 2 +- .../UploadImageMediaModal.tsx | 2 +- .../UploadModalMedia/UploadModalMedia.tsx | 2 +- .../UploadModalSplat/UploadModalSplat.tsx | 2 +- .../app/src/pages/PageScene/PageEditor.tsx | 119 ------------------ 6 files changed, 5 insertions(+), 124 deletions(-) diff --git a/frontend/apps/artcraft/app/src/components/reusable/UploadModal3D/UploadModal3D.tsx b/frontend/apps/artcraft/app/src/components/reusable/UploadModal3D/UploadModal3D.tsx index b7b00e9dc6..feeddceb7b 100644 --- a/frontend/apps/artcraft/app/src/components/reusable/UploadModal3D/UploadModal3D.tsx +++ b/frontend/apps/artcraft/app/src/components/reusable/UploadModal3D/UploadModal3D.tsx @@ -165,7 +165,7 @@ export function UploadModal3D(props: Props) { className="max-w-xl" showClose={true} > - + {UploaderModalContent()} ); } diff --git a/frontend/apps/artcraft/app/src/components/reusable/UploadModalImage/UploadModalImage.tsx b/frontend/apps/artcraft/app/src/components/reusable/UploadModalImage/UploadModalImage.tsx index c90c9d8a69..8fd43101a3 100644 --- a/frontend/apps/artcraft/app/src/components/reusable/UploadModalImage/UploadModalImage.tsx +++ b/frontend/apps/artcraft/app/src/components/reusable/UploadModalImage/UploadModalImage.tsx @@ -129,7 +129,7 @@ export function UploadModalImage(props: Props) { className="max-w-xl" showClose={true} > - + {UploaderModalContent()} ); } diff --git a/frontend/apps/artcraft/app/src/components/reusable/UploadModalMedia/UploadImageMediaModal.tsx b/frontend/apps/artcraft/app/src/components/reusable/UploadModalMedia/UploadImageMediaModal.tsx index 34e373287c..0e029e0cee 100644 --- a/frontend/apps/artcraft/app/src/components/reusable/UploadModalMedia/UploadImageMediaModal.tsx +++ b/frontend/apps/artcraft/app/src/components/reusable/UploadModalMedia/UploadImageMediaModal.tsx @@ -113,7 +113,7 @@ export function UploadImageMediaModal({ return ( - + {UploaderModalContent()} ); } diff --git a/frontend/apps/artcraft/app/src/components/reusable/UploadModalMedia/UploadModalMedia.tsx b/frontend/apps/artcraft/app/src/components/reusable/UploadModalMedia/UploadModalMedia.tsx index 53c75ebc99..10009a191c 100644 --- a/frontend/apps/artcraft/app/src/components/reusable/UploadModalMedia/UploadModalMedia.tsx +++ b/frontend/apps/artcraft/app/src/components/reusable/UploadModalMedia/UploadModalMedia.tsx @@ -100,7 +100,7 @@ export function UploadModalMedia({ return ( - + {UploaderModalContent()} ); } diff --git a/frontend/apps/artcraft/app/src/components/reusable/UploadModalSplat/UploadModalSplat.tsx b/frontend/apps/artcraft/app/src/components/reusable/UploadModalSplat/UploadModalSplat.tsx index 33099a2966..dd1adb5e2a 100644 --- a/frontend/apps/artcraft/app/src/components/reusable/UploadModalSplat/UploadModalSplat.tsx +++ b/frontend/apps/artcraft/app/src/components/reusable/UploadModalSplat/UploadModalSplat.tsx @@ -137,7 +137,7 @@ export function UploadModalSplat(props: Props) { className="max-w-xl" showClose={true} > - + {UploaderModalContent()} ); } diff --git a/frontend/apps/artcraft/app/src/pages/PageScene/PageEditor.tsx b/frontend/apps/artcraft/app/src/pages/PageScene/PageEditor.tsx index c3bfc595d3..0227f8e9db 100644 --- a/frontend/apps/artcraft/app/src/pages/PageScene/PageEditor.tsx +++ b/frontend/apps/artcraft/app/src/pages/PageScene/PageEditor.tsx @@ -123,125 +123,6 @@ export const PageEditor = () => { } }; - // TEMP debug: trace click / mousedown / pointerdown events at every - // listenable phase so we can locate whatever is silencing the - // upload-modal file-picker click. Remove once the bug is fixed. - useEffect(() => { - const describe = (t: EventTarget | null): string => { - if (!(t instanceof HTMLElement)) return String(t); - const id = t.id ? `#${t.id}` : ""; - const cls = - typeof t.className === "string" && t.className - ? `.${t.className.split(/\s+/).slice(0, 2).join(".")}` - : ""; - const label = - t.closest("label")?.getAttribute("for") ?? - (t.closest("label") ? "(unnamed)" : null); - return `${t.tagName.toLowerCase()}${id}${cls}${label ? ` [in label for=${label}]` : ""}`; - }; - - const trace = (phase: string, type: string) => (e: Event) => { - // eslint-disable-next-line no-console - console.debug( - `[click-trace] ${type} ${phase}`, - { - target: describe(e.target), - currentTarget: describe(e.currentTarget), - defaultPrevented: e.defaultPrevented, - bubbles: e.bubbles, - composedPath: (e as any).composedPath?.()?.slice(0, 6).map(describe), - }, - ); - }; - - const types: Array< - "click" | "mousedown" | "mouseup" | "pointerdown" | "pointerup" - > = ["click", "mousedown", "mouseup", "pointerdown", "pointerup"]; - const handlers = types.flatMap((type) => { - const cap = trace("CAPTURE", type); - const bub = trace("BUBBLE", type); - document.addEventListener(type, cap, true); - document.addEventListener(type, bub, false); - return [ - () => document.removeEventListener(type, cap, true), - () => document.removeEventListener(type, bub, false), - ]; - }); - - // Monkey-patch setPointerCapture to see who's grabbing the pointer - // (a mid-click capture explains the missing click — mouseup fires - // on the captured element instead of the original mousedown target). - const proto = Element.prototype as any; - const originalSet = proto.setPointerCapture; - proto.setPointerCapture = function (id: number) { - // eslint-disable-next-line no-console - console.debug("[click-trace] setPointerCapture", { - on: describe(this), - pointerId: id, - stack: new Error().stack?.split("\n").slice(1, 6).join("\n"), - }); - return originalSet.call(this, id); - }; - - // While a click is in flight (mousedown→mouseup), watch the entire - // body subtree for DOM mutations. If something re-renders / removes - // / repositions the element under the cursor mid-click, the mouseup - // target won't match mousedown — that's the actual root cause. - let activeObserver: MutationObserver | null = null; - let mutationCount = 0; - const onTrackedDown = (e: Event) => { - if (e.type !== "mousedown") return; - mutationCount = 0; - activeObserver = new MutationObserver((muts) => { - muts.forEach((m) => { - mutationCount += 1; - if (mutationCount > 30) return; - // eslint-disable-next-line no-console - console.debug("[click-trace] DOM mutation mid-click", { - type: m.type, - target: describe(m.target as any), - attributeName: m.attributeName, - oldValue: m.oldValue, - addedNodes: Array.from(m.addedNodes).map((n) => describe(n as any)), - removedNodes: Array.from(m.removedNodes).map((n) => describe(n as any)), - }); - }); - }); - activeObserver.observe(document.body, { - attributes: true, - attributeOldValue: true, - childList: true, - subtree: true, - characterData: true, - }); - }; - const onTrackedUp = (e: Event) => { - if (e.type !== "mouseup") return; - if (activeObserver) { - // eslint-disable-next-line no-console - console.debug( - `[click-trace] total mutations during click: ${mutationCount}`, - ); - activeObserver.disconnect(); - activeObserver = null; - } - }; - document.addEventListener("mousedown", onTrackedDown, true); - document.addEventListener("mouseup", onTrackedUp, true); - - // eslint-disable-next-line no-console - console.debug("[click-trace] installed"); - return () => { - handlers.forEach((off) => off()); - proto.setPointerCapture = originalSet; - document.removeEventListener("mousedown", onTrackedDown, true); - document.removeEventListener("mouseup", onTrackedUp, true); - activeObserver?.disconnect(); - // eslint-disable-next-line no-console - console.debug("[click-trace] removed"); - }; - }, []); - useEffect(() => { window.onbeforeunload = () => { return "You may have unsaved changes.";