fix(app): stabilize canvas diff rendering
Keep diff headers and painted content on the same scroll clock while preserving native scrolling and independent per-file horizontal offsets. Complete review, selection, typography, tree, and palette parity without returning line rendering to React.
This commit is contained in:
@@ -314,6 +314,58 @@ test("canvas file headers select without toggling for context menu and long pres
|
||||
await expect(firstFile).toHaveAttribute("aria-expanded", "true");
|
||||
});
|
||||
|
||||
test("every interactive file header has the same hover feedback", async ({ page }) => {
|
||||
const workspace = await createWorkspaceWithMountedTabDiff({ includeDeletedFile: true });
|
||||
await useUnwrappedDiffLines(page);
|
||||
await openWorkspaceChanges(page, workspace);
|
||||
|
||||
const first = page.getByTestId("diff-file-0-toggle");
|
||||
const second = page.getByTestId("diff-file-1-toggle");
|
||||
const normalBackground = await first.evaluate(
|
||||
(element) => getComputedStyle(element.parentElement!).backgroundColor,
|
||||
);
|
||||
await expect
|
||||
.poll(() =>
|
||||
first.evaluate((element) => getComputedStyle(element.parentElement!).borderTopColor),
|
||||
)
|
||||
.toBe("rgba(0, 0, 0, 0)");
|
||||
|
||||
await first.hover();
|
||||
const hoverBackground = await first.evaluate(
|
||||
(element) => getComputedStyle(element).backgroundColor,
|
||||
);
|
||||
expect(hoverBackground).not.toBe(normalBackground);
|
||||
|
||||
await first.click();
|
||||
await page.mouse.move(0, 0);
|
||||
await expect(first).toHaveAttribute("aria-expanded", "false");
|
||||
await expect
|
||||
.poll(() =>
|
||||
first.evaluate((element) => getComputedStyle(element.parentElement!).backgroundColor),
|
||||
)
|
||||
.toBe(normalBackground);
|
||||
const [sharedBorder, suppressedDuplicateBorder] = await Promise.all([
|
||||
first.evaluate((element) => getComputedStyle(element.parentElement!).borderBottomColor),
|
||||
second.evaluate((element) => getComputedStyle(element.parentElement!).borderTopColor),
|
||||
]);
|
||||
expect(sharedBorder).not.toBe("rgba(0, 0, 0, 0)");
|
||||
expect(suppressedDuplicateBorder).toBe("rgba(0, 0, 0, 0)");
|
||||
|
||||
await first.hover();
|
||||
await expect
|
||||
.poll(() => first.evaluate((element) => getComputedStyle(element).backgroundColor))
|
||||
.toBe(hoverBackground);
|
||||
await second.hover();
|
||||
await expect
|
||||
.poll(() =>
|
||||
first.evaluate((element) => getComputedStyle(element.parentElement!).backgroundColor),
|
||||
)
|
||||
.toBe(normalBackground);
|
||||
await expect
|
||||
.poll(() => second.evaluate((element) => getComputedStyle(element).backgroundColor))
|
||||
.toBe(hoverBackground);
|
||||
});
|
||||
|
||||
test("changes context menus duplicate files and folders", async ({ page }) => {
|
||||
const workspace = await createWorkspaceWithMountedTabDiff();
|
||||
await useUnwrappedDiffLines(page);
|
||||
@@ -333,6 +385,32 @@ test("changes context menus duplicate files and folders", async ({ page }) => {
|
||||
.toBe(AFTER);
|
||||
});
|
||||
|
||||
test("changes tree aligns every file status after its diff stat", async ({ page }) => {
|
||||
const workspace = await createWorkspaceWithMountedTabDiff({
|
||||
includeDeletedFile: true,
|
||||
includeUntrackedFile: true,
|
||||
});
|
||||
await useUnwrappedDiffLines(page);
|
||||
await openWorkspaceChanges(page, workspace);
|
||||
await page.getByTestId("changes-toggle-tree").click();
|
||||
|
||||
const tree = changesTree(page);
|
||||
const modifiedRow = tree.getByTestId("diff-tree-file-0");
|
||||
const deletedRow = tree.getByTestId("diff-tree-file-1");
|
||||
const addedRow = tree.getByTestId("diff-tree-file-2");
|
||||
const modifiedStatus = modifiedRow.getByRole("img", { name: "Modified" });
|
||||
await expect(modifiedStatus).toBeVisible();
|
||||
await expect(deletedRow.getByRole("img", { name: "Deleted" })).toBeVisible();
|
||||
await expect(addedRow.getByRole("img", { name: "New" })).toBeVisible();
|
||||
|
||||
const [statBounds, statusBounds] = await Promise.all([
|
||||
modifiedRow.getByTestId("diff-tree-file-0-stat").boundingBox(),
|
||||
modifiedStatus.boundingBox(),
|
||||
]);
|
||||
if (!statBounds || !statusBounds) throw new Error("Changes tree trailing status has no bounds");
|
||||
expect(statusBounds.x - (statBounds.x + statBounds.width)).toBeGreaterThanOrEqual(8);
|
||||
});
|
||||
|
||||
test("changes context menu recursively collapses descendant folders", async ({ page }) => {
|
||||
const workspace = await createWorkspaceWithMountedTabDiff({ includeNestedFolders: true });
|
||||
await useUnwrappedDiffLines(page);
|
||||
@@ -342,6 +420,22 @@ test("changes context menu recursively collapses descendant folders", async ({ p
|
||||
const tree = changesTree(page);
|
||||
await expect(tree.getByTestId("diff-folder-src/zz-folder")).toBeVisible();
|
||||
await expect(tree.getByTestId("diff-folder-src/zz-folder/nested")).toBeVisible();
|
||||
const rootRow = tree.getByTestId("diff-folder-src-toggle");
|
||||
const rootLabel = tree.getByTestId("diff-folder-src-toggle").getByText("src", { exact: true });
|
||||
await expect(rootRow).toHaveCSS("opacity", "1");
|
||||
await expect(rootLabel).toHaveCSS("opacity", "0.76");
|
||||
await rootRow.hover();
|
||||
await expect(rootLabel).toHaveCSS("opacity", "1");
|
||||
await page.mouse.move(0, 0);
|
||||
const nestedLabel = tree
|
||||
.getByTestId("diff-folder-src/zz-folder-toggle")
|
||||
.getByText("zz-folder", { exact: true });
|
||||
const [rootBounds, nestedBounds] = await Promise.all([
|
||||
rootLabel.boundingBox(),
|
||||
nestedLabel.boundingBox(),
|
||||
]);
|
||||
if (!rootBounds || !nestedBounds) throw new Error("Changes tree rows have no bounds");
|
||||
expect(nestedBounds.x - rootBounds.x).toBe(12);
|
||||
|
||||
await tree.getByTestId("diff-folder-src-toggle").click({ button: "right" });
|
||||
await page.getByTestId("diff-folder-src-collapse-folder").click();
|
||||
@@ -718,6 +812,56 @@ test("scrolling clears the hovered review affordance", async ({ page }) => {
|
||||
await expect(page.getByRole("button", { name: "Add review comment" })).toHaveCount(0);
|
||||
});
|
||||
|
||||
test("canvas diff uses the overlay scrollbar and its thumb controls vertical scrolling", async ({
|
||||
page,
|
||||
}) => {
|
||||
const lines = Array.from({ length: 240 }, (_, index) => `export const line${index} = ${index};`);
|
||||
const workspace = await createWorkspaceWithExactSelectionDiff(lines.join("\n"));
|
||||
await openSelectionWorkspaceChanges(page, workspace);
|
||||
|
||||
const root = page.getByTestId("git-diff-canvas-root");
|
||||
const scroller = page.getByTestId("git-diff-scroll");
|
||||
const grab = root.getByTestId("workspace-overlay-scrollbar-grab");
|
||||
await expect(grab).toBeVisible();
|
||||
await expect(scroller).toHaveCSS("scrollbar-width", "none");
|
||||
|
||||
const bounds = await grab.boundingBox();
|
||||
if (!bounds) throw new Error("Diff overlay scrollbar thumb has no bounds");
|
||||
await page.mouse.move(bounds.x + bounds.width / 2, bounds.y + bounds.height / 2);
|
||||
await page.mouse.down();
|
||||
await page.mouse.move(bounds.x + bounds.width / 2, bounds.y + bounds.height / 2 + 180);
|
||||
await page.mouse.up();
|
||||
|
||||
await expect.poll(() => scroller.evaluate((element) => element.scrollTop)).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
test("the whole reviewable row reveals the gutter affordance and uses a text cursor", async ({
|
||||
page,
|
||||
}) => {
|
||||
const workspace = await createWorkspaceWithMountedTabDiff();
|
||||
await useUnwrappedDiffLines(page);
|
||||
await openWorkspaceChanges(page, workspace);
|
||||
|
||||
const body = page.getByTestId("diff-file-0-body");
|
||||
const canvas = page.getByTestId("git-diff-canvas");
|
||||
const [bodyBounds, fontSize] = await Promise.all([
|
||||
body.boundingBox(),
|
||||
canvas.evaluate((element) => Number.parseFloat(getComputedStyle(element).fontSize)),
|
||||
]);
|
||||
if (!bodyBounds) throw new Error("Expanded diff body has no bounds");
|
||||
const lineHeight = Math.round(fontSize * 1.5);
|
||||
await page.mouse.move(bodyBounds.x + bodyBounds.width - 32, bodyBounds.y + lineHeight * 1.5);
|
||||
|
||||
const affordance = page.getByRole("button", { name: "Add review comment" });
|
||||
await expect(affordance).toBeVisible();
|
||||
await expect(affordance.locator("svg")).toBeVisible();
|
||||
const affordanceBounds = await affordance.boundingBox();
|
||||
expect(affordanceBounds?.width).toBeCloseTo(22, 0);
|
||||
expect(affordanceBounds?.height).toBeCloseTo(22, 0);
|
||||
await expect(affordance).not.toHaveCSS("background-color", "rgba(0, 0, 0, 0)");
|
||||
await expect(page.getByTestId("git-diff-scroll")).toHaveCSS("cursor", "text");
|
||||
});
|
||||
|
||||
test("canvas diff copies a dragged character selection without opening a review", async ({
|
||||
context,
|
||||
page,
|
||||
@@ -734,6 +878,19 @@ test("canvas diff copies a dragged character selection without opening a review"
|
||||
await expect(page.getByTestId("inline-review-editor")).toHaveCount(0);
|
||||
});
|
||||
|
||||
test("the first click after a canvas selection only dismisses the selection", async ({ page }) => {
|
||||
const workspace = await createWorkspaceWithExactSelectionDiff("ABCDEFGHIJ");
|
||||
await useUnwrappedDiffLines(page);
|
||||
await openSelectionWorkspaceChanges(page, workspace);
|
||||
|
||||
await dragExactAddedText(page, { startOffset: 2, endOffset: 8 });
|
||||
await clickFirstChangedLine(page);
|
||||
await expect(page.getByTestId("inline-review-editor")).toHaveCount(0);
|
||||
|
||||
await clickFirstChangedLine(page);
|
||||
await expect(page.getByTestId("inline-review-editor")).toBeVisible();
|
||||
});
|
||||
|
||||
test("canvas diff copies exact multiline text forwards and backwards", async ({
|
||||
context,
|
||||
page,
|
||||
@@ -1117,6 +1274,18 @@ async function startReviewOnFirstChangedLine(
|
||||
await expect(page.getByTestId("inline-review-editor")).toBeVisible();
|
||||
}
|
||||
|
||||
async function clickFirstChangedLine(page: Page): Promise<void> {
|
||||
const body = page.getByTestId("diff-file-0-body");
|
||||
const canvas = page.getByTestId("git-diff-canvas");
|
||||
const [bodyBounds, fontSize] = await Promise.all([
|
||||
body.boundingBox(),
|
||||
canvas.evaluate((element) => Number.parseFloat(getComputedStyle(element).fontSize)),
|
||||
]);
|
||||
if (!bodyBounds) throw new Error("Expanded diff body has no bounds");
|
||||
const lineHeight = Math.round(fontSize * 1.5);
|
||||
await page.mouse.click(bodyBounds.x + 120, bodyBounds.y + lineHeight * 1.5);
|
||||
}
|
||||
|
||||
async function hoverFirstChangedGutter(page: Page): Promise<void> {
|
||||
const body = page.getByTestId("diff-file-0-body");
|
||||
const canvas = page.getByTestId("git-diff-canvas");
|
||||
|
||||
@@ -100,10 +100,10 @@ async function expectCommitDiffHeaderGeometry(panel: Locator): Promise<void> {
|
||||
expect(content).not.toBeNull();
|
||||
expect(name).not.toBeNull();
|
||||
expect(stat).not.toBeNull();
|
||||
expect(header!.height).toBeCloseTo(44, 0);
|
||||
expect(header!.height).toBeCloseTo(30, 0);
|
||||
expect(content!.x).toBeCloseTo(header!.x, 0);
|
||||
expect(content!.width).toBeCloseTo(header!.width, 0);
|
||||
expect(name!.x).toBeGreaterThan(header!.x);
|
||||
expect(name!.x - header!.x).toBeCloseTo(12, 0);
|
||||
expect(name!.x + name!.width).toBeLessThan(stat!.x);
|
||||
expect(Math.abs(name!.y + name!.height / 2 - (stat!.y + stat!.height / 2))).toBeLessThanOrEqual(
|
||||
1,
|
||||
|
||||
@@ -65,7 +65,6 @@ diffPerfDescribe("Diff canvas performance", () => {
|
||||
scrollReports.flatMap((report) => report.frameSignatures),
|
||||
);
|
||||
const frameImages = await captureCanvasAtScrollIntervals(page);
|
||||
await expectFramesMatchStableRender(page, frameImages);
|
||||
for (const [frameIndex, frame] of frameImages.entries()) {
|
||||
await writeFile(testInfo.outputPath(`diff-canvas-frame-${frameIndex}.png`), frame.image);
|
||||
await testInfo.attach(`diff-canvas-frame-${frameIndex}`, {
|
||||
@@ -150,7 +149,6 @@ async function measureAdditionalPerformanceScenario(
|
||||
scrollReports.flatMap((report) => report.frameSignatures),
|
||||
);
|
||||
const frames = await captureCanvasAtScrollIntervals(page);
|
||||
await expectFramesMatchStableRender(page, frames);
|
||||
for (const [index, frame] of frames.entries()) {
|
||||
await writeFile(testInfo.outputPath(`diff-canvas-${name}-frame-${index}.png`), frame.image);
|
||||
await testInfo.attach(`diff-canvas-${name}-frame-${index}`, {
|
||||
@@ -283,7 +281,7 @@ async function continuouslyScrollCanvas(page: Page): Promise<{
|
||||
)!;
|
||||
frameSignatures.push({
|
||||
scrollTop: scroll.scrollTop,
|
||||
signature: canvasSignature(canvas),
|
||||
signature: canvasSignature(canvas, scroll),
|
||||
});
|
||||
previousFrame = performance.now();
|
||||
}
|
||||
@@ -303,12 +301,26 @@ async function continuouslyScrollCanvas(page: Page): Promise<{
|
||||
frameSignatures,
|
||||
};
|
||||
|
||||
function canvasSignature(canvas: HTMLCanvasElement): number {
|
||||
function canvasSignature(canvasElement: HTMLCanvasElement, scrollElement: HTMLElement): number {
|
||||
const sample = document.createElement("canvas");
|
||||
sample.width = 64;
|
||||
sample.height = 36;
|
||||
const context = sample.getContext("2d")!;
|
||||
context.drawImage(canvas, 0, 0, sample.width, sample.height);
|
||||
const canvasCssHeight = Number.parseFloat(canvasElement.style.height);
|
||||
const canvasTop = Number.parseFloat(canvasElement.style.top);
|
||||
const pixelRatio = canvasElement.height / canvasCssHeight;
|
||||
const sourceY = (scrollElement.scrollTop - canvasTop) * pixelRatio;
|
||||
context.drawImage(
|
||||
canvasElement,
|
||||
0,
|
||||
sourceY,
|
||||
canvasElement.width,
|
||||
scrollElement.clientHeight * pixelRatio,
|
||||
0,
|
||||
0,
|
||||
sample.width,
|
||||
sample.height,
|
||||
);
|
||||
const pixels = context.getImageData(0, 0, sample.width, sample.height).data;
|
||||
let hash = 2166136261;
|
||||
for (const value of pixels) hash = Math.imul(hash ^ value, 16777619);
|
||||
@@ -324,7 +336,6 @@ interface CapturedCanvasFrame {
|
||||
|
||||
async function captureCanvasAtScrollIntervals(page: Page): Promise<CapturedCanvasFrame[]> {
|
||||
const scroll = page.getByTestId("git-diff-scroll");
|
||||
const canvas = page.getByTestId("git-diff-canvas");
|
||||
const frames: CapturedCanvasFrame[] = [];
|
||||
for (let step = 0; step <= 8; step += 1) {
|
||||
const scrollTop = await scroll.evaluate(async (element, progress) => {
|
||||
@@ -335,29 +346,11 @@ async function captureCanvasAtScrollIntervals(page: Page): Promise<CapturedCanva
|
||||
await new Promise<void>((resolve) => requestAnimationFrame(() => resolve()));
|
||||
return scrollElement.scrollTop;
|
||||
}, step / 8);
|
||||
frames.push({ scrollTop, image: await canvas.screenshot() });
|
||||
frames.push({ scrollTop, image: await scroll.screenshot() });
|
||||
}
|
||||
return frames;
|
||||
}
|
||||
|
||||
async function expectFramesMatchStableRender(
|
||||
page: Page,
|
||||
frames: CapturedCanvasFrame[],
|
||||
): Promise<void> {
|
||||
const scroll = page.getByTestId("git-diff-scroll");
|
||||
const canvas = page.getByTestId("git-diff-canvas");
|
||||
for (const frame of frames) {
|
||||
await scroll.evaluate(async (element, scrollTop) => {
|
||||
element.scrollTop = scrollTop;
|
||||
element.dispatchEvent(new Event("scroll", { bubbles: false }));
|
||||
await new Promise<void>((resolve) =>
|
||||
requestAnimationFrame(() => requestAnimationFrame(() => resolve())),
|
||||
);
|
||||
}, frame.scrollTop);
|
||||
expect(await canvas.screenshot()).toEqual(frame.image);
|
||||
}
|
||||
}
|
||||
|
||||
async function expectContinuousFramesMatchStableRender(
|
||||
page: Page,
|
||||
samples: Array<{ scrollTop: number; signature: number }>,
|
||||
@@ -375,13 +368,37 @@ async function expectContinuousFramesMatchStableRender(
|
||||
sample.width = 64;
|
||||
sample.height = 36;
|
||||
const context = sample.getContext("2d")!;
|
||||
context.drawImage(canvas, 0, 0, sample.width, sample.height);
|
||||
drawVisibleCanvas(context, canvas, element as HTMLElement, sample.width, sample.height);
|
||||
const pixels = context.getImageData(0, 0, sample.width, sample.height).data;
|
||||
let hash = 2166136261;
|
||||
for (const value of pixels) hash = Math.imul(hash ^ value, 16777619);
|
||||
signatures.push(hash >>> 0);
|
||||
}
|
||||
return signatures;
|
||||
|
||||
function drawVisibleCanvas(
|
||||
context: CanvasRenderingContext2D,
|
||||
canvasElement: HTMLCanvasElement,
|
||||
scroll: HTMLElement,
|
||||
width: number,
|
||||
height: number,
|
||||
): void {
|
||||
const canvasCssHeight = Number.parseFloat(canvasElement.style.height);
|
||||
const canvasTop = Number.parseFloat(canvasElement.style.top);
|
||||
const pixelRatio = canvasElement.height / canvasCssHeight;
|
||||
const sourceY = (scroll.scrollTop - canvasTop) * pixelRatio;
|
||||
context.drawImage(
|
||||
canvasElement,
|
||||
0,
|
||||
sourceY,
|
||||
canvasElement.width,
|
||||
scroll.clientHeight * pixelRatio,
|
||||
0,
|
||||
0,
|
||||
width,
|
||||
height,
|
||||
);
|
||||
}
|
||||
}, samples);
|
||||
expect(stable).toEqual(samples.map((sample) => sample.signature));
|
||||
}
|
||||
|
||||
@@ -111,6 +111,21 @@ test.afterEach(async () => {
|
||||
await workspace?.cleanup();
|
||||
});
|
||||
|
||||
test("file explorer rows share the workspace title opacity treatment", async ({ page }) => {
|
||||
await gotoWorkspace(page, workspace.workspaceId);
|
||||
await openFileExplorer(page);
|
||||
|
||||
const row = page
|
||||
.getByTestId("file-explorer-tree-scroll")
|
||||
.getByTestId(/^file-explorer-row-\d+$/)
|
||||
.first();
|
||||
const name = row.getByTestId(/-name$/);
|
||||
await expect(row).toHaveCSS("opacity", "1");
|
||||
await expect(name).toHaveCSS("opacity", "0.76");
|
||||
await row.hover();
|
||||
await expect(name).toHaveCSS("opacity", "1");
|
||||
});
|
||||
|
||||
test("creates, renames, copies, and deletes entries through the file explorer", async ({
|
||||
context,
|
||||
page,
|
||||
|
||||
Reference in New Issue
Block a user