Skip to content

Commit f72d1b9

Browse files
authored
Clarify file section layout helper names (#148)
1 parent 7f7a4f0 commit f72d1b9

2 files changed

Lines changed: 42 additions & 30 deletions

File tree

src/ui/components/panes/DiffPane.tsx

Lines changed: 31 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@ import {
1717
type DiffSectionRowMetric,
1818
} from "../../lib/sectionHeights";
1919
import {
20-
buildSectionLayoutMetrics,
21-
findHeaderOwningSection,
22-
resolveFileHeaderTop,
23-
} from "../../lib/sectionLayout";
20+
buildFileSectionLayoutMetrics,
21+
findHeaderOwningFileSection,
22+
resolveFileSectionHeaderTop,
23+
} from "../../lib/fileSectionLayout";
2424
import { diffHunkId, diffSectionId } from "../../lib/ids";
2525
import type { AppTheme } from "../../themes";
2626
import { DiffSection } from "./DiffSection";
@@ -64,13 +64,13 @@ function findViewportRowAnchor(
6464
sectionMetrics: DiffSectionMetrics[],
6565
scrollTop: number,
6666
) {
67-
const sectionLayoutMetrics = buildSectionLayoutMetrics(
67+
const fileSectionLayoutMetrics = buildFileSectionLayoutMetrics(
6868
files,
6969
sectionMetrics.map((metrics) => metrics?.bodyHeight ?? 0),
7070
);
7171

7272
for (let index = 0; index < files.length; index += 1) {
73-
const layoutMetric = sectionLayoutMetrics[index];
73+
const layoutMetric = fileSectionLayoutMetrics[index];
7474
const bodyTop = layoutMetric?.bodyTop ?? 0;
7575
const metrics = sectionMetrics[index];
7676
const bodyHeight = metrics?.bodyHeight ?? 0;
@@ -97,13 +97,13 @@ function resolveViewportRowAnchorTop(
9797
sectionMetrics: DiffSectionMetrics[],
9898
anchor: ViewportRowAnchor,
9999
) {
100-
const sectionLayoutMetrics = buildSectionLayoutMetrics(
100+
const fileSectionLayoutMetrics = buildFileSectionLayoutMetrics(
101101
files,
102102
sectionMetrics.map((metrics) => metrics?.bodyHeight ?? 0),
103103
);
104104

105105
for (let index = 0; index < files.length; index += 1) {
106-
const layoutMetric = sectionLayoutMetrics[index];
106+
const layoutMetric = fileSectionLayoutMetrics[index];
107107
const bodyTop = layoutMetric?.bodyTop ?? 0;
108108
const file = files[index];
109109
const metrics = sectionMetrics[index];
@@ -287,8 +287,8 @@ export function DiffPane({
287287
() => baseSectionMetrics.map((metrics) => metrics.bodyHeight),
288288
[baseSectionMetrics],
289289
);
290-
const baseSectionLayoutMetrics = useMemo(
291-
() => buildSectionLayoutMetrics(files, baseEstimatedBodyHeights),
290+
const baseFileSectionLayoutMetrics = useMemo(
291+
() => buildFileSectionLayoutMetrics(files, baseEstimatedBodyHeights),
292292
[baseEstimatedBodyHeights, files],
293293
);
294294

@@ -297,11 +297,11 @@ export function DiffPane({
297297
const minVisibleY = Math.max(0, scrollViewport.top - overscanRows);
298298
const maxVisibleY = scrollViewport.top + scrollViewport.height + overscanRows;
299299
return new Set(
300-
baseSectionLayoutMetrics
300+
baseFileSectionLayoutMetrics
301301
.filter((metric) => metric.sectionBottom >= minVisibleY && metric.sectionTop <= maxVisibleY)
302302
.map((metric) => metric.fileId),
303303
);
304-
}, [baseSectionLayoutMetrics, scrollViewport.height, scrollViewport.top]);
304+
}, [baseFileSectionLayoutMetrics, scrollViewport.height, scrollViewport.top]);
305305

306306
const visibleAgentNotesByFile = useMemo(() => {
307307
const next = new Map<string, VisibleAgentNote[]>();
@@ -362,29 +362,29 @@ export function DiffPane({
362362
() => sectionMetrics.map((metrics) => metrics.bodyHeight),
363363
[sectionMetrics],
364364
);
365-
const sectionLayoutMetrics = useMemo(
366-
() => buildSectionLayoutMetrics(files, estimatedBodyHeights),
365+
const fileSectionLayoutMetrics = useMemo(
366+
() => buildFileSectionLayoutMetrics(files, estimatedBodyHeights),
367367
[estimatedBodyHeights, files],
368368
);
369369
// Read the live scroll box position during render so sticky-header ownership flips
370370
// immediately after imperative scrolls instead of waiting for the polled viewport snapshot.
371371
const effectiveScrollTop = scrollRef.current?.scrollTop ?? scrollViewport.top;
372372

373373
const totalContentHeight =
374-
sectionLayoutMetrics[sectionLayoutMetrics.length - 1]?.sectionBottom ?? 0;
374+
fileSectionLayoutMetrics[fileSectionLayoutMetrics.length - 1]?.sectionBottom ?? 0;
375375

376376
const stickyHeaderFile = useMemo(() => {
377377
if (files.length < 2) {
378378
return null;
379379
}
380380

381-
const owner = findHeaderOwningSection(sectionLayoutMetrics, effectiveScrollTop);
381+
const owner = findHeaderOwningFileSection(fileSectionLayoutMetrics, effectiveScrollTop);
382382
if (!owner || effectiveScrollTop <= owner.headerTop) {
383383
return null;
384384
}
385385

386386
return files[owner.sectionIndex] ?? null;
387-
}, [effectiveScrollTop, files, sectionLayoutMetrics]);
387+
}, [effectiveScrollTop, fileSectionLayoutMetrics, files]);
388388

389389
const visibleWindowedFileIds = useMemo(() => {
390390
if (!windowingEnabled) {
@@ -418,8 +418,8 @@ export function DiffPane({
418418
return null;
419419
}
420420

421-
const selectedSectionLayout = sectionLayoutMetrics[selectedFileIndex];
422-
if (!selectedSectionLayout) {
421+
const selectedFileSectionLayout = fileSectionLayoutMetrics[selectedFileIndex];
422+
if (!selectedFileSectionLayout) {
423423
return null;
424424
}
425425

@@ -433,13 +433,19 @@ export function DiffPane({
433433
}
434434

435435
return {
436-
top: selectedSectionLayout.bodyTop + hunkBounds.top,
436+
top: selectedFileSectionLayout.bodyTop + hunkBounds.top,
437437
height: hunkBounds.height,
438438
startRowId: hunkBounds.startRowId,
439439
endRowId: hunkBounds.endRowId,
440-
sectionTop: selectedSectionLayout.sectionTop,
440+
sectionTop: selectedFileSectionLayout.sectionTop,
441441
};
442-
}, [sectionLayoutMetrics, sectionMetrics, selectedFile, selectedFileIndex, selectedHunkIndex]);
442+
}, [
443+
fileSectionLayoutMetrics,
444+
sectionMetrics,
445+
selectedFile,
446+
selectedFileIndex,
447+
selectedHunkIndex,
448+
]);
443449

444450
/** Absolute scroll offset and height of the first inline note in the selected hunk, if any. */
445451
const selectedNoteBounds = useMemo(() => {
@@ -483,7 +489,7 @@ export function DiffPane({
483489
/** Scroll one file header to the top using the latest planned section geometry. */
484490
const scrollFileHeaderToTop = useCallback(
485491
(fileId: string) => {
486-
const headerTop = resolveFileHeaderTop(sectionLayoutMetrics, fileId);
492+
const headerTop = resolveFileSectionHeaderTop(fileSectionLayoutMetrics, fileId);
487493
if (headerTop == null) {
488494
return false;
489495
}
@@ -496,7 +502,7 @@ export function DiffPane({
496502
scrollBox.scrollTo(headerTop);
497503
return true;
498504
},
499-
[scrollRef, sectionLayoutMetrics],
505+
[fileSectionLayoutMetrics, scrollRef],
500506
);
501507

502508
useLayoutEffect(() => {
@@ -585,7 +591,7 @@ export function DiffPane({
585591
if (scrollFileHeaderToTop(pendingFileId)) {
586592
clearPendingFileTopAlign();
587593
}
588-
}, [clearPendingFileTopAlign, files, scrollFileHeaderToTop, sectionLayoutMetrics]);
594+
}, [clearPendingFileTopAlign, fileSectionLayoutMetrics, files, scrollFileHeaderToTop]);
589595

590596
useLayoutEffect(() => {
591597
if (suppressNextSelectionAutoScrollRef.current) {
Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { DiffFile } from "../../core/types";
22

33
/** Stream geometry for one file section in the main review pane. */
4-
export interface SectionLayoutMetric {
4+
export interface FileSectionLayoutMetric {
55
fileId: string;
66
sectionIndex: number;
77
sectionTop: number;
@@ -12,8 +12,8 @@ export interface SectionLayoutMetric {
1212
}
1313

1414
/** Build absolute section offsets from file order and measured body heights. */
15-
export function buildSectionLayoutMetrics(files: DiffFile[], bodyHeights: number[]) {
16-
const metrics: SectionLayoutMetric[] = [];
15+
export function buildFileSectionLayoutMetrics(files: DiffFile[], bodyHeights: number[]) {
16+
const metrics: FileSectionLayoutMetric[] = [];
1717
let cursor = 0;
1818

1919
files.forEach((file, index) => {
@@ -41,7 +41,10 @@ export function buildSectionLayoutMetrics(files: DiffFile[], bodyHeights: number
4141
}
4242

4343
/** Return the file section that owns the viewport top, switching at each next header row. */
44-
export function findHeaderOwningSection(sectionMetrics: SectionLayoutMetric[], scrollTop: number) {
44+
export function findHeaderOwningFileSection(
45+
sectionMetrics: FileSectionLayoutMetric[],
46+
scrollTop: number,
47+
) {
4548
if (sectionMetrics.length === 0) {
4649
return null;
4750
}
@@ -68,7 +71,10 @@ export function findHeaderOwningSection(sectionMetrics: SectionLayoutMetric[], s
6871
}
6972

7073
/** Resolve the scroll target needed to make one file header own the viewport top. */
71-
export function resolveFileHeaderTop(sectionMetrics: SectionLayoutMetric[], fileId: string) {
74+
export function resolveFileSectionHeaderTop(
75+
sectionMetrics: FileSectionLayoutMetric[],
76+
fileId: string,
77+
) {
7278
const targetSection = sectionMetrics.find((metric) => metric.fileId === fileId);
7379
if (!targetSection) {
7480
return null;

0 commit comments

Comments
 (0)