From ba83ad6df055d5a35b147fd8967304acf68ec45b Mon Sep 17 00:00:00 2001 From: dev_agent Date: Wed, 7 Oct 2026 04:39:55 -0400 Subject: [PATCH] Keep focus in a confirmation when a disabled button drops it to the page Confirm moved focus to its dialog only when the focused control stayed focused while disabled. A browser that moves focus to the page as it disables the control left the confirmation open with focus outside it. Confirm now also takes focus back to the dialog when busy or dismissible changes and focus is on the page. Co-Authored-By: Claude Opus 5.5 --- web/src/discover/DiscoverPage.test.tsx | 19 +++++++++++++++++-- web/src/settings/LeaveGuard.tsx | 8 +++++--- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/web/src/discover/DiscoverPage.test.tsx b/web/src/discover/DiscoverPage.test.tsx index ef3e8d7..f1f73f0 100644 --- a/web/src/discover/DiscoverPage.test.tsx +++ b/web/src/discover/DiscoverPage.test.tsx @@ -1,7 +1,7 @@ import { QueryClientProvider } from '@tanstack/react-query' import { act, fireEvent, render, screen, waitFor, within } from '@testing-library/react' import { createMemoryRouter, RouterProvider, useParams } from 'react-router' -import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, onTestFinished, vi, type Mock } from 'vitest' import { ApiProvider, createApiClient, createQueryClient, createVerifiedSession } from '../api/index.ts' import { createNetworkStatus } from '../pwa/networkStatus.ts' import { stubAnimate, type FakeAnimation } from '../test/animate.ts' @@ -685,7 +685,22 @@ describe('DiscoverPage', () => { expect(sent(server.fetchMock)).toEqual(['DELETE /api/v1/dinners/q1']) }) - it('keeps focus in the confirmation, off its disabled buttons, while a removal is under way', async () => { + it.each([ + ['keeps a disabled button focused', false], + ['moves focus off a disabled button to the page', true], + ])('keeps focus in the confirmation, off its disabled buttons, while a removal is under way when the browser %s', async (_, fixup) => { + // Some browsers move focus from a control to the page as it is disabled. + const setAttribute = Element.prototype.setAttribute + if (fixup) { + vi.spyOn(Element.prototype, 'setAttribute').mockImplementation(function (this: Element, name, value) { + // jsdom ignores blur() on a disabled control, so focus leaves just before. + if (name === 'disabled' && this === document.activeElement && this instanceof HTMLElement) this.blur() + setAttribute.call(this, name, value) + }) + } + onTestFinished(() => { + vi.restoreAllMocks() + }) const archive = deferred() const server = stubQuestLog([PAD_THAI, GUMBO], { 'DELETE /api/v1/dinners/q1': () => archive.promise }) await renderPage() diff --git a/web/src/settings/LeaveGuard.tsx b/web/src/settings/LeaveGuard.tsx index 8081cb2..26ded91 100644 --- a/web/src/settings/LeaveGuard.tsx +++ b/web/src/settings/LeaveGuard.tsx @@ -59,12 +59,14 @@ export function Confirm({ }: ConfirmProps) { const titleId = useId() const sheetRef = useRef(null) - // A focused button that becomes disabled loses focus to the page, outside - // the dialog; the dialog takes it instead. + // A focused button that becomes disabled either keeps focus while disabled or + // loses it to the page, depending on the browser; the dialog takes it instead. useLayoutEffect(() => { const dialog = sheetRef.current?.closest('[role="dialog"]') + if (!dialog) return const active = document.activeElement - if (dialog && active instanceof HTMLElement && active.matches(':disabled') && dialog.contains(active)) dialog.focus() + const lost = active === null || active === document.body || (dialog.contains(active) && active.matches(':disabled')) + if (lost) dialog.focus() }, [busy, dismissible]) function dismiss() { if (dismissible) onCancel()