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
164 changes: 164 additions & 0 deletions web/e2e/lane-inline-create.spec.ts
Original file line number Diff line number Diff line change
@@ -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<void> {
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<string> {
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);
});
});
26 changes: 18 additions & 8 deletions web/src/lib/components/collections/BoardView.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -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<unknown> | void;
onCreateInColumn?: (groupValue: string, title: string, reveal: boolean) => Promise<unknown> | void;
/**
* Inline draft state (TASK-1676) — owned by the page (which holds
* the leave guard + dialog) so a draft survives a board↔list view
Expand Down Expand Up @@ -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;
Expand Down
33 changes: 25 additions & 8 deletions web/src/routes/[username]/[workspace]/[collection]/+page.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -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<Item | null> {
if (!wsSlug || !collSlug) return null;
const trimmed = title.trim();
Expand All @@ -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) {
Expand Down