Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions apps/web/src/DiffWorkspace.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ export function DiffWorkspace() {
const { collapsed, setCollapsed } = useDiffCollapse();
const {
files,
activePath,
selectVisibleFile,
filter,
setFilter,
navigationTarget,
Expand Down Expand Up @@ -655,6 +657,27 @@ export function DiffWorkspace() {
containerRef={viewport}
ref={viewer}
items={items}
onScroll={(scrollTop, instance) => {
const height = viewport.current?.clientHeight ?? 0;
// Use Pierre's full item offsets, including unmounted files.
// Keep the top file until its body (not just its header) leaves.
const visible = items.find((item, index) => {
const top = instance.getTopForItem(item.id);
const next = items[index + 1];
const nextTop = next
? instance.getTopForItem(next.id)
: undefined;
const bottom =
nextTop === undefined ? Infinity : nextTop - fileSpacing;
return (
top !== undefined &&
top < scrollTop + height &&
bottom > scrollTop
);
});
if (visible && visible.id !== activePath)
selectVisibleFile(visible.id);
}}
options={options}
style={{ height: "100%", width: "100%", overflow: "auto" }}
renderHeaderPrefix={(item) => (
Expand Down
41 changes: 23 additions & 18 deletions apps/web/src/use-file-navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,34 +72,37 @@ export function useFileNavigation({
const activePath = files.some((file) => file.path === selected)
? selected
: (files[0]?.path ?? "");
const selectVisibleFile = useCallback((path: string) => {
setSelected(path);
const expandParents = (previous: Set<string>) => {
const parents = ancestorPaths(path);
if (!parents.some((parent) => previous.has(parent))) return previous;
const next = new Set(previous);
for (const parent of parents) next.delete(parent);
return next;
};
setClosed(expandParents);
setFilteredClosed(expandParents);
requestAnimationFrame(() => {
document
.querySelector<HTMLElement>(
`#file-tree .file-row[data-path="${CSS.escape(path)}"]`,
)
?.scrollIntoView({ block: "nearest" });
});
}, []);
const revealFile = useCallback(
(path: string) => {
setSelected(path);
const expandParents = (previous: Set<string>) => {
const parents = ancestorPaths(path);
if (!parents.some((parent) => previous.has(parent))) return previous;
const next = new Set(previous);
for (const parent of parents) next.delete(parent);
return next;
};
setClosed(expandParents);
setFilteredClosed(expandParents);
selectVisibleFile(path);
setCollapsed((previous) => {
if (!previous.has(path)) return previous;
const next = new Set(previous);
next.delete(path);
return next;
});
closeMobile();
requestAnimationFrame(() => {
document
.querySelector<HTMLElement>(
`#file-tree .file-row[data-path="${CSS.escape(path)}"]`,
)
?.scrollIntoView({ block: "nearest" });
});
},
[closeMobile, setCollapsed],
[selectVisibleFile, closeMobile, setCollapsed],
);
const selectFile = useCallback(
(path: string) => {
Expand Down Expand Up @@ -256,6 +259,7 @@ export function useFileNavigation({
filteredClosed,
setFilteredClosed,
selectFile,
selectVisibleFile,
search,
viewer,
navigateComment,
Expand All @@ -270,6 +274,7 @@ export function useFileNavigation({
closed,
filteredClosed,
selectFile,
selectVisibleFile,
navigateComment,
],
);
Expand Down
89 changes: 89 additions & 0 deletions apps/web/test/browser/app.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,95 @@ test("file navigation scrolls to and retriggers a destination cue", async ({
await expect(cue).toHaveCount(0);
});

test("scrolling selects the top file only when the previous diff leaves", async ({
page,
}) => {
await page.setViewportSize({ width: 1280, height: 420 });
await page.goto("/");
const first = page.locator('#file-tree [data-path="config/diffx.json"]');
const second = page.locator('#file-tree [data-path="docs/legacy.md"]');
const firstDiff = page
.locator("#viewer diffs-container")
.filter({ hasText: "config/diffx.json" });
await expect(firstDiff.locator("[data-diffs-header]")).toBeVisible();
await first.click();
const boundary = await firstDiff.evaluate((element) => {
const scroller = document.querySelector(".diff-code-view");
if (!scroller) throw new Error("Missing diff scroller");
return (
element.getBoundingClientRect().bottom -
scroller.getBoundingClientRect().top +
scroller.scrollTop
);
});
const scroller = page.locator(".diff-code-view");
await scroller.evaluate((element, top) => {
element.scrollTop = top;
}, boundary - 1);
await expect(first).toHaveAttribute("aria-current", "true");
await expect(second).toHaveAttribute("aria-current", "false");
// The next header is already visible, but the final pixel still belongs to the first file.
await expect(
page
.locator("#viewer diffs-container")
.filter({ hasText: "docs/legacy.md" })
.locator("[data-diffs-header]"),
).toBeVisible();
await scroller.evaluate((element, top) => {
element.scrollTop = top;
}, boundary + 1);
await expect(second).toHaveAttribute("aria-current", "true");
expect(await scroller.evaluate((element) => element.scrollTop)).toBeCloseTo(
boundary + 1,
0,
);
await scroller.evaluate((element, top) => {
element.scrollTop = top;
}, boundary - 1);
await expect(first).toHaveAttribute("aria-current", "true");
await scroller.evaluate((element) => {
element.scrollTop = element.scrollHeight;
});
await expect(first).toHaveAttribute("aria-current", "false");
await scroller.evaluate((element) => {
element.scrollTop = 0;
});
await expect(first).toHaveAttribute("aria-current", "true");
});

test("scroll selection preserves collapsed diffs and works with filtered files", async ({
page,
}) => {
await page.setViewportSize({ width: 1280, height: 420 });
await page.goto("/");
await page.getByRole("searchbox", { name: "Filter files" }).fill("src/");
const badge = page.locator(
'#file-tree [data-path="src/components/Badge.tsx"]',
);
const button = page.locator(
'#file-tree [data-path="src/components/Button.tsx"]',
);
await badge.click();
await page
.getByRole("button", {
name: "Collapse src/components/Badge.tsx",
exact: true,
})
.click();
await button.click();
await expect(button).toHaveAttribute("aria-current", "true");
await page.locator(".diff-code-view").evaluate((element) => {
element.scrollTop = 0;
});
await expect(badge).toHaveAttribute("aria-current", "true");
await expect(
page.getByRole("button", {
name: "Expand src/components/Badge.tsx",
exact: true,
}),
).toBeVisible();
});

test("file frames preserve measured gutters, gaps, and virtual geometry", async ({
page,
}) => {
Expand Down
3 changes: 3 additions & 0 deletions docs/development.md
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,9 @@ and `observation.head`; reading them never resolves current Git refs.
versions, worker options, and measured geometry. `use-review.ts` and
`use-reviewed-files.ts` own their REST requests and recovery; reads cannot settle
across writes or owner disposal. Keep section-only state within its component.
Scrolling the diff selects the top visible file in navigation; selection advances
only after that file’s entire diff leaves the viewport. Scroll-driven selection
reveals its tree row without expanding the diff or navigating again.

See [architecture.md](architecture.md) for repository boundaries and dependency
rules.
Expand Down

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Loading
Loading