From 92b753072930a860da051a38d514be17c3fc12c9 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-UIUX Date: Thu, 14 May 2026 12:50:37 +0000 Subject: [PATCH] fix(canvas/ThemeToggle): resolve 5 pre-existing INDEX_SIZE_ERR test errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: handleKeyDown used querySelectorAll("> [role=radio]") to find the next radio button after a key press. jsdom's selector parser throws INDEX_SIZE_ERR on the child-combinator selector in test environments, which @asamuzakjp/dom-selector surfaces as SyntaxError. The error always fired after the last keyboard-navigation test in each describe block (ArrowRight, ArrowLeft, ArrowDown, Home, End = 5 errors) and was non-fatal to the test pass count (18/18 still passed). Fix: 1. Replace querySelectorAll("> [role=radio]") with Array.from(radiogroup.children).filter(el => el.tagName === "BUTTON" && el.getAttribute("role") === "radio" ) — avoids the child-combinator selector entirely. 2. Guard the focus call with isConnected check to survive React StrictMode double-invocation of the handler during re-render. 3. Add bounds check (next < btns.length) before accessing btns[next]. Result: 18/18 pass, 0 errors (was 18/18 pass, 5 errors). Co-Authored-By: Claude Opus 4.7 --- canvas/src/components/ThemeToggle.tsx | 14 ++++++++++++-- .../components/__tests__/ThemeToggle.test.tsx | 18 +++++++++++------- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/canvas/src/components/ThemeToggle.tsx b/canvas/src/components/ThemeToggle.tsx index 5c8cfaec..12bf310f 100644 --- a/canvas/src/components/ThemeToggle.tsx +++ b/canvas/src/components/ThemeToggle.tsx @@ -65,9 +65,19 @@ export function ThemeToggle({ className = "" }: { className?: string }) { // Use direct-child query to scope strictly to this radiogroup's buttons // and avoid accidentally focusing unrelated [role=radio] elements // elsewhere in the DOM (e.g. React Flow canvas nodes). + // Guard: skip focus if the current target is no longer in the document + // (e.g. React StrictMode double-invokes handlers during re-render). + if (!e.currentTarget.isConnected) return; const radiogroup = e.currentTarget.closest("[role=radiogroup]") as HTMLElement | null; - const btns = radiogroup?.querySelectorAll("> [role=radio]"); - btns?.[next]?.focus(); + // Use children[] instead of querySelectorAll("> [role=radio]") to avoid + // jsdom's child-combinator selector parsing issues in test environments. + const btns = radiogroup + ? Array.from(radiogroup.children).filter( + (el): el is HTMLButtonElement => + el.tagName === "BUTTON" && el.getAttribute("role") === "radio" + ) + : []; + if (next < btns.length) btns[next]?.focus(); }, [] ); diff --git a/canvas/src/components/__tests__/ThemeToggle.test.tsx b/canvas/src/components/__tests__/ThemeToggle.test.tsx index 4128d3d7..08b875a4 100644 --- a/canvas/src/components/__tests__/ThemeToggle.test.tsx +++ b/canvas/src/components/__tests__/ThemeToggle.test.tsx @@ -24,8 +24,12 @@ vi.mock("@/lib/theme-provider", () => ({ })), })); +// Wrap cleanup in act() so any pending React state updates (e.g. from +// keyDown handlers that call setTheme) flush before DOM unmount. Without +// this, cleanup() can race against pending renders and cause INDEX_SIZE_ERR +// when the handleKeyDown callback tries to query the DOM mid-teardown. afterEach(() => { - cleanup(); + act(() => { cleanup(); }); vi.clearAllMocks(); }); @@ -146,7 +150,7 @@ describe("ThemeToggle — keyboard navigation (WCAG 2.1.1 / ARIA radiogroup)", ( const radios = screen.getAllByRole("radio"); // dark (index 2) is current; ArrowRight should wrap to light (index 0) act(() => { radios[2].focus(); }); - fireEvent.keyDown(radios[2], { key: "ArrowRight" }); + act(() => { fireEvent.keyDown(radios[2], { key: "ArrowRight" }); }); expect(mockSetTheme).toHaveBeenCalledWith("light"); }); @@ -160,7 +164,7 @@ describe("ThemeToggle — keyboard navigation (WCAG 2.1.1 / ARIA radiogroup)", ( const radios = screen.getAllByRole("radio"); // light (index 0) is current; ArrowLeft should go to dark (index 2) act(() => { radios[0].focus(); }); - fireEvent.keyDown(radios[0], { key: "ArrowLeft" }); + act(() => { fireEvent.keyDown(radios[0], { key: "ArrowLeft" }); }); expect(mockSetTheme).toHaveBeenCalledWith("dark"); }); @@ -174,7 +178,7 @@ describe("ThemeToggle — keyboard navigation (WCAG 2.1.1 / ARIA radiogroup)", ( const radios = screen.getAllByRole("radio"); // light (index 0) is current; ArrowDown should go to system (index 1) act(() => { radios[0].focus(); }); - fireEvent.keyDown(radios[0], { key: "ArrowDown" }); + act(() => { fireEvent.keyDown(radios[0], { key: "ArrowDown" }); }); expect(mockSetTheme).toHaveBeenCalledWith("system"); }); @@ -187,7 +191,7 @@ describe("ThemeToggle — keyboard navigation (WCAG 2.1.1 / ARIA radiogroup)", ( render(); const radios = screen.getAllByRole("radio"); act(() => { radios[2].focus(); }); - fireEvent.keyDown(radios[2], { key: "Home" }); + act(() => { fireEvent.keyDown(radios[2], { key: "Home" }); }); expect(mockSetTheme).toHaveBeenCalledWith("light"); }); @@ -200,14 +204,14 @@ describe("ThemeToggle — keyboard navigation (WCAG 2.1.1 / ARIA radiogroup)", ( render(); const radios = screen.getAllByRole("radio"); act(() => { radios[0].focus(); }); - fireEvent.keyDown(radios[0], { key: "End" }); + act(() => { fireEvent.keyDown(radios[0], { key: "End" }); }); expect(mockSetTheme).toHaveBeenCalledWith("dark"); }); it("does nothing on unrelated keys", () => { render(); const radios = screen.getAllByRole("radio"); - fireEvent.keyDown(radios[0], { key: "Enter" }); + act(() => { fireEvent.keyDown(radios[0], { key: "Enter" }); }); expect(mockSetTheme).not.toHaveBeenCalled(); }); });