From 1317c4df8155fcbdc6913706af7de8fe78305cf8 Mon Sep 17 00:00:00 2001 From: Mitch5000 Date: Mon, 27 Jul 2026 09:30:53 +0100 Subject: [PATCH] ConfirmDialog's openDialogCount FIX --- src/components/ConfirmDialog.test.tsx | 32 +++++++++++++++++++++++-- src/components/ConfirmDialog.tsx | 16 ++++++++----- src/hooks/useFocusShortcut.test.ts | 34 +++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 8 deletions(-) diff --git a/src/components/ConfirmDialog.test.tsx b/src/components/ConfirmDialog.test.tsx index 8a336b6..71c3d0f 100644 --- a/src/components/ConfirmDialog.test.tsx +++ b/src/components/ConfirmDialog.test.tsx @@ -1,6 +1,14 @@ -import { describe, it, expect, vi } from "vitest"; +import { describe, it, expect, vi, afterEach } from "vitest"; import { render, screen, fireEvent } from "@testing-library/react"; import { ConfirmDialog } from "./ConfirmDialog"; +import { + isConfirmDialogOpen, + resetConfirmDialogOpenState, +} from "./confirmDialogOpenState"; + +afterEach(() => { + resetConfirmDialogOpenState(); +}); describe("ConfirmDialog", () => { it("renders nothing when closed", () => { @@ -31,6 +39,24 @@ describe("ConfirmDialog", () => { expect(screen.getByText('Deactivate anchor "a"?')).toBeInTheDocument(); }); + it("clears the tracked open-dialog state when unmounted while open", () => { + const { unmount } = render( + {}} + onCancel={() => {}} + />, + ); + + expect(isConfirmDialogOpen()).toBe(true); + + unmount(); + + expect(isConfirmDialogOpen()).toBe(false); + }); + it("uses default button labels unless overridden", () => { render( { onCancel={onCancel} />, ); - fireEvent.click(screen.getByRole("alertdialog").parentElement as HTMLElement); + fireEvent.click( + screen.getByRole("alertdialog").parentElement as HTMLElement, + ); expect(onCancel).toHaveBeenCalledTimes(1); }); diff --git a/src/components/ConfirmDialog.tsx b/src/components/ConfirmDialog.tsx index 3133cf0..71810f3 100644 --- a/src/components/ConfirmDialog.tsx +++ b/src/components/ConfirmDialog.tsx @@ -28,14 +28,18 @@ export function ConfirmDialog({ const confirmRef = useRef(null); const triggerRef = useRef(null); - // Capture the currently focused element when dialog opens + // Capture the currently focused element when dialog opens, and keep the + // module-level open-dialog counter balanced if this component unmounts while + // still open (for example, during navigation away from the current route). useEffect(() => { - if (open) { - triggerRef.current = document.activeElement as HTMLElement; - markConfirmDialogOpen(); - } else { + if (!open) return; + + triggerRef.current = document.activeElement as HTMLElement; + markConfirmDialogOpen(); + + return () => { markConfirmDialogClosed(); - } + }; }, [open]); // Focus management diff --git a/src/hooks/useFocusShortcut.test.ts b/src/hooks/useFocusShortcut.test.ts index 190bdb3..30d0deb 100644 --- a/src/hooks/useFocusShortcut.test.ts +++ b/src/hooks/useFocusShortcut.test.ts @@ -100,6 +100,40 @@ describe("useFocusShortcut", () => { document.body.removeChild(outsideButton); }); + it("focuses the target element after an open confirm dialog unmounts during navigation", () => { + const searchInput = document.createElement("input"); + const outsideButton = document.createElement("button"); + document.body.appendChild(searchInput); + document.body.appendChild(outsideButton); + outsideButton.focus(); + + const ref = createRef(); + ref.current = searchInput; + + renderHook(() => useFocusShortcut("/", ref)); + const dialog = render( + createElement(ConfirmDialog, { + open: true, + title: "Deactivate anchor", + message: "Are you sure?", + onConfirm: () => {}, + onCancel: () => {}, + }), + ); + + pressKey("/"); + expect(document.activeElement).toBe(screen.getByText("Cancel")); + + dialog.unmount(); + + outsideButton.focus(); + pressKey("/"); + expect(document.activeElement).toBe(searchInput); + + document.body.removeChild(searchInput); + document.body.removeChild(outsideButton); + }); + it("when multiple instances are bound to the same key, the last mounted instance retains focus", () => { const input1 = document.createElement("input"); const input2 = document.createElement("input");