From d35f36ae73ababb3fe898a41b5678c4a0d56e5b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Sat, 30 May 2026 19:16:26 +0100 Subject: [PATCH] fix(web): make left sidebar width global + steady monitor-switch settling (#1140) --- .../components/task/dockview-layout-setup.ts | 30 +++---- .../task/layout-preset-selector.tsx | 49 ++++++----- apps/web/e2e/helpers/dockview-resize.ts | 5 ++ .../tests/layout/pane-resize-sidebar.spec.ts | 81 +++++++++++++++++++ apps/web/lib/local-storage.test.ts | 37 +++++++++ apps/web/lib/local-storage.ts | 22 +++++ .../state/dockview-env-switch-action.test.ts | 3 + .../state/dockview-env-switch-pinned.test.ts | 32 ++++---- apps/web/lib/state/dockview-env-switch.ts | 7 +- .../dockview-layout-builders-fixups.test.ts | 47 ++++++++++- .../web/lib/state/dockview-layout-builders.ts | 58 +++++++++++-- apps/web/lib/state/dockview-pinned-enforce.ts | 15 +++- apps/web/lib/state/dockview-store.test.ts | 60 +++++++++++--- apps/web/lib/state/dockview-store.ts | 51 +++++++----- .../lib/state/layout-manager/sizing.test.ts | 49 +++++++++++ apps/web/lib/state/layout-manager/sizing.ts | 13 ++- 16 files changed, 466 insertions(+), 93 deletions(-) create mode 100644 apps/web/lib/state/layout-manager/sizing.test.ts diff --git a/apps/web/components/task/dockview-layout-setup.ts b/apps/web/components/task/dockview-layout-setup.ts index 74cc736081..bc8fc87a3d 100644 --- a/apps/web/components/task/dockview-layout-setup.ts +++ b/apps/web/components/task/dockview-layout-setup.ts @@ -11,7 +11,7 @@ import { RIGHT_BOTTOM_GROUP, setPinnedTarget, } from "@/lib/state/layout-manager"; -import { setEnvLayout } from "@/lib/local-storage"; +import { setEnvLayout, setGlobalSidebarWidth } from "@/lib/local-storage"; import { panelPortalManager } from "@/lib/layout/panel-portal-manager"; import { stopVscode } from "@/lib/api/domains/vscode-api"; import { parkUserShell, stopUserShell } from "@/lib/api/domains/user-shell-api"; @@ -123,7 +123,15 @@ export function setupSashDragCapToggle(api: DockviewReadyEvent["api"]): () => vo const sv = getRootSplitview(api); if (!sv) return; const store = useDockviewStore.getState(); - if (store.sidebarVisible) setPinnedTarget("sidebar", sv.getViewSize(0)); + if (store.sidebarVisible) { + // A genuine drag is the ONLY writer of the global sidebar width pref. + // Persist it globally and mirror into the store's pinnedWidths so the + // override path (resolvePresetPinnedWidths / classifyColumns) agrees. + const sidebarW = sv.getViewSize(0); + setPinnedTarget("sidebar", sidebarW); + setGlobalSidebarWidth(sidebarW); + store.setPinnedWidth("sidebar", sidebarW); + } if (store.rightPanelsVisible) setPinnedTarget("right", sv.getViewSize(sv.length - 1)); if (IS_DEBUG) { debugWidths(`sash-drag-end ${formatWidthsSnapshot(snapshotColumnWidths(api))}`); @@ -152,18 +160,12 @@ function trackPinnedWidths(api: DockviewReadyEvent["api"]): void { const sv = getRootSplitview(api); if (!sv || sv.length < 2) return; try { - // Sidebar is grid index 0 *only when sidebar is visible*. Without the - // visibility guard, hiding the sidebar makes index 0 the center column, - // and we'd persist the center width as the sidebar's preferred width. - if (store.sidebarVisible) { - const sidebarW = sv.getViewSize(0); - if (sidebarW > 50) { - const current = store.pinnedWidths.get("sidebar"); - if (current !== sidebarW) { - store.setPinnedWidth("sidebar", sidebarW); - } - } - } + // The sidebar width is NOT tracked here: it is a global pref written only + // by a genuine sash drag (see setupSashDragCapToggle). Capturing the live + // width on every layout change would persist dockview's transient + // proportional-rebalance widths (e.g. 320→213 on a monitor shrink) and + // fight enforcePinnedTargets. Right column is still mirrored below. + // // Right column is the last grid index when present. Skip when there is // no right column (compact preset, rightPanelsVisible=false). if (store.rightPanelsVisible) { diff --git a/apps/web/components/task/layout-preset-selector.tsx b/apps/web/components/task/layout-preset-selector.tsx index 4e6a70fd12..108bfb8629 100644 --- a/apps/web/components/task/layout-preset-selector.tsx +++ b/apps/web/components/task/layout-preset-selector.tsx @@ -208,6 +208,29 @@ function SavedLayoutItems({ )); } +/** The built-in preset menu items. Applying any preset resets widths + * (resetWidths=true), which is how a user clears a custom sidebar width. */ +function BuiltInPresetItems({ onApply }: { onApply: (presetId: BuiltInPreset) => void }) { + return BUILT_IN_PRESETS.map((preset) => { + const Icon = preset.icon; + return ( + onApply(preset.id)} + className="cursor-pointer" + > + +
+ {preset.label} + {preset.description} +
+ + ); + }); +} + export function LayoutPresetSelector() { const [saveDialogOpen, setSaveDialogOpen] = useState(false); const [dropdownOpen, setDropdownOpen] = useState(false); @@ -261,7 +284,12 @@ export function LayoutPresetSelector() { - @@ -271,24 +299,7 @@ export function LayoutPresetSelector() { Presets - {BUILT_IN_PRESETS.map((preset) => { - const Icon = preset.icon; - return ( - applyBuiltInPreset(preset.id, true)} - className="cursor-pointer" - > - -
- {preset.label} - - {preset.description} - -
- - ); - })} + applyBuiltInPreset(id, true)} /> diff --git a/apps/web/e2e/helpers/dockview-resize.ts b/apps/web/e2e/helpers/dockview-resize.ts index 98e200a0e7..4c02b42797 100644 --- a/apps/web/e2e/helpers/dockview-resize.ts +++ b/apps/web/e2e/helpers/dockview-resize.ts @@ -184,6 +184,11 @@ export async function resizeColumnViaSplitview( if (typeof w.__setPinnedTarget__ === "function") { w.__setPinnedTarget__(col, actual); } + // The sidebar width is a global pref persisted on a real drag's mouseup. + // Mirror that so cross-task / reload assertions see the same width. + if (col === "sidebar" && typeof w.__setGlobalSidebarWidth__ === "function") { + w.__setGlobalSidebarWidth__(actual); + } // `sv.resizeView` alone does not fire `onDidLayoutChange` in dockview // 4.x — without that, the debounced layout-persistence handler never // saves the new width. Call the exposed test helper to force-flush the 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 9bad07c90b..6391c76204 100644 --- a/apps/web/e2e/tests/layout/pane-resize-sidebar.spec.ts +++ b/apps/web/e2e/tests/layout/pane-resize-sidebar.spec.ts @@ -58,3 +58,84 @@ test.describe("Sidebar resize — viewport-proportional cap", () => { expectApproxWidth(after, before, 20); }); }); + +test.describe("Sidebar width — global across tasks", () => { + test("a width set in one task applies to a different task", async ({ + testPage, + apiClient, + seedData, + }) => { + // Task A: drag the sidebar to a distinctive width. + const sessionA = await openWideTask(testPage, apiClient, seedData, "Sidebar global A"); + const widthA = await resizeColumnViaSplitview(testPage, "sidebar", 440); + + // Task B is a separate task (separate env). It must open at task A's width + // because the left sidebar width is now a single global preference. + const taskB = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Sidebar global B", + seedData.agentProfileId, + { + description: "/e2e:simple-message", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + await testPage.goto(`/t/${taskB.id}`); + await sessionA.waitForLoad(); + await sessionA.waitForDockviewReady(); + + const widthB = await getDockviewGroupWidth(testPage, "sidebar"); + expectApproxWidth(widthB, widthA, 12); + }); + + test("clamps to fit a smaller screen, then restores on a wider one", async ({ + testPage, + apiClient, + seedData, + }) => { + // Set a wide sidebar on a 1600px screen. + const session = await openWideTask(testPage, apiClient, seedData, "Sidebar clamp"); + const wide = await resizeColumnViaSplitview(testPage, "sidebar", 520); + expect(wide).toBeGreaterThan(450); + + // Shrink the viewport. The sidebar must settle within the smaller screen's + // cap (max(350, 30% of 1100) = 350, bounded by viewport-300) — not stay at + // 520 and not drift. + await testPage.setViewportSize({ width: 1100, height: 800 }); + await session.waitForDockviewReady(); + await testPage.waitForTimeout(400); + const small = await getDockviewGroupWidth(testPage, "sidebar"); + const smallCap = Math.max(350, Math.round(1100 * 0.3)); + expect(small).toBeLessThanOrEqual(smallCap + 12); + + // Back to a wide screen: the raw 520 preference is restored (clamp does not + // overwrite stored width). + await testPage.setViewportSize({ width: 1600, height: 900 }); + await session.waitForDockviewReady(); + await testPage.waitForTimeout(400); + const restored = await getDockviewGroupWidth(testPage, "sidebar"); + expectApproxWidth(restored, wide, 16); + }); + + test("Default layout preset resets the custom width", async ({ + testPage, + apiClient, + seedData, + }) => { + const session = await openWideTask(testPage, apiClient, seedData, "Sidebar reset"); + const custom = await resizeColumnViaSplitview(testPage, "sidebar", 480); + expect(custom).toBeGreaterThan(420); + + // Apply the "Default" layout preset (resetWidths=true). + await testPage.getByTestId("layout-preset-trigger").click(); + await testPage.locator('[data-testid="layout-preset-item"][data-preset-id="default"]').click(); + await session.waitForDockviewReady(); + await testPage.waitForTimeout(400); + + // Sidebar returns to the ratio default (1600*0.25=400, clamped to 350). + const afterReset = await getDockviewGroupWidth(testPage, "sidebar"); + expectApproxWidth(afterReset, 350, 16); + }); +}); diff --git a/apps/web/lib/local-storage.test.ts b/apps/web/lib/local-storage.test.ts index 6a4a0bf4cc..ade72f0ed0 100644 --- a/apps/web/lib/local-storage.test.ts +++ b/apps/web/lib/local-storage.test.ts @@ -1,8 +1,11 @@ import { beforeEach, describe, expect, it } from "vitest"; import { cleanupTaskStorage, + clearGlobalSidebarWidth, + getGlobalSidebarWidth, markPRClosedBannerDismissed, markPRMergedBannerDismissed, + setGlobalSidebarWidth, wasPRClosedBannerDismissed, wasPRMergedBannerDismissed, } from "./local-storage"; @@ -66,3 +69,37 @@ describe("PR closed banner dismissal storage", () => { expect(wasPRClosedBannerDismissed("task-b")).toBe(true); }); }); + +describe("global sidebar width storage", () => { + beforeEach(() => { + window.localStorage.clear(); + }); + + it("returns null when no width has been stored", () => { + expect(getGlobalSidebarWidth()).toBeNull(); + }); + + it("persists a width globally and reads it back (rounded)", () => { + setGlobalSidebarWidth(412.6); + expect(getGlobalSidebarWidth()).toBe(413); + }); + + it("ignores non-positive or non-finite widths", () => { + setGlobalSidebarWidth(0); + setGlobalSidebarWidth(-100); + setGlobalSidebarWidth(Number.NaN); + expect(getGlobalSidebarWidth()).toBeNull(); + }); + + it("clears the stored width", () => { + setGlobalSidebarWidth(320); + clearGlobalSidebarWidth(); + expect(getGlobalSidebarWidth()).toBeNull(); + }); + + it("is NOT removed by cleanupTaskStorage (it is global, not task-scoped)", () => { + setGlobalSidebarWidth(320); + cleanupTaskStorage("task-a", []); + expect(getGlobalSidebarWidth()).toBe(320); + }); +}); diff --git a/apps/web/lib/local-storage.ts b/apps/web/lib/local-storage.ts index 8bf3d86588..81b880efdd 100644 --- a/apps/web/lib/local-storage.ts +++ b/apps/web/lib/local-storage.ts @@ -288,6 +288,28 @@ export function setEnvLayout(envId: string, layout: object): void { } } +// --- Dockview global left-sidebar width (localStorage) --- +// The LEFT sidebar width is a single GLOBAL preference shared across every +// task env (unlike the per-env layout above, which keys widths by envId). +// Stores the user's raw, unclamped width — clamping to the current screen +// happens at apply time. Written only by a genuine sash drag; read by every +// layout build/restore/switch via getPinnedWidth. +const DOCKVIEW_GLOBAL_SIDEBAR_WIDTH_KEY = "kandev.dockview.sidebar-width"; + +export function getGlobalSidebarWidth(): number | null { + const v = getLocalStorage(DOCKVIEW_GLOBAL_SIDEBAR_WIDTH_KEY, null); + return typeof v === "number" && Number.isFinite(v) && v > 0 ? v : null; +} + +export function setGlobalSidebarWidth(width: number): void { + if (!Number.isFinite(width) || width <= 0) return; + setLocalStorage(DOCKVIEW_GLOBAL_SIDEBAR_WIDTH_KEY, Math.round(width)); +} + +export function clearGlobalSidebarWidth(): void { + removeLocalStorage(DOCKVIEW_GLOBAL_SIDEBAR_WIDTH_KEY); +} + // --- Dockview per-env maximize state (sessionStorage) --- // v2: bumped alongside DOCKVIEW_ENV_LAYOUT_PREFIX. The maximize blob // references the pre-maximize layout, which can carry the same oversized diff --git a/apps/web/lib/state/dockview-env-switch-action.test.ts b/apps/web/lib/state/dockview-env-switch-action.test.ts index be202ff0dd..e7821c3cc2 100644 --- a/apps/web/lib/state/dockview-env-switch-action.test.ts +++ b/apps/web/lib/state/dockview-env-switch-action.test.ts @@ -8,6 +8,9 @@ vi.mock("@/lib/local-storage", () => ({ getEnvMaximizeState: vi.fn(() => null), setEnvMaximizeState: vi.fn(), removeEnvMaximizeState: vi.fn(), + getGlobalSidebarWidth: vi.fn(() => null), + setGlobalSidebarWidth: vi.fn(), + clearGlobalSidebarWidth: vi.fn(), })); vi.mock("@/lib/layout/panel-portal-manager", () => ({ diff --git a/apps/web/lib/state/dockview-env-switch-pinned.test.ts b/apps/web/lib/state/dockview-env-switch-pinned.test.ts index 203d1682af..d9d0ceefe8 100644 --- a/apps/web/lib/state/dockview-env-switch-pinned.test.ts +++ b/apps/web/lib/state/dockview-env-switch-pinned.test.ts @@ -30,7 +30,10 @@ vi.mock("./layout-manager", () => { savedLayoutMatchesLive: vi.fn(() => false), layoutStructuresMatch: vi.fn(() => true), getRootSplitview: vi.fn(), - getPinnedWidth: vi.fn(() => 350), + // Distinct per-column defaults so we can prove the sidebar branch ignores + // saved per-env sizes (it routes through getPinnedWidth, which honors the + // global pref) while the right branch keeps using saved sizes. + getPinnedWidth: vi.fn((col: { id: string }) => (col.id === "sidebar" ? 300 : 350)), // Used by applyPinnedColumnSizes and (indirectly) by the // formatWidthsSnapshot debug log it now emits. setPinnedTarget: vi.fn(), @@ -87,15 +90,16 @@ describe("performEnvSwitch — pinned column resize after fast-path", () => { performEnvSwitch(makeParams()); - expect(resizeView).toHaveBeenCalledWith(0, 350); - expect(resizeView).toHaveBeenCalledWith(2, 350); + expect(resizeView).toHaveBeenCalledWith(0, 300); // sidebar default (global) + expect(resizeView).toHaveBeenCalledWith(2, 350); // right default // center column is at index 1 and is not pinned — must not be resized. expect(resizeView).not.toHaveBeenCalledWith(1, expect.anything()); }); - it("uses the saved layout's per-column sizes when present", () => { - // When a saved layout exists for the incoming env, its serialized - // grid.root.data[i].size wins over the ratio-based default. + it("ignores the saved sidebar size and uses the global width instead", () => { + // The sidebar is a GLOBAL pref shared across tasks, so the incoming env's + // saved sidebar size (420 below) must NOT win — it routes through + // getPinnedWidth (mocked to 300 for the sidebar). const savedLayout = { grid: { root: { @@ -131,15 +135,12 @@ describe("performEnvSwitch — pinned column resize after fast-path", () => { performEnvSwitch(makeParams()); - expect(resizeView).toHaveBeenCalledWith(0, 420); - // Only sidebar/right are resized; with 2 sv slots and 3 columns, the - // loop bound is min(3, 2) = 2, so center (index 1) is the last iterated - // but since columns[1] = "center" (not pinned), no resize fires. Right - // is at index 2 in the column list which exceeds sv.length so isn't - // hit; this scenario covers the saved-sidebar restoration path only. + // Saved sidebar size (420) is ignored; the global default (300) wins. + expect(resizeView).toHaveBeenCalledWith(0, 300); + expect(resizeView).not.toHaveBeenCalledWith(0, 420); }); - it("applies saved widths to both sidebar AND right when sv.length matches", () => { + it("uses the saved size for RIGHT but the global width for the sidebar", () => { // Cover the 3-column saved-layout path so the right column's // saved-size branch is exercised. The previous test exits the loop // before reaching the right column because sv.length = 2. @@ -181,9 +182,10 @@ describe("performEnvSwitch — pinned column resize after fast-path", () => { performEnvSwitch(makeParams()); - // Both pinned columns get their saved size; center (index 1) is not + // Sidebar ignores its saved size (420) → global default 300; the right + // column still restores its saved size (420). Center (index 1) is not // pinned and is skipped. - expect(resizeView).toHaveBeenCalledWith(0, 420); + expect(resizeView).toHaveBeenCalledWith(0, 300); expect(resizeView).toHaveBeenCalledWith(2, 420); expect(resizeView).not.toHaveBeenCalledWith(1, expect.anything()); }); diff --git a/apps/web/lib/state/dockview-env-switch.ts b/apps/web/lib/state/dockview-env-switch.ts index 4653fa41f7..a2d611e3b1 100644 --- a/apps/web/lib/state/dockview-env-switch.ts +++ b/apps/web/lib/state/dockview-env-switch.ts @@ -386,7 +386,12 @@ function applyPinnedColumnSizes( for (let i = 0; i < liveLayout.columns.length && i < sv.length; i++) { const col = liveLayout.columns[i]; if (col.id !== "sidebar" && col.id !== "right") continue; - const target = targetPinnedWidth(col, i, savedSizes, totalWidth); + // Sidebar uses the GLOBAL width pref (single source of truth across tasks), + // so it ignores this env's saved size. Right keeps per-env saved sizes. + const target = + col.id === "sidebar" + ? getPinnedWidth(col, totalWidth, undefined) + : targetPinnedWidth(col, i, savedSizes, totalWidth); if (typeof target !== "number" || target <= 0) continue; try { sv.resizeView(i, target); diff --git a/apps/web/lib/state/dockview-layout-builders-fixups.test.ts b/apps/web/lib/state/dockview-layout-builders-fixups.test.ts index ba0c1e8cf8..525718d740 100644 --- a/apps/web/lib/state/dockview-layout-builders-fixups.test.ts +++ b/apps/web/lib/state/dockview-layout-builders-fixups.test.ts @@ -15,6 +15,10 @@ vi.mock("@/lib/debug/log", () => ({ IS_DEBUG: false, })); +vi.mock("@/lib/local-storage", () => ({ + getGlobalSidebarWidth: vi.fn(() => null), +})); + vi.mock("./layout-manager", () => ({ SIDEBAR_LOCK: "no-drop-target", SIDEBAR_GROUP: "group-sidebar", @@ -24,6 +28,7 @@ vi.mock("./layout-manager", () => ({ LAYOUT_PINNED_MIN_PX: 180, computeSidebarMaxPx: vi.fn(() => SIDEBAR_CAP), computeRightMaxPx: vi.fn(() => RIGHT_CAP), + getPinnedWidth: vi.fn(() => 350), getRootSplitview: vi.fn(), resolveGroupIds: vi.fn(() => ({ sidebarGroupId: "group-sidebar", @@ -35,7 +40,15 @@ vi.mock("./layout-manager", () => ({ })); import { applyLayoutFixups } from "./dockview-layout-builders"; -import { getRootSplitview, setPinnedTarget, RIGHT_TOP_GROUP } from "./layout-manager"; +import { + getRootSplitview, + setPinnedTarget, + computeSidebarMaxPx, + computeRightMaxPx, + getPinnedWidth, + RIGHT_TOP_GROUP, +} from "./layout-manager"; +import { getGlobalSidebarWidth } from "@/lib/local-storage"; const SIDEBAR_GROUP = "group-sidebar"; const CENTER_GROUP = "group-center"; @@ -65,6 +78,8 @@ function makeApi(groupIds: string[]): DockviewApi { describe("applyLayoutFixups — pinned target capture", () => { beforeEach(() => { vi.clearAllMocks(); + vi.mocked(getGlobalSidebarWidth).mockReturnValue(null); + vi.mocked(getPinnedWidth).mockReturnValue(350); }); it("does NOT record a right target in the 2-column fallback (center is last)", () => { @@ -80,9 +95,20 @@ describe("applyLayoutFixups — pinned target capture", () => { expect(setPinnedTarget).not.toHaveBeenCalledWith("right", expect.anything()); }); - it("clamps an over-cap sidebar target down to the cap", () => { - // sidebar live (474) exceeds the cap (441); the constraint pins it at the - // cap, so the target must be clamped or enforcePinnedTargets spins forever. + it("uses the default sidebar target instead of an env-saved live width when no global pref exists", () => { + // Slow fromJSON restore can bring back an env-specific sidebar width. With + // sidebar now a global pref, no pref means "use the default", not the + // env-saved live width. + mockSplitview([420, 1050]); + const api = makeApi([SIDEBAR_GROUP, CENTER_GROUP]); + + applyLayoutFixups(api); + + expect(setPinnedTarget).toHaveBeenCalledWith("sidebar", 350); + }); + + it("clamps a global sidebar pref down to the cap", () => { + vi.mocked(getGlobalSidebarWidth).mockReturnValue(900); mockSplitview([474, 996]); const api = makeApi([SIDEBAR_GROUP, CENTER_GROUP]); @@ -101,6 +127,19 @@ describe("applyLayoutFixups — pinned target capture", () => { expect(setPinnedTarget).toHaveBeenCalledWith("right", 400); }); + it("derives the caps from api.width, not the window.innerWidth fallback", () => { + // Regression: caps computed without the measured width fall back to + // window.innerWidth, which can be transiently stale and clamp the captured + // target too narrow. They must be derived from api.width (1470 here). + mockSplitview([350, 720, 400]); + const api = makeApi([SIDEBAR_GROUP, CENTER_GROUP, RIGHT_TOP_GROUP]); + + applyLayoutFixups(api); + + expect(computeSidebarMaxPx).toHaveBeenCalledWith(1470); + expect(computeRightMaxPx).toHaveBeenCalledWith(1470); + }); + it("records the side-column target for a 3-column preset without RIGHT_TOP_GROUP", () => { // Regression: vscode/preview/plan presets put their side column in a group // with a generated id (not RIGHT_TOP_GROUP). The target must still be diff --git a/apps/web/lib/state/dockview-layout-builders.ts b/apps/web/lib/state/dockview-layout-builders.ts index b0fa17ccc0..5c82a359cd 100644 --- a/apps/web/lib/state/dockview-layout-builders.ts +++ b/apps/web/lib/state/dockview-layout-builders.ts @@ -8,11 +8,13 @@ import { LAYOUT_PINNED_MIN_PX, computeSidebarMaxPx, computeRightMaxPx, + getPinnedWidth, getRootSplitview as getRootSplitviewImpl, resolveGroupIds, setPinnedTarget, } from "./layout-manager"; import type { LayoutGroupIds } from "./layout-manager"; +import { getGlobalSidebarWidth } from "@/lib/local-storage"; import { createDebugLogger, IS_DEBUG } from "@/lib/debug/log"; // Re-export for consumers that import from this module @@ -20,6 +22,13 @@ export { getRootSplitview } from "./layout-manager"; const debugWidths = createDebugLogger("dockview:widths"); +/** Dockview's measured grid width, or undefined when not yet laid out — passed + * to the cap helpers so they don't fall back to a possibly-stale + * `window.innerWidth`. */ +function layoutWidth(api: DockviewApi): number | undefined { + return api.width > 0 ? api.width : undefined; +} + /** Best-effort caller chain for the fixups-capture debug log: pull the first * few stack frames above `applyLayoutFixups` so we can see WHICH layout path * (env-switch / restore / custom-layout / maximize) recorded a given target. @@ -63,6 +72,20 @@ export function applyLayoutFixups(api: DockviewApi): LayoutGroupIds { return resolveGroupIds(api); } +/** Resolve the sidebar's target width (clamped to the cap): the GLOBAL width + * pref when set, else the default width for the current measured layout. */ +function resolveSidebarTarget(cap: number, totalWidth: number | undefined): number | undefined { + const pref = getGlobalSidebarWidth(); + if (pref !== null) return Math.min(pref, cap); + const width = + totalWidth ?? + (typeof window !== "undefined" && window.innerWidth > 0 ? window.innerWidth : cap); + return Math.min( + getPinnedWidth({ id: "sidebar", pinned: true, groups: [] }, width, undefined), + cap, + ); +} + /** Lock + constrain the sidebar group and record its target width, clamped to * the cap. The constraint pins the column at `sidebarCap`, so a target above * it is unreachable and makes `enforcePinnedTargets` spin forever. */ @@ -70,13 +93,31 @@ export function applyLayoutFixups(api: DockviewApi): LayoutGroupIds { function captureSidebarTarget(api: DockviewApi, sv: any): void { const sb = api.getPanel("sidebar"); if (!sb) return; - const sidebarCap = computeSidebarMaxPx(); + // Derive the cap from dockview's measured grid width, not the implicit + // window.innerWidth fallback: innerWidth can read transiently stale during + // route transitions / devtools toggles, yielding a too-small cap that would + // clamp the captured target too narrow and persist it — the width drift this + // pipeline exists to prevent. + const measuredWidth = layoutWidth(api); + const sidebarCap = computeSidebarMaxPx(measuredWidth); sb.group.locked = SIDEBAR_LOCK; sb.group.header.hidden = false; sb.group.api.setConstraints({ maximumWidth: sidebarCap, minimumWidth: LAYOUT_PINNED_MIN_PX }); - const live = sv?.getViewSize?.(0) ?? sb.group.width; - if (typeof live === "number" && live > 0) { - setPinnedTarget("sidebar", Math.min(live, sidebarCap)); + // Slow-path env restore: fromJSON brought back this env's saved sidebar + // pixel width, but the sidebar is a GLOBAL pref. Seed the target from the + // pref (clamped to fit) and resize the column so the restore honors it; fall + // back to the default sidebar width when no pref exists. + const target = resolveSidebarTarget(sidebarCap, measuredWidth); + if (target !== undefined) { + const cur = sv?.getViewSize?.(0); + if (typeof cur === "number" && cur > 0 && Math.abs(cur - target) > 1) { + try { + sv?.resizeView?.(0, target); + } catch { + /* dockview rejects unreachable sizes — ignore */ + } + } + setPinnedTarget("sidebar", target); } } @@ -99,7 +140,9 @@ function captureSidebarTarget(api: DockviewApi, sv: any): void { function captureRightTarget(api: DockviewApi, sv: any): void { // Constrain the default preset's right column groups (stable well-known IDs). // Other presets' side columns aren't pinned and carry no max-width cap. - const rightCap = computeRightMaxPx(); + // Use the measured grid width, not the window.innerWidth fallback (see + // captureSidebarTarget). + const rightCap = computeRightMaxPx(layoutWidth(api)); for (const gid of [RIGHT_TOP_GROUP, RIGHT_BOTTOM_GROUP]) { const group = api.groups.find((g) => g.id === gid); if (group) { @@ -123,7 +166,8 @@ function logFixupsCapture(api: DockviewApi, sv: any): void { // that makes `enforcePinnedTargets` spin forever. `cols` shows whether the // layout was complete at capture (cols<3 → no real right column). api.width // vs window.innerWidth surfaces the window-fallback cap divergence. - const sidebarCap = computeSidebarMaxPx(); + const w = layoutWidth(api); + const sidebarCap = computeSidebarMaxPx(w); const innerW = typeof window !== "undefined" ? window.innerWidth : -1; const liveSidebar = sv?.getViewSize?.(0); // Threshold matches captureRightTarget (>= 3): in a 2-column layout the last @@ -134,7 +178,7 @@ function logFixupsCapture(api: DockviewApi, sv: any): void { debugWidths( `fixups-capture caller=${captureCallerChain()} apiW=${api.width} innerW=${innerW} ` + `cols=${sv?.length ?? 0} sidebarCap=${Math.round(sidebarCap)} liveSidebar=${r(liveSidebar)} ` + - `rightCap=${Math.round(computeRightMaxPx())} liveRight=${r(liveRight)} ` + + `rightCap=${Math.round(computeRightMaxPx(w))} liveRight=${r(liveRight)} ` + `sidebarOverCap=${typeof liveSidebar === "number" && liveSidebar > sidebarCap + 1}`, ); } diff --git a/apps/web/lib/state/dockview-pinned-enforce.ts b/apps/web/lib/state/dockview-pinned-enforce.ts index 9f87e1108a..139ec57a60 100644 --- a/apps/web/lib/state/dockview-pinned-enforce.ts +++ b/apps/web/lib/state/dockview-pinned-enforce.ts @@ -19,7 +19,7 @@ * `sv.resizeView`. */ import type { DockviewApi } from "dockview-react"; -import { getRootSplitview, getPinnedTarget } from "./layout-manager"; +import { getRootSplitview, getPinnedTarget, computeSidebarMaxPx } from "./layout-manager"; import { createDebugLogger, IS_DEBUG } from "@/lib/debug/log"; const debugWidths = createDebugLogger("dockview:widths"); @@ -97,7 +97,18 @@ export function enforcePinnedTargets(api: DockviewApi, ctx: EnforcePinnedTargets if (!sv || sv.length < 2) return; enforcing = true; try { - if (ctx.sidebarVisible) restoreColumnToTarget(sv, 0, getPinnedTarget("sidebar")); + if (ctx.sidebarVisible) { + // Clamp the (global, raw) sidebar target to the current screen so a width + // set on a wide monitor fits a narrower one. Storage keeps the raw value, + // so returning to the wide monitor restores it. + const raw = getPinnedTarget("sidebar"); + // Guard against an unmeasured container (api.width === 0): passing 0 to + // computeSidebarMaxPx bypasses its window.innerWidth fallback and clamps + // the target down to LAYOUT_PINNED_MIN_PX. undefined lets it fall back. + const safeWidth = api.width > 0 ? api.width : undefined; + const clamped = raw === undefined ? undefined : Math.min(raw, computeSidebarMaxPx(safeWidth)); + restoreColumnToTarget(sv, 0, clamped); + } if (ctx.rightPanelsVisible) { restoreColumnToTarget(sv, sv.length - 1, getPinnedTarget("right")); } diff --git a/apps/web/lib/state/dockview-store.test.ts b/apps/web/lib/state/dockview-store.test.ts index 9186587e97..84a68290a5 100644 --- a/apps/web/lib/state/dockview-store.test.ts +++ b/apps/web/lib/state/dockview-store.test.ts @@ -5,6 +5,8 @@ import { resolvePresetPinnedWidths, collectPinnedWidthUpdates, } from "./dockview-store"; +import { getGlobalSidebarWidth, setGlobalSidebarWidth } from "@/lib/local-storage"; +import { getPinnedTarget, setPinnedTarget, clearPinnedTarget } from "./layout-manager"; type ActivePanelEvent = { id: string }; type CapturedHandlers = { @@ -122,7 +124,7 @@ describe("resolvePresetPinnedWidths", () => { expect(result.has("center")).toBe(false); // not pinned }); - it("keeps live widths for columns in the target layout when not resetting", () => { + it("keeps right live widths for columns in the target layout when not resetting", () => { const live = new Map([ ["sidebar", 519], ["right", 900], @@ -130,10 +132,24 @@ describe("resolvePresetPinnedWidths", () => { const result = resolvePresetPinnedWidths(live, cols, 1600, false); - expect(result.get("sidebar")).toBe(519); + expect(result.has("sidebar")).toBe(false); expect(result.get("right")).toBe(900); }); + it("does not carry a live sidebar override on non-reset switches", () => { + setGlobalSidebarWidth(520); + const live = new Map([ + ["sidebar", 350], + ["right", 900], + ]); + + const result = resolvePresetPinnedWidths(live, cols, 1600, false); + + expect(result.has("sidebar")).toBe(false); + expect(result.get("right")).toBe(900); + expect(getGlobalSidebarWidth()).toBe(520); + }); + it("drops live overrides for columns absent from the target layout", () => { // e.g. switching to a layout without a "right" column must not leak the // old right width into the new layout. @@ -148,7 +164,7 @@ describe("resolvePresetPinnedWidths", () => { const result = resolvePresetPinnedWidths(live, noRight, 1600, false); - expect(result.get("sidebar")).toBe(300); + expect(result.has("sidebar")).toBe(false); expect(result.has("right")).toBe(false); }); @@ -157,20 +173,48 @@ describe("resolvePresetPinnedWidths", () => { resolvePresetPinnedWidths(live, cols, 1600, false); expect(live.get("sidebar")).toBe(300); }); + + describe("Default layout reset clears the global sidebar width", () => { + beforeEach(() => { + window.localStorage.clear(); + clearPinnedTarget("sidebar"); + }); + + it("clears the global pref + runtime target and uses the ratio default", () => { + setGlobalSidebarWidth(520); + setPinnedTarget("sidebar", 520); + + const result = resolvePresetPinnedWidths(new Map(), cols, 1600, true); + + // pref cleared → ratio default (1600*0.25=400, clamped to sidebar cap 350) + expect(result.get("sidebar")).toBe(350); + expect(getGlobalSidebarWidth()).toBeNull(); + expect(getPinnedTarget("sidebar")).toBeUndefined(); + }); + + it("does NOT clear the pref/target on a non-reset (programmatic) switch", () => { + setGlobalSidebarWidth(520); + setPinnedTarget("sidebar", 520); + + resolvePresetPinnedWidths(new Map([["sidebar", 480]]), cols, 1600, false); + + expect(getGlobalSidebarWidth()).toBe(520); + expect(getPinnedTarget("sidebar")).toBe(520); + }); + }); }); describe("collectPinnedWidthUpdates", () => { const size = (i: number) => [350, 560, 560][i]; // sidebar, center, last - it("tracks sidebar + right when both are visible", () => { + it("tracks right but not sidebar when both are visible", () => { const columns = [{ id: "sidebar" }, { id: "center" }, { id: "right" }]; const updates = collectPinnedWidthUpdates(columns, size, { - sidebarVisible: true, rightPanelsVisible: true, }); - expect(updates.get("sidebar")).toBe(350); + expect(updates.has("sidebar")).toBe(false); expect(updates.get("right")).toBe(560); }); @@ -182,19 +226,17 @@ describe("collectPinnedWidthUpdates", () => { const columns = [{ id: "sidebar" }, { id: "center" }, { id: "right" }]; const updates = collectPinnedWidthUpdates(columns, size, { - sidebarVisible: true, rightPanelsVisible: false, }); expect(updates.has("right")).toBe(false); - expect(updates.get("sidebar")).toBe(350); + expect(updates.has("sidebar")).toBe(false); }); it("skips collapsed/transient widths <= 50px", () => { const columns = [{ id: "sidebar" }, { id: "right" }]; const updates = collectPinnedWidthUpdates(columns, () => 40, { - sidebarVisible: true, rightPanelsVisible: true, }); diff --git a/apps/web/lib/state/dockview-store.ts b/apps/web/lib/state/dockview-store.ts index 859b4143a4..20d0a63266 100644 --- a/apps/web/lib/state/dockview-store.ts +++ b/apps/web/lib/state/dockview-store.ts @@ -6,8 +6,10 @@ import { getEnvMaximizeState, setEnvMaximizeState, removeEnvMaximizeState, + clearGlobalSidebarWidth, + setGlobalSidebarWidth, } from "@/lib/local-storage"; -import { setPinnedTarget } from "./layout-manager"; +import { setPinnedTarget, clearPinnedTarget } from "./layout-manager"; import { applyLayoutFixups, focusOrAddPanel } from "./dockview-layout-builders"; import { SIDEBAR_GROUP, @@ -217,37 +219,36 @@ function applyDeferredPanelActions(api: DockviewApi, actions: DeferredPanelActio } } -/** Read live column widths from dockview's splitview and persist them as pinned overrides. - * Only syncs widths for columns identified as "sidebar" or "right" to avoid - * capturing plan/preview/vscode column widths as stale "right" overrides. */ /** - * Build the pinnedWidths updates for a width sync, tracking a column only when - * it is the VISIBLE default sidebar/right. + * Build the pinnedWidths updates for a width sync, tracking only the VISIBLE + * default right column. * * In plan/preview/vscode layouts the side column inherits merged files/changes * panels and `fromDockviewApi` mislabels it "right"; storing its width as the * right override would then leak into the default layout when toggling back * (e.g. plan-mode off snapping the right column to the plan column's width). - * Mirrors the visibility guards in `trackPinnedWidths`. Widths <= 50px are - * treated as transient/collapsed and skipped. + * + * Sidebar is intentionally excluded: its persisted width is a global pref + * written only by explicit sash drag, and syncing live layout-change widths + * would overwrite the raw pref with viewport-clamped transient widths. + * Widths <= 50px are treated as transient/collapsed and skipped. */ export function collectPinnedWidthUpdates( columns: { id: string }[], getSize: (index: number) => number, - visibility: { sidebarVisible: boolean; rightPanelsVisible: boolean }, + visibility: { rightPanelsVisible: boolean }, ): Map { const updates = new Map(); columns.forEach((col, i) => { - const tracked = - (col.id === "sidebar" && visibility.sidebarVisible) || - (col.id === "right" && visibility.rightPanelsVisible); - if (!tracked) return; + if (col.id !== "right" || !visibility.rightPanelsVisible) return; const w = getSize(i); if (w > 50) updates.set(col.id, w); }); return updates; } +/** Read live column widths from dockview's splitview and persist the visible + * right width as a pinned override (see `collectPinnedWidthUpdates`). */ function syncPinnedWidthsFromApi(api: DockviewApi, set: StoreSet): void { if (api.hasMaximizedGroup()) return; const sv = getRootSplitview(api); @@ -255,9 +256,8 @@ function syncPinnedWidthsFromApi(api: DockviewApi, set: StoreSet): void { try { const state = fromDockviewApi(api); if (state.columns.length !== sv.length) return; - const { sidebarVisible, rightPanelsVisible } = useDockviewStore.getState(); + const { rightPanelsVisible } = useDockviewStore.getState(); const updates = collectPinnedWidthUpdates(state.columns, (i) => sv.getViewSize(i), { - sidebarVisible, rightPanelsVisible, }); if (updates.size > 0) { @@ -278,7 +278,6 @@ function syncPinnedWidthsFromApi(api: DockviewApi, set: StoreSet): void { } } -/** Capture the live sidebar/right pixel widths into pinnedWidths before a layout rebuild. */ /** * Decide which pinned-width overrides to apply when switching to a preset. * @@ -290,9 +289,10 @@ function syncPinnedWidthsFromApi(api: DockviewApi, set: StoreSet): void { * `fromJSON` out at a transient narrower width, and that shrunken size would * be captured as the pinned target and then enforced, leaving the columns * too narrow. - * - otherwise (programmatic switch, e.g. plan-mode toggle): keep the live - * widths, minus overrides for columns absent in the target layout, so the - * user's current sidebar/right widths carry across the switch. + * - otherwise (programmatic switch, e.g. plan-mode toggle): keep the live right + * width, minus overrides for columns absent in the target layout. Sidebar is + * always resolved from the global pref/default instead of an in-memory + * override. */ export function resolvePresetPinnedWidths( liveWidths: Map, @@ -301,6 +301,11 @@ export function resolvePresetPinnedWidths( resetWidths: boolean, ): Map { if (resetWidths) { + // "Default layout" is the reset gesture: drop any custom global sidebar + // width (and its runtime target) so getPinnedWidth returns the fresh + // ratio default for the current screen instead of re-reading the pref. + clearGlobalSidebarWidth(); + clearPinnedTarget("sidebar"); const defaults = new Map(); for (const col of columns) { if (col.pinned) defaults.set(col.id, getPinnedWidth(col, totalWidth, undefined)); @@ -310,11 +315,12 @@ export function resolvePresetPinnedWidths( const targetColumnIds = new Set(columns.map((c) => c.id)); const cleaned = new Map(liveWidths); for (const key of cleaned.keys()) { - if (!targetColumnIds.has(key)) cleaned.delete(key); + if (key === "sidebar" || !targetColumnIds.has(key)) cleaned.delete(key); } return cleaned; } +/** Capture the live right pixel width into pinnedWidths before a layout rebuild. */ function captureLiveWidths(api: DockviewApi, set: StoreSet): Map { if (api.hasMaximizedGroup()) { api.exitMaximizedGroup(); @@ -868,12 +874,15 @@ export const useDockviewStore = create((set, get) => ({ type TestWindow = { __dockviewApi__: DockviewApi | null; __setPinnedTarget__?: typeof setPinnedTarget; + __setGlobalSidebarWidth__?: typeof setGlobalSidebarWidth; }; const w = window as unknown as TestWindow; w.__dockviewApi__ = api; // E2E test helpers: let `resizeColumnViaSplitview` update the target - // width after a programmatic resize (mirroring the sash-drag mouseup). + // width after a programmatic resize (mirroring the sash-drag mouseup), + // including persisting the global sidebar-width pref like a real drag. w.__setPinnedTarget__ = setPinnedTarget; + w.__setGlobalSidebarWidth__ = setGlobalSidebarWidth; } if (api) { const resolveFilePath = (panelId: string | undefined): string | null => { diff --git a/apps/web/lib/state/layout-manager/sizing.test.ts b/apps/web/lib/state/layout-manager/sizing.test.ts new file mode 100644 index 0000000000..a86bdf9da9 --- /dev/null +++ b/apps/web/lib/state/layout-manager/sizing.test.ts @@ -0,0 +1,49 @@ +import { beforeEach, describe, expect, it } from "vitest"; +import { getPinnedWidth } from "./sizing"; +import { computeSidebarMaxPx } from "./caps"; +import type { LayoutColumn } from "./types"; +import { getGlobalSidebarWidth, setGlobalSidebarWidth } from "@/lib/local-storage"; + +const sidebar = (extra: Partial = {}): LayoutColumn => + ({ id: "sidebar", pinned: true, groups: [], ...extra }) as LayoutColumn; +const right = (extra: Partial = {}): LayoutColumn => + ({ id: "right", pinned: true, groups: [], ...extra }) as LayoutColumn; + +const TOTAL = 1600; // computeSidebarMaxPx(1600) = max(350, 480) bounded = 480 + +describe("getPinnedWidth — global sidebar width pref", () => { + beforeEach(() => { + window.localStorage.clear(); + }); + + it("uses the stored pref for the sidebar when it fits the screen", () => { + setGlobalSidebarWidth(400); // < cap 480 + expect(getPinnedWidth(sidebar(), TOTAL, undefined)).toBe(400); + }); + + it("clamps the sidebar pref to the screen cap without mutating storage", () => { + const cap = computeSidebarMaxPx(TOTAL); + setGlobalSidebarWidth(900); // > cap → clamp to fit + expect(getPinnedWidth(sidebar(), TOTAL, undefined)).toBe(cap); + // Raw value preserved so a wider monitor restores it. + expect(getGlobalSidebarWidth()).toBe(900); + }); + + it("falls back to the legacy ratio default when no pref is set", () => { + // ratio 0.25 * 1600 = 400, clamped to the legacy sidebar initial cap (350). + expect(getGlobalSidebarWidth()).toBeNull(); + expect(getPinnedWidth(sidebar(), TOTAL, undefined)).toBe(350); + }); + + it("lets an explicit override win over the pref", () => { + setGlobalSidebarWidth(400); + // maxWidth pins the runtime cap so the override isn't clamped by innerWidth. + expect(getPinnedWidth(sidebar({ maxWidth: 600 }), TOTAL, 500)).toBe(500); + }); + + it("does NOT apply the sidebar pref to the right column", () => { + setGlobalSidebarWidth(200); + // right: ratio 0.25 * 1600 = 400, clamped to legacy right cap (450) → 400. + expect(getPinnedWidth(right(), TOTAL, undefined)).toBe(400); + }); +}); diff --git a/apps/web/lib/state/layout-manager/sizing.ts b/apps/web/lib/state/layout-manager/sizing.ts index bc1ea0b956..e69cb5507e 100644 --- a/apps/web/lib/state/layout-manager/sizing.ts +++ b/apps/web/lib/state/layout-manager/sizing.ts @@ -1,6 +1,7 @@ import type { LayoutColumn, LayoutGroup } from "./types"; import { LAYOUT_INITIAL_RATIO } from "./constants"; -import { computePinnedMaxPxFor, LAYOUT_PINNED_MIN_PX } from "./caps"; +import { computePinnedMaxPxFor, computeSidebarMaxPx, LAYOUT_PINNED_MIN_PX } from "./caps"; +import { getGlobalSidebarWidth } from "@/lib/local-storage"; // Legacy hard caps used to clamp the *initial* default width. Users can still // drag past these via setConstraints (which uses the larger runtime cap), @@ -27,6 +28,16 @@ export function getPinnedWidth( if (override !== undefined) { return Math.max(min, Math.min(override, runtimeMax)); } + // No override: the left sidebar honors the persisted GLOBAL width pref so it + // opens identically across every task. Clamp-to-fit against the current + // screen (the raw value stays in storage, so a wider monitor restores it). + if (column.id === "sidebar") { + const pref = getGlobalSidebarWidth(); + if (pref !== null) { + const max = column.maxWidth ?? computeSidebarMaxPx(totalWidth); + return Math.max(min, Math.min(pref, max)); + } + } const ratioWidth = Math.round(totalWidth * LAYOUT_INITIAL_RATIO); const initialCap = column.maxWidth ??