From 52834088fe3336ed29f400d853278a02eaeb5fca Mon Sep 17 00:00:00 2001 From: Ning YuanLin <57001533+NingYuanLin@users.noreply.github.com> Date: Sat, 31 Oct 2026 05:20:39 +0800 Subject: [PATCH] fix(tree): preserve horizontal scroll when expanding unrelated nodes (#504) The Project view kept its widest row only until the rows changed, so expanding an unrelated folder while the wide row was out of the virtual window shrank the scroll range and moved the view back to the left edge. Row widths are now kept by row key, and a width goes away only when its row is removed or collapsed, or is measured narrower. Co-authored-by: NingYuanLin <57001533+NingYuanLin@users.noreply.github.com> --- e2e/treeWidth.fixture.tsx | 65 ++++++++++++++++ e2e/treeWidth.html | 11 +++ e2e/treeWidth.spec.ts | 158 ++++++++++++++++++++++++++++++++++++++ src/ui/Tree.test.tsx | 55 ++++++++++--- src/ui/Tree.tsx | 74 +++++++++++++++--- 5 files changed, 341 insertions(+), 22 deletions(-) create mode 100644 e2e/treeWidth.fixture.tsx create mode 100644 e2e/treeWidth.html create mode 100644 e2e/treeWidth.spec.ts diff --git a/e2e/treeWidth.fixture.tsx b/e2e/treeWidth.fixture.tsx new file mode 100644 index 00000000..81fa3749 --- /dev/null +++ b/e2e/treeWidth.fixture.tsx @@ -0,0 +1,65 @@ +import { useState } from "react"; +import { createRoot } from "react-dom/client"; +import { Tree } from "../src/ui/Tree"; + +interface Node { + key: string; + label: string; + children?: Node[]; +} +const longLabel = `z_${"long_file_name_".repeat(16)}.ts`; +const initial: Node[] = [ + { key: "src", label: "src", children: [{ key: "child", label: "child.ts" }] }, + ...Array.from({ length: 160 }, (_, i) => ({ key: `file-${i}`, label: `file_${i}.ts` })), + { key: "wide", label: longLabel }, +]; +const shorten = (nodes: Node[]): Node[] => + nodes.map((node) => ({ + ...node, + label: node.key === "wide" ? "short.ts" : node.label, + children: node.children ? shorten(node.children) : undefined, + })); +const getKey = (node: Node) => node.key; +const getLabel = (node: Node) => node.label; +const getChildren = (node: Node) => node.children; +const isExpandable = (node: Node) => !!node.children; + +function Fixture() { + const [roots, setRoots] = useState(initial); + const [width, setWidth] = useState(240); + const [font, setFont] = useState(14); + const [indent, setIndent] = useState(16); + const [hidden, setHidden] = useState(false); + const [horizontal, setHorizontal] = useState(true); + return ( + <> + + + + + + + + + + + + ); +} + +createRoot(document.getElementById("root")!).render(); diff --git a/e2e/treeWidth.html b/e2e/treeWidth.html new file mode 100644 index 00000000..c68029b7 --- /dev/null +++ b/e2e/treeWidth.html @@ -0,0 +1,11 @@ + + + + + Tree width regression + + +
+ + + diff --git a/e2e/treeWidth.spec.ts b/e2e/treeWidth.spec.ts new file mode 100644 index 00000000..e9a73a40 --- /dev/null +++ b/e2e/treeWidth.spec.ts @@ -0,0 +1,158 @@ +import { expect, test, type Locator, type Page } from "@playwright/test"; + +const metrics = (tree: Locator) => + tree.evaluate((node) => ({ width: node.scrollWidth, left: node.scrollLeft, viewport: node.clientWidth })); +async function open(page: Page) { + await page.goto("/e2e/treeWidth.html"); + const tree = page.getByRole("tree", { name: "Width fixture" }); + await expect(tree.getByRole("treeitem", { name: "src", exact: true })).toBeVisible(); + return tree; +} +async function wheel(page: Page, tree: Locator, x: number, y: number) { + await tree.hover(); + await page.mouse.wheel(x, y); + await expect + .poll(async () => tree.evaluate((node) => node.scrollTop === node.scrollHeight - node.clientHeight)) + .toBe(y > 0); + if (y < 0) await expect.poll(async () => tree.evaluate((node) => node.scrollTop)).toBe(0); +} +async function measureLong(page: Page, tree: Locator) { + await tree.getByRole("treeitem", { name: "src", exact: true }).click(); + await wheel(page, tree, 0, 10000); + await expect(tree.getByRole("treeitem", { name: /^z_long/ })).toBeVisible(); + await page.mouse.wheel(200, 0); + await expect.poll(async () => (await metrics(tree)).left).toBe(200); + return metrics(tree); +} + +test("keeps offscreen width when an unrelated directory expands, without transient clamping", async ({ page }) => { + const tree = await open(page); + const before = await measureLong(page, tree); + await wheel(page, tree, 0, -10000); + await expect(tree.getByRole("treeitem", { name: /^z_long/ })).toHaveCount(0); + expect(await metrics(tree)).toEqual(before); + await tree.evaluate((node) => { + const samples: number[] = []; + Object.assign(window, { widthSamples: samples }); + let frames = 0; + const sample = () => { + samples.push(node.scrollLeft); + if (++frames < 30) requestAnimationFrame(sample); + }; + requestAnimationFrame(sample); + }); + await page.keyboard.press("ArrowRight"); + await expect(tree.getByRole("treeitem", { name: "src", exact: true })).toHaveAttribute("aria-expanded", "true"); + await expect + .poll(async () => page.evaluate(() => (window as unknown as { widthSamples: number[] }).widthSamples.length)) + .toBe(30); + const after = await metrics(tree); + expect(after).toEqual(before); + expect(await page.evaluate(() => Math.min(...(window as unknown as { widthSamples: number[] }).widthSamples))).toBe( + 200, + ); +}); + +test("deleting, collapsing and filtering the widest row allow shrinkage", async ({ page }) => { + let tree = await open(page); + await measureLong(page, tree); + await wheel(page, tree, 0, -10000); + await page.getByRole("button", { name: "Delete widest" }).click(); + await expect.poll(async () => (await metrics(tree)).width).toBe(240); + tree = await open(page); + await measureLong(page, tree); + await wheel(page, tree, 0, -10000); + await page.getByRole("button", { name: "Nest rows" }).click(); + await tree.getByRole("treeitem", { name: "folder", exact: true }).click(); + await page.keyboard.press("ArrowLeft"); + await expect.poll(async () => (await metrics(tree)).width).toBe(240); + tree = await open(page); + await measureLong(page, tree); + await page.keyboard.type("src"); + await expect.poll(async () => (await metrics(tree)).width).toBe(240); +}); + +test("same-key content, depth, font and indent are remeasured", async ({ page }) => { + const tree = await open(page); + const initial = await measureLong(page, tree); + // An external style/font update need not cause a React render. + const fontStyle = await page.addStyleTag({ content: '[role="tree"] { font-size: 20px; }' }); + await expect.poll(async () => (await metrics(tree)).width).toBeGreaterThan(initial.width); + await fontStyle.evaluate((node) => node.parentNode!.removeChild(node)); + await expect.poll(async () => (await metrics(tree)).width).toBe(initial.width); + await page.getByRole("button", { name: "Change font" }).click(); + await expect.poll(async () => (await metrics(tree)).width).toBeGreaterThan(initial.width); + await page.getByRole("button", { name: "Change font" }).click(); + await expect.poll(async () => (await metrics(tree)).width).toBe(initial.width); + await page.getByRole("button", { name: "Nest rows" }).click(); + await tree.focus(); + await page.keyboard.press("End"); + await expect.poll(async () => (await metrics(tree)).width).toBe(initial.width + 16); + await page.getByRole("button", { name: "Change indent" }).click(); + await expect.poll(async () => (await metrics(tree)).width).toBe(initial.width + 40); + await page.getByRole("button", { name: "Shorten same key" }).click(); + await expect.poll(async () => (await metrics(tree)).width).toBe(240); +}); + +test("panel resizing does not cache the viewport floor, and horizontal mode can be disabled", async ({ page }) => { + const tree = await open(page); + await page.getByRole("button", { name: "Resize panel" }).click(); + await expect.poll(async () => (await metrics(tree)).viewport).toBe(2400); + await wheel(page, tree, 0, 10000); + await wheel(page, tree, 0, -10000); + await page.getByRole("button", { name: "Resize panel" }).click(); + await expect.poll(async () => (await metrics(tree)).viewport).toBe(240); + expect((await metrics(tree)).width).toBeLessThan(2400); + expect((await metrics(tree)).width).toBeGreaterThan(240); + await page.getByRole("button", { name: "Toggle horizontal" }).click(); + await expect(tree.locator("[data-content-width]")).toHaveCount(0); + await expect.poll(async () => (await metrics(tree)).width).toBe(240); + await wheel(page, tree, 0, 10000); + await expect.poll(async () => (await metrics(tree)).width).toBe(240); + await page.getByRole("button", { name: "Shorten same key" }).click(); + await page.getByRole("button", { name: "Toggle horizontal" }).click(); + await expect(tree.locator("[data-content-width]")).toHaveCount(1); + await expect.poll(async () => (await metrics(tree)).width).toBe(240); +}); + +test("offscreen same-key updates remain estimates until the row is mounted again", async ({ page }) => { + const tree = await open(page); + const before = await measureLong(page, tree); + await wheel(page, tree, 0, -10000); + await page.getByRole("button", { name: "Shorten same key" }).click(); + expect(await metrics(tree)).toEqual(before); + await wheel(page, tree, 0, 10000); + await expect(tree.getByRole("treeitem", { name: "short.ts", exact: true })).toBeVisible(); + await expect.poll(async () => (await metrics(tree)).width).toBe(240); + await wheel(page, tree, 0, -10000); + await page.getByRole("button", { name: "Resize panel" }).click(); + await wheel(page, tree, 0, 10000); + await page.getByRole("button", { name: "Resize panel" }).click(); + await expect.poll(async () => (await metrics(tree)).width).toBe(240); +}); + +test("hidden trees keep their widths and reconcile pending removals when shown", async ({ page }) => { + await open(page); + const tree = page.getByRole("tree", { name: "Width fixture", includeHidden: true }); + const before = await measureLong(page, tree); + const spacer = tree.locator("[data-content-width]"); + const stored = await spacer.evaluate((node) => (node as HTMLElement).style.width); + await page.getByRole("button", { name: "Toggle visibility" }).click(); + await expect(tree).toBeHidden(); + // Give ResizeObserver deliveries a chance to run while there is no layout. + await page.evaluate( + () => + new Promise((resolve) => { + requestAnimationFrame(() => requestAnimationFrame(() => requestAnimationFrame(() => resolve()))); + }), + ); + expect(await spacer.evaluate((node) => (node as HTMLElement).style.width)).toBe(stored); + await page.getByRole("button", { name: "Toggle visibility" }).click(); + await expect(tree).toBeVisible(); + await expect.poll(() => metrics(tree)).toEqual(before); + await page.getByRole("button", { name: "Toggle visibility" }).click(); + await page.getByRole("button", { name: "Delete widest" }).click(); + await page.getByRole("button", { name: "Change indent" }).click(); + await page.getByRole("button", { name: "Toggle visibility" }).click(); + await expect.poll(async () => (await metrics(tree)).width).toBe(240); +}); diff --git a/src/ui/Tree.test.tsx b/src/ui/Tree.test.tsx index 3f271398..8c0d04cd 100644 --- a/src/ui/Tree.test.tsx +++ b/src/ui/Tree.test.tsx @@ -223,19 +223,52 @@ describe("Tree horizontal scrolling", () => { expect(spacer(tree)).toBeNull(); }); - it("keeps the scroll range while scrolling and re-measures when the rows change", () => { + it("remeasures mounted rows with the same key, including narrower content", () => { const width = vi.spyOn(HTMLElement.prototype, "offsetWidth", "get").mockReturnValue(640); - const { tree, key } = setup({ horizontalScroll: true, defaultExpanded: ["src"] }); - expect(spacer(tree)?.style.width).toBe("640px"); + try { + const { tree, key } = setup({ horizontalScroll: true, defaultExpanded: ["src"] }); + expect(spacer(tree)?.style.width).toBe("640px"); + width.mockReturnValue(100); + key("ArrowDown"); + expect(spacer(tree)?.style.width).toBe("100px"); + } finally { + width.mockRestore(); + } + }); - // The wide row left the rendered window: the same rows, measured narrower. - width.mockReturnValue(100); - key("ArrowDown"); - expect(spacer(tree)?.style.width).toBe("640px"); + it("removes widths belonging to collapsed descendants", () => { + const width = vi.spyOn(HTMLElement.prototype, "offsetWidth", "get").mockImplementation(function ( + this: HTMLElement, + ) { + return this.textContent === "util.ts" ? 640 : 100; + }); + try { + const { tree, key } = setup({ horizontalScroll: true, defaultExpanded: ["src"] }); + expect(spacer(tree)?.style.width).toBe("640px"); + key("Home"); + key("ArrowLeft"); + expect(spacer(tree)?.style.width).toBe("100px"); + } finally { + width.mockRestore(); + } + }); - key("Home"); - key("ArrowLeft"); - expect(spacer(tree)?.style.width).toBe("100px"); - width.mockRestore(); + it("measures without the viewport minimum and restores it before returning", () => { + const minimums: string[] = []; + const width = vi.spyOn(HTMLElement.prototype, "offsetWidth", "get").mockImplementation(function ( + this: HTMLElement, + ) { + if (this.hasAttribute("data-row")) minimums.push(this.style.minWidth); + return parseFloat(this.style.minWidth) === 0 ? 100 : 800; + }); + try { + const { tree } = setup({ horizontalScroll: true }); + expect(spacer(tree)?.style.width).toBe("100px"); + expect(minimums.length).toBeGreaterThan(0); + expect(minimums.every((value) => parseFloat(value) === 0)).toBe(true); + for (const row of tree.querySelectorAll("[data-row]")) expect(row.style.minWidth).toBe(""); + } finally { + width.mockRestore(); + } }); }); diff --git a/src/ui/Tree.tsx b/src/ui/Tree.tsx index cf5de5fe..13258ddc 100644 --- a/src/ui/Tree.tsx +++ b/src/ui/Tree.tsx @@ -117,20 +117,72 @@ export interface TreeProps { reorder?: TreeReorder; } -/** Widest row seen since `rows` last changed; rows are virtualised, so the scroll range must outlive them. */ -function useContentWidth(scroller: RefObject, rows: unknown, enabled: boolean): number { +/** Remember intrinsic widths by identity, including rows outside the virtual window. */ +function useContentWidth( + scroller: RefObject, + rows: readonly Row[], + enabled: boolean, +): number { const [width, setWidth] = useState(0); - const measuredRows = useRef(rows); + const cache = useRef(new Map()); + const measuredRows = useRef[] | null>(null); + const maximum = useRef(0); useLayoutEffect(() => { - if (!enabled) return; - let widest = 0; - scroller.current?.querySelectorAll("[data-row]").forEach((row) => { - widest = Math.max(widest, row.offsetWidth); - }); - const kept = measuredRows.current === rows; - measuredRows.current = rows; - setWidth((current) => (kept ? Math.max(current, widest) : widest)); + if (!enabled) { + cache.current.clear(); + measuredRows.current = null; + maximum.current = 0; + return; + } + const element = scroller.current; + if (!element) return; + const mounted = [...element.querySelectorAll("[data-row]")]; + const measure = () => { + // Hidden tool windows have no layout. Keep their estimates until shown; + // the observer remains attached so showing the window resumes measurement. + if (element.clientWidth === 0) return; + let recompute = false; + if (measuredRows.current !== rows) { + const keys = new Set(rows.map((row) => row.key)); + for (const [key, value] of cache.current) { + if (!keys.has(key)) { + cache.current.delete(key); + if (value === maximum.current) recompute = true; + } + } + measuredRows.current = rows; + } + // Measure max-content, not the viewport's min-width floor. Batch writes and + // restore before paint so selection backgrounds still fill the viewport. + const minimums = mounted.map((row) => row.style.minWidth); + for (const row of mounted) row.style.minWidth = "0"; + const widths = mounted.map((row) => row.offsetWidth); + mounted.forEach((row, index) => { + row.style.minWidth = minimums[index]!; + const key = rows[Number(row.dataset.row)]!.key; + const value = widths[index]!; + if (cache.current.get(key) === maximum.current && value < maximum.current) recompute = true; + cache.current.set(key, value); + maximum.current = Math.max(maximum.current, value); + }); + // Ordinary scrolling only touches mounted rows. Scan cached widths only + // when the previous maximum was removed or actually became narrower. + if (recompute) { + maximum.current = 0; + for (const value of cache.current.values()) maximum.current = Math.max(maximum.current, value); + recompute = false; + } + // Reconcile and measure together: never publish an intermediate shrink. + setWidth(maximum.current); + }; + measure(); + const observer = new ResizeObserver(measure); + observer.observe(element); + for (const row of mounted) observer.observe(row); + return () => observer.disconnect(); }); + // Offscreen widths are estimates after label/font/indent changes; mounting + // measures them afresh, including decreases for the same key. return width; }