diff --git a/web/e2e/lane-inline-create.spec.ts b/web/e2e/lane-inline-create.spec.ts new file mode 100644 index 00000000..b712f2da --- /dev/null +++ b/web/e2e/lane-inline-create.spec.ts @@ -0,0 +1,164 @@ +import { test, expect } from './fixtures'; +import { browserLogin, seedDoc } from './lib/collab-helpers'; +import type { APIRequestContext, Page } from '@playwright/test'; +import type { SuiteFixture } from './fixtures'; + +/** + * Board lane inline-create destination e2e (IDEA-2298). + * + * The lane `+` opens a Trello-style draft card (TASK-1676); Enter creates the + * item. TASK-1676 predates the split pane (PLAN-2105), so it hardcoded a + * FULL-PAGE `goto(.../{item}?new=1)` — which meant the one gesture that already + * knows the title ejected you off the board and re-opened the title editor on + * it, while clicking any EXISTING card opened the same item in the pane. This + * spec pins the corrected destinations: + * + * - desktop → the split pane (`?item=`), same entry point a card click uses + * - mobile → nothing opens; the card just appears in its lane + * - both → the pathname NEVER changes, and `?new=1` is never set + * + * The pathname assertion is the load-bearing one on both viewports: a + * regression back to a full-page navigate changes the pathname, and no amount + * of pane/lane styling can hide that. + * + * Viewport is driven explicitly with `setViewportSize` (the widths straddle the + * inclusive 768px breakpoint), so both cases are project-independent and we run + * them on one project rather than doubling the work — matching + * `pane-mobile-overlay.spec.ts`. + */ + +const SPLIT = { width: 1280, height: 900 }; // over the breakpoint → split pane +const OVERLAY = { width: 768, height: 1024 }; // == breakpoint (max-width is inclusive) → mobile + +/** The docs board groups by `status`; 'draft' is its first lane. */ +const LANE = 'Draft'; + +function boardUrl(fixture: { adminUsername: string; workspaceSlug: string }): string { + return `/${fixture.adminUsername}/${fixture.workspaceSlug}/docs?view=board`; +} + +function collectionPathname(fixture: { adminUsername: string; workspaceSlug: string }): string { + return `/${fixture.adminUsername}/${fixture.workspaceSlug}/docs`; +} + +/** + * Open the docs board with at least one item in the target lane. + * + * An EMPTY collection renders a "No docs yet" empty state instead of the + * lanes, so there'd be no lane `+` to click at all — seed one `status=draft` + * doc first so the Draft lane exists. Waits for that card so the board (fed by + * the local index) is known-rendered before the draft flow starts. + */ +async function openBoardWithLane( + page: Page, + fixture: SuiteFixture, + request: APIRequestContext, +): Promise { + await seedDoc(fixture, request, 'Lane anchor', { status: 'draft' }); + await page.goto(boardUrl(fixture)); + await expect(page.locator('.item-card', { hasText: 'Lane anchor' }).first()).toBeVisible(); +} + +/** + * Drive the lane `+` → type → Enter flow, returning the title used. Waits for + * the created card to render in the lane, which is the shared postcondition on + * both viewports (the local-index upsert) and the sync point for the + * destination assertions that follow. + */ +async function createViaLaneDraft(page: Page, lane: string): Promise { + const title = `Lane draft ${Date.now()}`; + await page.getByRole('button', { name: `Add item to ${lane}` }).click(); + + const input = page.locator('.lane-draft-input'); + await expect(input).toBeFocused(); + await input.fill(title); + await input.press('Enter'); + + // The item is upserted into the local index on BOTH viewports, so the card + // appears in the lane either way. + await expect(page.locator('.item-card', { hasText: title })).toBeVisible(); + return title; +} + +test.describe('board lane inline-create destination (IDEA-2298)', () => { + test('desktop: Enter opens the new item in the split pane without leaving the board', async ({ + page, + fixture, + request, + }, testInfo) => { + test.skip( + testInfo.project.name !== 'desktop-chromium', + 'viewport is driven explicitly; one project is enough', + ); + await page.setViewportSize(SPLIT); + await browserLogin(page); + await openBoardWithLane(page, fixture, request); + + const title = await createViaLaneDraft(page, LANE); + + // Wait for the pane open to actually land before reading the URL. The + // created card renders off the SYNCHRONOUS local-index upsert, so it is + // NOT a sync point for the navigation that follows it — asserting + // straight after it would race, and a pathname assertion in particular + // would pass vacuously against a full-page `goto` that simply hadn't + // resolved yet (caught by mutating the fix back out). + await page.waitForURL((u) => new URL(u).searchParams.has('item')); + + // Opened in the pane — NOT a full-page navigate. `?item=` is the pane's + // single source of truth, and the pathname is still the collection route. + const url = new URL(page.url()); + expect(url.pathname).toBe(collectionPathname(fixture)); + expect(url.searchParams.get('item')).toBeTruthy(); + // `?new=1` drives ItemDetail's auto-edit-title flow. The lane draft + // already captured the title, so re-opening that editor is the exact + // inconsistency IDEA-2298 filed — it must never be set on this path. + expect(url.searchParams.get('new')).toBeNull(); + + const pane = page.locator('.item-pane'); + await expect(pane).toBeVisible(); + await expect(pane).toContainText(title); + + // The board stayed mounted behind the pane (it's a split, not a + // navigation), and the composer closed on submit — uniform on every + // viewport, since here the pane takes focus anyway. + await expect(page.locator('.list-column')).toBeVisible(); + await expect(page.locator('.lane-draft-input')).toHaveCount(0); + }); + + test('mobile: Enter creates the card in the lane and opens nothing', async ({ + page, + fixture, + request, + }, testInfo) => { + test.skip( + testInfo.project.name !== 'desktop-chromium', + 'viewport is driven explicitly; one project is enough', + ); + await page.setViewportSize(OVERLAY); + await browserLogin(page); + await openBoardWithLane(page, fixture, request); + + await createViaLaneDraft(page, LANE); + + // Composer closed on submit here too (uniform close, IDEA-2298). + await expect(page.locator('.lane-draft-input')).toHaveCount(0); + + // "Nothing opened" is an ABSENCE, and absences can't be polled — a bare + // URL assertion here would pass against a full-page `goto` that just + // hadn't resolved yet. So prove it POSITIVELY instead: re-open the lane + // composer. That click can only succeed if the board is still mounted and + // interactive, and awaiting it gives any would-be navigation a real chance + // to land first. It also encodes the actual UX win — on mobile you can add + // a second card immediately instead of navigating back. + await page.getByRole('button', { name: `Add item to ${LANE}` }).click(); + await expect(page.locator('.lane-draft-input')).toBeVisible(); + await page.locator('.lane-draft-input').press('Escape'); + + // Still on the board, with nothing opened. + const url = new URL(page.url()); + expect(url.pathname).toBe(collectionPathname(fixture)); + expect(url.searchParams.get('item')).toBeNull(); + expect(url.searchParams.get('new')).toBeNull(); + await expect(page.locator('.item-pane')).toHaveCount(0); + }); +}); diff --git a/web/src/lib/components/collections/BoardView.svelte b/web/src/lib/components/collections/BoardView.svelte index 519cc4ea..b3718521 100644 --- a/web/src/lib/components/collections/BoardView.svelte +++ b/web/src/lib/components/collections/BoardView.svelte @@ -26,12 +26,17 @@ oncreate?: () => void; /** * Create an item in this lane from the inline draft card - * (TASK-1676), pre-filling the lane's group value. `navigate` true - * (Enter) opens the new item; false (nav-guard Save) just lands it - * in the lane. Throws on failure so the draft can be restored. - * Gated behind `canEdit`. When wired, the `+`/menu open a draft. + * (TASK-1676), pre-filling the lane's group value. Throws on failure + * so the draft can be restored. Gated behind `canEdit`. When wired, + * the `+`/menu open a draft. + * + * `reveal` reports INTENT, not a destination: true means "the user + * submitted this draft, show them the item", false means "create it + * quietly" (the page's nav-guard Save-all). What revealing does — and + * whether the viewport gets one at all — is the page's call, so this + * component stays unaware of the split pane (IDEA-2298). */ - onCreateInColumn?: (groupValue: string, title: string, navigate: boolean) => Promise | void; + onCreateInColumn?: (groupValue: string, title: string, reveal: boolean) => Promise | void; /** * Inline draft state (TASK-1676) — owned by the page (which holds * the leave guard + dialog) so a draft survives a board↔list view @@ -149,9 +154,14 @@ const title = (draftText[col] ?? '').trim(); if (!title || savingDraft || !onCreateInColumn) return; savingDraft = true; - // Clear optimistically BEFORE the create navigates, so the page's - // leave guard (fires when onCreateInColumn opens the new item) - // doesn't re-flag this lane. + // The composer closes on submit on EVERY viewport (IDEA-2298) — on + // desktop the pane takes focus anyway, so leaving it open for + // Trello-style rapid entry would just fight the pane for the cursor. + // + // Clear the text optimistically, BEFORE the await, so the page's + // unsaved-draft leave guard can't see this lane as dirty while the + // create is in flight. The desktop reveal is a same-pathname `?item=` + // push, which that guard skips regardless — belt and suspenders. const prior = draftText[col]; delete draftText[col]; draftOpen[col] = false; diff --git a/web/src/routes/[username]/[workspace]/[collection]/+page.svelte b/web/src/routes/[username]/[workspace]/[collection]/+page.svelte index cfc69c8d..847e570c 100644 --- a/web/src/routes/[username]/[workspace]/[collection]/+page.svelte +++ b/web/src/routes/[username]/[workspace]/[collection]/+page.svelte @@ -1800,14 +1800,29 @@ // Create an item in a board lane from the inline draft card // (TASK-1676, refines TASK-1671 / IDEA-1159). Pre-fills the lane's - // group field so the item lands in that lane. `navigate` true (Enter - // in the draft) opens the new item; false (nav-guard "Save") just - // upserts it into the local index and stays put. Throws on failure so + // group field so the item lands in that lane. Throws on failure so // BoardView can restore the draft. Returns the created item. + // + // `reveal` is the caller's INTENT — "the user just submitted this draft, + // show them the item" (Enter / "Add card") vs. "create it quietly" (the + // nav-guard's Save-all, which must never open anything on the way out). + // This page owns what revealing MEANS, so BoardView doesn't have to know + // the pane exists (IDEA-2298): + // - desktop → open the split pane, exactly like clicking an existing card + // - mobile → nothing; the card appears in the lane and the user taps it + // if they want it + // Either way the item is upserted into the local index so it renders in + // its lane immediately, and either way the draft composer closes + // (BoardView.submitDraft) — on desktop the pane takes focus. + // + // Deliberately NOT the `?new=1` full-page goto this used to do. That flow + // exists to drop you into the title editor of a fresh "Untitled" item + // (`createNewItem`), and re-opening the title editor on a title the user + // just finished typing is the inconsistency IDEA-2298 filed. async function quickCreateInColumn( groupValue: string, title: string, - navigate: boolean + reveal: boolean ): Promise { if (!wsSlug || !collSlug) return null; const trimmed = title.trim(); @@ -1831,10 +1846,12 @@ fields: JSON.stringify(defaultFields), source: 'web' }); - if (navigate) { - goto(`/${username}/${wsSlug}/${collSlug}/${itemUrlId(item)}?new=1`); - } else { - localIndex.upsert(wsSlug, item, epoch); + localIndex.upsert(wsSlug, item, epoch); + // Desktop reveal only — `openItemPane` is the same entry point a + // plain left-click on an existing card uses, so a created card and + // a clicked card land in the same place. + if (reveal && !viewport.isMobile) { + openItemPane(item); } return item; } catch (err: any) {