fix(app): stabilize diff reviews and workspace tabs
Reuse measured diff geometry when review controls open, keeping the interaction responsive on large changes. Preserve complete tab-track snapshots while exact content widths settle so new tabs and retained workspaces do not flash or truncate.
This commit is contained in:
@@ -272,6 +272,54 @@ describe("diff document model", () => {
|
||||
expect(reviewRow?.kind === "line" ? reviewRow.reviewHeight : 0).toBe(148);
|
||||
});
|
||||
|
||||
it("reflows review geometry without remeasuring unchanged text", () => {
|
||||
const files = [file("a.ts"), file("b.ts")];
|
||||
let measurementCount = 0;
|
||||
const countingMeasurer: TextMeasurer = {
|
||||
measure(text) {
|
||||
measurementCount += 1;
|
||||
return Array.from(text).length * 10;
|
||||
},
|
||||
};
|
||||
const base = buildDiffDocumentModel(input({ files, measureText: countingMeasurer }));
|
||||
const initialMeasurementCount = measurementCount;
|
||||
const reviewCell = base.rows
|
||||
.filter((row) => row.kind === "line")
|
||||
.flatMap((row) => row.cells)
|
||||
.find((cell) => cell?.reviewTarget);
|
||||
if (!reviewCell?.reviewTarget) throw new Error("Expected a reviewable cell");
|
||||
const baseReviewRow = rowForCell(base, reviewCell);
|
||||
const baseSecondFileTop = base.files[1]!.top;
|
||||
|
||||
const withEditor = buildDiffDocumentModel(
|
||||
input({
|
||||
files,
|
||||
measureText: countingMeasurer,
|
||||
reviewActions: reviewActionsWithEditor(reviewCell.reviewTarget),
|
||||
reuseFrom: [base],
|
||||
}),
|
||||
);
|
||||
const editorRow = rowForReviewTarget(withEditor, reviewCell.reviewTarget.key);
|
||||
|
||||
expect(measurementCount).toBe(initialMeasurementCount);
|
||||
expect(editorRow.reviewHeight).toBe(148);
|
||||
expect(editorRow.cells).toBe(baseReviewRow.cells);
|
||||
expect(withEditor.files[1]!.top).toBe(baseSecondFileTop + 148);
|
||||
|
||||
const withoutEditor = buildDiffDocumentModel(
|
||||
input({
|
||||
files,
|
||||
measureText: countingMeasurer,
|
||||
reviewActions: reviewActionsWithEditor(null),
|
||||
reuseFrom: [base, withEditor],
|
||||
}),
|
||||
);
|
||||
|
||||
expect(measurementCount).toBe(initialMeasurementCount);
|
||||
expect(withoutEditor.height).toBe(base.height);
|
||||
expect(withoutEditor.files[1]!.top).toBe(baseSecondFileTop);
|
||||
});
|
||||
|
||||
it("preserves a logical source anchor through width and font metric relayout", () => {
|
||||
const narrow = buildDiffDocumentModel(
|
||||
input({ files: [fileWithChangedLine("abcdefghij")], viewportWidth: 90 }),
|
||||
@@ -492,3 +540,26 @@ function rowForCell(
|
||||
if (!row || row.kind !== "line") throw new Error("Expected the cell row");
|
||||
return row;
|
||||
}
|
||||
|
||||
function rowForReviewTarget(model: ReturnType<typeof buildDiffDocumentModel>, key: string) {
|
||||
const row = model.rows.find(
|
||||
(candidate) =>
|
||||
candidate.kind === "line" && candidate.cells.some((cell) => cell?.reviewTarget?.key === key),
|
||||
);
|
||||
if (!row || row.kind !== "line") throw new Error("Expected the review row");
|
||||
return row;
|
||||
}
|
||||
|
||||
function reviewActionsWithEditor(
|
||||
target: NonNullable<ReturnType<typeof addedCell>["reviewTarget"]> | null,
|
||||
): NonNullable<BuildDiffDocumentModelInput["reviewActions"]> {
|
||||
return {
|
||||
commentsByTarget: new Map(),
|
||||
editor: target ? { target, body: "", commentId: null } : null,
|
||||
onStartComment() {},
|
||||
onCancelEditor() {},
|
||||
onSaveEditor() {},
|
||||
onEditComment() {},
|
||||
onDeleteComment() {},
|
||||
};
|
||||
}
|
||||
|
||||
@@ -60,14 +60,28 @@ export function buildDiffDocumentModel(input: BuildDiffDocumentModelInput): Diff
|
||||
if (!isCollapsed && reusableFile) {
|
||||
const reusedRows = reusableModel!.rows.slice(reusableFile.rowStart, reusableFile.rowEnd);
|
||||
for (const reusedRow of reusedRows) {
|
||||
const height = reusedRow.height;
|
||||
if (reusedRow.kind === "line") {
|
||||
const reviewHeight = reviewHeightForCells(reusedRow.cells, input);
|
||||
const textHeight = reusedRow.height - reusedRow.reviewHeight;
|
||||
const height = textHeight + reviewHeight;
|
||||
rows.push({
|
||||
...reusedRow,
|
||||
index: rows.length,
|
||||
fileIndex,
|
||||
top: documentTop,
|
||||
height,
|
||||
reviewHeight,
|
||||
});
|
||||
documentTop += height;
|
||||
continue;
|
||||
}
|
||||
rows.push({
|
||||
...reusedRow,
|
||||
index: rows.length,
|
||||
fileIndex,
|
||||
top: documentTop,
|
||||
});
|
||||
documentTop += height;
|
||||
documentTop += reusedRow.height;
|
||||
}
|
||||
maximumHorizontalOverflow = Math.max(0, reusableFile.contentWidth - input.viewportWidth);
|
||||
documentTop += BODY_BORDER_HEIGHT;
|
||||
|
||||
@@ -93,7 +93,6 @@ export function DiffSurface(props: DiffSurfaceProps) {
|
||||
typography,
|
||||
measurement,
|
||||
props.palette,
|
||||
reviewActions,
|
||||
t,
|
||||
] as const;
|
||||
const previous = reusableModelRef.current;
|
||||
|
||||
@@ -95,7 +95,6 @@ export function DiffSurface(props: DiffSurfaceProps) {
|
||||
loadedTypography,
|
||||
measurement,
|
||||
props.palette,
|
||||
reviewActions,
|
||||
t,
|
||||
] as const;
|
||||
const previous = reusableModelRef.current;
|
||||
@@ -210,7 +209,15 @@ export function DiffSurface(props: DiffSurfaceProps) {
|
||||
}, [loadedTypography, measurement, props.palette, viewport.height, viewport.width]);
|
||||
const schedulePaint = useCallback(
|
||||
(force = true) => {
|
||||
if (force) forcePaintRef.current = true;
|
||||
if (force) {
|
||||
forcePaintRef.current = true;
|
||||
// Fast Refresh preserves refs. A request owned by the previous module can leave a stale
|
||||
// id behind, which would otherwise prevent every subsequent canvas repaint.
|
||||
if (frameRef.current !== null) {
|
||||
cancelAnimationFrame(frameRef.current);
|
||||
frameRef.current = null;
|
||||
}
|
||||
}
|
||||
if (frameRef.current === null) frameRef.current = requestAnimationFrame(paint);
|
||||
},
|
||||
[paint],
|
||||
@@ -260,6 +267,7 @@ export function DiffSurface(props: DiffSurfaceProps) {
|
||||
useEffect(
|
||||
() => () => {
|
||||
if (frameRef.current !== null) cancelAnimationFrame(frameRef.current);
|
||||
frameRef.current = null;
|
||||
},
|
||||
[],
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user