fix: preserve file-tree rows after hidden panel restore (#3744)
This commit is contained in:
@@ -28,6 +28,7 @@ import type { FileInfo } from "@/lib/state/store";
|
||||
import type { WorkspaceRestorationAttempt } from "@/lib/state/slices/session-runtime/workspace-restoration";
|
||||
import type { FileBrowserRow } from "./file-browser-hooks";
|
||||
import { areTreeNodeRowPropsEqual, type TreeNodeRowProps } from "./file-tree-row-props";
|
||||
import { measureFileTreeElement } from "./file-tree-measurement";
|
||||
import { InlineFileInput } from "./inline-file-input";
|
||||
import {
|
||||
FileContextMenu,
|
||||
@@ -545,6 +546,7 @@ function VirtualizedFileTreeView(props: FileBrowserContentAreaProps) {
|
||||
const row = virtualRows[index];
|
||||
return row?.type === "create" ? `create:${row.parentPath}` : (row?.row.path ?? index);
|
||||
},
|
||||
measureElement: measureFileTreeElement,
|
||||
overscan: 5,
|
||||
});
|
||||
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
fileTreeGeometryIssues,
|
||||
type FileTreeViewportGeometry,
|
||||
} from "../../e2e/tests/task/file-tree-geometry";
|
||||
|
||||
function geometry(overrides: Partial<FileTreeViewportGeometry>): FileTreeViewportGeometry {
|
||||
return {
|
||||
bottom: 100,
|
||||
clientHeight: 100,
|
||||
rows: [{ path: "row-0", top: 0, bottom: 28 }],
|
||||
scrollTop: 0,
|
||||
top: 0,
|
||||
treeHeight: 28,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe("file-tree viewport geometry assertions", () => {
|
||||
it("rejects a blank top edge", () => {
|
||||
expect(
|
||||
fileTreeGeometryIssues(
|
||||
geometry({
|
||||
rows: [{ path: "row-4", top: 30, bottom: 58 }],
|
||||
treeHeight: 300,
|
||||
}),
|
||||
),
|
||||
).toContainEqual(expect.stringContaining("blank gap at the viewport top"));
|
||||
});
|
||||
|
||||
it("rejects a blank bottom edge when overflowing content continues below", () => {
|
||||
expect(
|
||||
fileTreeGeometryIssues(
|
||||
geometry({
|
||||
rows: [
|
||||
{ path: "row-0", top: 0, bottom: 28 },
|
||||
{ path: "row-1", top: 28, bottom: 80 },
|
||||
],
|
||||
treeHeight: 300,
|
||||
}),
|
||||
),
|
||||
).toContainEqual(expect.stringContaining("blank gap at the viewport bottom"));
|
||||
});
|
||||
|
||||
it("allows trailing space for a short tree", () => {
|
||||
expect(
|
||||
fileTreeGeometryIssues(
|
||||
geometry({
|
||||
rows: [{ path: "folder", top: 0, bottom: 28 }],
|
||||
treeHeight: 99,
|
||||
}),
|
||||
),
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it("allows the configured end padding at the end of overflowing content", () => {
|
||||
expect(
|
||||
fileTreeGeometryIssues(
|
||||
geometry({
|
||||
rows: [
|
||||
{ path: "row-4", top: -20, bottom: 8 },
|
||||
{ path: "row-5", top: 8, bottom: 92 },
|
||||
],
|
||||
scrollTop: 200,
|
||||
treeHeight: 300,
|
||||
}),
|
||||
),
|
||||
).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,55 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { Virtualizer } from "@tanstack/react-virtual";
|
||||
import { measureFileTreeElement } from "./file-tree-measurement";
|
||||
|
||||
function createVirtualizer(estimateSize: number): Virtualizer<HTMLDivElement, HTMLDivElement> {
|
||||
return new Virtualizer<HTMLDivElement, HTMLDivElement>({
|
||||
count: 1,
|
||||
getScrollElement: () => null,
|
||||
estimateSize: () => estimateSize,
|
||||
scrollToFn: () => {},
|
||||
observeElementRect: () => {},
|
||||
observeElementOffset: () => {},
|
||||
getItemKey: () => "row-a",
|
||||
});
|
||||
}
|
||||
|
||||
function resizeEntry(element: HTMLDivElement, blockSize: number): ResizeObserverEntry {
|
||||
return {
|
||||
target: element,
|
||||
borderBoxSize: [{ blockSize, inlineSize: 320 }],
|
||||
} as unknown as ResizeObserverEntry;
|
||||
}
|
||||
|
||||
describe("file-tree row measurements", () => {
|
||||
it("retains a positive cached row size when a hidden row reports zero height", () => {
|
||||
const element = document.createElement("div");
|
||||
element.dataset.index = "0";
|
||||
const virtualizer = createVirtualizer(28);
|
||||
virtualizer.itemSizeCache.set("row-a", 32);
|
||||
|
||||
expect(measureFileTreeElement(element, resizeEntry(element, 0), virtualizer)).toBe(32);
|
||||
});
|
||||
|
||||
it.each([28, 44])(
|
||||
"uses the current estimate when no positive size exists (%dpx)",
|
||||
(estimateSize) => {
|
||||
const element = document.createElement("div");
|
||||
element.dataset.index = "0";
|
||||
const virtualizer = createVirtualizer(estimateSize);
|
||||
|
||||
expect(measureFileTreeElement(element, resizeEntry(element, 0), virtualizer)).toBe(
|
||||
estimateSize,
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
it("accepts a later positive measurement after a cached fallback", () => {
|
||||
const element = document.createElement("div");
|
||||
element.dataset.index = "0";
|
||||
const virtualizer = createVirtualizer(28);
|
||||
virtualizer.itemSizeCache.set("row-a", 28);
|
||||
|
||||
expect(measureFileTreeElement(element, resizeEntry(element, 48), virtualizer)).toBe(48);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,19 @@
|
||||
import { measureElement as tanstackMeasureElement } from "@tanstack/react-virtual";
|
||||
import type { Virtualizer } from "@tanstack/react-virtual";
|
||||
|
||||
export function measureFileTreeElement(
|
||||
element: HTMLDivElement,
|
||||
entry: ResizeObserverEntry | undefined,
|
||||
instance: Virtualizer<HTMLDivElement, HTMLDivElement>,
|
||||
): number {
|
||||
const measuredSize = tanstackMeasureElement(element, entry, instance);
|
||||
if (measuredSize > 0) return measuredSize;
|
||||
|
||||
const index = instance.indexFromElement(element);
|
||||
const key = instance.options.getItemKey(index);
|
||||
// Hidden rows must retain positive cached geometry until the browser can measure them again.
|
||||
const cachedSize = instance.itemSizeCache.get(key);
|
||||
return cachedSize !== undefined && cachedSize > 0
|
||||
? cachedSize
|
||||
: instance.options.estimateSize(index);
|
||||
}
|
||||
@@ -0,0 +1,112 @@
|
||||
export type FileTreeRowBounds = Readonly<{
|
||||
bottom: number;
|
||||
path: string | null;
|
||||
top: number;
|
||||
}>;
|
||||
|
||||
export type FileTreeViewportGeometry = Readonly<{
|
||||
bottom: number;
|
||||
clientHeight: number;
|
||||
rows: readonly FileTreeRowBounds[];
|
||||
scrollTop: number;
|
||||
top: number;
|
||||
treeHeight: number;
|
||||
}>;
|
||||
|
||||
export type FileTreeGeometryOptions = Readonly<{
|
||||
contentTopInset?: number;
|
||||
endPadding?: number;
|
||||
expectedPaths?: readonly string[];
|
||||
tolerance?: number;
|
||||
}>;
|
||||
|
||||
export const FILE_TREE_ROW_EDGE_TOLERANCE_PX = 2;
|
||||
export const FILE_TREE_END_PADDING_PX = 8;
|
||||
|
||||
function expectedPathIssues(
|
||||
rows: readonly FileTreeRowBounds[],
|
||||
expectedPaths: readonly string[] | undefined,
|
||||
): string[] {
|
||||
if (!expectedPaths) return [];
|
||||
const actualPaths = rows.flatMap((row) => (row.path === null ? [] : [row.path]));
|
||||
if (
|
||||
actualPaths.length === expectedPaths.length &&
|
||||
actualPaths.every((path, index) => path === expectedPaths[index])
|
||||
) {
|
||||
return [];
|
||||
}
|
||||
return [
|
||||
`file tree paths differ: expected ${JSON.stringify(expectedPaths)}, got ${JSON.stringify(actualPaths)}`,
|
||||
];
|
||||
}
|
||||
|
||||
function viewportEdgeIssues(
|
||||
geometry: FileTreeViewportGeometry,
|
||||
options: FileTreeGeometryOptions = {},
|
||||
): string[] {
|
||||
const tolerance = options.tolerance ?? FILE_TREE_ROW_EDGE_TOLERANCE_PX;
|
||||
const contentTopInset = options.contentTopInset ?? 0;
|
||||
const endPadding = options.endPadding ?? FILE_TREE_END_PADDING_PX;
|
||||
const issues: string[] = [];
|
||||
const [firstRow] = geometry.rows;
|
||||
const lastRow = geometry.rows.at(-1);
|
||||
|
||||
if (!firstRow || !lastRow) return issues;
|
||||
|
||||
const contentTop = geometry.top + contentTopInset;
|
||||
if (firstRow.top > contentTop + tolerance) {
|
||||
issues.push(
|
||||
`file tree has a blank gap at the viewport top: first row starts ${firstRow.top - contentTop}px below content top`,
|
||||
);
|
||||
}
|
||||
|
||||
const treeBottom = geometry.top - geometry.scrollTop + geometry.treeHeight;
|
||||
const treeIsShort = geometry.treeHeight < geometry.clientHeight;
|
||||
if (!treeIsShort) {
|
||||
const treeEndsAtViewport = treeBottom <= geometry.bottom + endPadding + tolerance;
|
||||
const contentBottom = treeEndsAtViewport
|
||||
? Math.min(geometry.bottom, treeBottom) - endPadding
|
||||
: geometry.bottom;
|
||||
if (lastRow.bottom < contentBottom - tolerance) {
|
||||
issues.push(
|
||||
`file tree has a blank gap at the viewport bottom: last row ends ${contentBottom - lastRow.bottom}px before content bottom`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
return issues;
|
||||
}
|
||||
|
||||
function adjacentRowIssues(rows: readonly FileTreeRowBounds[], tolerance: number): string[] {
|
||||
const issues: string[] = [];
|
||||
for (let index = 1; index < rows.length; index += 1) {
|
||||
const previous = rows[index - 1];
|
||||
const current = rows[index];
|
||||
if (!previous || !current) continue;
|
||||
const gap = current.top - previous.bottom;
|
||||
if (gap < -tolerance) {
|
||||
issues.push(
|
||||
`file tree rows ${previous.path ?? "unknown"} and ${current.path ?? "unknown"} overlap by ${-gap}px`,
|
||||
);
|
||||
} else if (gap > tolerance) {
|
||||
issues.push(
|
||||
`file tree rows ${previous.path ?? "unknown"} and ${current.path ?? "unknown"} have a blank gap of ${gap}px`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
return issues;
|
||||
}
|
||||
|
||||
export function fileTreeGeometryIssues(
|
||||
geometry: FileTreeViewportGeometry,
|
||||
options: FileTreeGeometryOptions = {},
|
||||
): string[] {
|
||||
if (geometry.rows.length === 0) return ["file tree has no mounted rows in its viewport"];
|
||||
const tolerance = options.tolerance ?? FILE_TREE_ROW_EDGE_TOLERANCE_PX;
|
||||
return [
|
||||
...expectedPathIssues(geometry.rows, options.expectedPaths),
|
||||
...viewportEdgeIssues(geometry, options),
|
||||
...adjacentRowIssues(geometry.rows, tolerance),
|
||||
];
|
||||
}
|
||||
@@ -1,11 +1,16 @@
|
||||
import path from "node:path";
|
||||
import type { Locator } from "@playwright/test";
|
||||
import { expect, type Locator } from "@playwright/test";
|
||||
import type { Page } from "@playwright/test";
|
||||
import type { SeedData } from "../../fixtures/test-base";
|
||||
import type { ApiClient } from "../../helpers/api-client";
|
||||
import type { BackendContext } from "../../fixtures/backend";
|
||||
import { GitHelper, makeGitEnv } from "../../helpers/git-helper";
|
||||
import { SessionPage } from "../../pages/session-page";
|
||||
import {
|
||||
fileTreeGeometryIssues,
|
||||
type FileTreeGeometryOptions,
|
||||
type FileTreeViewportGeometry,
|
||||
} from "./file-tree-geometry";
|
||||
|
||||
export const LARGE_FILE_TREE_FOLDER = "large-file-tree";
|
||||
export const LARGE_FILE_TREE_COUNT = 600;
|
||||
@@ -14,6 +19,66 @@ export function largeFileTreePath(index: number): string {
|
||||
return `${LARGE_FILE_TREE_FOLDER}/entry-${index.toString().padStart(4, "0")}.txt`;
|
||||
}
|
||||
|
||||
/** Wait for a hidden-panel transition to pass through the browser's layout observers. */
|
||||
export async function waitForFileTreeLayoutSettle(page: Page): Promise<void> {
|
||||
await page.evaluate(
|
||||
() =>
|
||||
new Promise<void>((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))),
|
||||
);
|
||||
}
|
||||
|
||||
async function readFileTreeViewportGeometry(viewport: Locator): Promise<FileTreeViewportGeometry> {
|
||||
return viewport.evaluate((element) => {
|
||||
const viewportRect = element.getBoundingClientRect();
|
||||
const firstVirtualRow = element.querySelector<HTMLElement>("[data-index]");
|
||||
const treeContainer = firstVirtualRow?.parentElement;
|
||||
const rows = Array.from(element.querySelectorAll<HTMLElement>("[data-index]"))
|
||||
.map((row) => {
|
||||
const rect = row.getBoundingClientRect();
|
||||
return {
|
||||
bottom: rect.bottom,
|
||||
path:
|
||||
row.querySelector<HTMLElement>('[data-testid="file-tree-node"]')?.dataset.path ?? null,
|
||||
top: rect.top,
|
||||
};
|
||||
})
|
||||
.filter((row) => row.bottom > viewportRect.top && row.top < viewportRect.bottom)
|
||||
.sort((left, right) => left.top - right.top);
|
||||
return {
|
||||
bottom: viewportRect.bottom,
|
||||
clientHeight: element.clientHeight,
|
||||
rows,
|
||||
scrollTop: element.scrollTop,
|
||||
top: viewportRect.top,
|
||||
treeHeight: treeContainer?.getBoundingClientRect().height ?? 0,
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
/** Return the ordered rows currently intersecting the file-tree viewport. */
|
||||
export async function visibleFileTreePaths(viewport: Locator): Promise<string[]> {
|
||||
const geometry = await readFileTreeViewportGeometry(viewport);
|
||||
return geometry.rows.flatMap((row) => (row.path === null ? [] : [row.path]));
|
||||
}
|
||||
|
||||
/** Assert that mounted rows cover the visible viewport without blank intervals. */
|
||||
export async function expectContiguousVisibleFileTreeRows(
|
||||
viewport: Locator,
|
||||
options: FileTreeGeometryOptions = {},
|
||||
): Promise<void> {
|
||||
const geometry = await readFileTreeViewportGeometry(viewport);
|
||||
const issues = fileTreeGeometryIssues(geometry, options);
|
||||
expect(issues, issues.join("; ")).toEqual([]);
|
||||
}
|
||||
|
||||
/** Compare an expected ordered visible window after a panel transition. */
|
||||
export async function expectVisibleFileTreePaths(
|
||||
viewport: Locator,
|
||||
expectedPaths: readonly string[],
|
||||
): Promise<void> {
|
||||
expect(await visibleFileTreePaths(viewport)).toEqual(expectedPaths);
|
||||
}
|
||||
|
||||
/** Reveal the final seeded row even when later tests have added rows after the fixture folder. */
|
||||
export async function scrollToLastLargeFile(lastFile: Locator, viewport: Locator): Promise<void> {
|
||||
await viewport.evaluate((element) => {
|
||||
|
||||
@@ -2,9 +2,13 @@ import { test, expect } from "../../fixtures/test-base";
|
||||
import {
|
||||
LARGE_FILE_TREE_FOLDER,
|
||||
LARGE_FILE_TREE_COUNT,
|
||||
expectContiguousVisibleFileTreeRows,
|
||||
expectVisibleFileTreePaths,
|
||||
largeFileTreePath,
|
||||
scrollToLastLargeFile,
|
||||
setupLargeFileTreeTask,
|
||||
visibleFileTreePaths,
|
||||
waitForFileTreeLayoutSettle,
|
||||
} from "./large-file-tree-virtualization-helpers";
|
||||
|
||||
test.describe("Large file tree virtualization", () => {
|
||||
@@ -82,4 +86,66 @@ test.describe("Large file tree virtualization", () => {
|
||||
await expect(input).toBeVisible({ timeout: 15_000 });
|
||||
await expect(input).toBeFocused({ timeout: 5_000 });
|
||||
});
|
||||
|
||||
test("restores contiguous rows after Files is hidden and reopened", async ({
|
||||
apiClient,
|
||||
seedData,
|
||||
backend,
|
||||
testPage,
|
||||
}) => {
|
||||
test.setTimeout(120_000);
|
||||
const session = await setupLargeFileTreeTask({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
backend,
|
||||
title: "Large file tree restoration",
|
||||
});
|
||||
|
||||
await session.clickTab("Files");
|
||||
const folder = session.fileTreeNode(LARGE_FILE_TREE_FOLDER);
|
||||
const viewport = session.fileTreeScrollViewport();
|
||||
await expect(folder).toBeVisible({ timeout: 15_000 });
|
||||
await expect(viewport).toBeVisible({ timeout: 15_000 });
|
||||
await waitForFileTreeLayoutSettle(testPage);
|
||||
await expectContiguousVisibleFileTreeRows(viewport);
|
||||
const collapsedPaths = await visibleFileTreePaths(viewport);
|
||||
|
||||
await session.clickTab("Changes");
|
||||
await expect(session.files).toBeHidden();
|
||||
await waitForFileTreeLayoutSettle(testPage);
|
||||
await session.clickTab("Files");
|
||||
await expect(viewport).toBeVisible({ timeout: 15_000 });
|
||||
await expect(folder).toBeVisible({ timeout: 15_000 });
|
||||
await expectContiguousVisibleFileTreeRows(viewport);
|
||||
await expectVisibleFileTreePaths(viewport, collapsedPaths);
|
||||
|
||||
await folder.click();
|
||||
const firstFile = largeFileTreePath(0);
|
||||
await expect(session.fileTreeNode(firstFile)).toBeVisible({ timeout: 15_000 });
|
||||
await expectContiguousVisibleFileTreeRows(viewport);
|
||||
const expandedTopPaths = await visibleFileTreePaths(viewport);
|
||||
|
||||
await session.clickTab("Changes");
|
||||
await expect(session.files).toBeHidden();
|
||||
await waitForFileTreeLayoutSettle(testPage);
|
||||
await session.clickTab("Files");
|
||||
await expect(session.fileTreeNode(firstFile)).toBeVisible({ timeout: 15_000 });
|
||||
await expectContiguousVisibleFileTreeRows(viewport);
|
||||
await expectVisibleFileTreePaths(viewport, expandedTopPaths);
|
||||
|
||||
const lastFile = largeFileTreePath(LARGE_FILE_TREE_COUNT - 1);
|
||||
await scrollToLastLargeFile(session.fileTreeNode(lastFile), viewport);
|
||||
await expect(session.fileTreeNode(lastFile)).toBeVisible({ timeout: 15_000 });
|
||||
await expectContiguousVisibleFileTreeRows(viewport);
|
||||
const scrolledPaths = await visibleFileTreePaths(viewport);
|
||||
|
||||
await session.clickTab("Changes");
|
||||
await expect(session.files).toBeHidden();
|
||||
await waitForFileTreeLayoutSettle(testPage);
|
||||
await session.clickTab("Files");
|
||||
await expect(session.fileTreeNode(lastFile)).toBeVisible({ timeout: 15_000 });
|
||||
await expectContiguousVisibleFileTreeRows(viewport);
|
||||
await expectVisibleFileTreePaths(viewport, scrolledPaths);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,10 +1,15 @@
|
||||
import { test, expect } from "../../fixtures/test-base";
|
||||
import { assertNoDocumentHorizontalOverflow } from "../../helpers/layout-assertions";
|
||||
import {
|
||||
LARGE_FILE_TREE_COUNT,
|
||||
LARGE_FILE_TREE_FOLDER,
|
||||
expectContiguousVisibleFileTreeRows,
|
||||
expectVisibleFileTreePaths,
|
||||
largeFileTreePath,
|
||||
scrollToLastLargeFile,
|
||||
setupLargeFileTreeTask,
|
||||
visibleFileTreePaths,
|
||||
waitForFileTreeLayoutSettle,
|
||||
} from "./large-file-tree-virtualization-helpers";
|
||||
|
||||
test.describe("Mobile large file tree virtualization", () => {
|
||||
@@ -25,9 +30,23 @@ test.describe("Mobile large file tree virtualization", () => {
|
||||
|
||||
await testPage.getByRole("button", { name: "Files" }).tap();
|
||||
const folder = session.fileTreeNode(LARGE_FILE_TREE_FOLDER);
|
||||
const viewport = session.fileTreeScrollViewport();
|
||||
await expect(folder).toBeVisible({ timeout: 15_000 });
|
||||
await expect(viewport).toBeVisible({ timeout: 15_000 });
|
||||
await waitForFileTreeLayoutSettle(testPage);
|
||||
await expectContiguousVisibleFileTreeRows(viewport);
|
||||
const collapsedPaths = await visibleFileTreePaths(viewport);
|
||||
|
||||
await testPage.getByRole("button", { name: "Chat" }).tap();
|
||||
await expect(viewport).toBeHidden();
|
||||
await testPage.getByRole("button", { name: "Files" }).tap();
|
||||
await expect(folder).toBeVisible({ timeout: 15_000 });
|
||||
await expectContiguousVisibleFileTreeRows(viewport);
|
||||
await expectVisibleFileTreePaths(viewport, collapsedPaths);
|
||||
|
||||
await folder.tap();
|
||||
await expect(session.fileTreeNode(largeFileTreePath(0))).toBeVisible({ timeout: 15_000 });
|
||||
await expectContiguousVisibleFileTreeRows(viewport);
|
||||
await expect
|
||||
.poll(() => session.visibleFileTreeNodes().count(), { timeout: 5_000 })
|
||||
.toBeLessThan(80);
|
||||
@@ -39,8 +58,6 @@ test.describe("Mobile large file tree virtualization", () => {
|
||||
expect(firstActionsBox!.width).toBeGreaterThanOrEqual(44);
|
||||
expect(firstActionsBox!.height).toBeGreaterThanOrEqual(44);
|
||||
|
||||
const viewport = session.fileTreeScrollViewport();
|
||||
await expect(viewport).toBeVisible();
|
||||
await scrollToLastLargeFile(
|
||||
session.fileTreeNode(largeFileTreePath(LARGE_FILE_TREE_COUNT - 1)),
|
||||
viewport,
|
||||
@@ -54,6 +71,8 @@ test.describe("Mobile large file tree virtualization", () => {
|
||||
expect(lastActionsBox).not.toBeNull();
|
||||
expect(lastActionsBox!.width).toBeGreaterThanOrEqual(44);
|
||||
expect(lastActionsBox!.height).toBeGreaterThanOrEqual(44);
|
||||
await expectContiguousVisibleFileTreeRows(viewport);
|
||||
await assertNoDocumentHorizontalOverflow(testPage, "mobile large file tree");
|
||||
|
||||
await session.fileTreeNode(lastFile).tap();
|
||||
const viewer = testPage.getByTestId("mobile-file-viewer-panel");
|
||||
|
||||
@@ -0,0 +1,198 @@
|
||||
---
|
||||
created: 2026-09-16
|
||||
status: implemented
|
||||
requirements:
|
||||
- REQ-UI-FILE-TREE-CHAT-CONTEXT-001
|
||||
system_design:
|
||||
- ../../specs/ui/system-design/task-surface-render-isolation.md
|
||||
legacy_specs: []
|
||||
---
|
||||
|
||||
# Implementation Plan: File Tree Hidden Measurements
|
||||
|
||||
## Overview
|
||||
|
||||
Keep visible file-tree rows contiguous after the Files panel hides and reopens.
|
||||
One work order adds a measurement guard and regression coverage.
|
||||
The implementation is complete.
|
||||
|
||||
## Ownership and assumptions
|
||||
|
||||
UI owns row geometry and virtualization. Filesystem data and task lifecycle retain their existing owners.
|
||||
The user supplied the intended complete tree and requested reproduction and repair.
|
||||
The repair preserves file operations, row order, scroll ownership, and mobile navigation.
|
||||
No material product choice remains unresolved.
|
||||
|
||||
## Confirmed cause and reproduction
|
||||
|
||||
`VirtualizedFileTreeView` in `apps/web/components/task/file-browser-parts.tsx`
|
||||
uses the default TanStack row measurement through `ref={virtualizer.measureElement}`.
|
||||
That measurement accepts zero-height `ResizeObserver` entries from hidden rows.
|
||||
The entries replace valid cached heights and disrupt the virtual range on restoration.
|
||||
Desktop Dockview uses `defaultRenderer="always"`, so inactive panels can retain mounted content.
|
||||
|
||||
A temporary browser fixture used the actual `FileTreeView`, shared `ScrollArea`,
|
||||
row components, providers, and application CSS. Its viewport height was 560px.
|
||||
The fixture supplied 18 directory rows and a button that toggled the ancestor's
|
||||
`display` between `flex` and `none`. Two animation frames let each visibility
|
||||
transition reach the browser observers. An initial owner update connected the
|
||||
fixture's viewport ref, equivalent to the real tree's asynchronous data arrival.
|
||||
|
||||
| Scenario | First mounted index after restoration | Blank space at viewport top | Mounted rows |
|
||||
| --- | --- | --- | --- |
|
||||
| Original, compact rows | 12 | 234px | 6 |
|
||||
| Temporary guard, compact rows, three cycles | 0 | 0px | 18 |
|
||||
| Original, 44px touch rows at 393px browser width | 12 | 296px | 6 |
|
||||
| Temporary guard, 44px touch rows at 393px browser width | 0 | 0px | 18 |
|
||||
|
||||
Desktop `scrollTop` remained zero. The broken state persisted after layout settled.
|
||||
The touch reproduction changed `scrollTop` to 100px. The temporary guard retained zero.
|
||||
The touch experiment exercised row sizing in the same fixture, not the complete mobile task layout.
|
||||
|
||||
The experiment changed only a temporary copy of the renderer. It kept a cached
|
||||
row height, or its estimate, when default measurement returned zero.
|
||||
This proves a matching failure path, while the permanent implementation and
|
||||
integration coverage below verify the complete interaction.
|
||||
|
||||
## Scope
|
||||
|
||||
### In scope
|
||||
|
||||
- Guard unavailable row measurements in the file-tree virtualizer.
|
||||
- Preserve positive dynamic measurements for normal rows and creation controls.
|
||||
- Cover hidden-panel restoration and existing large-tree navigation on desktop and phone.
|
||||
|
||||
### Out of scope
|
||||
|
||||
- Dependency upgrades, changes to shared ScrollArea, or other virtualized lists.
|
||||
- File loading, backend APIs, persisted layout, selection, and file operations.
|
||||
- A new mobile composition or public documentation.
|
||||
|
||||
## Technical approach
|
||||
|
||||
Add `apps/web/components/task/file-tree-measurement.ts` for the typed measurement
|
||||
callback. Delegate actual measurement to the installed TanStack function.
|
||||
Accept positive measured heights. Otherwise retain the positive cached value
|
||||
for `instance.options.getItemKey(index)`, or use `estimateSize(index)`.
|
||||
Wire this callback into `VirtualizedFileTreeView` without changing its overscan,
|
||||
row keys, viewport, reveal effects, or mounted-row bound.
|
||||
|
||||
Keep dynamic measurement. Fixed heights cannot cover every inline control or
|
||||
responsive transition. Increasing overscan can conceal the defect without
|
||||
protecting the measurement cache.
|
||||
|
||||
## ASCII UI preview
|
||||
|
||||
UI-01: Files panel after restoration, with the scroll position at the top.
|
||||
|
||||
```text
|
||||
Observed desktop Corrected desktop Corrected phone Files
|
||||
+-------------------+ +-------------------+ +----------------------+
|
||||
| Files toolbar | | Files toolbar | | Files toolbar |
|
||||
| | | > folder-00 | | > folder-00 [...]|
|
||||
| blank gap | | > folder-01 | | > folder-01 [...]|
|
||||
| | | > folder-02 | | > folder-02 [...]|
|
||||
| > folder-12 | | ... | | ... |
|
||||
| > folder-13 | | > folder-17 | +----------------------+
|
||||
+-------------------+ +-------------------+ | Existing bottom nav |
|
||||
+----------------------+
|
||||
```
|
||||
|
||||
The toolbar and phone navigation retain their current positions.
|
||||
The existing Files viewport is the only vertical scroll owner.
|
||||
All rows intersecting that viewport must appear without gaps or overlaps
|
||||
(`AC-UI-FILE-TREE-CHAT-CONTEXT-001.10`). Names and spacing in this sketch are illustrative.
|
||||
|
||||
The mobile exemplar is `components/task/mobile/session-mobile-layout.tsx`.
|
||||
Users enter through the Files bottom-navigation action and tap a row to open it.
|
||||
The existing full-height surface supports repeated file exploration.
|
||||
It retains touch actions, safe-area clearance, and the current shared tree model.
|
||||
Mobile panel switching currently unmounts Files, so tests cover that real path
|
||||
in addition to hidden-measurement component coverage with touch-sized rows.
|
||||
|
||||
## Tests
|
||||
|
||||
`components/task/file-tree-measurement.test.ts` covers the measurement contract:
|
||||
|
||||
- A zero observer entry retains the previous positive height.
|
||||
- A zero first measurement uses the node or creation-row estimate.
|
||||
- A later positive measurement replaces the fallback after restoration or a touch-size change.
|
||||
|
||||
These cases support `.10` while preserving compact spacing `.9` and touch actions `.7`.
|
||||
Use the real TanStack measurement function with controlled element geometry.
|
||||
The zero-height regression must fail before the guard is implemented.
|
||||
|
||||
`components/task/file-tree-geometry.test.ts` covers the shared
|
||||
viewport validator. It rejects a blank top edge, rejects a blank bottom edge when
|
||||
the tree overflows the viewport, and permits legitimate trailing space for a short
|
||||
tree or the file-tree container's end padding.
|
||||
|
||||
## E2E tests
|
||||
|
||||
Extend the existing desktop and mobile virtualization specs, preserving their current scenarios.
|
||||
|
||||
- Desktop: hide Files through an actual Dockview tab transition, then restore it.
|
||||
Cover a short tree at the top and a large tree after scrolling.
|
||||
Assert the first visible row reaches the viewport content top, the last visible
|
||||
row reaches the bottom when overflowing content continues below, and adjacent
|
||||
rows have no gaps or overlaps. The short-tree transition compares its ordered
|
||||
visible path window before and after restoration.
|
||||
- Phone: enter Files, navigate away and return, then scroll and open a file.
|
||||
Reuse the same top, bottom, and adjacent-row validator, compare the ordered
|
||||
short-tree path window after Files to Chat to Files navigation, and assert
|
||||
reachable touch actions and no horizontal document overflow.
|
||||
- Preserve the fewer-than-80 mounted-row bound for the 600-entry fixture,
|
||||
last-file access, and create-input reveal.
|
||||
|
||||
The implementation work order contains the exact build and test commands.
|
||||
|
||||
## Work orders
|
||||
|
||||
- [x] [Task 01: Preserve valid file-tree measurements](task-01-preserve-measurements.md)
|
||||
|
||||
This work order is sequential and has no dependencies.
|
||||
|
||||
## Verification results
|
||||
|
||||
Temporary browser reproduction and guard comparison passed as recorded above.
|
||||
The production measurement callback now retains a positive row-keyed cache entry,
|
||||
or the current row estimate, when TanStack receives a zero-height observer entry.
|
||||
Positive measurements remain authoritative for compact rows, touch rows, and
|
||||
inline creation controls.
|
||||
|
||||
- Focused Vitest: 4 files and 14 tests passed, including four shared viewport
|
||||
geometry cases for blank top, overflowing blank bottom, short-tree trailing
|
||||
space, and end padding.
|
||||
- Frontend typecheck passed.
|
||||
- Targeted ESLint passed for the measurement implementation, renderer, and E2E helpers/specs.
|
||||
- Production-served desktop E2E: 3 tests passed, including collapsed, expanded-top,
|
||||
and scrolled Files restoration through Dockview. The restoration assertions
|
||||
compare ordered visible path windows at the same scroll offsets.
|
||||
- Production-served mobile E2E: 3 tests passed, including the large-tree touch flow
|
||||
and the two existing file-tree chat-context scenarios. The large-tree flow uses
|
||||
the shared viewport validator and compares the collapsed path window after
|
||||
Files navigation.
|
||||
- E2E sleep lint passed.
|
||||
- `python3 scripts/list-docs.py validate`: passed (281 decisions, 973 specifications).
|
||||
- `python3 scripts/lint-spec-files.py --all`: passed.
|
||||
- `git diff --check`: passed.
|
||||
|
||||
The temporary browser fixture, copied renderer, and browser configuration were
|
||||
removed after the experiment. The named browser and isolated app were stopped.
|
||||
Public documentation does not change.
|
||||
|
||||
## Risks
|
||||
|
||||
- Disabling measurement entirely would break variable-height controls.
|
||||
- A fallback keyed by index can reuse another row's size after a tree change.
|
||||
- Shared viewport checks must track intentional content insets and end padding if
|
||||
the Files layout changes.
|
||||
- The isolated fixture remains diagnostic evidence; the production E2E scenarios
|
||||
cover the Dockview and mobile navigation lifecycles.
|
||||
|
||||
## Related implementation records
|
||||
|
||||
- [Original virtualization work order](../task-surface-render-isolation/task-02-virtualize-file-tree.md).
|
||||
- [Responsive row spacing](../file-tree-responsive-spacing/plan.md).
|
||||
|
||||
Their historical results remain unchanged. This package owns the new visibility regression.
|
||||
@@ -0,0 +1,161 @@
|
||||
---
|
||||
id: "01-preserve-measurements"
|
||||
title: "Preserve valid file-tree measurements"
|
||||
status: done
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-UI-FILE-TREE-CHAT-CONTEXT-001
|
||||
acceptance_criteria:
|
||||
- AC-UI-FILE-TREE-CHAT-CONTEXT-001.7
|
||||
- AC-UI-FILE-TREE-CHAT-CONTEXT-001.9
|
||||
- AC-UI-FILE-TREE-CHAT-CONTEXT-001.10
|
||||
system_design:
|
||||
- ../../specs/ui/system-design/task-surface-render-isolation.md
|
||||
---
|
||||
|
||||
# Task 01: Preserve Valid File Tree Measurements
|
||||
|
||||
## Summary
|
||||
|
||||
Reject zero-height measurements from hidden file-tree rows.
|
||||
Retain valid cached geometry until the browser can measure each row again.
|
||||
The [plan](plan.md#confirmed-cause-and-reproduction) records the matching browser reproduction.
|
||||
|
||||
## In scope
|
||||
|
||||
- A typed file-tree measurement callback and focused regression tests.
|
||||
- Wiring the callback into `VirtualizedFileTreeView`.
|
||||
- Desktop panel-restoration and phone Files-navigation E2E coverage.
|
||||
|
||||
## Out of scope
|
||||
|
||||
Dependency upgrades, global observer changes, panel remount policies, backend
|
||||
changes, public copy, and persisted state.
|
||||
|
||||
## Acceptance
|
||||
|
||||
1. Unavailable measurements use a positive cache entry for the current row key,
|
||||
or its current estimate. Positive measurements remain authoritative.
|
||||
2. Files restoration shows every expected viewport row without gaps or overlaps,
|
||||
including compact and touch-sized rows (`.9`, `.10`).
|
||||
3. Large trees retain bounded mounting, last-file access, create reveal, and
|
||||
reachable 44px touch actions (`.7`).
|
||||
|
||||
## ASCII UI preview
|
||||
|
||||
UI-01: Files restored at the top. Full comparison: [plan preview](plan.md#ascii-ui-preview).
|
||||
|
||||
```text
|
||||
Desktop Files Phone Files
|
||||
+-------------------+ +----------------------+
|
||||
| Files toolbar | | Files toolbar |
|
||||
| > folder-00 | | > folder-00 [...]|
|
||||
| > folder-01 | | > folder-01 [...]|
|
||||
| > folder-02 | | > folder-02 [...]|
|
||||
| ... | | ... |
|
||||
+-------------------+ +----------------------+
|
||||
| Existing bottom nav |
|
||||
+----------------------+
|
||||
```
|
||||
|
||||
The existing viewport owns scrolling. Rows remain contiguous after restoration.
|
||||
Phone touch actions retain their current geometry. This implements `.7`, `.9`, and `.10`.
|
||||
|
||||
## Implementation sequence
|
||||
|
||||
1. Mark this work order `in_progress`.
|
||||
2. Add `file-tree-measurement.test.ts` with the zero-height regression.
|
||||
3. Run it against a callback that delegates to the current default measurement.
|
||||
4. Record the behavioral failure: expected cached height or estimate, received zero.
|
||||
5. Implement the fallback and wire the callback into the real virtualizer.
|
||||
6. Extend both existing virtualization specs with the plan's transition scenarios.
|
||||
7. Add shared viewport geometry assertions and focused cases for blank top, overflowing
|
||||
blank bottom, legitimate short-tree trailing space, and end padding.
|
||||
8. Run every verification command and record the results.
|
||||
9. Compare the rendered desktop and phone geometry with UI-01.
|
||||
10. Mark this work order `done` and synchronize the plan.
|
||||
|
||||
## Verification
|
||||
|
||||
Run from the repository root. If dependencies are absent, first run
|
||||
`(cd apps && pnpm install --frozen-lockfile)`.
|
||||
|
||||
```bash
|
||||
(cd apps/web && pnpm exec vitest run components/task/file-tree-measurement.test.ts components/task/file-browser-render-identity.test.tsx components/task/file-browser-responsive.test.tsx components/task/file-tree-geometry.test.ts)
|
||||
(cd apps/web && pnpm run typecheck)
|
||||
(cd apps/web && pnpm exec eslint components/task/file-tree-measurement.ts components/task/file-tree-measurement.test.ts components/task/file-tree-geometry.test.ts components/task/file-browser-parts.tsx e2e/tests/task/file-tree-geometry.ts e2e/tests/task/large-file-tree-virtualization-helpers.ts e2e/tests/task/large-file-tree-virtualization.spec.ts e2e/tests/task/mobile-large-file-tree-virtualization.spec.ts)
|
||||
(cd apps/web && pnpm e2e:run --project chromium tests/task/large-file-tree-virtualization.spec.ts)
|
||||
(cd apps/web && pnpm e2e:run --project mobile-chrome tests/task/mobile-large-file-tree-virtualization.spec.ts tests/task/mobile-file-tree-chat-context.spec.ts)
|
||||
(cd apps/web && pnpm run lint:e2e-sleeps -- e2e/tests/task/large-file-tree-virtualization.spec.ts e2e/tests/task/mobile-large-file-tree-virtualization.spec.ts)
|
||||
python3 scripts/list-docs.py validate
|
||||
python3 scripts/lint-spec-files.py --all
|
||||
git diff --check
|
||||
```
|
||||
|
||||
The managed E2E commands build the current production sources.
|
||||
Run desktop and mobile commands sequentially.
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/web/components/task/file-tree-measurement.ts` (new).
|
||||
- `apps/web/components/task/file-tree-measurement.test.ts` (new).
|
||||
- `apps/web/components/task/file-browser-parts.tsx`.
|
||||
- `apps/web/e2e/tests/task/file-tree-geometry.ts` (new).
|
||||
- `apps/web/components/task/file-tree-geometry.test.ts` (new).
|
||||
- `apps/web/e2e/tests/task/large-file-tree-virtualization.spec.ts`.
|
||||
- `apps/web/e2e/tests/task/mobile-large-file-tree-virtualization.spec.ts`.
|
||||
- `apps/web/e2e/tests/task/large-file-tree-virtualization-helpers.ts`.
|
||||
|
||||
## Dependencies
|
||||
|
||||
None.
|
||||
|
||||
## Risks
|
||||
|
||||
Cache identity must follow the row key. Accept later positive measurements so
|
||||
responsive sizes and inline controls can change height.
|
||||
Actual mobile Files navigation unmounts the tree. It does not reproduce desktop
|
||||
Dockview lifetime, so retain the controlled hidden-measurement regression too.
|
||||
|
||||
## Parallelism
|
||||
|
||||
`sequential`
|
||||
|
||||
## Inputs
|
||||
|
||||
- [Requirements](../../specs/ui/requirements/file-tree-chat-context.md), criteria `.7`, `.9`, and `.10`.
|
||||
- [System design](../../specs/ui/system-design/task-surface-render-isolation.md#file-tree-virtualization).
|
||||
- Existing desktop and mobile virtualization specs and their shared fixture.
|
||||
- `file-browser-render-identity.test.tsx` for nearby component-test conventions.
|
||||
|
||||
## Results
|
||||
|
||||
- RED: the focused measurement test against TanStack's default callback returned
|
||||
`0` for a hidden row where the positive cached height was `32`.
|
||||
- GREEN: `file-tree-measurement.ts` delegates positive measurements to TanStack
|
||||
and otherwise retains the current row-keyed positive cache entry or estimate.
|
||||
`VirtualizedFileTreeView` uses the callback without changing its scroll owner,
|
||||
row keys, estimates, overscan, or reveal effects.
|
||||
- Review RED: the original intersection-filtered viewport-edge checks accepted
|
||||
synthetic blank top and bottom gaps because every retained row satisfied the
|
||||
later edge comparisons.
|
||||
- Review GREEN: the shared pure viewport validator checks the first row's top,
|
||||
the last row's bottom for overflowing trees, and adjacent row bounds. It skips
|
||||
the bottom fill requirement for short trees and allows the container's end
|
||||
padding. Four focused geometry cases passed.
|
||||
- Focused Vitest passed 4 files and 14 tests. It covers cached zero measurements,
|
||||
first-measurement estimates, later positive measurements, and the shared
|
||||
blank-edge/short-tree geometry contract.
|
||||
- Frontend typecheck and targeted ESLint passed.
|
||||
- Desktop production-served E2E passed 3 tests. The new scenario restores the
|
||||
collapsed tree, expanded tree at the top, and expanded tree after scrolling
|
||||
through actual Dockview tab transitions. It compares ordered visible path
|
||||
windows before and after restoration and checks visible row bounds.
|
||||
- Mobile production-served E2E passed 3 tests. The large-tree scenario checks
|
||||
Files to Chat to Files navigation, the shared contiguous-row validator, the
|
||||
ordered collapsed path window, 44px actions, last-file access, and document
|
||||
overflow, alongside the existing chat-context tests.
|
||||
- E2E sleep lint, specification validation, specification lint, and `git diff --check`
|
||||
all passed.
|
||||
@@ -28,6 +28,7 @@ Users inspecting a task's Files tab must currently return to the chat composer a
|
||||
- **AC-UI-FILE-TREE-CHAT-CONTEXT-001.7:** On phone, tablet, and coarse-pointer desktop-workbench compositions, each eligible file-tree row exposes a visible, accessible action trigger. The trigger opens the existing responsive menu treatment with a touch target at least 44px high; long press or right-click is not required.
|
||||
- **AC-UI-FILE-TREE-CHAT-CONTEXT-001.8:** All new visible labels and feedback use the existing localization catalogs.
|
||||
- **AC-UI-FILE-TREE-CHAT-CONTEXT-001.9:** File-tree rows shall retain compact single-line spacing in every responsive composition. A visible coarse-pointer action, including in the desktop workbench, shall not cause wrapping; long names shall truncate instead.
|
||||
- **AC-UI-FILE-TREE-CHAT-CONTEXT-001.10:** After the Files panel reopens or changes size, every row within its scroll viewport shall remain visible and ordered without blank gaps. Recovery shall not require a manual scroll, file operation, or page reload. This applies to desktop and touch layouts.
|
||||
|
||||
## Migrated source detail
|
||||
|
||||
|
||||
@@ -76,6 +76,16 @@ Rows use measured sizes because editing controls and touch layouts can change ro
|
||||
virtualizer uses an estimated size before measurement. It updates positions after a measured size
|
||||
changes.
|
||||
|
||||
Only positive row measurements can replace cached sizes. A hidden panel can
|
||||
produce zero-height observer entries even though its rows remain connected.
|
||||
Those entries represent unavailable geometry, not a row with no content.
|
||||
The measurement callback retains the last positive size for the row key, or
|
||||
uses the current estimate when no positive measurement exists.
|
||||
When the panel becomes visible, normal measurement accepts its actual row sizes.
|
||||
This applies to node rows and inline creation controls in both pointer modes.
|
||||
It implements `AC-UI-FILE-TREE-CHAT-CONTEXT-001.10` without another scroll owner
|
||||
or changes to Dockview panel lifetime.
|
||||
|
||||
Active-file reveal and restored scroll operations resolve a row index from `visibleRows`. They use
|
||||
the virtualizer scroll API before they query a row element. This sequence makes an offscreen row
|
||||
available before focus or selection work uses its element.
|
||||
@@ -148,6 +158,11 @@ task data as the current complete list.
|
||||
Unit tests record render counts for controlled updates. Browser E2E tests record the number of
|
||||
mounted rows for a large generated file tree.
|
||||
|
||||
Measurement tests cover positive sizes, unavailable sizes with a cached value,
|
||||
and unavailable sizes before the first measurement. Browser regressions hide
|
||||
and restore the Files panel, then compare visible row bounds with the viewport.
|
||||
Coverage includes compact desktop rows, touch rows, and scrolled large trees.
|
||||
|
||||
Final performance evaluation uses a warmed production build without React DevTools or CPU profiler
|
||||
startup in the measured window. The report includes main-thread work, long tasks, and mounted-row
|
||||
counts. Development traces remain diagnostic evidence only.
|
||||
|
||||
Reference in New Issue
Block a user