Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 126 additions & 0 deletions web/e2e/quickactions-inline-form.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
import { test, expect } from './fixtures';
import { browserLogin } from './lib/collab-helpers';
import type { APIRequestContext } from '@playwright/test';
import type { SuiteFixture } from './fixtures';

/**
* BUG-2281 regression e2e — the inline "New quick action" form must open
* and stay open.
*
* Found during the BUG-2129 switch-safety reconciliation. Clicking the
* footer "+ New quick action" flips `showCreateForm=true`, which unmounts
* the footer's {:else} branch (the very button clicked). Svelte 5
* flushSyncs after a delegated event handler, so by the time that click
* bubbles on to the <svelte:window> click-outside handler the button is
* DETACHED — `target.closest('.quick-actions-menu')` returns null and
* handleWindowClick treats it as an outside click, closing the whole menu
* and wiping the create form the instant it opens. `handleTriggerClick`
* already guards this with `e.stopPropagation()`; `handleOpenCreateForm`
* and the create-form Cancel button did not.
*
* jsdom (the unit test) doesn't reproduce the mid-bubble detach, and no
* e2e ever CLICKED "New quick action" (the capstone only asserts it's
* visible) — so this closes that gap in a real Chromium event pipeline.
* Pre-fix: the create form never appears. Post-fix (stopPropagation): it
* opens and stays, and Cancel returns to the action list instead of
* closing the whole menu.
*/

function authHeaders(fixture: SuiteFixture) {
return { Authorization: `Bearer ${fixture.apiToken}`, 'Content-Type': 'application/json' };
}

// A plain collection with a minimal valid schema. Letters-only prefix and a
// timestamped name so the server-derived slug stays in sync with the name
// (see pane-collection-migration-race.spec.ts::seedMigratableCollection for
// why numeric-derived prefixes 404 by-ref lookups and out-of-sync name/slug
// reassign the slug on the next save).
async function seedCollection(
fixture: SuiteFixture,
request: APIRequestContext,
namePrefix: string,
itemPrefix: string,
) {
const name = `${namePrefix} ${Date.now()}`;
const schema = JSON.stringify({
fields: [
{
key: 'status',
label: 'Status',
type: 'select',
options: ['draft', 'published'],
default: 'draft',
required: true,
},
],
});
const resp = await request.post(`/api/v1/workspaces/${fixture.workspaceSlug}/collections`, {
headers: authHeaders(fixture),
data: { name, prefix: itemPrefix, schema },
});
if (!resp.ok()) throw new Error(`collection create failed (${resp.status()}): ${await resp.text()}`);
return (await resp.json()) as { id: string; slug: string; name: string };
}

async function seedItem(
fixture: SuiteFixture,
request: APIRequestContext,
collSlug: string,
title: string,
) {
const resp = await request.post(
`/api/v1/workspaces/${fixture.workspaceSlug}/collections/${collSlug}/items`,
{
headers: authHeaders(fixture),
data: { title, fields: JSON.stringify({ status: 'draft' }), content: '' },
},
);
if (!resp.ok()) throw new Error(`item create failed (${resp.status()}): ${await resp.text()}`);
return (await resp.json()) as { id: string; slug: string };
}

test("the inline 'New quick action' form opens and stays open (BUG-2281)", async ({
page,
fixture,
request,
}, testInfo) => {
test.skip(
testInfo.project.name !== 'desktop-chromium',
'event-propagation behavior is viewport-agnostic; one project is enough',
);
test.setTimeout(30_000);

// A brand-new collection has NO quick actions yet, so the dropdown opens
// straight to the footer (New quick action / Manage actions) — the exact
// state a user hits when adding their first action.
const coll = await seedCollection(fixture, request, 'bug2281-form', 'BFRM');
const it = await seedItem(fixture, request, coll.slug, 'BUG-2281 form item');

await browserLogin(page);
await page.goto(`/${fixture.adminUsername}/${fixture.workspaceSlug}/${coll.slug}/${it.slug}`);
await expect(page.locator('.title', { hasText: 'BUG-2281 form item' })).toBeVisible();

await page.locator('button.trigger-btn[title="Quick actions"]').click();
const openFormBtn = page.locator('button.action-item.footer-row', { hasText: 'New quick action' });
await expect(openFormBtn).toBeVisible();
await openFormBtn.click();

// Pre-fix, the click reached the <svelte:window> click-outside handler on a
// footer button that Svelte's flushSync had already detached, so the menu
// closed instantly and the create form never appeared. Post-fix
// (stopPropagation on handleOpenCreateForm), the form opens and stays.
await expect(page.locator('input.qa-label-input')).toBeVisible();
await expect(page.locator('textarea.qa-prompt-input')).toBeVisible();
// A short settle to prove it didn't merely flash open then close.
await page.waitForTimeout(200);
await expect(page.locator('input.qa-label-input')).toBeVisible();

// Cancel returns to the action list (not: closes the whole menu) — same
// stopPropagation fix on the create-form Cancel button.
await page.locator('button.qa-btn.qa-btn-cancel', { hasText: 'Cancel' }).click();
await expect(page.locator('input.qa-label-input')).toHaveCount(0);
await expect(
page.locator('button.action-item.footer-row', { hasText: 'New quick action' }),
'Cancel returns to the action list rather than closing the whole menu',
).toBeVisible();
});
24 changes: 22 additions & 2 deletions web/src/lib/components/common/QuickActionsMenu.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,30 @@
newIcon = '';
}

function handleOpenCreateForm() {
function handleOpenCreateForm(e: MouseEvent) {
// stopPropagation, mirroring handleTriggerClick (BUG-2281): flipping
// showCreateForm unmounts the footer's {:else} branch (this very
// button). Svelte 5 flushSyncs after a delegated event handler, so by
// the time this click bubbles on to the <svelte:window> click-outside
// handler the button is DETACHED — `target.closest('.quick-actions-menu')`
// then returns null, and handleWindowClick treats it as an outside
// click and closes the whole menu (open=false + resetCreateForm),
// wiping the create form the instant it opens. Stopping propagation
// keeps the click from reaching the window handler at all.
e.stopPropagation();
showCreateForm = true;
}

function handleCancelCreateForm(e: MouseEvent) {
// Same detach-then-window-close race as handleOpenCreateForm (BUG-2281):
// resetCreateForm unmounts the create form (this Cancel button), so
// without stopPropagation the bubbling click would hit the window
// handler on a detached target and close the whole menu instead of
// returning to the action list.
e.stopPropagation();
resetCreateForm();
}

function handleManage() {
// Recheck `canEdit` at dispatch time (mirrors handleSaveNewAction): if it
// flips false while the menu is open — e.g. the master-freeze passing
Expand Down Expand Up @@ -279,7 +299,7 @@
Template variables: {'{ref}'} {'{title}'} {'{status}'} {'{priority}'} {'{collection}'} {'{content}'} {'{fields}'}
</div>
<div class="qa-actions">
<button class="qa-btn qa-btn-cancel" type="button" onclick={resetCreateForm}>
<button class="qa-btn qa-btn-cancel" type="button" onclick={handleCancelCreateForm}>
Cancel
</button>
<button
Expand Down
42 changes: 30 additions & 12 deletions web/src/lib/components/items/ItemDetail.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -4284,18 +4284,15 @@
writes are confined to the active side, the prompts stay on both. -->
{#if collection && (quickActions.length > 0 || isOwner)}
<!-- {#key itemSlug}: structural containment (PLAN-2105 / TASK-2112).
Remount this item-scoped menu on every item switch so any
in-flight quick-action continuation is discarded. Keyed on
itemSlug (the URL/ref identity), NOT item.id, so it resets the
instant the ref changes rather than waiting for B to load. -->
Remount this item-scoped menu on every item switch so the
child's OWN local state (open form, in-progress inputs) resets.
Keyed on itemSlug (the URL/ref identity), NOT item.id, so it
resets the instant the ref changes rather than waiting for B to
load. NOTE: the remount does NOT cancel a destroyed instance's
in-flight save promise — that promise still fires
oncollectionupdated into this persistent parent (see the
switch-safety note on the callback). -->
{#key itemSlug}
<!-- Capture the item identity at THIS render (frozen for this
{#key} instance). A destroyed instance's in-flight async
still fires its callback into the PERSISTENT parent, and
its OWN prop check passes (frozen at its item) — so the
PARENT-side guard below drops the write when the pane has
since moved on (PLAN-2105 / TASK-2112; coordinator). -->
{@const keyedSlug = itemSlug}
<QuickActionsMenu
actions={quickActions}
{item}
Expand All @@ -4308,7 +4305,28 @@
editCollectionOpen = true;
}}
oncollectionupdated={(updated) => {
if (keyedSlug !== itemSlug) return;
// Switch-safety note (BUG-2280 — investigated, NOT a live
// bug; do NOT add a template-side {@const keyedSlug =
// itemSlug} "snapshot" fence here). A quick-action save can
// resolve AFTER an A->B switch and fire this callback from a
// destroyed {#key} instance, but it's already safe by two
// independent layers, so no parent fence is needed:
// 1. QuickActionsMenu's OWN child-side guard
// (`collection?.id !== baseCollection.id`, captured
// pre-await) drops the callback on a CROSS-collection
// switch: a destroyed instance's `collection` prop reads
// the LIVE parent value (B's collection), not a frozen
// A — so the guard fails and oncollectionupdated is
// never invoked. (A {@const} snapshot would NOT freeze
// in Svelte 5 — it's a lazily-pulled derived that reads
// the current itemSlug — so a keyedSlug fence here is a
// no-op: the literal BUG-2129 trap. Verified empirically.)
// 2. On a SAME-collection switch the callback DOES fire,
// but `updated` is that same collection, so assigning it
// is correct; and loadData's collection write
// (`collGen === collectionGen || collection?.id !==
// collData.id`) forces the right collection regardless
// of the collectionGen bump below.
// Bump the unified fence so an in-flight stale load/refresh
// can't revert this fresh write (Codex).
collectionGen++;
Expand Down