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}