mirror of
https://github.com/storytold/artcraft.git
synced 2026-11-04 17:38:10 +00:00
Upload modals: stop remounting children on every parent re-render
`UploaderModalContent` was defined inline in each upload modal's
function body and rendered as `<UploaderModalContent />`. 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 `<u>` ("Upload a file" link) is destroyed mid-click,
mouseup lands on the modal wrapper instead of `<u>`, 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).
This commit is contained in:
1 parent
e8143057b6
commit
2a4f849940
6 files changed
+5
-124
No files matched your search
@@ -165,7 +165,7 @@ export function UploadModal3D(props: Props) {
|
||||
className="max-w-xl"
|
||||
showClose={true}
|
||||
>
|
||||
<UploaderModalContent />
|
||||
{UploaderModalContent()}
|
||||
</Modal>
|
||||
);
|
||||
}
|
||||
+1
-1
@@ -129,7 +129,7 @@ export function UploadModalImage(props: Props) {
|
||||
className="max-w-xl"
|
||||
showClose={true}
|
||||
>
|
||||
<UploaderModalContent />
|
||||
{UploaderModalContent()}
|
||||
</Modal>
|
||||
);
|
||||
}
|
||||
+1
-1
@@ -113,7 +113,7 @@ export function UploadImageMediaModal({
|
||||
|
||||
return (
|
||||
<Modal isOpen={isOpen} onClose={onClose} title={title}>
|
||||
<UploaderModalContent />
|
||||
{UploaderModalContent()}
|
||||
</Modal>
|
||||
);
|
||||
}
|
||||
+1
-1
@@ -100,7 +100,7 @@ export function UploadModalMedia({
|
||||
|
||||
return (
|
||||
<Modal isOpen={isOpen} onClose={onClose} title={title}>
|
||||
<UploaderModalContent />
|
||||
{UploaderModalContent()}
|
||||
</Modal>
|
||||
);
|
||||
}
|
||||
+1
-1
@@ -137,7 +137,7 @@ export function UploadModalSplat(props: Props) {
|
||||
className="max-w-xl"
|
||||
showClose={true}
|
||||
>
|
||||
<UploaderModalContent />
|
||||
{UploaderModalContent()}
|
||||
</Modal>
|
||||
);
|
||||
}
|
||||
@@ -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.";
|
||||
|
||||
Reference in new issue
Block a user