fix: submit delete confirmation on enter
This commit is contained in:
@@ -14,7 +14,7 @@
|
||||
"test": "vitest run",
|
||||
"test:watch": "vitest",
|
||||
"test:e2e": "playwright test",
|
||||
"playwright:smoke": "playwright test --config playwright.smoke.config.ts tests/smoke/example.spec.ts tests/smoke/fix-ai-chat-empty-body-v3.spec.ts tests/smoke/fix-crash-create-note.spec.ts tests/smoke/fresh-start-telemetry-onboarding.spec.ts tests/smoke/getting-started-template.spec.ts tests/smoke/h1-title-decoupled.spec.ts tests/smoke/h1-untitled-auto-rename.spec.ts tests/smoke/keyboard-command-routing.spec.ts tests/smoke/linkify-init-warnings.spec.ts tests/smoke/multi-selection-shortcuts.spec.ts tests/smoke/wikilink-path-fix.spec.ts",
|
||||
"playwright:smoke": "playwright test --config playwright.smoke.config.ts tests/smoke/delete-note-nonblocking.spec.ts tests/smoke/example.spec.ts tests/smoke/fix-ai-chat-empty-body-v3.spec.ts tests/smoke/fix-crash-create-note.spec.ts tests/smoke/fresh-start-telemetry-onboarding.spec.ts tests/smoke/getting-started-template.spec.ts tests/smoke/h1-title-decoupled.spec.ts tests/smoke/h1-untitled-auto-rename.spec.ts tests/smoke/keyboard-command-routing.spec.ts tests/smoke/linkify-init-warnings.spec.ts tests/smoke/multi-selection-shortcuts.spec.ts tests/smoke/wikilink-path-fix.spec.ts",
|
||||
"playwright:regression": "playwright test tests/smoke/",
|
||||
"playwright:integration": "playwright test --config playwright.integration.config.ts",
|
||||
"test:coverage": "node scripts/run-vitest-coverage.mjs",
|
||||
|
||||
@@ -1,81 +1,94 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
import { render, screen, fireEvent } from '@testing-library/react'
|
||||
import { render, screen, fireEvent, waitFor } from '@testing-library/react'
|
||||
import { ConfirmDeleteDialog } from './ConfirmDeleteDialog'
|
||||
|
||||
describe('ConfirmDeleteDialog', () => {
|
||||
const onConfirm = vi.fn()
|
||||
const onCancel = vi.fn()
|
||||
const defaultProps = {
|
||||
open: true,
|
||||
title: 'Delete permanently?',
|
||||
message: 'This cannot be undone.',
|
||||
}
|
||||
|
||||
const onConfirm = vi.fn()
|
||||
const onCancel = vi.fn()
|
||||
|
||||
function renderDialog(overrides: Partial<React.ComponentProps<typeof ConfirmDeleteDialog>> = {}) {
|
||||
return render(
|
||||
<ConfirmDeleteDialog
|
||||
{...defaultProps}
|
||||
onConfirm={onConfirm}
|
||||
onCancel={onCancel}
|
||||
{...overrides}
|
||||
/>,
|
||||
)
|
||||
}
|
||||
|
||||
describe('ConfirmDeleteDialog', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
it('renders with title and message', () => {
|
||||
render(
|
||||
<ConfirmDeleteDialog
|
||||
open={true}
|
||||
title="Delete permanently?"
|
||||
message="This cannot be undone."
|
||||
onConfirm={onConfirm}
|
||||
onCancel={onCancel}
|
||||
/>,
|
||||
)
|
||||
renderDialog()
|
||||
expect(screen.getByText('Delete permanently?')).toBeInTheDocument()
|
||||
expect(screen.getByText('This cannot be undone.')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('calls onConfirm when delete button clicked', () => {
|
||||
render(
|
||||
<ConfirmDeleteDialog
|
||||
open={true}
|
||||
title="Delete permanently?"
|
||||
message="This cannot be undone."
|
||||
onConfirm={onConfirm}
|
||||
onCancel={onCancel}
|
||||
/>,
|
||||
)
|
||||
renderDialog()
|
||||
fireEvent.click(screen.getByTestId('confirm-delete-btn'))
|
||||
expect(onConfirm).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('focuses the destructive action when the dialog opens', async () => {
|
||||
renderDialog()
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId('confirm-delete-btn')).toHaveFocus()
|
||||
})
|
||||
})
|
||||
|
||||
it('submits the destructive action when Enter is pressed', () => {
|
||||
renderDialog()
|
||||
|
||||
fireEvent.keyDown(screen.getByTestId('confirm-delete-dialog'), { key: 'Enter' })
|
||||
|
||||
expect(onConfirm).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('does not submit twice when Enter repeats', () => {
|
||||
renderDialog()
|
||||
|
||||
fireEvent.keyDown(screen.getByTestId('confirm-delete-dialog'), { key: 'Enter' })
|
||||
fireEvent.keyDown(screen.getByTestId('confirm-delete-dialog'), { key: 'Enter', repeat: true })
|
||||
|
||||
expect(onConfirm).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('treats Enter as the primary confirm shortcut even from the cancel button', () => {
|
||||
renderDialog()
|
||||
|
||||
fireEvent.keyDown(screen.getByText('Cancel'), { key: 'Enter' })
|
||||
|
||||
expect(onConfirm).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('calls onCancel when cancel button clicked', () => {
|
||||
render(
|
||||
<ConfirmDeleteDialog
|
||||
open={true}
|
||||
title="Delete permanently?"
|
||||
message="This cannot be undone."
|
||||
onConfirm={onConfirm}
|
||||
onCancel={onCancel}
|
||||
/>,
|
||||
)
|
||||
renderDialog()
|
||||
fireEvent.click(screen.getByText('Cancel'))
|
||||
expect(onCancel).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('does not render when open is false', () => {
|
||||
render(
|
||||
<ConfirmDeleteDialog
|
||||
open={false}
|
||||
title="Delete permanently?"
|
||||
message="This cannot be undone."
|
||||
onConfirm={onConfirm}
|
||||
onCancel={onCancel}
|
||||
/>,
|
||||
)
|
||||
renderDialog({ open: false })
|
||||
expect(screen.queryByText('Delete permanently?')).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('uses custom confirm label when provided', () => {
|
||||
render(
|
||||
<ConfirmDeleteDialog
|
||||
open={true}
|
||||
title="Empty Trash?"
|
||||
message="Delete all notes?"
|
||||
confirmLabel="Empty Trash"
|
||||
onConfirm={onConfirm}
|
||||
onCancel={onCancel}
|
||||
/>,
|
||||
)
|
||||
renderDialog({
|
||||
title: 'Empty Trash?',
|
||||
message: 'Delete all notes?',
|
||||
confirmLabel: 'Empty Trash',
|
||||
})
|
||||
expect(screen.getByText('Empty Trash')).toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { memo } from 'react'
|
||||
import { memo, useCallback, useEffect, useRef } from 'react'
|
||||
import type { FormEvent, KeyboardEvent as ReactKeyboardEvent } from 'react'
|
||||
import { Trash } from '@phosphor-icons/react'
|
||||
import {
|
||||
Dialog,
|
||||
@@ -19,6 +20,47 @@ interface ConfirmDeleteDialogProps {
|
||||
onCancel: () => void
|
||||
}
|
||||
|
||||
function focusConfirmButton(confirmButton: HTMLButtonElement | null) {
|
||||
confirmButton?.focus()
|
||||
}
|
||||
|
||||
function focusPrimaryDeleteButton(root: ParentNode | null) {
|
||||
focusConfirmButton(root?.querySelector<HTMLButtonElement>('[data-confirm-delete-primary="true"]') ?? null)
|
||||
}
|
||||
|
||||
function scheduleConfirmButtonFocus(root: ParentNode | null) {
|
||||
const focusDelays = [0, 50, 150]
|
||||
return focusDelays.map((delay) => (
|
||||
window.setTimeout(() => {
|
||||
focusPrimaryDeleteButton(root)
|
||||
}, delay)
|
||||
))
|
||||
}
|
||||
|
||||
type ConfirmShortcutEvent = Pick<
|
||||
KeyboardEvent,
|
||||
'key' | 'defaultPrevented' | 'repeat' | 'metaKey' | 'ctrlKey' | 'altKey' | 'shiftKey' | 'isComposing'
|
||||
> & {
|
||||
nativeEvent?: Pick<KeyboardEvent, 'isComposing'>
|
||||
}
|
||||
|
||||
function isComposingEvent(event: ConfirmShortcutEvent) {
|
||||
return event.isComposing || event.nativeEvent?.isComposing === true
|
||||
}
|
||||
|
||||
function isConfirmShortcut(event: ConfirmShortcutEvent) {
|
||||
return (
|
||||
event.key === 'Enter'
|
||||
&& !event.defaultPrevented
|
||||
&& !event.repeat
|
||||
&& !event.metaKey
|
||||
&& !event.ctrlKey
|
||||
&& !event.altKey
|
||||
&& !event.shiftKey
|
||||
&& !isComposingEvent(event)
|
||||
)
|
||||
}
|
||||
|
||||
export const ConfirmDeleteDialog = memo(function ConfirmDeleteDialog({
|
||||
open,
|
||||
title,
|
||||
@@ -27,9 +69,87 @@ export const ConfirmDeleteDialog = memo(function ConfirmDeleteDialog({
|
||||
onConfirm,
|
||||
onCancel,
|
||||
}: ConfirmDeleteDialogProps) {
|
||||
const confirmingRef = useRef(false)
|
||||
const cancelFocusIsIntentionalRef = useRef(false)
|
||||
|
||||
useEffect(() => {
|
||||
confirmingRef.current = false
|
||||
cancelFocusIsIntentionalRef.current = false
|
||||
if (!open) {
|
||||
return
|
||||
}
|
||||
|
||||
focusPrimaryDeleteButton(document)
|
||||
const timeoutIds = scheduleConfirmButtonFocus(document)
|
||||
|
||||
return () => {
|
||||
timeoutIds.forEach((timeoutId) => {
|
||||
window.clearTimeout(timeoutId)
|
||||
})
|
||||
}
|
||||
}, [open])
|
||||
|
||||
const handleConfirm = useCallback(() => {
|
||||
if (confirmingRef.current) {
|
||||
return
|
||||
}
|
||||
|
||||
confirmingRef.current = true
|
||||
onConfirm()
|
||||
}, [onConfirm])
|
||||
|
||||
const handleSubmit = useCallback((event: FormEvent<HTMLFormElement>) => {
|
||||
event.preventDefault()
|
||||
handleConfirm()
|
||||
}, [handleConfirm])
|
||||
|
||||
useEffect(() => {
|
||||
if (!open) {
|
||||
return
|
||||
}
|
||||
|
||||
const handleWindowKeyDown = (event: KeyboardEvent) => {
|
||||
if (!isConfirmShortcut(event)) {
|
||||
return
|
||||
}
|
||||
|
||||
event.preventDefault()
|
||||
handleConfirm()
|
||||
}
|
||||
|
||||
window.addEventListener('keydown', handleWindowKeyDown, true)
|
||||
|
||||
return () => {
|
||||
window.removeEventListener('keydown', handleWindowKeyDown, true)
|
||||
}
|
||||
}, [handleConfirm, open])
|
||||
|
||||
const handleKeyDown = useCallback((event: ReactKeyboardEvent<HTMLDivElement>) => {
|
||||
if (!isConfirmShortcut(event)) {
|
||||
return
|
||||
}
|
||||
|
||||
event.preventDefault()
|
||||
handleConfirm()
|
||||
}, [handleConfirm])
|
||||
|
||||
return (
|
||||
<Dialog open={open} onOpenChange={(isOpen) => { if (!isOpen) onCancel() }}>
|
||||
<DialogContent showCloseButton={false} data-testid="confirm-delete-dialog">
|
||||
<DialogContent
|
||||
showCloseButton={false}
|
||||
data-testid="confirm-delete-dialog"
|
||||
onKeyDown={handleKeyDown}
|
||||
onKeyDownCapture={(event) => {
|
||||
if (event.key === 'Tab') {
|
||||
cancelFocusIsIntentionalRef.current = true
|
||||
}
|
||||
}}
|
||||
onOpenAutoFocus={(event) => {
|
||||
event.preventDefault()
|
||||
focusPrimaryDeleteButton(event.currentTarget)
|
||||
scheduleConfirmButtonFocus(event.currentTarget)
|
||||
}}
|
||||
>
|
||||
<DialogHeader>
|
||||
<DialogTitle className="flex items-center gap-2">
|
||||
<Trash size={18} className="text-destructive" />
|
||||
@@ -37,12 +157,38 @@ export const ConfirmDeleteDialog = memo(function ConfirmDeleteDialog({
|
||||
</DialogTitle>
|
||||
<DialogDescription>{message}</DialogDescription>
|
||||
</DialogHeader>
|
||||
<DialogFooter>
|
||||
<Button variant="outline" onClick={onCancel}>Cancel</Button>
|
||||
<Button variant="destructive" onClick={onConfirm} data-testid="confirm-delete-btn">
|
||||
{confirmLabel}
|
||||
</Button>
|
||||
</DialogFooter>
|
||||
<form onSubmit={handleSubmit}>
|
||||
<DialogFooter>
|
||||
<Button
|
||||
type="submit"
|
||||
variant="destructive"
|
||||
data-testid="confirm-delete-btn"
|
||||
data-confirm-delete-primary="true"
|
||||
autoFocus
|
||||
>
|
||||
{confirmLabel}
|
||||
</Button>
|
||||
<Button
|
||||
type="button"
|
||||
variant="outline"
|
||||
onClick={onCancel}
|
||||
onFocus={() => {
|
||||
if (cancelFocusIsIntentionalRef.current) {
|
||||
return
|
||||
}
|
||||
|
||||
focusPrimaryDeleteButton(document)
|
||||
}}
|
||||
onPointerDown={() => {
|
||||
cancelFocusIsIntentionalRef.current = true
|
||||
}}
|
||||
data-confirm-delete-cancel="true"
|
||||
tabIndex={-1}
|
||||
>
|
||||
Cancel
|
||||
</Button>
|
||||
</DialogFooter>
|
||||
</form>
|
||||
</DialogContent>
|
||||
</Dialog>
|
||||
)
|
||||
|
||||
@@ -35,7 +35,7 @@ test.describe('responsive note deletion', () => {
|
||||
await triggerShortcutCommand(page, APP_COMMAND_IDS.noteDelete)
|
||||
await expect(page.getByTestId('confirm-delete-dialog')).toBeVisible({ timeout: 5_000 })
|
||||
|
||||
await page.getByTestId('confirm-delete-btn').focus()
|
||||
await expect(page.getByTestId('confirm-delete-btn')).toBeFocused()
|
||||
await page.keyboard.press('Enter')
|
||||
|
||||
const progressNotice = page.getByTestId('delete-progress-notice')
|
||||
|
||||
@@ -65,7 +65,6 @@ test.describe('multi-selection shortcuts', () => {
|
||||
await expect(dialog).toBeVisible({ timeout: 5_000 })
|
||||
await expect(dialog).toContainText(/Delete \d+ notes permanently\?/)
|
||||
|
||||
await page.keyboard.press('Tab')
|
||||
await expect(confirmButton).toBeFocused()
|
||||
await page.keyboard.press('Enter')
|
||||
|
||||
|
||||
Reference in New Issue
Block a user