From c6fb09a785a66cd8cfd7afc89902c3abb6a17f0c Mon Sep 17 00:00:00 2001 From: dev_agent Date: Wed, 7 Oct 2026 04:32:18 -0400 Subject: [PATCH] Latch a Quest log removal as Remove is activated and keep focus in its confirmation The pending state reached the confirmation only on a later render, so Keep it, Escape, the scrim or a second Remove straight after Remove could still close the confirmation or send a second DELETE. QuestLogSheet now latches the removal synchronously in a ref, refuses dismissal and a repeated confirmation while it is set, and renders busy from it until the removal settles. A focused button that becomes disabled loses focus to the page, outside the dialog. Confirm now moves focus to its dialog when the focused control is disabled by busy or dismissible changing. Co-Authored-By: Claude Opus 5.5 --- web/src/discover/DiscoverPage.test.tsx | 46 +++++++++++++++++++++----- web/src/discover/QuestLogSheet.tsx | 38 +++++++++++++++------ web/src/settings/LeaveGuard.tsx | 12 +++++-- 3 files changed, 75 insertions(+), 21 deletions(-) diff --git a/web/src/discover/DiscoverPage.test.tsx b/web/src/discover/DiscoverPage.test.tsx index f4591f2..ef3e8d7 100644 --- a/web/src/discover/DiscoverPage.test.tsx +++ b/web/src/discover/DiscoverPage.test.tsx @@ -654,7 +654,11 @@ describe('DiscoverPage', () => { expect(screen.queryByRole('dialog')).not.toBeInTheDocument() }) - it('keeps the confirmation open while a removal is under way: Keep it, Escape and the scrim do nothing', async () => { + it.each([ + ['Keep it', (confirm: HTMLElement) => fireEvent.click(within(confirm).getByRole('button', { name: 'Keep it' }))], + ['Escape', (confirm: HTMLElement) => fireEvent.keyDown(confirm, { key: 'Escape' })], + ['the scrim', (confirm: HTMLElement) => fireEvent.click(confirm.previousElementSibling!)], + ])('keeps the confirmation and sends one removal once Remove is activated: %s and a second Remove at once do nothing', async (_, dismiss) => { const archive = deferred() const server = stubQuestLog([PAD_THAI, GUMBO], { 'DELETE /api/v1/dinners/q1': () => archive.promise }) await renderPage() @@ -662,20 +666,46 @@ describe('DiscoverPage', () => { fireEvent.click(within(sheet).getByRole('button', { name: 'Remove Pad thai' })) const confirm = screen.getByRole('dialog', { name: 'Remove Pad thai?' }) - fireEvent.click(within(confirm).getByRole('button', { name: 'Remove' })) - expect(await within(confirm).findByRole('button', { name: 'Removing…' })).toBeDisabled() - const keepIt = within(confirm).getByRole('button', { name: 'Keep it' }) - expect(keepIt).toBeDisabled() + const removeButton = within(confirm).getByRole('button', { name: 'Remove' }) + // Straight after Remove, without waiting for the pending request to render. + fireEvent.click(removeButton) + fireEvent.click(removeButton) + dismiss(confirm) + expect(screen.getByRole('dialog', { name: 'Remove Pad thai?' })).toBe(confirm) - fireEvent.click(keepIt) - fireEvent.keyDown(confirm, { key: 'Escape' }) - fireEvent.click(confirm.previousElementSibling!) + expect(await within(confirm).findByRole('button', { name: 'Removing…' })).toBeDisabled() + expect(within(confirm).getByRole('button', { name: 'Keep it' })).toBeDisabled() + dismiss(confirm) expect(screen.getByRole('dialog', { name: 'Remove Pad thai?' })).toBe(confirm) server.questLogDinners.dinners = [GUMBO] await act(async () => archive.resolve(jsonResponse(200, { ...PAD_THAI, status: 'archived' }))) await waitFor(() => expect(screen.queryByRole('dialog', { name: 'Remove Pad thai?' })).not.toBeInTheDocument()) expect(names(sheet)).toEqual(['Shrimp gumbo']) + 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 () => { + const archive = deferred() + const server = stubQuestLog([PAD_THAI, GUMBO], { 'DELETE /api/v1/dinners/q1': () => archive.promise }) + await renderPage() + const sheet = await openSheet(2) + + fireEvent.click(within(sheet).getByRole('button', { name: 'Remove Pad thai' })) + const confirm = screen.getByRole('dialog', { name: 'Remove Pad thai?' }) + const removeButton = within(confirm).getByRole('button', { name: 'Remove' }) + removeButton.focus() + fireEvent.click(removeButton) + expect(removeButton).toBeDisabled() + expect(confirm).toHaveFocus() + + expect(await within(confirm).findByRole('button', { name: 'Removing…' })).toBeDisabled() + expect(confirm).toHaveFocus() + + server.questLogDinners.dinners = [GUMBO] + await act(async () => archive.resolve(jsonResponse(200, { ...PAD_THAI, status: 'archived' }))) + await waitFor(() => expect(screen.queryByRole('dialog', { name: 'Remove Pad thai?' })).not.toBeInTheDocument()) + expect(within(sheet).getByRole('button', { name: 'Remove Shrimp gumbo' })).toHaveFocus() }) it('keeps a dinner whose removal the server refuses, saying why', async () => { diff --git a/web/src/discover/QuestLogSheet.tsx b/web/src/discover/QuestLogSheet.tsx index 2e63c20..cafdfa7 100644 --- a/web/src/discover/QuestLogSheet.tsx +++ b/web/src/discover/QuestLogSheet.tsx @@ -42,6 +42,11 @@ export function QuestLogSheet({ open, dinners, onClose }: QuestLogSheetProps) { const removed = useRef<{ id: string; index: number } | null>(null) const removeButtons = useRef(new Map()) const sheetRef = useRef(null) + // Whether a removal has been sent and not yet settled. The ref is set as + // Remove is activated, so a dismissal or a second Remove that arrives before + // the next render is already refused; the state renders it. + const sending = useRef(false) + const [sendingShown, setSendingShown] = useState(false) // Once a removal's confirmation has closed and the dinner has left the list, // focus goes to the remove button of the dinner now in its place, or of the @@ -57,11 +62,29 @@ export function QuestLogSheet({ open, dinners, onClose }: QuestLogSheetProps) { else sheetRef.current!.closest('[role="dialog"]')!.focus() }, [removing, dinners]) + function setSending(value: boolean) { + sending.current = value + setSendingShown(value) + } + function cancelRemove() { + if (sending.current) return remove.reset() setRemoving(null) } + function confirmRemove() { + if (sending.current) return + setSending(true) + remove.mutate(removing!, { + onSuccess: (_data, dinner) => { + removed.current = { id: dinner.id, index: dinners.findIndex((d) => d.id === dinner.id) } + setRemoving(null) + }, + onSettled: () => setSending(false), + }) + } + return ( <> @@ -108,21 +131,14 @@ export function QuestLogSheet({ open, dinners, onClose }: QuestLogSheetProps) { - remove.mutate(removing!, { - onSuccess: (_data, dinner) => { - removed.current = { id: dinner.id, index: dinners.findIndex((d) => d.id === dinner.id) } - setRemoving(null) - }, - }) - } + onConfirm={confirmRemove} > It leaves your Quest log for good. diff --git a/web/src/settings/LeaveGuard.tsx b/web/src/settings/LeaveGuard.tsx index 5938f1c..8081cb2 100644 --- a/web/src/settings/LeaveGuard.tsx +++ b/web/src/settings/LeaveGuard.tsx @@ -1,4 +1,4 @@ -import { useEffect, useId, type ReactNode } from 'react' +import { useEffect, useId, useLayoutEffect, useRef, type ReactNode } from 'react' import { useBlocker } from 'react-router' import { Button } from '../ui/Button.tsx' import { Sheet } from '../ui/Sheet.tsx' @@ -58,12 +58,20 @@ export function Confirm({ onConfirm, }: 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. + useLayoutEffect(() => { + const dialog = sheetRef.current?.closest('[role="dialog"]') + const active = document.activeElement + if (dialog && active instanceof HTMLElement && active.matches(':disabled') && dialog.contains(active)) dialog.focus() + }, [busy, dismissible]) function dismiss() { if (dismissible) onCancel() } return ( -
+

{title}