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({

)}
-