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; }