From 5b334c9181c2c0821d1891248d798f9154db67da Mon Sep 17 00:00:00 2001 From: dev_agent Date: Wed, 7 Oct 2026 04:23:33 -0400 Subject: [PATCH] Keep focus and the confirmation in place around a Quest log removal After a removal, focus moves to the remove button of the dinner now in the removed one's place, the one before it when it was the last, or the Quest log sheet when none is left, instead of the removed row's detached button. While the DELETE is pending, the confirmation can no longer be dismissed: Confirm gains a dismissible prop that disables the cancel button and ignores Escape and the scrim. Co-Authored-By: Claude Opus 5.5 --- web/src/discover/DiscoverPage.test.tsx | 60 ++++++++++++++++++++++++++ web/src/discover/QuestLogSheet.tsx | 44 +++++++++++++++++-- web/src/settings/LeaveGuard.tsx | 11 ++++- 3 files changed, 109 insertions(+), 6 deletions(-) diff --git a/web/src/discover/DiscoverPage.test.tsx b/web/src/discover/DiscoverPage.test.tsx index 6060513..f4591f2 100644 --- a/web/src/discover/DiscoverPage.test.tsx +++ b/web/src/discover/DiscoverPage.test.tsx @@ -618,6 +618,66 @@ describe('DiscoverPage', () => { expect(screen.getByRole('button', { name: 'Quest log · 1' })).toBeInTheDocument() }) + it('keeps focus in the open Quest log after a removal: on the next row, the one before the last, then the sheet', async () => { + const archive = (id: string) => () => { + server.questLogDinners.dinners = server.questLogDinners.dinners.filter((dinner) => dinner !== byId[id]) + return jsonResponse(200, { ...byId[id]!, status: 'archived' }) + } + const byId: Record = { q1: PAD_THAI, q2: GUMBO, q3: RISOTTO } + const server = stubQuestLog([PAD_THAI, GUMBO, RISOTTO], { + 'DELETE /api/v1/dinners/q1': archive('q1'), + 'DELETE /api/v1/dinners/q2': archive('q2'), + 'DELETE /api/v1/dinners/q3': archive('q3'), + }) + await renderPage() + const sheet = await openSheet(3) + + async function removeAndExpectFocus(name: string, expected: () => HTMLElement) { + within(sheet).getByRole('button', { name: `Remove ${name}` }).focus() + fireEvent.click(within(sheet).getByRole('button', { name: `Remove ${name}` })) + fireEvent.click(within(screen.getByRole('dialog', { name: `Remove ${name}?` })).getByRole('button', { name: 'Remove' })) + await waitFor(() => expect(screen.queryByRole('dialog', { name: `Remove ${name}?` })).not.toBeInTheDocument()) + expect(expected()).toHaveFocus() + expect(sheet).toContainElement(document.activeElement as HTMLElement) + } + + // A middle row: focus goes to the row now in its place. + await removeAndExpectFocus('Shrimp gumbo', () => within(sheet).getByRole('button', { name: 'Remove Mushroom risotto' })) + // The last row: focus goes to the row before it. + await removeAndExpectFocus('Mushroom risotto', () => within(sheet).getByRole('button', { name: 'Remove Pad thai' })) + // The only row: focus goes to the sheet itself. + await removeAndExpectFocus('Pad thai', () => sheet) + expect(within(sheet).getByText('Your Quest log is empty — swipe right on a side quest to add one.')).toBeInTheDocument() + + // From there Escape still reaches the Quest log, and closes it. + fireEvent.keyDown(document.activeElement!, { key: 'Escape' }) + 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 () => { + 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?' }) + 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() + + fireEvent.click(keepIt) + fireEvent.keyDown(confirm, { key: 'Escape' }) + fireEvent.click(confirm.previousElementSibling!) + 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']) + }) + it('keeps a dinner whose removal the server refuses, saying why', async () => { const server = stubQuestLog([PAD_THAI, GUMBO], { 'DELETE /api/v1/dinners/q1': () => errorResponse(500, 'internal') }) await renderPage() diff --git a/web/src/discover/QuestLogSheet.tsx b/web/src/discover/QuestLogSheet.tsx index 2f34608..2e63c20 100644 --- a/web/src/discover/QuestLogSheet.tsx +++ b/web/src/discover/QuestLogSheet.tsx @@ -1,5 +1,5 @@ import { Trash2 } from 'lucide-react' -import { useId, useState } from 'react' +import { useEffect, useId, useRef, useState } from 'react' import { useNavigate } from 'react-router' import { Confirm } from '../settings/LeaveGuard.tsx' import { Icon } from '../ui/Icon.tsx' @@ -37,6 +37,25 @@ export function QuestLogSheet({ open, dinners, onClose }: QuestLogSheetProps) { const remove = useRemoveFromQuestLog() // The dinner whose removal is being confirmed. const [removing, setRemoving] = useState(null) + // The dinner just removed and its place in the list, until focus has + // moved on from it. + const removed = useRef<{ id: string; index: number } | null>(null) + const removeButtons = useRef(new Map()) + const sheetRef = useRef(null) + + // 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 + // one before it when it was the last, or to the Quest log sheet itself when + // none is left. Otherwise it would be left on the removed dinner's button, + // which the confirmation hands it back to, and which is then gone. + useEffect(() => { + const done = removed.current + if (removing !== null || done === null || dinners.some((d) => d.id === done.id)) return + removed.current = null + const next = dinners[Math.min(done.index, dinners.length - 1)] + if (next) removeButtons.current.get(next.id)!.focus() + else sheetRef.current!.closest('[role="dialog"]')!.focus() + }, [removing, dinners]) function cancelRemove() { remove.reset() @@ -46,7 +65,7 @@ export function QuestLogSheet({ open, dinners, onClose }: QuestLogSheetProps) { return ( <> -
+

Quest log

@@ -67,7 +86,15 @@ export function QuestLogSheet({ open, dinners, onClose }: QuestLogSheetProps) { meta={details(dinner)} onClick={() => navigate(`/dinner/${encodeURIComponent(dinner.id)}`)} trailing={ - setRemoving(dinner)}> + { + if (button) removeButtons.current.set(dinner.id, button) + else removeButtons.current.delete(dinner.id) + }} + label={`Remove ${dinner.name}`} + tone="neutral" + onClick={() => setRemoving(dinner)} + > } @@ -84,9 +111,18 @@ export function QuestLogSheet({ open, dinners, onClose }: QuestLogSheetProps) { confirmLabel={remove.isPending ? 'Removing…' : 'Remove'} cancelLabel="Keep it" busy={remove.isPending} + // Once the removal is sent, it stands: the confirmation stays until it settles. + dismissible={!remove.isPending} error={remove.isError ? `Couldn't remove it. ${remove.error.message}` : null} onCancel={cancelRemove} - onConfirm={() => remove.mutate(removing!, { onSuccess: () => setRemoving(null) })} + onConfirm={() => + remove.mutate(removing!, { + onSuccess: (_data, dinner) => { + removed.current = { id: dinner.id, index: dinners.findIndex((d) => d.id === dinner.id) } + setRemoving(null) + }, + }) + } > It leaves your Quest log for good. diff --git a/web/src/settings/LeaveGuard.tsx b/web/src/settings/LeaveGuard.tsx index 35ae711..5938f1c 100644 --- a/web/src/settings/LeaveGuard.tsx +++ b/web/src/settings/LeaveGuard.tsx @@ -36,6 +36,9 @@ type ConfirmProps = { confirmLabel: string cancelLabel?: string busy?: boolean + // When false, the cancel button, Escape and the scrim do nothing, so the + // sheet stays until the action settles. + dismissible?: boolean error?: string | null onCancel: () => void onConfirm: () => void @@ -49,13 +52,17 @@ export function Confirm({ confirmLabel, cancelLabel = 'Cancel', busy = false, + dismissible = true, error = null, onCancel, onConfirm, }: ConfirmProps) { const titleId = useId() + function dismiss() { + if (dismissible) onCancel() + } return ( - +

{title} @@ -67,7 +74,7 @@ export function Confirm({

)}
-