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 <noreply@anthropic.com>
This commit is contained in:
@@ -654,7 +654,11 @@ describe('DiscoverPage', () => {
|
|||||||
expect(screen.queryByRole('dialog')).not.toBeInTheDocument()
|
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 archive = deferred()
|
||||||
const server = stubQuestLog([PAD_THAI, GUMBO], { 'DELETE /api/v1/dinners/q1': () => archive.promise })
|
const server = stubQuestLog([PAD_THAI, GUMBO], { 'DELETE /api/v1/dinners/q1': () => archive.promise })
|
||||||
await renderPage()
|
await renderPage()
|
||||||
@@ -662,20 +666,46 @@ describe('DiscoverPage', () => {
|
|||||||
|
|
||||||
fireEvent.click(within(sheet).getByRole('button', { name: 'Remove Pad thai' }))
|
fireEvent.click(within(sheet).getByRole('button', { name: 'Remove Pad thai' }))
|
||||||
const confirm = screen.getByRole('dialog', { name: 'Remove Pad thai?' })
|
const confirm = screen.getByRole('dialog', { name: 'Remove Pad thai?' })
|
||||||
fireEvent.click(within(confirm).getByRole('button', { name: 'Remove' }))
|
const removeButton = within(confirm).getByRole('button', { name: 'Remove' })
|
||||||
expect(await within(confirm).findByRole('button', { name: 'Removing…' })).toBeDisabled()
|
// Straight after Remove, without waiting for the pending request to render.
|
||||||
const keepIt = within(confirm).getByRole('button', { name: 'Keep it' })
|
fireEvent.click(removeButton)
|
||||||
expect(keepIt).toBeDisabled()
|
fireEvent.click(removeButton)
|
||||||
|
dismiss(confirm)
|
||||||
|
expect(screen.getByRole('dialog', { name: 'Remove Pad thai?' })).toBe(confirm)
|
||||||
|
|
||||||
fireEvent.click(keepIt)
|
expect(await within(confirm).findByRole('button', { name: 'Removing…' })).toBeDisabled()
|
||||||
fireEvent.keyDown(confirm, { key: 'Escape' })
|
expect(within(confirm).getByRole('button', { name: 'Keep it' })).toBeDisabled()
|
||||||
fireEvent.click(confirm.previousElementSibling!)
|
dismiss(confirm)
|
||||||
expect(screen.getByRole('dialog', { name: 'Remove Pad thai?' })).toBe(confirm)
|
expect(screen.getByRole('dialog', { name: 'Remove Pad thai?' })).toBe(confirm)
|
||||||
|
|
||||||
server.questLogDinners.dinners = [GUMBO]
|
server.questLogDinners.dinners = [GUMBO]
|
||||||
await act(async () => archive.resolve(jsonResponse(200, { ...PAD_THAI, status: 'archived' })))
|
await act(async () => archive.resolve(jsonResponse(200, { ...PAD_THAI, status: 'archived' })))
|
||||||
await waitFor(() => expect(screen.queryByRole('dialog', { name: 'Remove Pad thai?' })).not.toBeInTheDocument())
|
await waitFor(() => expect(screen.queryByRole('dialog', { name: 'Remove Pad thai?' })).not.toBeInTheDocument())
|
||||||
expect(names(sheet)).toEqual(['Shrimp gumbo'])
|
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 () => {
|
it('keeps a dinner whose removal the server refuses, saying why', async () => {
|
||||||
|
|||||||
@@ -42,6 +42,11 @@ export function QuestLogSheet({ open, dinners, onClose }: QuestLogSheetProps) {
|
|||||||
const removed = useRef<{ id: string; index: number } | null>(null)
|
const removed = useRef<{ id: string; index: number } | null>(null)
|
||||||
const removeButtons = useRef(new Map<string, HTMLButtonElement>())
|
const removeButtons = useRef(new Map<string, HTMLButtonElement>())
|
||||||
const sheetRef = useRef<HTMLDivElement>(null)
|
const sheetRef = useRef<HTMLDivElement>(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,
|
// 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
|
// 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<HTMLElement>('[role="dialog"]')!.focus()
|
else sheetRef.current!.closest<HTMLElement>('[role="dialog"]')!.focus()
|
||||||
}, [removing, dinners])
|
}, [removing, dinners])
|
||||||
|
|
||||||
|
function setSending(value: boolean) {
|
||||||
|
sending.current = value
|
||||||
|
setSendingShown(value)
|
||||||
|
}
|
||||||
|
|
||||||
function cancelRemove() {
|
function cancelRemove() {
|
||||||
|
if (sending.current) return
|
||||||
remove.reset()
|
remove.reset()
|
||||||
setRemoving(null)
|
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 (
|
return (
|
||||||
<>
|
<>
|
||||||
<Sheet open={open} onClose={onClose} aria-labelledby={titleId}>
|
<Sheet open={open} onClose={onClose} aria-labelledby={titleId}>
|
||||||
@@ -108,21 +131,14 @@ export function QuestLogSheet({ open, dinners, onClose }: QuestLogSheetProps) {
|
|||||||
<Confirm
|
<Confirm
|
||||||
open={removing !== null}
|
open={removing !== null}
|
||||||
title={`Remove ${removing?.name ?? ''}?`}
|
title={`Remove ${removing?.name ?? ''}?`}
|
||||||
confirmLabel={remove.isPending ? 'Removing…' : 'Remove'}
|
confirmLabel={sendingShown ? 'Removing…' : 'Remove'}
|
||||||
cancelLabel="Keep it"
|
cancelLabel="Keep it"
|
||||||
busy={remove.isPending}
|
busy={sendingShown}
|
||||||
// Once the removal is sent, it stands: the confirmation stays until it settles.
|
// Once the removal is sent, it stands: the confirmation stays until it settles.
|
||||||
dismissible={!remove.isPending}
|
dismissible={!sendingShown}
|
||||||
error={remove.isError ? `Couldn't remove it. ${remove.error.message}` : null}
|
error={remove.isError ? `Couldn't remove it. ${remove.error.message}` : null}
|
||||||
onCancel={cancelRemove}
|
onCancel={cancelRemove}
|
||||||
onConfirm={() =>
|
onConfirm={confirmRemove}
|
||||||
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.
|
It leaves your Quest log for good.
|
||||||
</Confirm>
|
</Confirm>
|
||||||
|
|||||||
@@ -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 { useBlocker } from 'react-router'
|
||||||
import { Button } from '../ui/Button.tsx'
|
import { Button } from '../ui/Button.tsx'
|
||||||
import { Sheet } from '../ui/Sheet.tsx'
|
import { Sheet } from '../ui/Sheet.tsx'
|
||||||
@@ -58,12 +58,20 @@ export function Confirm({
|
|||||||
onConfirm,
|
onConfirm,
|
||||||
}: ConfirmProps) {
|
}: ConfirmProps) {
|
||||||
const titleId = useId()
|
const titleId = useId()
|
||||||
|
const sheetRef = useRef<HTMLDivElement>(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<HTMLElement>('[role="dialog"]')
|
||||||
|
const active = document.activeElement
|
||||||
|
if (dialog && active instanceof HTMLElement && active.matches(':disabled') && dialog.contains(active)) dialog.focus()
|
||||||
|
}, [busy, dismissible])
|
||||||
function dismiss() {
|
function dismiss() {
|
||||||
if (dismissible) onCancel()
|
if (dismissible) onCancel()
|
||||||
}
|
}
|
||||||
return (
|
return (
|
||||||
<Sheet open={open} onClose={dismiss} aria-labelledby={titleId}>
|
<Sheet open={open} onClose={dismiss} aria-labelledby={titleId}>
|
||||||
<div className={styles.sheet}>
|
<div ref={sheetRef} className={styles.sheet}>
|
||||||
<h2 id={titleId} className={styles.title}>
|
<h2 id={titleId} className={styles.title}>
|
||||||
{title}
|
{title}
|
||||||
</h2>
|
</h2>
|
||||||
|
|||||||
Reference in New Issue
Block a user