From c0154aa42164a83e4ea580bc2979595eddaec366 Mon Sep 17 00:00:00 2001 From: kolaente Date: Fri, 19 Jun 2026 21:34:01 +0200 Subject: [PATCH] =?UTF-8?q?fix(editor):=20make=20link=20prompt=20a=20sub-m?= =?UTF-8?q?odal=20=E2=80=94=20Escape=20cancels=20it=20without=20closing=20?= =?UTF-8?q?the=20task=20dialog?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review point (#2950, comment 3444116036): when the surrounding task 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 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. --- frontend/src/helpers/inputPrompt.ts | 35 +++++++++--- .../editor/link-prompt-kanban-popup.spec.ts | 57 +++++++++++++++++++ 2 files changed, 83 insertions(+), 9 deletions(-) diff --git a/frontend/src/helpers/inputPrompt.ts b/frontend/src/helpers/inputPrompt.ts index 77b70e089..d8de526ca 100644 --- a/frontend/src/helpers/inputPrompt.ts +++ b/frontend/src/helpers/inputPrompt.ts @@ -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 . 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 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) diff --git a/frontend/tests/e2e/editor/link-prompt-kanban-popup.spec.ts b/frontend/tests/e2e/editor/link-prompt-kanban-popup.spec.ts index 43fe85c75..26e15ae0f 100644 --- a/frontend/tests/e2e/editor/link-prompt-kanban-popup.spec.ts +++ b/frontend/tests/e2e/editor/link-prompt-kanban-popup.spec.ts @@ -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 : pressing Escape while + // it is open must cancel only the prompt and leave the task dialog open, + // instead of falling through to the native '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() + }) })