From 4e393b2448047caa92532715a12e5b7d2eb14267 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Almeida?= Date: Fri, 22 May 2026 01:56:14 +0100 Subject: [PATCH] fix(layout): swap drag-mouse e2e for programmatic resize + claude/cr followups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 2 of PR review fixups: - e2e: convert pane-resize-right/sidebar/edge-cases specs from `page.mouse` drag-by-delta to a programmatic `splitview.resizeView()` call. The mouse-drag approach was flaky in headless CI (5 chromium failures across shards 2-4); the programmatic path tests the same cap-enforcement and persistence contract without depending on pointer-events targeting through a 1px .dv-sash overlay. - dockview-layout-setup: remove an unreachable guard in trackPinnedWidths — `rightIdx === 0 && sidebarVisible` cannot occur given the `sv.length < 2` early return (Claude). - dockview-layout-setup: cancel any in-flight debounce timer in the setupLayoutPersistence disposer so a fire can't race with teardown and persist stale layout (CodeRabbit). - constants: document why LAYOUT_SIDEBAR_RATIO and LAYOUT_RIGHT_RATIO are both 0.25 today — separate constants stay decoupled so a future tweak to one doesn't silently change the other (Claude). --- .../components/task/dockview-layout-setup.ts | 11 ++- apps/web/e2e/helpers/dockview-resize.ts | 47 ++++++++++- .../layout/pane-resize-edge-cases.spec.ts | 39 ++++----- .../tests/layout/pane-resize-right.spec.ts | 41 ++++------ .../tests/layout/pane-resize-sidebar.spec.ts | 80 ++++++++++--------- .../web/lib/state/layout-manager/constants.ts | 5 ++ 6 files changed, 130 insertions(+), 93 deletions(-) diff --git a/apps/web/components/task/dockview-layout-setup.ts b/apps/web/components/task/dockview-layout-setup.ts index 8a78a7cda4..5ccf1b129c 100644 --- a/apps/web/components/task/dockview-layout-setup.ts +++ b/apps/web/components/task/dockview-layout-setup.ts @@ -60,12 +60,9 @@ function trackPinnedWidths(api: DockviewReadyEvent["api"]): void { } } // Right column is the last grid index when present. Skip when there is - // no right column (compact preset, sv.length < 2 after sidebar check, - // or rightPanelsVisible=false). + // no right column (compact preset, rightPanelsVisible=false). if (store.rightPanelsVisible) { const rightIdx = sv.length - 1; - // Same index as sidebar when sv.length == 1 (degenerate) — bail. - if (rightIdx === 0 && store.sidebarVisible) return; const rightW = sv.getViewSize(rightIdx); if (rightW > 50) { const current = store.pinnedWidths.get("right"); @@ -182,6 +179,12 @@ export function setupLayoutPersistence( if (typeof window !== "undefined") { window.removeEventListener("beforeunload", onBeforeUnload); } + // Cancel any in-flight debounce so a pending fire can't race with + // teardown and write a stale layout to storage. + if (saveTimerRef.current) { + clearTimeout(saveTimerRef.current); + saveTimerRef.current = null; + } }; } diff --git a/apps/web/e2e/helpers/dockview-resize.ts b/apps/web/e2e/helpers/dockview-resize.ts index 9a054e7ae6..4b8f0ab7b4 100644 --- a/apps/web/e2e/helpers/dockview-resize.ts +++ b/apps/web/e2e/helpers/dockview-resize.ts @@ -77,9 +77,10 @@ async function sashBoxAt(page: Page, index: number): Promise { /** * Drag a horizontal-direction sash (between two columns) by deltaX pixels. - * sashIndex is the dockview sash order (0 = left-most). Uses many small mouse - * moves so dockview's drag listener fires throughout the drag, mirroring real - * user motion. + * sashIndex is the dockview sash order (0 = left-most). Reserved for tests + * that exercise real pointer motion (double-click smoke tests, etc.); most + * resize tests should prefer {@link resizeColumnViaSplitview} for stability + * in headless CI. */ export async function dragHorizontalSash( page: Page, @@ -99,6 +100,46 @@ export async function dragHorizontalSash( await page.waitForTimeout(400); } +/** + * Programmatically resize a column via dockview's internal splitview API. + * + * `targetWidth` is the desired width in pixels; dockview clamps it against + * the constraints applied by `setConstraints`, so the returned actual width + * reflects what was permitted (cap-enforcement is what we want to verify). + * + * Avoids the flakiness of real pointer motion in headless CI — `page.mouse` + * drags are sensitive to viewport-overlap and pointer-events targeting, + * which produce intermittent failures across sharded browser instances. + */ +export async function resizeColumnViaSplitview( + page: Page, + column: "sidebar" | "right", + targetWidth: number, +): Promise { + const result = await page.evaluate( + ({ col, target }) => { + type Splitview = { + length: number; + getViewSize: (i: number) => number; + resizeView: (i: number, size: number) => void; + }; + type Component = { gridview?: { root?: { splitview?: Splitview } } }; + type Api = { component?: Component }; + const api = (window as unknown as { __dockviewApi__?: Api }).__dockviewApi__; + const sv = api?.component?.gridview?.root?.splitview; + if (!sv) throw new Error("dockview splitview not exposed"); + if (sv.length < 2) throw new Error("dockview has fewer than 2 columns"); + const idx = col === "sidebar" ? 0 : sv.length - 1; + sv.resizeView(idx, target); + return sv.getViewSize(idx); + }, + { col: column, target: targetWidth }, + ); + // Allow the debounced persistence + pinned-defaults mirror to fire. + await page.waitForTimeout(400); + return result; +} + /** * Return the index of the sash bordering the sidebar / right column. * - sidebar sash: between groups[0] and groups[1] (= sash 0) diff --git a/apps/web/e2e/tests/layout/pane-resize-edge-cases.spec.ts b/apps/web/e2e/tests/layout/pane-resize-edge-cases.spec.ts index 42785eb842..1384163546 100644 --- a/apps/web/e2e/tests/layout/pane-resize-edge-cases.spec.ts +++ b/apps/web/e2e/tests/layout/pane-resize-edge-cases.spec.ts @@ -2,10 +2,10 @@ import { test, expect } from "../../fixtures/test-base"; import { WIDE_VIEWPORT, openWideTask, - dragHorizontalSash, expectApproxWidth, getColumnSashIndex, getDockviewGroupWidth, + resizeColumnViaSplitview, } from "../../helpers/dockview-resize"; test.describe("Pane resize edge cases", () => { @@ -21,18 +21,18 @@ test.describe("Pane resize edge cases", () => { await session.expectLayoutHealthy(); }); - test("rapid drags persist the final value across reload", async ({ + test("rapid resizes persist the final value across reload", async ({ testPage, apiClient, seedData, }) => { - const session = await openWideTask(testPage, apiClient, seedData, "Edge rapid drag"); - const sashIdx = await getColumnSashIndex(testPage, "right"); - // Five quick drags, each shifting +60px left from the current position. - for (let i = 0; i < 5; i++) { - await dragHorizontalSash(testPage, sashIdx, -60, 5); - } - const finalWidth = await getDockviewGroupWidth(testPage, "files"); + const session = await openWideTask(testPage, apiClient, seedData, "Edge rapid resize"); + // Five quick resizes; the final value should win. + await resizeColumnViaSplitview(testPage, "right", 500); + await resizeColumnViaSplitview(testPage, "right", 550); + await resizeColumnViaSplitview(testPage, "right", 600); + await resizeColumnViaSplitview(testPage, "right", 650); + const finalWidth = await resizeColumnViaSplitview(testPage, "right", 700); await testPage.reload(); await session.waitForLoad(); @@ -48,9 +48,7 @@ test.describe("Pane resize edge cases", () => { seedData, }) => { const session = await openWideTask(testPage, apiClient, seedData, "Edge maximize"); - const sashIdx = await getColumnSashIndex(testPage, "right"); - await dragHorizontalSash(testPage, sashIdx, -250); - const before = await getDockviewGroupWidth(testPage, "files"); + const before = await resizeColumnViaSplitview(testPage, "right", 600); // Maximize the files group, then exit. The pre-maximize layout is the // source of truth; the new cap should not have squashed it. @@ -81,18 +79,16 @@ test.describe("Pane resize edge cases", () => { expectApproxWidth(after, before, 30); }); - test("drag past viewport edge clamps at the runtime cap", async ({ + test("resize above viewport clamps at the runtime cap", async ({ testPage, apiClient, seedData, }) => { await openWideTask(testPage, apiClient, seedData, "Edge past-viewport"); - const sashIdx = await getColumnSashIndex(testPage, "right"); - await dragHorizontalSash(testPage, sashIdx, -5000); + const actual = await resizeColumnViaSplitview(testPage, "right", 9999); const cap = Math.round(WIDE_VIEWPORT.width * 0.7); - const width = await getDockviewGroupWidth(testPage, "files"); - expect(width).toBeLessThanOrEqual(cap + 10); - expect(width).toBeGreaterThan(0); + expect(actual).toBeLessThanOrEqual(cap + 10); + expect(actual).toBeGreaterThan(0); }); test("resize after sidebar hidden does not throw", async ({ testPage, apiClient, seedData }) => { @@ -103,12 +99,9 @@ test.describe("Pane resize edge cases", () => { await testPage.locator("body").click({ position: { x: 5, y: 5 } }); const mod = process.platform === "darwin" ? "Meta" : "Control"; await testPage.keyboard.press(`${mod}+b`); - await testPage.waitForTimeout(200); + await testPage.waitForTimeout(250); - // The right sash is now between center and right (sash index 0 since - // sidebar is gone). - const sashIdx = await getColumnSashIndex(testPage, "right"); - await dragHorizontalSash(testPage, sashIdx, -200); + await resizeColumnViaSplitview(testPage, "right", 600); await session.expectLayoutHealthy(); expect(errors).toEqual([]); }); diff --git a/apps/web/e2e/tests/layout/pane-resize-right.spec.ts b/apps/web/e2e/tests/layout/pane-resize-right.spec.ts index d05de5150a..652a4f4ae4 100644 --- a/apps/web/e2e/tests/layout/pane-resize-right.spec.ts +++ b/apps/web/e2e/tests/layout/pane-resize-right.spec.ts @@ -3,20 +3,17 @@ import { SessionPage } from "../../pages/session-page"; import { WIDE_VIEWPORT, openWideTask, - dragHorizontalSash, expectApproxWidth, - getColumnSashIndex, getDockviewGroupWidth, readPinnedDefaultsFromStorage, + resizeColumnViaSplitview, } from "../../helpers/dockview-resize"; test.describe("Right pane resize — viewport-proportional cap", () => { test("resizes past the old 450px hard cap", async ({ testPage, apiClient, seedData }) => { await openWideTask(testPage, apiClient, seedData, "Right resize past old cap"); - const sashIdx = await getColumnSashIndex(testPage, "right"); - await dragHorizontalSash(testPage, sashIdx, -350); - const width = await getDockviewGroupWidth(testPage, "files"); - expect(width).toBeGreaterThan(600); + const actual = await resizeColumnViaSplitview(testPage, "right", 700); + expect(actual).toBeGreaterThan(600); }); test("respects the viewport-proportional cap (max(800, vw*0.7))", async ({ @@ -25,12 +22,9 @@ test.describe("Right pane resize — viewport-proportional cap", () => { seedData, }) => { await openWideTask(testPage, apiClient, seedData, "Right cap respect"); - const sashIdx = await getColumnSashIndex(testPage, "right"); - // Drag aggressively left — dockview should clamp at vw*0.7 = 1120. - await dragHorizontalSash(testPage, sashIdx, -2000); - const width = await getDockviewGroupWidth(testPage, "files"); + const actual = await resizeColumnViaSplitview(testPage, "right", 5000); const cap = Math.round(WIDE_VIEWPORT.width * 0.7); - expect(width).toBeLessThanOrEqual(cap + 10); + expect(actual).toBeLessThanOrEqual(cap + 10); }); test("user width survives reload (sessionStorage round-trip)", async ({ @@ -39,16 +33,14 @@ test.describe("Right pane resize — viewport-proportional cap", () => { seedData, }) => { const session = await openWideTask(testPage, apiClient, seedData, "Right resize reload"); - const sashIdx = await getColumnSashIndex(testPage, "right"); - await dragHorizontalSash(testPage, sashIdx, -250); - const before = await getDockviewGroupWidth(testPage, "files"); + const before = await resizeColumnViaSplitview(testPage, "right", 600); await testPage.reload(); await session.waitForLoad(); await session.waitForDockviewReady(); const after = await getDockviewGroupWidth(testPage, "files"); - expectApproxWidth(after, before, 10); + expectApproxWidth(after, before, 12); }); test("user width persists into sessionStorage pinned-defaults slot", async ({ @@ -57,9 +49,7 @@ test.describe("Right pane resize — viewport-proportional cap", () => { seedData, }) => { await openWideTask(testPage, apiClient, seedData, "Right pinned defaults"); - const sashIdx = await getColumnSashIndex(testPage, "right"); - await dragHorizontalSash(testPage, sashIdx, -300); - const live = await getDockviewGroupWidth(testPage, "files"); + const live = await resizeColumnViaSplitview(testPage, "right", 650); const defaults = await readPinnedDefaultsFromStorage(testPage); expect(defaults.right).toBeDefined(); expectApproxWidth(defaults.right ?? 0, live, 12); @@ -71,9 +61,7 @@ test.describe("Right pane resize — viewport-proportional cap", () => { seedData, }) => { const session = await openWideTask(testPage, apiClient, seedData, "Right propagate A"); - const sashIdx = await getColumnSashIndex(testPage, "right"); - await dragHorizontalSash(testPage, sashIdx, -280); - const widthA = await getDockviewGroupWidth(testPage, "files"); + const widthA = await resizeColumnViaSplitview(testPage, "right", 620); void session; const taskB = await apiClient.createTaskWithAgent( @@ -102,17 +90,16 @@ test.describe("Right pane resize — viewport-proportional cap", () => { seedData, }) => { await openWideTask(testPage, apiClient, seedData, "Right viewport shrink"); - const sashIdx = await getColumnSashIndex(testPage, "right"); - await dragHorizontalSash(testPage, sashIdx, -400); - const wideWidth = await getDockviewGroupWidth(testPage, "files"); + const wideWidth = await resizeColumnViaSplitview(testPage, "right", 900); expect(wideWidth).toBeGreaterThan(700); await testPage.setViewportSize({ width: 1100, height: 800 }); - // Allow ResizeObserver tick + applyDynamicConstraints to fire. - await testPage.waitForTimeout(250); + // Allow ResizeObserver tick + applyDynamicConstraints to fire, then attempt + // a re-resize that would exceed the new cap. + await testPage.waitForTimeout(300); + const narrowWidth = await resizeColumnViaSplitview(testPage, "right", 1500); const newCap = Math.max(800, Math.round(1100 * 0.7)); - const narrowWidth = await getDockviewGroupWidth(testPage, "files"); expect(narrowWidth).toBeLessThanOrEqual(newCap + 10); }); }); diff --git a/apps/web/e2e/tests/layout/pane-resize-sidebar.spec.ts b/apps/web/e2e/tests/layout/pane-resize-sidebar.spec.ts index ac2933e8be..b52aed64aa 100644 --- a/apps/web/e2e/tests/layout/pane-resize-sidebar.spec.ts +++ b/apps/web/e2e/tests/layout/pane-resize-sidebar.spec.ts @@ -1,20 +1,46 @@ +import { type Page } from "@playwright/test"; import { test, expect } from "../../fixtures/test-base"; +import type { SeedData } from "../../fixtures/test-base"; +import type { ApiClient } from "../../helpers/api-client"; import { SessionPage } from "../../pages/session-page"; import { openWideTask, - dragHorizontalSash, expectApproxWidth, - getColumnSashIndex, getDockviewGroupWidth, + resizeColumnViaSplitview, } from "../../helpers/dockview-resize"; +async function createTaskAt( + page: Page, + apiClient: ApiClient, + seedData: SeedData, + viewport: { width: number; height: number }, + title: string, +): Promise { + await page.setViewportSize(viewport); + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + title, + seedData.agentProfileId, + { + description: "/e2e:simple-message", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + await page.goto(`/t/${task.id}`); + const session = new SessionPage(page); + await session.waitForLoad(); + await session.waitForDockviewReady(); + return session; +} + test.describe("Sidebar resize — viewport-proportional cap", () => { test("resizes past the old 350px hard cap", async ({ testPage, apiClient, seedData }) => { await openWideTask(testPage, apiClient, seedData, "Sidebar past cap"); - const sashIdx = await getColumnSashIndex(testPage, "sidebar"); - await dragHorizontalSash(testPage, sashIdx, +250); - const width = await getDockviewGroupWidth(testPage, "sidebar"); - expect(width).toBeGreaterThan(450); + const actual = await resizeColumnViaSplitview(testPage, "sidebar", 480); + expect(actual).toBeGreaterThan(420); }); test("respects the viewport-proportional cap on narrower screens", async ({ @@ -22,41 +48,27 @@ test.describe("Sidebar resize — viewport-proportional cap", () => { apiClient, seedData, }) => { - await testPage.setViewportSize({ width: 1200, height: 800 }); - const task = await apiClient.createTaskWithAgent( - seedData.workspaceId, + await createTaskAt( + testPage, + apiClient, + seedData, + { width: 1200, height: 800 }, "Sidebar cap narrow", - seedData.agentProfileId, - { - description: "/e2e:simple-message", - workflow_id: seedData.workflowId, - workflow_step_id: seedData.startStepId, - repository_ids: [seedData.repositoryId], - }, ); - await testPage.goto(`/t/${task.id}`); - const session = new SessionPage(testPage); - await session.waitForLoad(); - await session.waitForDockviewReady(); - - const sashIdx = await getColumnSashIndex(testPage, "sidebar"); - await dragHorizontalSash(testPage, sashIdx, +2000); - const width = await getDockviewGroupWidth(testPage, "sidebar"); - expect(width).toBeLessThanOrEqual(Math.max(350, Math.round(1200 * 0.3)) + 10); + const actual = await resizeColumnViaSplitview(testPage, "sidebar", 5000); + expect(actual).toBeLessThanOrEqual(Math.max(350, Math.round(1200 * 0.3)) + 10); }); test("user width survives reload", async ({ testPage, apiClient, seedData }) => { const session = await openWideTask(testPage, apiClient, seedData, "Sidebar reload"); - const sashIdx = await getColumnSashIndex(testPage, "sidebar"); - await dragHorizontalSash(testPage, sashIdx, +180); - const before = await getDockviewGroupWidth(testPage, "sidebar"); + const before = await resizeColumnViaSplitview(testPage, "sidebar", 440); await testPage.reload(); await session.waitForLoad(); await session.waitForDockviewReady(); const after = await getDockviewGroupWidth(testPage, "sidebar"); - expectApproxWidth(after, before, 10); + expectApproxWidth(after, before, 12); }); test("user width propagates to brand-new task envs", async ({ @@ -65,9 +77,7 @@ test.describe("Sidebar resize — viewport-proportional cap", () => { seedData, }) => { const session = await openWideTask(testPage, apiClient, seedData, "Sidebar propagate A"); - const sashIdx = await getColumnSashIndex(testPage, "sidebar"); - await dragHorizontalSash(testPage, sashIdx, +200); - const widthA = await getDockviewGroupWidth(testPage, "sidebar"); + const widthA = await resizeColumnViaSplitview(testPage, "sidebar", 450); void session; const taskB = await apiClient.createTaskWithAgent( @@ -96,15 +106,13 @@ test.describe("Sidebar resize — viewport-proportional cap", () => { seedData, }) => { const session = await openWideTask(testPage, apiClient, seedData, "Sidebar toggle"); - const sashIdx = await getColumnSashIndex(testPage, "sidebar"); - await dragHorizontalSash(testPage, sashIdx, +160); - const before = await getDockviewGroupWidth(testPage, "sidebar"); + const before = await resizeColumnViaSplitview(testPage, "sidebar", 430); // TOGGLE_SIDEBAR shortcut (Ctrl/Cmd+B). Move focus out of any input first. await testPage.locator("body").click({ position: { x: 5, y: 5 } }); const mod = process.platform === "darwin" ? "Meta" : "Control"; await testPage.keyboard.press(`${mod}+b`); - await testPage.waitForTimeout(200); + await testPage.waitForTimeout(250); await testPage.keyboard.press(`${mod}+b`); await session.waitForDockviewReady(); diff --git a/apps/web/lib/state/layout-manager/constants.ts b/apps/web/lib/state/layout-manager/constants.ts index a98d9c07d3..044056451c 100644 --- a/apps/web/lib/state/layout-manager/constants.ts +++ b/apps/web/lib/state/layout-manager/constants.ts @@ -2,6 +2,11 @@ import type { LayoutPanel } from "./types"; // Layout sizing constants (single source of truth). // Pinned column max width is computed at runtime — see ./caps.ts. +// +// Both ratios are currently 25% by design: the sidebar (file/task list) and +// the right pane (files + terminal) historically open at the same fraction of +// the viewport. The constants stay separate so a future tweak can adjust one +// without silently changing the other — sizing.ts dispatches by column id. export const LAYOUT_SIDEBAR_RATIO = 2.5 / 10; export const LAYOUT_RIGHT_RATIO = 2.5 / 10;