diff --git a/web/e2e/pane-collection-migration-race.spec.ts b/web/e2e/pane-collection-migration-race.spec.ts index 26af656b..77c9f65d 100644 --- a/web/e2e/pane-collection-migration-race.spec.ts +++ b/web/e2e/pane-collection-migration-race.spec.ts @@ -425,7 +425,8 @@ test('a collection migration completing after a cross-collection navigation does .locator('.relationship-group', { hasText: 'Related' }) .locator('a.link-target', { hasText: bTitle }) .click(); - // B opens in the docked pane beside the (now peeking) A master. + // B opens in the docked pane beside the (still-editable, focus-follows) A + // master; the pane is a read-only preview until clicked into (PLAN-2179 DR-2). const pane = page.locator('.item-pane'); await expect(pane).toBeVisible(); await expect(pane.locator('.title', { hasText: bTitle })).toBeVisible(); diff --git a/web/e2e/pane-full-page-capstone.spec.ts b/web/e2e/pane-full-page-capstone.spec.ts index bffb0aa8..34ec3dcb 100644 --- a/web/e2e/pane-full-page-capstone.spec.ts +++ b/web/e2e/pane-full-page-capstone.spec.ts @@ -304,11 +304,17 @@ test.describe('full-page pane host CAPSTONE (PLAN-2154 Phase 2 / TASK-2175)', () await expect(masterEditor).toBeVisible({ timeout: SYNC_TIMEOUT }); await expect(masterEditor).toHaveAttribute('contenteditable', 'true'); - // ── Open the pane → the master goes PEEKING (retain-alive read-only). ── + // ── Open the pane → focus-follows-editing (PLAN-2179 DR-2 / TASK-2181) keeps + // the MASTER editable (the pane opens as a read-only PREVIEW). Then click + // INTO the pane → the master goes PEEKING (retain-alive read-only). ── await openPaneViaRelated(page, 'FP freeze target'); const pane = page.locator('.item-pane'); await expect(pane).toBeVisible(); await expect(pane.locator('.title', { hasText: 'FP freeze target' })).toBeVisible(); + // DR-2: opening alone does NOT freeze the master — it stays the active side. + await expect(editableTitle).toBeVisible(); + // Activate the pane (click its title) → the master becomes the frozen side. + await pane.locator('.title', { hasText: 'FP freeze target' }).click(); // ── Peeking: every NEW-edit-initiation surface is gone/disabled. ── // Title: click-to-edit button replaced by a non-editable

. @@ -415,11 +421,13 @@ test.describe('full-page pane host CAPSTONE (PLAN-2154 Phase 2 / TASK-2175)', () .poll(liveRoomsObj, { timeout: SYNC_TIMEOUT }) .toEqual({ [master.id]: 1, [related.id]: 1 }); - // Drill related → grandchild INSIDE the pane (a real `.child-row` click). - // The pane provider RE-TARGETS: `related`'s socket is torn down, the - // grandchild's minted — master + grandchild = STILL 2 distinct rooms, not - // {master, related, grandchild}. This is "one pane provider that - // re-targets, not N". + // Drill related → grandchild INSIDE the pane (a real `.child-row` click). The + // pane opened as a read-only PREVIEW (master active, DR-2), but a content-link + // / child-row drills on the FIRST click and activates the pane — no + // pane-activation pre-click, no dndzone-swallow (PLAN-2179 / TASK-2181). The + // pane provider RE-TARGETS: `related`'s socket is torn down, the grandchild's + // minted — master + grandchild = STILL 2 distinct rooms, not + // {master, related, grandchild}. This is "one pane provider that re-targets, not N". await pane.locator('.child-row', { hasText: 'FP ws grandchild' }).click(); await expect(pane.locator('.title', { hasText: 'FP ws grandchild' })).toBeVisible(); await expect diff --git a/web/e2e/pane-full-page-host.spec.ts b/web/e2e/pane-full-page-host.spec.ts index 94e9fd05..c21f4e17 100644 --- a/web/e2e/pane-full-page-host.spec.ts +++ b/web/e2e/pane-full-page-host.spec.ts @@ -109,7 +109,7 @@ test.describe('full-page pane host (PLAN-2154 Phase 2 / TASK-2174)', () => { ); }); - test('a master content-link opens a pane beside the (read-only, still-live) master; the pane drills, in-pane Back and ✕ close cleanly', async ({ + test('focus follows editing: open keeps the master editable (pane = read-only preview); clicking a side activates it and freezes the other; drill/back keep the pane active (PLAN-2179 DR-2/DR-3 / TASK-2181)', async ({ page, fixture, request, @@ -126,12 +126,17 @@ test.describe('full-page pane host (PLAN-2154 Phase 2 / TASK-2174)', () => { const relatedRef = await itemRef(fixture, request, related.slug); const grandchildRef = await itemRef(fixture, request, grandchild.slug); + // Editable/frozen probes: a `button.title` is the click-to-edit title + // (editable side); an `h1.title.title-readonly` is the frozen side. + const masterEditable = page.locator('button.title', { hasText: 'FP host master' }); + const masterFrozen = page.locator('h1.title.title-readonly', { hasText: 'FP host master' }); + // Land on the MASTER full page. No pane yet: the flex-row host is present, // the master title is EDITABLE (a click-to-edit button — not peeking), and // there's no `?item=`. await page.goto(fullPageUrl(fixture, master.slug)); await expect(page.locator('.item-page-host')).toBeVisible(); - await expect(page.locator('button.title', { hasText: 'FP host master' })).toBeVisible(); + await expect(masterEditable).toBeVisible(); await expect(page.locator('.item-pane')).toHaveCount(0); expect(openItemParam(page)).toBeNull(); @@ -143,41 +148,68 @@ test.describe('full-page pane host (PLAN-2154 Phase 2 / TASK-2174)', () => { const pane = page.locator('.item-pane'); await expect(pane).toBeVisible(); - await expect(pane.locator('.title', { hasText: 'FP host related' })).toBeVisible(); await expect.poll(() => openItemParam(page)).toBe(relatedRef); - // The master is RETAIN-ALIVE READ-ONLY (peeking): its content is still - // rendered (not torn down), but the title is now a non-editable

- // (`title-readonly`) instead of the click-to-edit button — the freeze. - await expect(page.locator('h1.title.title-readonly', { hasText: 'FP host master' })).toBeVisible(); - await expect(page.locator('button.title', { hasText: 'FP host master' })).toHaveCount(0); + // DR-2: opening does NOT freeze the master. Focus-follows-editing means the + // MASTER stays the active/editable side (openItemPaneByRef never moves focus + // into the pane), and the pane opens as a READ-ONLY PREVIEW — pane title is a + // non-editable

, no click-to-edit button. + await expect(masterEditable).toBeVisible(); + await expect(masterFrozen).toHaveCount(0); + await expect(pane.locator('h1.title.title-readonly', { hasText: 'FP host related' })).toBeVisible(); + await expect(pane.locator('button.title')).toHaveCount(0); // Depth 0: the pane's Back chevron is hidden. await expect(pane.locator('button.pane-back-btn')).toHaveCount(0); - // Click the CHILD row INSIDE the pane → DRILL in place (same pathname, - // `?item=` swaps to the child, the Back chevron appears at depth>0). The - // master stays put + read-only underneath. + // Drill a CHILD row directly from the FROZEN preview — on the FIRST click. + // Content-link / child-row navigation stays live while the master is active + // (the mini-browser preview), so the pointerdown activator EXCLUDES navigable + // targets: the freeze-flip can't re-init ChildItems' dndzone and swallow the + // click. The drill is pane-internal, so it ALSO activates the pane — pane + // editable, master frozen (PLAN-2179 DR-2 / TASK-2181). No pre-activation click. const masterPathname = new URL(page.url()).pathname; await pane.locator('.child-row', { hasText: 'FP host grandchild' }).click(); await expect.poll(() => openItemParam(page)).toBe(grandchildRef); expect(new URL(page.url()).pathname).toBe(masterPathname); await expect(pane.locator('button.pane-back-btn')).toBeVisible(); - await expect(pane.locator('.title', { hasText: 'FP host grandchild' })).toBeVisible(); - await expect(page.locator('h1.title.title-readonly', { hasText: 'FP host master' })).toBeVisible(); + await expect(pane.locator('button.title', { hasText: 'FP host grandchild' })).toBeVisible(); + await expect(masterFrozen).toBeVisible(); + await expect(masterEditable).toHaveCount(0); - // Browser BACK → pops one drill level back to B in the pane. + // Browser BACK → pops one drill level back to B in the pane. A drill-pop is + // still pane-internal, so `activePane` stays 'pane' — pane editable, master + // frozen. await page.goBack(); await expect.poll(() => openItemParam(page)).toBe(relatedRef); - await expect(pane.locator('.title', { hasText: 'FP host related' })).toBeVisible(); + await expect(pane.locator('button.title', { hasText: 'FP host related' })).toBeVisible(); await expect(pane.locator('button.pane-back-btn')).toHaveCount(0); + await expect(masterFrozen).toBeVisible(); + await expect(masterEditable).toHaveCount(0); + + // Click BACK into the MASTER — on its now-read-only

title, a NON-focusable, + // NON-navigable element that drops focus to . The pointerdown activator + // re-activates the master, and the desktop backstop must NOT yank focus back to + // the pane. Master editable again; pane freezes. Exactly one side editable. + await masterFrozen.click(); + await expect(masterEditable).toBeVisible(); + await expect(masterFrozen).toHaveCount(0); + await expect(pane.locator('h1.title.title-readonly', { hasText: 'FP host related' })).toBeVisible(); + await expect(pane.locator('button.title')).toHaveCount(0); + + // Click into the PANE on its read-only title (a NON-navigable target) → the + // pointerdown activator makes the pane the active side; the master freezes. + await pane.locator('.title', { hasText: 'FP host related' }).click(); + await expect(pane.locator('button.title', { hasText: 'FP host related' })).toBeVisible(); + await expect(masterFrozen).toBeVisible(); + await expect(masterEditable).toHaveCount(0); // Close (✕) → the pane unmounts cleanly, `?item=` drops, and the master is // EDITABLE again (no longer peeking → click-to-edit button returns). await pane.locator('button[title="Close pane"]').click(); await expect(page.locator('.item-pane')).toHaveCount(0); await expect.poll(() => openItemParam(page)).toBeNull(); - await expect(page.locator('button.title', { hasText: 'FP host master' })).toBeVisible(); + await expect(masterEditable).toBeVisible(); await expect(page.locator('h1.title.title-readonly')).toHaveCount(0); }); @@ -211,22 +243,27 @@ test.describe('full-page pane host (PLAN-2154 Phase 2 / TASK-2174)', () => { expect(editorNode).not.toBeNull(); expect(await editorNode!.evaluate((el) => el.isConnected)).toBe(true); - // Open the pane → the master goes peeking (read-only). + // Open the pane → focus-follows keeps the MASTER active (DR-2), so its editor + // stays editable and the SAME DOM node stays connected — no open-driven remount. + const pane = page.locator('.item-pane'); await page .locator('.relationship-group', { hasText: 'Related' }) .locator('a.link-target', { hasText: 'FP host reactive-freeze related' }) .click(); - await expect(page.locator('.item-pane')).toBeVisible(); + await expect(pane).toBeVisible(); await expect.poll(() => openItemParam(page)).toBe(relatedRef); + expect(await editorNode!.evaluate((el) => el.isConnected)).toBe(true); + await expect(masterMainEditor).toHaveAttribute('contenteditable', 'true'); + + // Click INTO the pane → the master FREEZES. The freeze is REACTIVE: the SAME + // editor DOM node is still connected (a `{#key}`-driven remount — the OLD + // peeking-in-the-key behavior — would have detached this handle, isConnected → + // false), and it merely flipped contenteditable=false in place. This is the + // whole point of PLAN-2179 DR-1: freeze without destroying/recreating the editor. + await pane.locator('.title', { hasText: 'FP host reactive-freeze related' }).click(); await expect( page.locator('h1.title.title-readonly', { hasText: 'FP host reactive-freeze master' }), ).toBeVisible(); - - // The freeze is REACTIVE: the SAME editor DOM node is still connected (a - // `{#key}`-driven remount — the OLD peeking-in-the-key behavior — would - // have detached this handle, isConnected → false), and it merely flipped - // contenteditable=false in place. This is the whole point of PLAN-2179 - // DR-1: freeze without destroying/recreating the editor. expect(await editorNode!.evaluate((el) => el.isConnected)).toBe(true); await expect(masterMainEditor).toHaveAttribute('contenteditable', 'false'); @@ -274,15 +311,18 @@ test.describe('full-page pane host (PLAN-2154 Phase 2 / TASK-2174)', () => { await masterMain.locator('p').first().hover(); await expect.poll(handleDisplay, { timeout: 3000 }).not.toBe('none'); - // PEEK: open the pane → the master freezes (contenteditable=false). Hovering - // the SAME paragraph must NOT reveal the handle — the reactive-editable - // choke (onMouseMove/update bail on !editorView.editable) keeps it hidden. + // PEEK: open the pane (focus-follows keeps the master editable — DR-2), then + // click INTO the pane → the master FREEZES (contenteditable=false). Hovering + // the SAME paragraph must NOT reveal the handle — the reactive-editable choke + // (onMouseMove/update bail on !editorView.editable) keeps it hidden. + const pane = page.locator('.item-pane'); await page .locator('.relationship-group', { hasText: 'Related' }) .locator('a.link-target', { hasText: 'FP host drag-handle related' }) .click(); - await expect(page.locator('.item-pane')).toBeVisible(); + await expect(pane).toBeVisible(); await expect.poll(() => openItemParam(page)).toBe(relatedRef); + await pane.locator('.title', { hasText: 'FP host drag-handle related' }).click(); await expect(masterMain).toHaveAttribute('contenteditable', 'false'); await page.mouse.move(5, 5); // leave the editor first await masterMain.locator('p').first().hover({ force: true }); @@ -343,8 +383,12 @@ test.describe('full-page pane host (PLAN-2154 Phase 2 / TASK-2174)', () => { await expect(pane).toBeVisible(); await expect(pane.locator('.title', { hasText: 'FP host cold other' })).toBeVisible(); await expect.poll(() => openItemParam(page)).toBe(otherRef); - // The master is present + peeking (read-only) beside the pane. - await expect(page.locator('h1.title.title-readonly', { hasText: 'FP host cold master' })).toBeVisible(); + // DR-2 cold-load initializer (desktop): no focusin fires on a `?item=` deep + // load, so `activePane` seeds to the MASTER — it's EDITABLE beside the pane, + // which opens as a read-only PREVIEW (frozen title). + await expect(page.locator('button.title', { hasText: 'FP host cold master' })).toBeVisible(); + await expect(page.locator('h1.title.title-readonly', { hasText: 'FP host cold master' })).toHaveCount(0); + await expect(pane.locator('h1.title.title-readonly', { hasText: 'FP host cold other' })).toBeVisible(); }); test('a cold-loaded `?item=` is stripped (server slug-fallback self-collision)', async ({ @@ -411,8 +455,13 @@ test.describe('full-page pane host (PLAN-2154 Phase 2 / TASK-2174)', () => { await page.goBack(); await expect(page).toHaveURL(paneUrl); await expect(pane).toBeVisible(); - await expect(pane.locator('.title', { hasText: 'FP host expand related' })).toBeVisible(); await expect.poll(() => openItemParam(page)).toBe(relatedRef); + // Clicking "Expand to full page" is a click INSIDE the pane, so `activePane` + // latched to 'pane' and PERSISTS across the expand + Back (this route + // component is REUSED, never remounted). The restored pane is therefore the + // ACTIVE/editable side and the master A is frozen — the pane "stays active" + // across browser Back (PLAN-2179 DR-2). + await expect(pane.locator('button.title', { hasText: 'FP host expand related' })).toBeVisible(); await expect(page.locator('h1.title.title-readonly', { hasText: 'FP host expand master' })).toBeVisible(); }); }); diff --git a/web/src/lib/collections/paneFocus.ts b/web/src/lib/collections/paneFocus.ts index aaa99ea0..7e0ebf15 100644 --- a/web/src/lib/collections/paneFocus.ts +++ b/web/src/lib/collections/paneFocus.ts @@ -45,6 +45,27 @@ export function paneFocusables( ); } +/** + * Portalled / self-trapping surfaces that legitimately overlay EITHER pane + * region — a native modal ``, an ARIA dialog / menu / listbox, or the + * editor's imperative block context menu (no ARIA role, matched by class). They + * own their own focus + keyboard and sit in NEITHER the master column nor the + * pane region (most portal out to ``), so both call sites treat a focus / + * pointer landing on them as "not a region event": + * • the mobile focus trap must not hijack their Tab or yank focus off them, and + * • the full-page host's focus-follows-editing classifier (PLAN-2179 / DR-2) + * must not read a focus / pointerdown on them as a master↔pane switch. + * ONE definition shared by both call sites (PaneHost's mobile trap + the host + * route's activePane classifier) so the two can't drift. + */ +export const PANE_EXEMPT_SURFACE_SELECTOR = + 'dialog, [role="dialog"], [role="menu"], [role="listbox"], .block-context-menu'; + +/** True when `el` (or an ancestor) is one of the {@link PANE_EXEMPT_SURFACE_SELECTOR} overlays. */ +export function inExemptSurface(el: Element | null | undefined): boolean { + return !!el?.closest?.(PANE_EXEMPT_SURFACE_SELECTOR); +} + /** * Where a Tab / Shift+Tab should send focus while the pane is TRAPPING (the * mobile full-screen overlay). Returns the element to focus, or `null` to let diff --git a/web/src/lib/components/collections/PaneHost.svelte b/web/src/lib/components/collections/PaneHost.svelte index 590e8686..f5eb6c72 100644 --- a/web/src/lib/components/collections/PaneHost.svelte +++ b/web/src/lib/components/collections/PaneHost.svelte @@ -26,7 +26,7 @@ import { onDestroy, untrack } from 'svelte'; import ItemDetail from '$lib/components/items/ItemDetail.svelte'; import { viewport } from '$lib/stores/breakpoint.svelte'; - import { paneFocusables, nextTrapTarget } from '$lib/collections/paneFocus'; + import { paneFocusables, nextTrapTarget, inExemptSurface } from '$lib/collections/paneFocus'; import type { PaneTarget } from '$lib/types'; interface Props { @@ -39,6 +39,14 @@ /** The coalesced provider-mint ref (TASK-2166) the host computes and passes * in; the inner `` `ref` keys off THIS, not raw `openItemRef`. */ paneMintForRoute: string | null; + /** Focus-follows-editing (PLAN-2179 DR-2/DR-3, TASK-2181) — OPTIONAL. The + * full-page item host owns an `activePane: 'master'|'pane'` model and passes + * it here so the pane's inner ItemDetail freezes (`peeking`) while the MASTER + * is the active side, and the desktop focusin backstop stands down instead of + * fighting a click into the master. LEFT UNSET by the collection route (its + * master is the non-editable list) → pane stays always-editable + the backstop + * keeps its original always-pull-back behavior there (byte-identical). */ + activePane?: 'master' | 'pane'; onClose: () => void; onGone: () => void; onNavigateAway: (url: string) => void; @@ -52,6 +60,7 @@ wsSlug, collSlug, paneMintForRoute, + activePane, onClose, onGone, onNavigateAway, @@ -106,6 +115,16 @@ paneEl.focus({ preventScroll: true }); } + // The stable `.item-pane` region element (PLAN-2179 / TASK-2181). Exposed so + // the full-page host's focus-follows classifier can test membership with + // `paneEl.contains(target)` against the BOUND element — the `.item-page` / + // `.item-pane` class pair alone can't disambiguate (ItemDetail's inner content + // wrapper reuses `.item-page`), which is exactly why the host binds its own + // column too. Returns null before mount. + export function getPaneRegion(): HTMLElement | null { + return paneEl; + } + // ── Resizable detail pane + persisted width (PLAN-2105 / TASK-2114) ─ // No resize primitive existed in the repo, so this is built from // scratch: a draggable divider between the list column and the pane, @@ -323,23 +342,14 @@ // closure boundary otherwise). const region = paneEl; // Surfaces that OWN their own focus/keyboard and legitimately overlay the - // pane — exempt from the trap so it neither hijacks their Tab nor yanks - // focus off them the instant they open. Recognised by role/tag rather than - // per-component class, so PANE-OWNED popups that portal out to - // (outside `.item-pane` in the DOM) are all covered without whack-a-mole: - // • native modal (Share / Edit Collection / Open-Children - // confirm) — top layer, self-trapping. - // • [role="dialog"] — BottomSheets opened FROM WITHIN the pane (field - // selects, Quick Actions, Move To); they render in the pane's own - // stacking context, ABOVE its content, and own their Escape (Codex P1). - // • [role="menu"] / [role="listbox"] — the item-actions reorder menu, the - // editor slash/turn-into menus, select dropdowns, etc. (Codex P2). - // • the editor block context menu — imperative, no ARIA role, so matched - // by class (block-drag-handle.ts). + // pane are exempt from the trap so it neither hijacks their Tab nor yanks + // focus off them the instant they open (native , [role="dialog"] + // BottomSheets, [role="menu"] / [role="listbox"], the editor block context + // menu). Recognised by role/tag/class so pane-owned popups that portal out + // to are all covered — the shared `inExemptSurface` set (paneFocus.ts) + // is the SAME one the host's focus-follows classifier reuses (PLAN-2179). // (Focus already inside `.item-pane` is handled by the `region.contains` // check at each call site.) - const inExemptSurface = (el: Element | null | undefined): boolean => - !!el?.closest?.('dialog, [role="dialog"], [role="menu"], [role="listbox"], .block-context-menu'); function onTrapKeydown(e: KeyboardEvent) { if (e.key !== 'Tab' || e.defaultPrevented) return; if (inExemptSurface(e.target as Element | null)) return; @@ -383,6 +393,15 @@ // exactly the R1 case — the element was REMOVED, so it's already gone — so // we remember whether the last settled focus was inside the pane ourselves // rather than reading it off the event. + // + // Focus-follows-editing reconciliation (PLAN-2179 DR-2 / TASK-2181): on the + // full-page host this backstop must DEFER to `activePane`. Clicking + // non-focusable MASTER text drops focus to ``; if the last focus was in + // the pane this backstop would yank it straight back — fighting "click master + // → activate master". So the pull-back only fires while the PANE is the active + // side (`activePane !== 'master'`). When `activePane` is UNSET (the collection + // route never passes it) `undefined !== 'master'` is true → the original + // always-pull-back R1 behavior is byte-identical there. $effect(() => { if (!browser) return; if (!paneEl || viewport.isMobile || !openItemRef) return; @@ -395,14 +414,18 @@ return; } // Dropped to out of the pane → pull it back into the stable - // region. `region.focus()` re-fires focusin with the region as target, - // which the branch above treats as "inside" (no loop). - if (t === document.body && lastFocusWasInPane) { + // region, but ONLY while the pane is the active side (else a click into + // the master would be immediately overridden). `region.focus()` re-fires + // focusin with the region as target, which the branch above treats as + // "inside" (no loop). Read `activePane` live at event time (not tracked + // by this effect) so the listener isn't re-registered on every flip. + if (t === document.body && lastFocusWasInPane && activePane !== 'master') { region.focus({ preventScroll: true }); return; } // A real element outside the pane (a list row/control the user Tabbed - // or clicked to) — legitimate on the desktop split; stand down. + // or clicked to), OR a body drop while the MASTER is active — legitimate + // on the desktop split; stand down. lastFocusWasInPane = false; } document.addEventListener('focusin', onFocusIn); @@ -556,6 +579,7 @@ { - // Un-peek: the master reclaims the shared activeItem (R9). Nothing - // to re-flush — under Option A pre-pane saves already completed on - // their own while peeking; the freeze only ever blocked NEW edits. - if (item) { + // Un-peek: this side is now the active/editable one, so it reclaims the + // shared activeItem (R9). Nothing to re-flush — under Option A pre-pane + // saves already completed on their own while peeking; the freeze only + // ever blocked NEW edits. Gate on `itemMatchesRef` (PLAN-2179 / TASK-2181; + // Codex review): during a retarget (a drill that flips this side active + // while `ref` already points at the NEXT item but `item` still holds the + // previous one) reclaiming `item` would point activeItem at a stale item. + // Skip until the load resolves — loadData's now-unfrozen `!peeking` + // setActiveItem then claims the freshly-loaded item. + if (item && itemMatchesRef) { collectionStore.setActiveItem(item); activeItemOwnedId = item.id; + // This side owns the singleton editorStore now too — restore its + // scalars from THIS instance's own shadow so +layout's self-save + // suppression reads the ACTIVE side's real dirty/save state, not the + // side we just froze (PLAN-2179 / TASK-2181; Codex review). Mirrors the + // load-time / teardown ownership gates. + editorStore.setDirty(localDirty); + editorStore.setLastSaveTime(localLastSaveTime); } }); } @@ -985,7 +998,18 @@ // (PLAN-2105 / TASK-2112; Codex round 2 P1). onDestroy runs // synchronously on unmount, before the awaited fetch continuations. loadGeneration++; - editorStore.resetForDoc(); + // A FROZEN (peeking) instance is NOT the active editor — it never owned the + // singleton `editorStore`, so its teardown must not reset the scalars the + // ACTIVE side owns (PLAN-2179 / TASK-2181; Codex review). Closing a read-only + // preview beside a dirty editable master would otherwise wipe the master's + // global dirty flag and degrade +layout's self-save suppression. Symmetric + // with the load-time gate and the clear-if-owner for activeItem below. + // `wasPeeking` (the transition effect's plain prev-flag) is this instance's + // last frozen state; constant false for every non-host caller → resetForDoc + // runs, byte-identical. + if (!wasPeeking) { + editorStore.resetForDoc(); + } localDirty = false; // R9 (TASK-2172): clear-if-owner. `collectionStore.activeItem` is a // module singleton shared with the layout (self-save suppression) and @@ -1201,9 +1225,38 @@ } else { collection = collData; } - collectionStore.setActiveItem(itemData); - activeItemOwnedId = itemData.id; - editorStore.resetForDoc(); + // A FROZEN (peeking) instance must NOT write the SINGLETON global stores + // the ACTIVE side owns (PLAN-2179 DR-2 / TASK-2181). Both are module + // singletons shared across the two concurrently-mounted ItemDetail + // instances (master + docked pane): + // • `collectionStore.activeItem` — feeds the layout self-save-suppression + // + ChildItems.defaultCollSlug, which must track the EDITABLE side. + // • `editorStore` scalars (mode/dirty/externalChange) — `resetForDoc()` + // clears `dirty`; a frozen preview opening beside a DIRTY editable + // master would otherwise wipe the master's dirty flag and degrade + // +layout's self-save suppression (Codex review). + // On the focus-follows host the pane LOADS while the master stays active + // (`activePane='master'`); without this gate the pane's load would clobber + // both singletons. The freeze END transition reclaims them the instant THIS + // side becomes editable, so ownership always follows the active side. + // `peeking` defaults false → byte-identical for every non-host caller (the + // collection-route pane included — it never peeks). The per-instance + // `localDirty` shadow (below) resets unconditionally — it's what THIS + // instance's own SSE guards read (TASK-2156). + if (!peeking) { + collectionStore.setActiveItem(itemData); + activeItemOwnedId = itemData.id; + editorStore.resetForDoc(); + // resetForDoc() clears mode/dirty/externalChange but NOT lastSaveTime, + // so a load-time ownership claim would otherwise inherit the PREVIOUS + // owner's save timestamp (e.g. a pane activated mid-load — when the END + // reclaim was skipped by the itemMatchesRef gate — inheriting the frozen + // master's). Establish THIS instance's own lastSaveTime, mirroring the + // END reclaim (Codex review). For a single-instance caller this is a + // no-op: localLastSaveTime already equals editorStore.lastSaveTime (they + // mirror on every write) → byte-identical for the collection route. + editorStore.setLastSaveTime(localLastSaveTime); + } localDirty = false; // Fetch child item progress for any item (generalized parent/child) @@ -2557,8 +2610,13 @@ // that canonical remote state as a background snapshot is exactly the // retain-alive posture (the acceptance carves out remote sync); gating // it here would half-tear-down the provider the freeze must keep whole. + // The per-instance SHADOW (`localDirty`) likewise updates unconditionally. + // But the SINGLETON `editorStore.dirty` IS peeking-gated (PLAN-2179 DR-2 / + // TASK-2181): it's owned by the ACTIVE side, so a FROZEN instance's remote + // traffic must not flip it — else a preview's background sync corrupts + // +layout's self-save suppression for the ACTIVE item. if (collabProvider) { - editorStore.setDirty(true); + if (!peeking) editorStore.setDirty(true); localDirty = true; collabFlusher.schedule(activeCollabContext, markdown); return; @@ -2726,8 +2784,11 @@ if (isForegroundCurrent()) { saveStatus = 'saving'; - editorStore.setLastSaveTime(Date.now()); localLastSaveTime = Date.now(); + // Singleton editorStore is owned by the ACTIVE side only: a FROZEN + // (peeking) instance stamps its own shadow above but must not touch the + // singleton (PLAN-2179 DR-2 / TASK-2181). `saveStatus` is per-instance. + if (!peeking) editorStore.setLastSaveTime(Date.now()); } try { // Pass the provider's per-tab op-log cursor (TASK-1319) so the @@ -2758,10 +2819,14 @@ // round 10 [P1]. if (forceRefreshInFlight) return 'skipped'; if (isForegroundCurrent()) { - editorStore.setLastSaveTime(Date.now()); localLastSaveTime = Date.now(); - editorStore.setDirty(false); localDirty = false; + // Singleton editorStore: active side only (PLAN-2179 / TASK-2181) — + // a frozen preview's remote-op flush updates only its shadow above. + if (!peeking) { + editorStore.setLastSaveTime(Date.now()); + editorStore.setDirty(false); + } showSaved(); } return 'flushed'; diff --git a/web/src/routes/[username]/[workspace]/+layout.svelte b/web/src/routes/[username]/[workspace]/+layout.svelte index 6b31ea33..cce7bd87 100644 --- a/web/src/routes/[username]/[workspace]/+layout.svelte +++ b/web/src/routes/[username]/[workspace]/+layout.svelte @@ -187,7 +187,15 @@ } else { try { const updated = await api.items.get(wsSlug, activeItem.slug); - collectionStore.setActiveItem(updated); + // Fence the late continuation (PLAN-2179 / TASK-2181): on the + // focus-follows-editing host `collectionStore.activeItem` + // ping-pongs master↔pane on each click, so the active item may + // have SWITCHED during this await. Don't clobber the new active + // side's reclaim with the item this event was for; the list + // refresh below is still valid for the fetched item regardless. + if (collectionStore.activeItem?.id === activeItem.id) { + collectionStore.setActiveItem(updated); + } collectionStore.updateItemInList(updated); } catch {} } diff --git a/web/src/routes/[username]/[workspace]/[collection]/[slug]/+page.svelte b/web/src/routes/[username]/[workspace]/[collection]/[slug]/+page.svelte index ff4d1f3f..848ab6a4 100644 --- a/web/src/routes/[username]/[workspace]/[collection]/[slug]/+page.svelte +++ b/web/src/routes/[username]/[workspace]/[collection]/[slug]/+page.svelte @@ -10,6 +10,7 @@ import { type ResolvedPaneState } from '$lib/collections/paneController'; import { createPaneMintSettle, PANE_MINT_SETTLE_MS } from '$lib/collections/paneMintSettle'; import { resolvePaneTarget, isSamePaneTarget, type PaneGuardItem } from '$lib/collections/paneTarget'; + import { inExemptSurface } from '$lib/collections/paneFocus'; import { viewport } from '$lib/stores/breakpoint.svelte'; import { runTopEscape, topEscapePriority, ESCAPE_PRIORITY } from '$lib/stores/escapeStack'; import type { PaneTarget, ResolvedItemIdentity } from '$lib/types'; @@ -114,6 +115,34 @@ // The controller's `focusPaneRegion` dep resolves it lazily at hop time. let paneHostEl = $state(); + // ── Focus-follows-editing (PLAN-2179 DR-2/DR-3 / TASK-2181) ──────────── + // Which side is the ACTIVE (editable) one. The master freezes while the PANE + // is active and vice-versa (via the `peeking` derivations below) so exactly + // ONE side is ever editable — no two-editor collision, by construction. The + // TASK-2180 reactive freeze makes flipping this a cheap toggle (no remount). + // + // Cold-load INITIALIZER: no focusin fires on a `?item=` deep-load, so seed the + // active side from viewport reality — desktop first-open/cold-load focus is the + // MASTER (openItemPaneByRef does NOT move focus into the pane; the pane opens as + // a read-only PREVIEW per DR-2), and mobile is pane-only (the master is inert + // behind the full-screen overlay). Persists across same-route navigations (this + // route component is REUSED on expand / browser Back), so a drilled-then-Back + // pane keeps `activePane='pane'`; a fresh first-open re-seeds it (below). + let activePane = $state<'master' | 'pane'>(viewport.isMobile ? 'pane' : 'master'); + + // Mobile is pane-only: crossing INTO the mobile breakpoint must force the pane + // active (the master goes `inert` behind the overlay — an "active" master there + // would be an un-editable dead end). Reads only `viewport.isMobile` (a plain + // prev-flag tracks the edge), writes only `activePane` → CONVE-1688-safe; fires + // on the transition, not every render. Desktop→mobile only; leaving mobile keeps + // whatever side was active. + let wasMobile = viewport.isMobile; + $effect(() => { + const nowMobile = viewport.isMobile; + if (nowMobile && !wasMobile) activePane = 'pane'; + wasMobile = nowMobile; + }); + // ── Pane-navigation controller (PLAN-2154 Architecture A/E) ──────────── // The SAME controller the collection page mounts — one depth/ownership state // machine, one three-way close. The full-page host has NO list, so the @@ -122,7 +151,17 @@ const paneController = createPaneController({ getOpenItemRef: () => openItemRef, cancelFollow: () => {}, - focusPaneRegion: () => paneHostEl?.focusPaneRegion(), + // Focus-follows-editing (PLAN-2179 DR-2 / TASK-2181): the controller calls + // this on EVERY pane-internal hop — a drill (`navigatePaneTo`), an in-pane + // Back (`handlePaneBack`), and the depth-aware ESC pop. All are pane-internal, + // so each makes the PANE the active/editable side. Setting `activePane` here + // (the single wire point) keeps it consistent with the synchronous + // `focusPaneRegion` focus move — the imminent focusin lands in the pane and + // the classifier agrees, no desync — and covers the test-hook drill path too. + focusPaneRegion: () => { + activePane = 'pane'; + paneHostEl?.focusPaneRegion(); + }, captureReturnFocus: () => {}, setBypassNavGuard: () => {}, }); @@ -238,6 +277,14 @@ // slug / ref-number (the conservative D2 guard — err toward a match). const resolved = resolvePaneTarget(target, masterItem); if (!resolved || isMasterRef(resolved)) return; + // First-open re-seeds the active side (PLAN-2179 DR-2 / TASK-2181): the pane + // opens as a read-only PREVIEW beside the still-editable master on desktop + // (pane-only on mobile). Explicit because `activePane` PERSISTS across the + // route's lifetime — a prior in-pane session may have left it 'pane', and a + // brand-new open must not inherit that (it would freeze the master on open). + // `openItemPaneByRef` does NOT move focus into the pane, so no focusin/hop + // fights this. + activePane = viewport.isMobile ? 'pane' : 'master'; openItemPaneByRef(resolved); } @@ -252,6 +299,79 @@ navigatePaneTo(resolved); } + // ── Focus-follows-editing detectors (PLAN-2179 DR-2/DR-3 / TASK-2181) ─── + // Classify which region a focus / pointer event landed in, so `activePane` + // tracks where the user is working: + // • inside the MASTER column (`itemPageEl`, the bound `.item-page`) → 'master' + // • inside the PANE region (`paneEl`, PaneHost's bound `.item-pane`) → 'pane' + // • a portalled/self-trapping overlay (dialog/menu/listbox/block-context-menu, + // the SHARED `inExemptSurface` set) → DON'T flip (it's in NEITHER region and + // owns its own focus) + // • a bare `` drop or anything else → DON'T flip + // Membership is tested against the BOUND elements, never the `.item-page` / + // `.item-pane` CLASSES — ItemDetail's inner content wrapper reuses `.item-page`, + // so a class match inside the pane would misclassify (which is exactly why the + // host binds its own column and PaneHost exposes `getPaneRegion`). + function classifyPaneRegion(target: EventTarget | null): 'master' | 'pane' | null { + const el = target instanceof Element ? target : null; + if (!el) return null; + // Exempt FIRST: an overlay portalled out of a region (or rendered above it) + // is never a region switch, even if it happens to sit inside one in the DOM. + if (inExemptSurface(el)) return null; + if (itemPageEl && itemPageEl.contains(el)) return 'master'; + const paneEl = paneHostEl?.getPaneRegion() ?? null; + if (paneEl && paneEl.contains(el)) return 'pane'; + return null; + } + + // A pointer/focus target that is (or is inside) a navigable DRILL surface — + // a content link, child row, relationship link, breadcrumb, or graph "Open" + // (all ``, routed through `shouldOpenInPane` → `onOpenTarget`). NEITHER + // detector may flip `activePane` for these: flipping mid-gesture un-freezes the + // pane, which re-inits ChildItems' live `dndzone` (`dragDisabled` tracks the + // freeze) and SWALLOWS the very click that would drill (PLAN-2179 DR-2 / + // TASK-2181). Instead we let the click's own drill fire — `navigatePaneTo` + // ALREADY sets `activePane='pane'` (via focusPaneRegion) — so a content link in + // a frozen preview drills on the FIRST click AND activates the pane, no dndzone + // churn. (Chromium/Firefox focus an `` on mouse-click, so the exclusion + // must cover the focusin path too, not just pointerdown.) + function isNavigableDrillTarget(target: EventTarget | null): boolean { + const el = target instanceof Element ? target : null; + return !!el?.closest('a[href]'); + } + + // The detectors flip `activePane` ONLY on a CHANGED region (no begin/end + // transition churn on the frozen/active ItemDetail pair), and BOTH exclude + // navigable drill targets (their own click owns the transition — see above): + // • focusin — the primary classifier: Tab / programmatic focus / a click that + // lands focus on a focusable control fires it with a real region target. + // Chromium/Firefox DO focus an `` on mouse-click, so this fires for a + // content-link drill too — hence the same navigable exclusion here, or the + // focus-driven flip would re-init the dndzone and swallow the drill. + // • pointerdown (CAPTURE) — the activator for NON-focusable text/background: + // clicking plain master/pane body text drops focus to `` and fires NO + // region focusin, so a pointer landing on the region must flip regardless of + // the target's focusability. This is what makes "click into the master + // activates it". Capture-phase so a child `stopPropagation` can't swallow it. + // Only mounted while a pane is open (`paneOpen`) — with no pane there is no + // second side to arbitrate. Reading the boolean (not raw `openItemRef`) keeps + // the effect from re-registering on every drill. + let paneOpen = $derived(!!openItemRef); + $effect(() => { + if (!browser || !paneOpen) return; + function onRegionEvent(e: Event) { + if (isNavigableDrillTarget(e.target)) return; + const region = classifyPaneRegion(e.target); + if (region && region !== activePane) activePane = region; + } + document.addEventListener('focusin', onRegionEvent); + document.addEventListener('pointerdown', onRegionEvent, true); + return () => { + document.removeEventListener('focusin', onRegionEvent); + document.removeEventListener('pointerdown', onRegionEvent, true); + }; + }); + // Cold-load strip (PLAN-2154 Architecture E). A hand-crafted / shared // `?item=` URL must NOT mount a pane on the master itself // (two providers, one room, one shared itemID-only cursor). Once the master's @@ -407,7 +527,7 @@ {wsSlug} {collSlug} {ref} - peeking={!!openItemRef} + peeking={!!openItemRef && activePane === 'pane'} onReady={(r) => (scrollReady = r)} onIdentity={(id) => { masterIdentity = id; @@ -427,6 +547,7 @@ {wsSlug} {collSlug} {paneMintForRoute} + {activePane} onClose={closeItemPane} onGone={closeItemPane} onNavigateAway={handlePaneNavigateAway}