fix(editor): make link prompt a sub-modal — Escape cancels it without closing the task dialog
Review point (#2950, comment 3444116036): when the surrounding task <dialog> closed while the link prompt was open, the prompt was orphaned and cleanup() never ran, leaking listeners and an unresolved promise. Treat the prompt as a sub-modal of the task dialog: pressing Escape while it is open now preventDefault()/stopPropagation()s the keydown so the native modal <dialog> does not close on Escape, resolves the prompt with '' (cancel) and runs cleanup() — only the prompt is dismissed, the task dialog stays open. A one-shot 'cancel' listener on the enclosing dialog backs this up in case the keydown handling is insufficient in some browser. Tighten cleanup() so the prompt fully tears down regardless of how it closes (Enter / Escape / click-outside): it now removes the scroll listener, the document click listener and the dialog cancel listener, and removes the element. handleClickOutside was hoisted so cleanup() can remove it, closing the leaked-listener gap directly. Adds an e2e asserting Escape cancels the prompt while the task dialog stays open; the existing 'Enter creates the link' case still passes.
This commit is contained in:
parent
84dc57c562
commit
0e17556a16
|
|
@ -89,8 +89,24 @@ export default function inputPrompt(pos: ClientRect, oldValue: string = '', edit
|
|||
|
||||
nextTick(() => document.getElementById(id)?.focus())
|
||||
|
||||
// The prompt is a sub-modal of the enclosing task <dialog>. Native modal
|
||||
// dialogs close themselves on Escape ("cancel"); swallow that while the
|
||||
// prompt is open so Escape only dismisses the prompt, not the task dialog.
|
||||
const dialog = container.closest('dialog') as HTMLDialogElement | null
|
||||
const handleDialogCancel = (event: Event) => event.preventDefault()
|
||||
dialog?.addEventListener('cancel', handleDialogCancel)
|
||||
|
||||
const handleClickOutside = (event: MouseEvent) => {
|
||||
if (!popupElement.contains(event.target as Node)) {
|
||||
resolve('')
|
||||
cleanup()
|
||||
}
|
||||
}
|
||||
|
||||
const cleanup = () => {
|
||||
window.removeEventListener('scroll', handleScroll, true)
|
||||
document.removeEventListener('click', handleClickOutside)
|
||||
dialog?.removeEventListener('cancel', handleDialogCancel)
|
||||
if (container.contains(popupElement)) {
|
||||
container.removeChild(popupElement)
|
||||
}
|
||||
|
|
@ -98,6 +114,16 @@ export default function inputPrompt(pos: ClientRect, oldValue: string = '', edit
|
|||
|
||||
document.getElementById(id)?.addEventListener('keydown', event => {
|
||||
const shortcutString = eventToShortcutString(event)
|
||||
|
||||
if (shortcutString === 'Escape') {
|
||||
// Stop the native <dialog> from closing on Escape; cancel the prompt only.
|
||||
event.preventDefault()
|
||||
event.stopPropagation()
|
||||
resolve('')
|
||||
cleanup()
|
||||
return
|
||||
}
|
||||
|
||||
if (shortcutString !== 'Enter') {
|
||||
return
|
||||
}
|
||||
|
|
@ -112,15 +138,6 @@ export default function inputPrompt(pos: ClientRect, oldValue: string = '', edit
|
|||
cleanup()
|
||||
})
|
||||
|
||||
// Close on click outside
|
||||
const handleClickOutside = (event: MouseEvent) => {
|
||||
if (!popupElement.contains(event.target as Node)) {
|
||||
resolve('')
|
||||
cleanup()
|
||||
document.removeEventListener('click', handleClickOutside)
|
||||
}
|
||||
}
|
||||
|
||||
// Add slight delay to prevent immediate closing
|
||||
setTimeout(() => {
|
||||
document.addEventListener('click', handleClickOutside)
|
||||
|
|
|
|||
|
|
@ -64,4 +64,61 @@ test.describe('Editor link prompt inside the Kanban task popup', () => {
|
|||
await expect(link).toBeVisible()
|
||||
await expect(link).toHaveText('link me')
|
||||
})
|
||||
|
||||
// The link prompt is a sub-modal of the task <dialog>: pressing Escape while
|
||||
// it is open must cancel only the prompt and leave the task dialog open,
|
||||
// instead of falling through to the native <dialog>'s Escape-to-close.
|
||||
test('Escape cancels the link prompt without closing the task dialog', async ({authenticatedPage: page}) => {
|
||||
const projects = await ProjectFactory.create(1)
|
||||
const views = await ProjectViewFactory.create(1, {
|
||||
id: 1,
|
||||
project_id: projects[0].id,
|
||||
view_kind: 3,
|
||||
bucket_configuration_mode: 1,
|
||||
})
|
||||
const buckets = await BucketFactory.create(1, {
|
||||
project_view_id: views[0].id,
|
||||
})
|
||||
const tasks = await TaskFactory.create(1, {
|
||||
project_id: projects[0].id,
|
||||
description: 'link me',
|
||||
index: 1,
|
||||
})
|
||||
await TaskBucketFactory.create(1, {
|
||||
task_id: tasks[0].id,
|
||||
bucket_id: buckets[0].id,
|
||||
project_view_id: views[0].id,
|
||||
})
|
||||
|
||||
await page.goto(`/projects/${projects[0].id}/${views[0].id}`)
|
||||
|
||||
const card = page.locator('.kanban .bucket .tasks .task').filter({hasText: tasks[0].title})
|
||||
await expect(card).toBeVisible()
|
||||
await card.click()
|
||||
|
||||
const dialog = page.locator('dialog[open]')
|
||||
await expect(dialog).toBeVisible()
|
||||
await expect(dialog.locator('.task-view')).toBeVisible()
|
||||
|
||||
const editButton = dialog.locator('.details.content.description .tiptap button.done-edit').filter({hasText: 'Edit'})
|
||||
await expect(editButton).toBeVisible({timeout: 10000})
|
||||
await editButton.click()
|
||||
|
||||
const description = dialog.locator('.details.content.description')
|
||||
const editor = description.locator('[contenteditable="true"]').first()
|
||||
await expect(editor).toBeVisible({timeout: 10000})
|
||||
await editor.click()
|
||||
await page.keyboard.press('ControlOrMeta+a')
|
||||
|
||||
await description.locator('.editor-toolbar__button').filter({hasText: 'Link'}).click()
|
||||
|
||||
const urlInput = dialog.locator('input.input[placeholder="URL"]')
|
||||
await expect(urlInput).toBeVisible()
|
||||
await urlInput.press('Escape')
|
||||
|
||||
// The prompt is gone, but the task dialog stays open.
|
||||
await expect(urlInput).toBeHidden()
|
||||
await expect(dialog).toBeVisible()
|
||||
await expect(dialog.locator('.task-view')).toBeVisible()
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Reference in New Issue