diff --git a/src/client/EditorHost.tsx b/src/client/EditorHost.tsx index 984a1d7..c66a847 100644 --- a/src/client/EditorHost.tsx +++ b/src/client/EditorHost.tsx @@ -31,6 +31,7 @@ import { api, mediaUrl, type SessionScope } from './api.ts' import { BinaryDownload } from './binary-download.tsx' import { planFirstMatch, planFsReadOutcome, type EditorLoadAction } from './editor-load.ts' import { baseName } from './FileTree.tsx' +import { createFrameBatcher } from './frame-batcher.ts' import { openSidebarFile } from './intercept.tsx' import { TreePanel } from './TreePanel.tsx' import { t } from './locales.ts' @@ -165,8 +166,16 @@ export function EditorHost(props: { // (no window listeners — the captured pointer keeps tracking even off the // handle). Local width while dragging, persisted into meta.treeWidth on // release. The panel docks right, so dragging LEFT widens it. + // Moves are BATCHED per frame (createFrameBatcher): applying every + // pointermove is a setState that re-renders this host AND the editor + // viewer below it (CodeMirror re-lays out on the width change) at event + // cadence — the visible drag lag on slower CPUs (#315). The batch applies + // the latest width once per frame; release flushes it and commits. const [dragWidth, setDragWidth] = useState(null) const dragRef = useRef<{ startX: number; startWidth: number } | null>(null) + const pendingWidthRef = useRef(0) + const dragBatcher = useRef(createFrameBatcher()).current + useEffect(() => () => dragBatcher.dispose(), [dragBatcher]) const treeWidth = dragWidth ?? treeWidthOf(tab) const onResizeStart = (event: React.PointerEvent): void => { @@ -178,11 +187,17 @@ export function EditorHost(props: { const onResizeMove = (event: React.PointerEvent): void => { const drag = dragRef.current if (drag === null) return - setDragWidth(clampTreeWidth(drag.startWidth + (drag.startX - event.clientX))) + pendingWidthRef.current = clampTreeWidth(drag.startWidth + (drag.startX - event.clientX)) + dragBatcher.schedule(() => setDragWidth(pendingWidthRef.current)) } const onResizeEnd = (event: React.PointerEvent): void => { const drag = dragRef.current if (drag === null) return + // Flush the last pending frame (a release can land with the final move + // still queued; without the flush a stray frame would re-apply the + // drag width AFTER the null below). Both setStates batch into this same + // event, so the committed treeWidth wins visually. + dragBatcher.flushNow() dragRef.current = null setDragWidth(null) const finalWidth = clampTreeWidth(drag.startWidth + (drag.startX - event.clientX)) diff --git a/src/client/Sidebar.tsx b/src/client/Sidebar.tsx index b613254..2b5be43 100644 --- a/src/client/Sidebar.tsx +++ b/src/client/Sidebar.tsx @@ -28,7 +28,7 @@ * drawer floats). Widening does not migrate back: the tabs keep living in * the right tree. */ -import { createElement, useCallback, useEffect, useMemo, useRef, useState, type DragEvent, type ReactNode } from 'react' +import { createElement, memo, useCallback, useEffect, useMemo, useRef, useState, type DragEvent, type ReactNode } from 'react' import { useSyncExternalStore } from 'react' import clsx from 'clsx' import { IconCloseFill14, Tooltip } from '@deepseek-ai/dsh-client-ui-primitives' @@ -52,6 +52,7 @@ import type { TabDragPayload } from './TabBar.tsx' import { relativeTo } from './paths.ts' import { OrphanedTab } from './OrphanedTab.tsx' import { RenderBoundary } from './RenderBoundary.tsx' +import { tabContentCompare, type TabContentMemoKey } from './tab-content-memo.ts' import { detectNewDirectSubagent } from './subagent-detect.ts' import { detectNewJob } from './subagent-jobs.ts' import { t } from './locales.ts' @@ -113,23 +114,25 @@ function injectUserCss(attr: string, id: string, cssText: string): HTMLStyleElem return tag } -/** Render the content of one tab (dispatched by type). */ -function TabContent(props: { - tab: SidebarTab - sessionId: string - cwd: string | undefined - expanded: string[] +/** Props of one tab's content cell = the memo key (tab-content-memo.ts) plus + * the runtime objects/callbacks the cell renders with. The memo comparator + * is the pure `tabContentCompare`; anything in the key decides a re-render + * must propagate, anything outside it must be a stable object (ctx/store) + * or covered by a compared field (paneId covers onOpenDiff's captured + * pane; sessionId/cwd cover onReferenceFile). */ +interface TabContentProps extends TabContentMemoKey { onToggleDir: (path: string) => void onReferenceFile: (path: string) => void ctx: Context store: SidebarStore - /** Whether this tab is the active one AND the panel is open (live views pause otherwise). */ - visible: boolean /** Fired before a topology node jumps to its child session (see Sidebar). */ onSubagentJump: (childSessionId: string) => void /** Open a diff tab from the git panel (placement handled by the store). */ onOpenDiff: (tab: SidebarTab) => void -}) { +} + +/** Render the content of one tab (dispatched by type). */ +const TabContent = memo(function TabContent(props: TabContentProps) { const { tab, sessionId, cwd, expanded, onToggleDir, onReferenceFile, ctx, store, visible, onSubagentJump, onOpenDiff } = props const scope = { sessionId, cwd } const descriptor = ctx.betterSidebar?.getTab(tab.type) @@ -152,7 +155,7 @@ function TabContent(props: { onToggleDir, onReferenceFile, onOpenDiff, onSubagentJump, }), ) -} +}, tabContentCompare) /** The + menu options for the current state, driven by the tab registry. * Hidden tabs (editor/diff) never show; `available` returning false shows @@ -185,6 +188,18 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) { ) void localeRevision + // Tab-registry revision: TabContent memo cells must pick up a descriptor + // a plugin registers/disposes after mount (the + menu / icons already read + // the registry at render). Rare events (plugin (un)mount), so one full + // re-render per change is fine — this is what keeps the memoized cells + // from going stale, mirroring the localeRevision mechanism above. + const [tabsVersion, setTabsVersion] = useState(0) + useEffect(() => { + const service = ctx.betterSidebar + if (service === undefined) return + return service.subscribe(() => setTabsVersion(version => version + 1)) + }, [ctx]) + // Narrow (mobile) viewports collapse the two panels into one: the right // panel becomes a full-width drawer holding BOTH workbenches, the bottom // panel (and its toggle button) disappears, and the layout push is @@ -497,7 +512,16 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) { // horizontal edges — including the animated margin-right push while the // right panel opens/closes; a frame that never appears keeps the initial // zero-size fallback (the panel renders at 0 width until measured). - const [centerRect, setCenterRect] = useState({ left: 0, right: 0 }) + // The rect lives in a REF (not state): the open/close transition resizes + // the center column EVERY frame for its duration, and reacting per frame + // with setState re-renders the whole Sidebar (every mounted tab) at + // animation cadence — the visible toggle jank (#315). measureCenter + // writes the bottom panel's edges directly (same DOM-write pattern as + // applyDrag), so the panel still tracks the column per frame with zero + // React work; `centerMeasured` flips ONCE to gate the hidden→visible + // first-paint fallback. + const centerRectRef = useRef({ left: 0, right: 0 }) + const [centerMeasured, setCenterMeasured] = useState(false) // Refs keep the measure step stable across renders and let it skip work // mid-drag: during a width/corner drag the layout push resizes the center // column every frame, and reacting (setCenterRect → re-render) would @@ -518,13 +542,18 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) { return } const rect = col.getBoundingClientRect() - // The bottom panel only cares about the horizontal edges: a pure height - // change (the bottom panel itself opening/closing) must not re-render, - // so keep the previous object when left/right are unchanged. - setCenterRect(prev => - prev.left === rect.left && prev.right === rect.right - ? prev - : { left: rect.left, right: rect.right }) + // Ref + direct DOM write (see the centerRectRef comment): the bottom + // panel keeps tracking the center column per frame during the right + // panel's open/close animation without re-rendering the shell. The + // one-shot measured flip renders the panel visible once (a stale + // {0,0} fallback would flash full-width). + centerRectRef.current = { left: rect.left, right: rect.right } + const bottom = bottomRef.current + if (bottom !== null) { + bottom.style.setProperty('left', `${rect.left}px`) + bottom.style.setProperty('right', `${window.innerWidth - rect.right}px`) + } + setCenterMeasured(prev => (prev ? prev : true)) }, []) useEffect(() => { let disposed = false @@ -571,6 +600,13 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) { let locateFrame: number | null = null const scheduleLocate = (): void => { if (locateFrame !== null) return + // Mid-drag every frame writes --dsh-sidebar-* on 's style + // attribute, which is the mutation this watcher observes — relocating + // per drag frame is pointless (the center column node cannot change + // while the pointer is captured) and adds a querySelector to every + // frame's budget (#315). The 1.5s retry below still covers any node + // swap that somehow lands mid-drag. + if (draggingRef.current) return locateFrame = requestAnimationFrame(() => { locateFrame = null locate() @@ -708,7 +744,7 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) { // width (innerWidth - state.width - detailsWidth), so this equals // `width + detailsWidth` — derived from the measured column, keeping the // drag write-only (no React re-render mid-drag). - bottomRef.current?.style.setProperty('right', `${(window.innerWidth - centerRect.right) + (width - (state?.width ?? 0))}px`) + bottomRef.current?.style.setProperty('right', `${(window.innerWidth - centerRectRef.current.right) + (width - (state?.width ?? 0))}px`) writeGeometry(width, height) } @@ -1033,6 +1069,7 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) { const renderTab = (tab: SidebarTab, active: boolean, paneId: string, bottom = false) => ( { subagentJumpRef.current = childSessionId }} onOpenDiff={(diffTab) => { store.reduce(s => openDiffTab(s, paneId, diffTab)) }} + localeRevision={localeRevision} + tabsVersion={tabsVersion} /> ) @@ -1231,7 +1270,7 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) { data-dsh-bottom-panel style={{ height: Math.min(state.bottomHeight, window.innerHeight), - left: centerRect.left, + left: centerRectRef.current.left, // Keep the panel above the on-screen keyboard when the visual // viewport shrinks (see the keyboardInset effect). bottom: keyboardInset > 0 ? `${keyboardInset}px` : undefined, @@ -1240,14 +1279,14 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) { // details column's left edge (the details column sits between the // center and the right panel, and the right panel's margin-right // push is already baked into centerRect.right). - right: window.innerWidth - centerRect.right, + right: window.innerWidth - centerRectRef.current.right, // The seam against the open right panel needs its own hairline // (the right panel's border-left alone is covered by this panel's // fill — without it the corner looks cut off). borderRight: state.panelOpen ? '1px solid var(--dsw-alias-border-l2)' : undefined, // Unmeasured center column → keep the panel invisible (zero-size // geometry would flash full-width overflow instead). - visibility: centerRect.right > 0 ? undefined : 'hidden', + visibility: centerMeasured ? undefined : 'hidden', }} data-dragging={(draggingBottom || draggingCorner) || undefined} diff --git a/src/client/frame-batcher.ts b/src/client/frame-batcher.ts new file mode 100644 index 0000000..2eac7c9 --- /dev/null +++ b/src/client/frame-batcher.ts @@ -0,0 +1,56 @@ +/** + * Per-frame task coalescing for pointer-driven interactions. + * + * Pointer streams fire several times faster than the display refresh, and + * applying every event (a React setState, a store reduce) re-renders whole + * trees at event cadence — the visible drag lag on slower CPUs (#315). This + * batcher keeps the LATEST scheduled task, runs it at most once per + * animation frame, and lets the caller flush synchronously on release (so + * the final pointer position is never lost to a cancelled frame). + * + * Callers compose the pending value in a ref before scheduling; the + * scheduled task reads that ref, so replacing the task on every event is + * cheap (one ref write + one rAF check). + */ +export interface FrameBatcher { + /** Schedule (or replace) the per-frame task. Safe to call on every event. */ + schedule(task: () => void): void + /** Cancel any pending frame and run the scheduled task synchronously + * (no-op when nothing is scheduled). */ + flushNow(): void + /** Cancel any pending frame and drop the task without running it. */ + dispose(): void +} + +export function createFrameBatcher(): FrameBatcher { + let frame: number | null = null + let task: (() => void) | null = null + + const run = (): void => { + frame = null + const current = task + task = null + current?.() + } + + return { + schedule(next) { + task = next + if (frame === null) frame = requestAnimationFrame(run) + }, + flushNow() { + if (frame !== null) { + cancelAnimationFrame(frame) + frame = null + } + run() + }, + dispose() { + if (frame !== null) { + cancelAnimationFrame(frame) + frame = null + } + task = null + }, + } +} diff --git a/src/client/split-pane.tsx b/src/client/split-pane.tsx index 5388d55..06af947 100644 --- a/src/client/split-pane.tsx +++ b/src/client/split-pane.tsx @@ -15,6 +15,7 @@ import clsx from 'clsx' import type { SidebarState, SidebarTab, SplitNode } from './state.ts' import type { DropZone } from './state.ts' import { TabBar, type NewTabOption, parseDrag, type TabDragPayload } from './TabBar.tsx' +import { createFrameBatcher } from './frame-batcher.ts' import css from './sidebar.module.css' /** Actions the workbench needs (bound to the store by the sidebar shell). */ @@ -34,7 +35,18 @@ export interface WorkbenchActions { * Deltas are incremental — each move reports the displacement since the * previous move — because the store adds every reported delta to the pane * sizes; a cumulative (since-pointer-down) delta would be re-added on each - * move and the divider would run away from the cursor. */ + * move and the divider would run away from the cursor. + * + * The moves are BATCHED per frame (createFrameBatcher): a pointer stream + * fires faster than the display refresh, and applying each move is a store + * reduce that re-renders both workbenches (terminals, editors, trees) per + * event — the visible drag lag on slower CPUs (#315). The batch accumulates + * the incremental deltas in a ref and applies the summed fraction at most + * once per frame; the sum equals what the per-event application would have + * produced (the reducer clamps each application, and at a settled position + * a clamped sum is clamped to the same boundary), so the result is + * indistinguishable at rest and at most one frame behind the cursor. + */ function Divider(props: { dir: 'row' | 'col'; onResize: (deltaFrac: number) => void }) { const { dir, onResize } = props // `last` is the previous pointer position while dragging; `size` is the @@ -42,6 +54,9 @@ function Divider(props: { dir: 'row' | 'col'; onResize: (deltaFrac: number) => v // normalize the px delta into a fraction. const last = useRef({ x: 0, y: 0, size: 0 }) const [dragging, setDragging] = useState(false) + const pendingDelta = useRef(0) + const batcher = useRef(createFrameBatcher()).current + useEffect(() => () => batcher.dispose(), [batcher]) return (
v onPointerMove={(event) => { if (!event.currentTarget.hasPointerCapture(event.pointerId)) return const delta = dir === 'row' ? event.clientX - last.current.x : event.clientY - last.current.y - onResize(delta / Math.max(1, last.current.size)) + pendingDelta.current += delta + batcher.schedule(() => { + const accumulated = pendingDelta.current + pendingDelta.current = 0 + if (accumulated !== 0) onResize(accumulated / Math.max(1, last.current.size)) + }) last.current.x = event.clientX last.current.y = event.clientY }} onPointerUp={(event) => { if (!event.currentTarget.hasPointerCapture(event.pointerId)) return + // Flush any pending baked delta so the final pointer position lands + // (the release event is not a move; dropping it would lose the tail). + batcher.flushNow() event.currentTarget.releasePointerCapture(event.pointerId) setDragging(false) }} diff --git a/src/client/tab-content-memo.ts b/src/client/tab-content-memo.ts new file mode 100644 index 0000000..50ef171 --- /dev/null +++ b/src/client/tab-content-memo.ts @@ -0,0 +1,46 @@ +/** + * The memo comparator for one tab's content cell (TabContent in Sidebar.tsx). + * + * Kept as a pure module so the "what re-renders a tab cell" contract is unit + * testable: geometry/store re-renders of the Sidebar shell must NOT reconcile + * every mounted tab (xterm/CodeMirror/FileTree subtrees — issue #315), while + * locale switches (localeRevision), tab-registry updates (tabsVersion), and + * a tab MOVING BETWEEN PANES (paneId — onOpenDiff closes over the pane id, + * and moveTab reuses the SAME tab object, so only the pane id identifies the + * move) must re-render the cell. + */ +import type { SidebarTab } from './state.ts' + +/** The fields whose change invalidates a tab cell's memo. Anything a cell's + * render (or its callbacks) depends on must be listed here. */ +export interface TabContentMemoKey { + tab: SidebarTab + /** The pane this tab currently lives in. onOpenDiff closes over it, and + * moveTab reuses the same tab object when the tab moves panes — without + * comparing paneId the cell would keep the source pane's callback. */ + paneId: string + sessionId: string + cwd: string | undefined + visible: boolean + expanded: string[] + localeRevision: string + tabsVersion: number +} + +/** True when the cell may skip a re-render (all render-affecting fields + * unchanged). Callback/context identities are deliberately ignored: their + * captured dependencies are stable or covered by the compared fields + * (onReferenceFile → sessionId/cwd, onSubagentJump/onToggleDir → stable + * refs/closures, onOpenDiff → paneId). */ +export function tabContentCompare(prev: TabContentMemoKey, next: TabContentMemoKey): boolean { + return ( + prev.tab === next.tab && + prev.paneId === next.paneId && + prev.sessionId === next.sessionId && + prev.cwd === next.cwd && + prev.visible === next.visible && + prev.expanded === next.expanded && + prev.localeRevision === next.localeRevision && + prev.tabsVersion === next.tabsVersion + ) +} diff --git a/tests/e2e/toggle-layout.e2e.ts b/tests/e2e/toggle-layout.e2e.ts new file mode 100644 index 0000000..e391079 --- /dev/null +++ b/tests/e2e/toggle-layout.e2e.ts @@ -0,0 +1,199 @@ +/** + * Toggle-layout lane (issue #315): while the right panel slides open/closed + * with its 300ms layout push, the BOTTOM panel must keep tracking the center + * column's horizontal edges frame by frame. + * + * The shell changed the bottom panel's geometry source from React state + * (setCenterRect per ResizeObserver frame → full Sidebar re-render at + * animation cadence) to a ref + direct inline writes, so the panel still + * follows the animated center column with zero React work. This lane samples + * the transition and asserts the bottom panel's edges stay glued to the + * center column (left/right within a rounding + one-frame tolerance) — the + * visual-equivalence proof for that change. + * + * The server is booted by scripts/e2e-mount.sh; this spec only loads the + * page (own workspace, like drag-layout.e2e.ts). + */ +import { mkdirSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { test, expect, request, type APIRequestContext } from '@playwright/test' + +const BASE_URL = process.env.DSH_E2E_URL +if (!BASE_URL) { + throw new Error('DSH_E2E_URL is not set — boot a DSH web instance with the plugin mounted and point this lane at it (see scripts/e2e-mount.sh)') +} + +const WORKSPACE_PATH = process.env.DSH_E2E_TOGGLE_WORKSPACE ?? join(tmpdir(), 'dsh-e2e-toggle-workspace') + +let api: APIRequestContext + +async function seedSession(): Promise { + mkdirSync(WORKSPACE_PATH, { recursive: true }) + writeFileSync(join(WORKSPACE_PATH, 'seed.txt'), 'toggle lane\n') + const workspace = await api.post(`${BASE_URL}/api/workspace.create`, { + data: { type: 'client-request', rpcId: 'e2e-toggle-workspace', method: 'workspace.create', payload: { path: WORKSPACE_PATH } }, + }) + expect(workspace.ok(), `workspace.create: ${workspace.status()} ${await workspace.text()}`).toBe(true) + const workspaceBody = (await workspace.json()) as { + result: { ok: true; value: { workspace: { workspaceId: string } } } | { ok: false; error: unknown } + } + expect(workspaceBody.result.ok).toBe(true) + const workspaceId = (workspaceBody.result as { value: { workspace: { workspaceId: string } } }).value.workspace.workspaceId + const session = await api.post(`${BASE_URL}/api/session.create`, { + data: { type: 'client-request', rpcId: 'e2e-toggle-session', method: 'session.create', payload: { workspaceId } }, + }) + expect(session.ok(), `session.create: ${session.status()} ${await session.text()}`).toBe(true) +} + +test.beforeAll(async () => { + api = await request.newContext({ baseURL: BASE_URL }) + await seedSession() +}) + +test.afterAll(async () => { + await api?.dispose() +}) + +interface FrameSample { + t: number + centerLeft: number + centerRight: number + bottomLeft: number + bottomRight: number +} + +test('bottom panel tracks the center column during the right-panel toggle transition (issue #315)', async ({ page }) => { + await page.goto(BASE_URL, { waitUntil: 'domcontentloaded' }) + await expect(page.locator('#root > *')).not.toHaveCount(0, { timeout: 90_000 }) + const sidebar = page.locator('[data-dsh-better-sidebar]') + await expect(sidebar).toBeAttached({ timeout: 90_000 }) + + // Dismiss onboarding (same dance as the drag lane). + try { + await expect + .poll(() => page.getByRole('button', { name: /^(Continue|Configure later)$/ }).count(), { timeout: 60_000 }) + .toBeGreaterThan(0) + } catch { + console.warn('[e2e-toggle] no onboarding takeover appeared; proceeding') + } + for (let round = 0; round < 8; round++) { + let dismissed = false + for (const name of ['Continue', 'Configure later']) { + const button = page.getByRole('button', { name, exact: true }).first() + if ((await button.count()) === 0) continue + try { + await button.click({ timeout: 4_000 }) + dismissed = true + await page.waitForTimeout(800) + } catch { + // Masked by a takeover stacked above; retry in the next round. + } + } + if (!dismissed) break + } + + // Open the right panel (fresh sessions start collapsed). + const expandButton = sidebar.getByRole('button', { name: 'Expand sidebar' }) + await expect(expandButton).toHaveCount(1) + await expandButton.click() + await expect + .poll(async () => { + const value = await page.evaluate(() => document.documentElement.style.getPropertyValue('--dsh-sidebar-width')) + return value !== '' && value !== '0px' + }, { timeout: 90_000 }) + .toBe(true) + + // Open the bottom panel too: the twin panels make the toggle exercise the + // full geometry chain (bottom panel edges follow the animated push). + const expandBottom = sidebar.getByRole('button', { name: 'Expand bottom panel' }) + await expect(expandBottom).toHaveCount(1) + await expandBottom.click() + // Wait until the bottom panel is REALLY tracking the center column (not + // the hidden {0,0} fallback): the panel's box exists even while hidden, so + // wait for the computed visibility + a matched right edge before sampling. + await expect + .poll(async () => page.evaluate(() => { + const col = document.querySelector('#root [data-slot="conversation"]')?.parentElement + const bottom = document.querySelector('[data-dsh-bottom-panel]') + if (col === null || col === undefined || bottom === null) return false + const cr = col.getBoundingClientRect() + const br = bottom.getBoundingClientRect() + const visible = getComputedStyle(bottom).visibility !== 'hidden' + return cr.width > 100 && visible && Math.abs(br.right - cr.right) <= 3 + }), { timeout: 30_000 }) + .toBe(true) + await page.waitForTimeout(500) // let both open transitions settle + + // Per-frame sampler: center column (same anchor as layout.css) vs the + // bottom panel's visible edges. The loop re-resolves + // window.__toggleSamples every frame, so clearing it (sampleToggle) drops + // old frames without detaching the loop. + await page.evaluate(() => { + const center = document.querySelector('#root [data-slot="conversation"]')?.parentElement + const bottom = document.querySelector('[data-dsh-bottom-panel]') + ;(window as unknown as { __toggleSamples: FrameSample[] }).__toggleSamples = [] + const loop = (): void => { + const c = center?.getBoundingClientRect() ?? { left: 0, right: 0 } + const b = bottom?.getBoundingClientRect() ?? { left: 0, right: 0 } + ;(window as unknown as { __toggleSamples: FrameSample[] }).__toggleSamples.push({ + t: performance.now(), centerLeft: c.left, centerRight: c.right, bottomLeft: b.left, bottomRight: b.right, + }) + requestAnimationFrame(loop) + } + requestAnimationFrame(loop) + }) + + const collapseButton = sidebar.getByRole('button', { name: 'Collapse sidebar' }) + const expandButton2 = sidebar.getByRole('button', { name: 'Expand sidebar' }) + + async function sampleToggle(click: () => Promise): Promise { + // Clear accumulated frames so the sample window starts at the click. + await page.evaluate(() => { + ;(window as unknown as { __toggleSamples: FrameSample[] }).__toggleSamples = [] + }) + await click() + await page.waitForTimeout(800) + const samples = await page.evaluate(() => { + const data = (window as unknown as { __toggleSamples: FrameSample[] }).__toggleSamples + ;(window as unknown as { __toggleSamples: FrameSample[] }).__toggleSamples = [] + return data + }) + return samples + } + + function trackMiss(samples: FrameSample[], label: string): void { + // The bottom panel's geometry rides the center column's ResizeObserver: + // RO callbacks run AFTER layout, so a write there lands in the NEXT + // frame's paint — the panel is exactly ONE animation frame behind the + // column mid-transition (measured: bottom(t) === center(t-1)). + // Anything more is a real desync; the settled frames must agree exactly. + const valid = samples.filter(s => s.centerRight - s.centerLeft > 100) + expect(valid.length, `${label}: sampler must have captured the transition`).toBeGreaterThan(3) + + let maxOneFrame = 0 + for (let i = 1; i < valid.length; i++) { + maxOneFrame = Math.max( + maxOneFrame, + Math.abs(valid[i]!.bottomLeft - valid[i - 1]!.centerLeft), + Math.abs(valid[i]!.bottomRight - valid[i - 1]!.centerRight), + ) + } + const settled = valid.slice(-5) + const settleMiss = Math.max( + ...settled.map(s => Math.max( + Math.abs(s.bottomLeft - s.centerLeft), + Math.abs(s.bottomRight - s.centerRight), + )), + ) + console.log(`[e2e-toggle] ${label}: ${valid.length} frames, max one-frame miss=${maxOneFrame.toFixed(1)}px, settle miss=${settleMiss.toFixed(1)}px`) + expect(maxOneFrame, `${label}: bottom panel must track the center column one frame behind (RO cadence)`).toBeLessThanOrEqual(4) + expect(settleMiss, `${label}: bottom panel must settle exactly on the center column`).toBeLessThanOrEqual(3) + } + + const collapseSamples = await sampleToggle(() => collapseButton.click()) + trackMiss(collapseSamples, 'collapse') + + const expandSamples = await sampleToggle(() => expandButton2.click()) + trackMiss(expandSamples, 'expand') +}) diff --git a/tests/editor-host.spec.tsx b/tests/editor-host.spec.tsx index 1e060e7..34bdf7f 100644 --- a/tests/editor-host.spec.tsx +++ b/tests/editor-host.spec.tsx @@ -219,7 +219,7 @@ describe('EditorHost (files window)', () => { } }) - it('dragging the panel edge resizes the dock and persists meta.treeWidth on release', () => { + it('dragging the panel edge resizes the dock and persists meta.treeWidth on release', async () => { const { store, ctx, homeTab } = setup() store.setPrefs({ ...store.getPrefs(), editorExplorer: true }) const { container, unmount } = mountHost(ctx, store, homeTab) @@ -232,10 +232,15 @@ describe('EditorHost (files window)', () => { // Drag the left edge LEFT by 100px → the right-docked panel widens. // Pointer capture keeps move/up on the handle (jsdom: MouseEvent with // pointer* type names; setPointerCapture is absent and skipped). + // Moves are batched to one application per frame (#315), so flush the + // pending frame before asserting the width. act(() => { handle.dispatchEvent(new MouseEvent('pointerdown', { bubbles: true, clientX: 300 })) handle.dispatchEvent(new MouseEvent('pointermove', { bubbles: true, clientX: 200 })) }) + await act(async () => { + await new Promise(resolve => requestAnimationFrame(() => resolve())) + }) expect(dock.style.width).toBe('340px') // Release: the drag state clears and the width persists on the tab. act(() => { handle.dispatchEvent(new MouseEvent('pointerup', { bubbles: true, clientX: 200 })) }) diff --git a/tests/frame-batcher.spec.ts b/tests/frame-batcher.spec.ts new file mode 100644 index 0000000..f94bc58 --- /dev/null +++ b/tests/frame-batcher.spec.ts @@ -0,0 +1,93 @@ +/** + * Frame-batcher: coalesce bursty schedules into one task per animation + * frame, flush synchronously on release, and drop pending work on dispose. + * Uses a deterministic rAF stub so the coalescing contract is exact. + */ +// @vitest-environment jsdom +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { createFrameBatcher } from '../src/client/frame-batcher.ts' + +let frames: Map void> +let nextId: number + +function stepFrame(): void { + const callbacks = [...frames.values()] + frames.clear() + for (const callback of callbacks) callback() +} + +beforeEach(() => { + frames = new Map() + nextId = 1 + vi.stubGlobal('requestAnimationFrame', (callback: () => void) => { + frames.set(nextId, callback) + return nextId++ + }) + vi.stubGlobal('cancelAnimationFrame', (id: number) => { + frames.delete(id) + }) +}) + +afterEach(() => { + vi.unstubAllGlobals() +}) + +describe('createFrameBatcher', () => { + it('coalesces many schedules into one task per frame (latest wins)', () => { + const batcher = createFrameBatcher() + const calls: number[] = [] + let pending = 0 + const apply = (value: number): void => { calls.push(value) } + + // Caller pattern: compose into a ref, schedule a reader of that ref. + for (const value of [1, 2, 3, 4]) { + pending = value + batcher.schedule(() => apply(pending)) + } + expect(calls).toEqual([]) + stepFrame() + expect(calls).toEqual([4]) + expect(frames.size).toBe(0) + + // The next burst starts a fresh frame. + pending = 7 + batcher.schedule(() => apply(pending)) + stepFrame() + expect(calls).toEqual([4, 7]) + }) + + it('flushNow runs the pending task synchronously and cancels the frame', () => { + const batcher = createFrameBatcher() + const calls: number[] = [] + batcher.schedule(() => calls.push(1)) + batcher.schedule(() => calls.push(2)) + expect(frames.size).toBe(1) // still one pending frame + + batcher.flushNow() + expect(calls).toEqual([2]) // latest task, synchronously + expect(frames.size).toBe(0) // frame cancelled + + stepFrame() // the cancelled frame must not fire again + expect(calls).toEqual([2]) + + // flushNow when nothing is scheduled is a no-op. + batcher.flushNow() + expect(calls).toEqual([2]) + }) + + it('dispose drops the pending task without running it', () => { + const batcher = createFrameBatcher() + const calls: number[] = [] + batcher.schedule(() => calls.push(1)) + batcher.dispose() + expect(frames.size).toBe(0) + stepFrame() + expect(calls).toEqual([]) + + // A disposed batcher accepts new schedules (React strict-mode remounts + // reuse the same ref-created instance semantics). + batcher.schedule(() => calls.push(2)) + stepFrame() + expect(calls).toEqual([2]) + }) +}) diff --git a/tests/tab-content-memo.spec.ts b/tests/tab-content-memo.spec.ts new file mode 100644 index 0000000..ee62b6f --- /dev/null +++ b/tests/tab-content-memo.spec.ts @@ -0,0 +1,55 @@ +/** + * The TabContent memo comparator: geometry/store re-renders of the Sidebar + * shell must not reconcile every mounted tab (issue #315), while the fields + * a cell's output or callbacks depend on MUST invalidate it. The pane-move + * case (review P1): moveTab reuses the SAME tab object when a tab moves + * panes, so paneId — not tab identity — is what identifies the move and + * must be compared (onOpenDiff closes over the pane id). + */ +import { describe, expect, it } from 'vitest' +import { tabContentCompare, type TabContentMemoKey } from '../src/client/tab-content-memo.ts' +import type { SidebarTab } from '../src/client/state.ts' + +function makeKey(overrides: Partial = {}): TabContentMemoKey { + return { + tab: { id: 'tab:1', type: 'git', title: 'Git' } as SidebarTab, + paneId: 'pane:a', + sessionId: 'session:1', + cwd: '/workspace', + visible: true, + expanded: [], + localeRevision: 'en-US', + tabsVersion: 0, + ...overrides, + } +} + +describe('tabContentCompare', () => { + it('skips re-render when nothing render-affecting changed (callback identity alone is ignored)', () => { + const key = makeKey() + expect(tabContentCompare(key, { ...key })).toBe(true) + }) + + it('invalidates on a tab moving between panes even though the tab OBJECT is reused (moveTab P1)', () => { + const sameTab = { id: 'tab:1', type: 'git', title: 'Git' } as SidebarTab + const before = makeKey({ tab: sameTab, paneId: 'pane:a' }) + const after = makeKey({ tab: sameTab, paneId: 'pane:b' }) // moveTab reinserts the same object + expect(before.tab).toBe(after.tab) + expect(tabContentCompare(before, after)).toBe(false) + }) + + it('invalidates on locale switches (copy freshness)', () => { + expect(tabContentCompare(makeKey(), makeKey({ localeRevision: 'zh-CN' }))).toBe(false) + }) + + it('invalidates on tab-registry updates (descriptor refresh)', () => { + expect(tabContentCompare(makeKey(), makeKey({ tabsVersion: 1 }))).toBe(false) + }) + + it('invalidates on visible/expanded/session/cwd changes', () => { + expect(tabContentCompare(makeKey(), makeKey({ visible: false }))).toBe(false) + expect(tabContentCompare(makeKey(), makeKey({ expanded: ['/workspace'] }))).toBe(false) + expect(tabContentCompare(makeKey(), makeKey({ sessionId: 'session:2' }))).toBe(false) + expect(tabContentCompare(makeKey(), makeKey({ cwd: '/other' }))).toBe(false) + }) +})