Skip to content

Commit 44bb49e

Browse files
committed
fix(editor): a modal above a held source session no longer swallows every click
1 parent e376ebc commit 44bb49e

3 files changed

Lines changed: 60 additions & 14 deletions

File tree

‎src/components/Output/ZPLOutput.sourceEdit.test.tsx‎

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ vi.mock("./ZplCodeMirror", () => {
4848
return { default: MockCodeMirror };
4949
});
5050
import { ZPLOutput } from "./ZPLOutput";
51+
import { DialogShell } from "../ui/DialogShell";
5152
import { useLabelStore } from "../../store/labelStore";
5253
import type { LabelObject, Page } from "@zplab/core/types/Group";
5354

@@ -234,7 +235,7 @@ describe("leaving the editor", () => {
234235
const confirm = () => within(screen.getByRole("alertdialog"));
235236
expect(confirm().getByText(t().output.editSourceDiscardBody)).toBeTruthy();
236237
expect(review().getByRole("button", { name: t().output.editSourceConfirmApply })).toBeTruthy();
237-
// The deferred blur apply stays suspended across the handover. No apply, no second review.
238+
// Focus crossing from the review into the question must not apply, or a second review opens.
238239
await act(async () => {
239240
await new Promise((r) => setTimeout(r, 0));
240241
});
@@ -266,7 +267,7 @@ describe("leaving the editor", () => {
266267
expect(copiedTexts).toEqual(["^XA^FDedited^FS^XZ"]);
267268
});
268269

269-
it("asks before discarding a modified buffer, and the question suspends the blur apply", async () => {
270+
it("asks before discarding a modified buffer, and the question holds the blur apply", async () => {
270271
const { container } = render(<ZPLOutput onResizeMouseDown={vi.fn()} />);
271272
typeDraft("^XA^JZY^FO10,10^A0N,30,30^FDX^FS^XZ");
272273
fireEvent.click(screen.getByRole("button", { name: t().app.cancel }));
@@ -593,6 +594,54 @@ describe("outside pointerdown", () => {
593594
});
594595
});
595596

597+
describe("a modal above the panel", () => {
598+
const Modal = ({ onAction }: { onAction: () => void }) => (
599+
<DialogShell portal onClose={vi.fn()} describedBy="modal-body" boxClassName="">
600+
<p id="modal-body">settings</p>
601+
<button onClick={onAction}>setting</button>
602+
</DialogShell>
603+
);
604+
605+
it("takes clicks while an unappliable buffer holds the session", () => {
606+
const action = vi.fn();
607+
render(
608+
<>
609+
<Modal onAction={action} />
610+
<ZPLOutput onResizeMouseDown={vi.fn()} />
611+
</>,
612+
);
613+
typeDraft("^XA^FO10,10^A0N,30,30^FDbroken^FS");
614+
const setting = screen.getByRole("button", { name: "setting" });
615+
fireEvent.pointerDown(setting);
616+
fireEvent.click(setting);
617+
expect(action).toHaveBeenCalledTimes(1);
618+
expect(useLabelStore.getState().sourceEdit.status).toBe("editing");
619+
expect(useLabelStore.getState().sourceShadow?.refusal ?? null).toBeNull();
620+
});
621+
622+
it("keeps the focus it took instead of handing it back to the editor", async () => {
623+
const { container, rerender } = render(<ZPLOutput onResizeMouseDown={vi.fn()} />);
624+
typeDraft("^XA^FO10,10^A0N,30,30^FDbroken^FS");
625+
editor().focus();
626+
rerender(
627+
<>
628+
<Modal onAction={vi.fn()} />
629+
<ZPLOutput onResizeMouseDown={vi.fn()} />
630+
</>,
631+
);
632+
vi.spyOn(document, "hasFocus").mockReturnValue(true);
633+
const setting = screen.getByRole("button", { name: "setting" });
634+
setting.focus();
635+
fireEvent.focusOut(panelRoot(container));
636+
await act(async () => {
637+
await new Promise((r) => setTimeout(r, 0));
638+
});
639+
expect(useLabelStore.getState().sourceEdit.status).toBe("editing");
640+
expect(useLabelStore.getState().sourceShadow?.refusal ?? null).toBeNull();
641+
expect(document.activeElement).toBe(setting);
642+
});
643+
});
644+
596645
describe("keyboard focus after button exits", () => {
597646
it("returns to the editor on a clean footer apply", () => {
598647
render(<ZPLOutput onResizeMouseDown={vi.fn()} />);

‎src/components/Output/ZplSourceEditor.tsx‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ function SessionChrome({
199199
focusEditor();
200200
};
201201

202-
/** Returns whether the session ended (a held session swallows the click). */
202+
/** Returns whether the session ended. */
203203
const handleApply = (explicit = false): boolean => {
204204
// An untouched buffer must not commit: the reparse would still rename
205205
// variables and replace ids, pure loss for a no-op edit.
@@ -237,11 +237,7 @@ function SessionChrome({
237237
return false;
238238
};
239239

240-
useSessionExit(panelRef, {
241-
onExit: handleApply,
242-
onEscape: requestCancel,
243-
suspended: pendingPlan !== null || confirmDiscard || confirmNew,
244-
});
240+
useSessionExit(panelRef, { onExit: handleApply, onEscape: requestCancel });
245241

246242
return (
247243
<>

‎src/hooks/useSessionExit.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import { useEffect, useLayoutEffect, useRef, type RefObject } from 'react';
22

33
/** The source-edit session's exit surface on a panel root. Not useDismiss:
4-
* capture phase, focusout fallback, and a held exit (onExit false: refusal
5-
* or confirm dialog) swallows the click so nothing fires under it. */
4+
* capture phase, a focusout fallback, and a held exit that swallows the
5+
* click so nothing fires under it. */
66
export function useSessionExit(
77
panelRef: RefObject<HTMLElement | null>,
8-
handlers: { onExit: () => boolean; onEscape: () => void; suspended: boolean },
8+
handlers: { onExit: () => boolean; onEscape: () => void },
99
): void {
1010
// Latest closures for the raw DOM listeners (mounted once per session).
1111
// Layout effect: synced before paint, so no user event sees a stale closure.
@@ -18,14 +18,15 @@ export function useSessionExit(
1818
const root = panelRef.current;
1919
if (!root) return;
2020
let timer: ReturnType<typeof setTimeout> | undefined;
21+
// Under a modal's backdrop a held session would swallow every click, so a modal is no exit.
22+
const modalOpen = () => document.querySelector('[aria-modal="true"]') !== null;
2123
// Capture-phase, so the exit lands BEFORE the target's own handlers and
2224
// chrome/stage clicks act on the unfrozen store in the same click. (A
2325
// click on a canvas OBJECT still needs a second one: the apply re-ids
2426
// the Konva node under the pointer mid-gesture.)
2527
const onPointerDown = (e: PointerEvent) => {
26-
if (handlersRef.current.suspended) return;
2728
const target = e.target as Element | null;
28-
if (!target || root.contains(target)) return;
29+
if (!target || root.contains(target) || modalOpen()) return;
2930
if (!handlersRef.current.onExit()) {
3031
e.preventDefault();
3132
e.stopPropagation();
@@ -51,10 +52,10 @@ export function useSessionExit(
5152
// window blur (alt-tab) keeps the session; only focus moving elsewhere
5253
// in the app is "the one person now works the canvas".
5354
timer = setTimeout(() => {
54-
if (handlersRef.current.suspended) return;
5555
if (!document.hasFocus()) return;
5656
const active = document.activeElement;
5757
if (active && root.contains(active)) return;
58+
if (modalOpen()) return;
5859
handlersRef.current.onExit();
5960
}, 0);
6061
};

0 commit comments

Comments
 (0)