Fix diff gutter row alignment
This commit is contained in:
Mohamed Boudra
@@ -0,0 +1,505 @@
|
||||
import { writeFile } from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { type Page } from "@playwright/test";
|
||||
import { buildHostWorkspaceRoute, buildSettingsSectionRoute } from "../src/utils/host-routes";
|
||||
import { test, expect } from "./fixtures";
|
||||
import { getServerId } from "./helpers/server-id";
|
||||
import { connectSeedClient } from "./helpers/seed-client";
|
||||
import { createTempGitRepo } from "./helpers/workspace";
|
||||
import { waitForWorkspaceTabsVisible } from "./helpers/workspace-tabs";
|
||||
|
||||
interface DirtyWorkspace {
|
||||
id: string;
|
||||
}
|
||||
|
||||
interface CleanupTask {
|
||||
run: () => Promise<void>;
|
||||
}
|
||||
|
||||
const cleanupTasks: CleanupTask[] = [];
|
||||
const APP_SETTINGS_KEY = "@paseo:app-settings";
|
||||
const CHANGES_PREFERENCES_KEY = "@paseo:changes-preferences";
|
||||
|
||||
const BEFORE = `import { useLayoutEffect, useMemo, useRef, useState } from "react";
|
||||
|
||||
interface UseMountedTabSetInput {
|
||||
activeTabId: string | null;
|
||||
allTabIds: string[];
|
||||
cap: number;
|
||||
}
|
||||
|
||||
interface UseMountedTabSetResult {
|
||||
mountedTabIds: Set<string>;
|
||||
}
|
||||
|
||||
function createInitialMountedTabIds(input: UseMountedTabSetInput): Set<string> {
|
||||
if (!input.activeTabId || !input.allTabIds.includes(input.activeTabId)) {
|
||||
return new Set<string>();
|
||||
}
|
||||
return new Set<string>([input.activeTabId]);
|
||||
}
|
||||
|
||||
function setsEqual(left: Set<string>, right: Set<string>): boolean {
|
||||
if (left.size !== right.size) {
|
||||
return false;
|
||||
}
|
||||
for (const value of left) {
|
||||
if (!right.has(value)) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
export function useMountedTabSet(input: UseMountedTabSetInput): UseMountedTabSetResult {
|
||||
const { activeTabId, allTabIds, cap } = input;
|
||||
const allTabIdsKey = allTabIds.join("\\u0000");
|
||||
const availableTabIds = useMemo(() => {
|
||||
void allTabIdsKey;
|
||||
return new Set(allTabIds);
|
||||
}, [allTabIds, allTabIdsKey]);
|
||||
const [mountedTabIds, setMountedTabIds] = useState(() => createInitialMountedTabIds(input));
|
||||
const lruRef = useRef(activeTabId && allTabIds.includes(activeTabId) ? [activeTabId] : []);
|
||||
|
||||
useLayoutEffect(() => {
|
||||
const nextLru = lruRef.current.filter((tabId) => availableTabIds.has(tabId));
|
||||
if (activeTabId && availableTabIds.has(activeTabId)) {
|
||||
const existingIndex = nextLru.indexOf(activeTabId);
|
||||
if (existingIndex >= 0) {
|
||||
nextLru.splice(existingIndex, 1);
|
||||
}
|
||||
nextLru.unshift(activeTabId);
|
||||
}
|
||||
if (nextLru.length > cap) {
|
||||
nextLru.length = cap;
|
||||
}
|
||||
|
||||
lruRef.current = nextLru;
|
||||
setMountedTabIds((previousMountedTabIds) => {
|
||||
const nextMountedTabIds = new Set(nextLru);
|
||||
return setsEqual(previousMountedTabIds, nextMountedTabIds)
|
||||
? previousMountedTabIds
|
||||
: nextMountedTabIds;
|
||||
});
|
||||
}, [activeTabId, availableTabIds, cap]);
|
||||
|
||||
return { mountedTabIds };
|
||||
}
|
||||
`;
|
||||
|
||||
const AFTER = `import { useLayoutEffect, useMemo, useRef, useState } from "react";
|
||||
|
||||
interface UseMountedTabSetInput {
|
||||
activeTabId: string | null;
|
||||
allTabIds: string[];
|
||||
cap: number;
|
||||
}
|
||||
|
||||
interface UseMountedTabSetResult {
|
||||
mountedTabIds: Set<string>;
|
||||
}
|
||||
|
||||
interface DeriveRenderMountedTabIdsInput {
|
||||
activeTabId: string | null;
|
||||
availableTabIds: Set<string>;
|
||||
cap: number;
|
||||
mountedTabIds: Set<string>;
|
||||
}
|
||||
|
||||
function createInitialMountedTabIds(input: UseMountedTabSetInput): Set<string> {
|
||||
if (!input.activeTabId || !input.allTabIds.includes(input.activeTabId)) {
|
||||
return new Set<string>();
|
||||
}
|
||||
return new Set<string>([input.activeTabId]);
|
||||
}
|
||||
|
||||
function setsEqual(left: Set<string>, right: Set<string>): boolean {
|
||||
if (left.size !== right.size) {
|
||||
return false;
|
||||
}
|
||||
for (const value of left) {
|
||||
if (!right.has(value)) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
function deriveRenderMountedTabIds(input: DeriveRenderMountedTabIdsInput): Set<string> {
|
||||
const { activeTabId, availableTabIds, cap, mountedTabIds } = input;
|
||||
if (!activeTabId || !availableTabIds.has(activeTabId) || mountedTabIds.has(activeTabId)) {
|
||||
return mountedTabIds;
|
||||
}
|
||||
|
||||
const next = new Set<string>([activeTabId]);
|
||||
const maxSize = Math.max(1, cap);
|
||||
for (const tabId of mountedTabIds) {
|
||||
if (next.size >= maxSize) {
|
||||
break;
|
||||
}
|
||||
if (availableTabIds.has(tabId)) {
|
||||
next.add(tabId);
|
||||
}
|
||||
}
|
||||
return next;
|
||||
}
|
||||
|
||||
export function useMountedTabSet(input: UseMountedTabSetInput): UseMountedTabSetResult {
|
||||
const { activeTabId, allTabIds, cap } = input;
|
||||
const allTabIdsKey = allTabIds.join("\\u0000");
|
||||
const availableTabIds = useMemo(() => {
|
||||
void allTabIdsKey;
|
||||
return new Set(allTabIds);
|
||||
}, [allTabIds, allTabIdsKey]);
|
||||
const [mountedTabIds, setMountedTabIds] = useState(() => createInitialMountedTabIds(input));
|
||||
const lruRef = useRef(activeTabId && allTabIds.includes(activeTabId) ? [activeTabId] : []);
|
||||
const renderMountedTabIds = useMemo(
|
||||
() =>
|
||||
deriveRenderMountedTabIds({
|
||||
activeTabId,
|
||||
availableTabIds,
|
||||
cap,
|
||||
mountedTabIds,
|
||||
}),
|
||||
[activeTabId, availableTabIds, cap, mountedTabIds],
|
||||
);
|
||||
|
||||
useLayoutEffect(() => {
|
||||
const nextLru = lruRef.current.filter((tabId) => availableTabIds.has(tabId));
|
||||
if (activeTabId && availableTabIds.has(activeTabId)) {
|
||||
const existingIndex = nextLru.indexOf(activeTabId);
|
||||
if (existingIndex >= 0) {
|
||||
nextLru.splice(existingIndex, 1);
|
||||
}
|
||||
nextLru.unshift(activeTabId);
|
||||
}
|
||||
if (nextLru.length > cap) {
|
||||
nextLru.length = cap;
|
||||
}
|
||||
|
||||
lruRef.current = nextLru;
|
||||
setMountedTabIds((previousMountedTabIds) => {
|
||||
const nextMountedTabIds = new Set(nextLru);
|
||||
return setsEqual(previousMountedTabIds, nextMountedTabIds)
|
||||
? previousMountedTabIds
|
||||
: nextMountedTabIds;
|
||||
});
|
||||
}, [activeTabId, availableTabIds, cap]);
|
||||
|
||||
return { mountedTabIds: renderMountedTabIds };
|
||||
}
|
||||
`;
|
||||
|
||||
test.afterEach(async () => {
|
||||
for (const task of cleanupTasks.splice(0)) {
|
||||
await task.run();
|
||||
}
|
||||
});
|
||||
|
||||
test("changes diff keeps code rows aligned with the gutter", async ({ page }) => {
|
||||
const workspace = await createWorkspaceWithMountedTabDiff();
|
||||
await useCodeFont(page, 9);
|
||||
await useUnwrappedDiffLines(page);
|
||||
await openWorkspaceChanges(page, workspace);
|
||||
|
||||
await expectDiffCodeFontSize(page, 9);
|
||||
await expectVisibleDiffRowsAligned(page);
|
||||
await expectDiffCodeTextAlignedWithGutterText(page, [
|
||||
{
|
||||
codeText: "function createInitialMountedTabIds(input: UseMountedTabSetInput)",
|
||||
lineNumber: "20",
|
||||
},
|
||||
{ codeText: "return next;", lineNumber: "55" },
|
||||
{ codeText: "useLayoutEffect(() => {", lineNumber: "78" },
|
||||
]);
|
||||
await expectHoverCommentButtonAlignedWithCodeLine(page, {
|
||||
codeText: "function createInitialMountedTabIds(input: UseMountedTabSetInput)",
|
||||
lineNumber: "20",
|
||||
});
|
||||
});
|
||||
|
||||
test("changes diff keeps unwrapped gutter and code rows aligned after code size changes", async ({
|
||||
page,
|
||||
}) => {
|
||||
const workspace = await createWorkspaceWithMountedTabDiff();
|
||||
await useCodeFont(page, 12);
|
||||
await useUnwrappedDiffLines(page);
|
||||
await openWorkspaceChanges(page, workspace);
|
||||
|
||||
await changeCodeFontSizeFromSettings(page, 18);
|
||||
await returnToWorkspaceChanges(page);
|
||||
await scrollToLowerUnwrappedDiffRows(page);
|
||||
|
||||
await expectDiffCodeFontSize(page, 18);
|
||||
await expectVisibleDiffRowsShareTypography(page);
|
||||
await expectVisibleDiffRowsAligned(page);
|
||||
});
|
||||
|
||||
async function useCodeFont(page: Page, codeFontSize: number): Promise<void> {
|
||||
await page.addInitScript(
|
||||
({ settingsKey, fontSize }) => {
|
||||
localStorage.setItem(
|
||||
settingsKey,
|
||||
JSON.stringify({
|
||||
theme: "dark",
|
||||
sendBehavior: "interrupt",
|
||||
serviceUrlBehavior: "ask",
|
||||
terminalScrollbackLines: 10_000,
|
||||
uiFontFamily: "",
|
||||
monoFontFamily: "",
|
||||
uiFontSize: 16,
|
||||
codeFontSize: fontSize,
|
||||
syntaxTheme: "one",
|
||||
}),
|
||||
);
|
||||
},
|
||||
{ settingsKey: APP_SETTINGS_KEY, fontSize: codeFontSize },
|
||||
);
|
||||
}
|
||||
|
||||
async function useUnwrappedDiffLines(page: Page): Promise<void> {
|
||||
await page.addInitScript(
|
||||
({ preferencesKey }) => {
|
||||
localStorage.setItem(
|
||||
preferencesKey,
|
||||
JSON.stringify({ layout: "unified", wrapLines: false, hideWhitespace: false }),
|
||||
);
|
||||
},
|
||||
{ preferencesKey: CHANGES_PREFERENCES_KEY },
|
||||
);
|
||||
}
|
||||
|
||||
async function expectDiffCodeFontSize(page: Page, fontSize: number): Promise<void> {
|
||||
const actualFontSize = await page
|
||||
.getByTestId("diff-code-text-1")
|
||||
.evaluate((text) => Number.parseFloat(getComputedStyle(text).fontSize));
|
||||
expect(actualFontSize).toBe(fontSize);
|
||||
}
|
||||
|
||||
async function expectVisibleDiffRowsAligned(page: Page): Promise<void> {
|
||||
const geometry = await readVisibleDiffRowGeometry(page);
|
||||
expect(geometry.maxDelta, JSON.stringify(geometry.rows, null, 2)).toBeLessThanOrEqual(1);
|
||||
}
|
||||
|
||||
async function expectVisibleDiffRowsShareTypography(page: Page): Promise<void> {
|
||||
const geometry = await readVisibleDiffRowGeometry(page);
|
||||
expect(geometry.mismatchedTypography, JSON.stringify(geometry, null, 2)).toEqual([]);
|
||||
}
|
||||
|
||||
async function readVisibleDiffRowGeometry(page: Page): Promise<{
|
||||
maxDelta: number;
|
||||
mismatchedTypography: { index: number; gutterLineHeight: number; codeLineHeight: number }[];
|
||||
rows: {
|
||||
index: number;
|
||||
gutterTop: number;
|
||||
codeTop: number;
|
||||
delta: number;
|
||||
gutterLineHeight: number;
|
||||
codeLineHeight: number;
|
||||
}[];
|
||||
}> {
|
||||
return page.locator("body").evaluate(({ ownerDocument }) => {
|
||||
const root = ownerDocument.querySelector('[data-testid="diff-file-0-body"]');
|
||||
if (!root) {
|
||||
throw new Error("Expanded diff body is not mounted");
|
||||
}
|
||||
|
||||
const readRows = (prefix: string, textPrefix: string) =>
|
||||
Array.from(root.querySelectorAll<HTMLElement>(`[data-testid^="${prefix}"]`)).map((row) => {
|
||||
const testId = row.getAttribute("data-testid") ?? "";
|
||||
const index = Number(testId.slice(prefix.length));
|
||||
const rect = row.getBoundingClientRect();
|
||||
const text = root.querySelector<HTMLElement>(`[data-testid="${textPrefix}${index}"]`);
|
||||
const lineHeight = text ? Number.parseFloat(getComputedStyle(text).lineHeight) : 0;
|
||||
return { index, top: rect.top, height: rect.height, lineHeight };
|
||||
});
|
||||
|
||||
const gutters = new Map(
|
||||
readRows("diff-gutter-row-", "diff-gutter-text-").map((row) => [row.index, row]),
|
||||
);
|
||||
const codes = readRows("diff-code-row-", "diff-code-text-");
|
||||
const rows = codes
|
||||
.map((code) => {
|
||||
const gutter = gutters.get(code.index);
|
||||
if (!gutter) {
|
||||
throw new Error(`Missing gutter row ${code.index}`);
|
||||
}
|
||||
return {
|
||||
index: code.index,
|
||||
gutterTop: gutter.top,
|
||||
codeTop: code.top,
|
||||
delta: Math.abs(code.top - gutter.top),
|
||||
gutterLineHeight: gutter.lineHeight,
|
||||
codeLineHeight: code.lineHeight,
|
||||
};
|
||||
})
|
||||
.filter((row) => row.gutterTop >= 0 && row.codeTop >= 0);
|
||||
|
||||
return {
|
||||
maxDelta: Math.max(...rows.map((row) => row.delta)),
|
||||
mismatchedTypography: rows
|
||||
.filter((row) => Math.abs(row.gutterLineHeight - row.codeLineHeight) > 0.5)
|
||||
.map((row) => ({
|
||||
index: row.index,
|
||||
gutterLineHeight: row.gutterLineHeight,
|
||||
codeLineHeight: row.codeLineHeight,
|
||||
})),
|
||||
rows,
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
async function createWorkspaceWithMountedTabDiff(): Promise<DirtyWorkspace> {
|
||||
const repo = await createTempGitRepo("diff-row-alignment-", {
|
||||
files: [{ path: "src/use-mounted-tab-set.ts", content: BEFORE }],
|
||||
});
|
||||
const client = await connectSeedClient();
|
||||
cleanupTasks.push({
|
||||
run: async () => {
|
||||
await client.close().catch(() => undefined);
|
||||
await repo.cleanup().catch(() => undefined);
|
||||
},
|
||||
});
|
||||
|
||||
await writeFile(path.join(repo.path, "src/use-mounted-tab-set.ts"), AFTER);
|
||||
const opened = await client.openProject(repo.path);
|
||||
if (!opened.workspace) {
|
||||
throw new Error(opened.error ?? `Failed to open project ${repo.path}`);
|
||||
}
|
||||
return { id: opened.workspace.id };
|
||||
}
|
||||
|
||||
async function openWorkspaceChanges(page: Page, workspace: DirtyWorkspace): Promise<void> {
|
||||
await page.setViewportSize({ width: 1400, height: 900 });
|
||||
await page.goto(buildHostWorkspaceRoute(getServerId(), workspace.id));
|
||||
await waitForWorkspaceTabsVisible(page);
|
||||
await page.getByRole("button", { name: "Open explorer" }).click();
|
||||
await openChangesInVisibleExplorer(page);
|
||||
await page.getByTestId("diff-file-0").click();
|
||||
await expectExpandedMountedTabDiff(page);
|
||||
}
|
||||
|
||||
async function openChangesInVisibleExplorer(page: Page): Promise<void> {
|
||||
await expect(page.getByTestId("explorer-tab-changes")).toBeVisible({ timeout: 30_000 });
|
||||
await expect(page.getByText("use-mounted-tab-set.ts")).toBeVisible({ timeout: 30_000 });
|
||||
}
|
||||
|
||||
async function expectExpandedMountedTabDiff(page: Page): Promise<void> {
|
||||
await expect(page.getByTestId("diff-file-0-body")).toBeVisible({ timeout: 30_000 });
|
||||
await expect(page.getByText("function createInitialMountedTabIds")).toBeVisible({
|
||||
timeout: 30_000,
|
||||
});
|
||||
}
|
||||
|
||||
async function changeCodeFontSizeFromSettings(page: Page, codeFontSize: number): Promise<void> {
|
||||
await page.getByTestId("sidebar-settings").click();
|
||||
await expect(page).toHaveURL(new RegExp(`${buildSettingsSectionRoute("general")}|/settings$`));
|
||||
await page.getByRole("button", { name: "Appearance" }).click();
|
||||
await page.getByLabel("Code font size").fill(String(codeFontSize));
|
||||
await page.getByLabel("Code font size").press("Enter");
|
||||
await expect(page.getByLabel("Code font size")).toHaveValue(String(codeFontSize));
|
||||
}
|
||||
|
||||
async function returnToWorkspaceChanges(page: Page): Promise<void> {
|
||||
await page.getByTestId("settings-back-to-workspace").click();
|
||||
await waitForWorkspaceTabsVisible(page);
|
||||
await openChangesInVisibleExplorer(page);
|
||||
await expectExpandedMountedTabDiff(page);
|
||||
}
|
||||
|
||||
async function scrollToLowerUnwrappedDiffRows(page: Page): Promise<void> {
|
||||
const lastRowIndex = await page.getByTestId("diff-file-0-body").evaluate((root) => {
|
||||
const rows = Array.from(root.querySelectorAll<HTMLElement>('[data-testid^="diff-code-row-"]'));
|
||||
if (rows.length === 0) {
|
||||
throw new Error("No unwrapped code rows are mounted");
|
||||
}
|
||||
return Math.max(
|
||||
...rows.map((row) => Number((row.getAttribute("data-testid") ?? "").slice(14))),
|
||||
);
|
||||
});
|
||||
await page.getByTestId(`diff-code-row-${lastRowIndex}`).scrollIntoViewIfNeeded();
|
||||
await expect(page.getByTestId(`diff-code-row-${lastRowIndex}`)).toBeVisible();
|
||||
}
|
||||
|
||||
async function expectDiffCodeTextAlignedWithGutterText(
|
||||
page: Page,
|
||||
lines: { codeText: string; lineNumber: string }[],
|
||||
): Promise<void> {
|
||||
const geometries = await readDiffTextGeometry(page, lines);
|
||||
for (const geometry of geometries) {
|
||||
expect(geometry.codeTop, geometry.codeText).toBeCloseTo(geometry.gutterTop, 0);
|
||||
}
|
||||
}
|
||||
|
||||
async function expectHoverCommentButtonAlignedWithCodeLine(
|
||||
page: Page,
|
||||
line: { codeText: string; lineNumber: string },
|
||||
): Promise<void> {
|
||||
const target = await readDiffTextGeometry(page, [line]).then((rows) => rows[0]);
|
||||
if (!target) {
|
||||
throw new Error(`Could not find target line ${line.lineNumber}`);
|
||||
}
|
||||
await page.getByTestId(`diff-code-row-${target.index}`).hover();
|
||||
const geometry = await page
|
||||
.getByTestId(`diff-gutter-action-${target.index}`)
|
||||
.evaluate((action, expectedCodeCenterY) => {
|
||||
const rect = action.getBoundingClientRect();
|
||||
return {
|
||||
actionCenterY: rect.top + rect.height / 2,
|
||||
codeCenterY: expectedCodeCenterY,
|
||||
};
|
||||
}, target.codeCenterY);
|
||||
expect(geometry.actionCenterY).toBeCloseTo(geometry.codeCenterY, 0);
|
||||
}
|
||||
|
||||
async function readDiffTextGeometry(
|
||||
page: Page,
|
||||
lines: { codeText: string; lineNumber: string }[],
|
||||
): Promise<
|
||||
{ index: number; codeText: string; codeTop: number; gutterTop: number; codeCenterY: number }[]
|
||||
> {
|
||||
return page.locator("body").evaluate(({ ownerDocument }, targets) => {
|
||||
const root = ownerDocument.querySelector('[data-testid="explorer-content-area"]');
|
||||
if (!root) {
|
||||
throw new Error("Changes panel is not mounted");
|
||||
}
|
||||
|
||||
const readIndexedElements = (prefix: string) =>
|
||||
Array.from(root.querySelectorAll<HTMLElement>(`[data-testid^="${prefix}"]`)).map(
|
||||
(element) => {
|
||||
const testId = element.getAttribute("data-testid") ?? "";
|
||||
return { index: Number(testId.slice(prefix.length)), element };
|
||||
},
|
||||
);
|
||||
|
||||
const gutterTexts = readIndexedElements("diff-gutter-text-");
|
||||
const codeTexts = readIndexedElements("diff-code-text-");
|
||||
|
||||
return targets.map((target) => {
|
||||
const gutter = gutterTexts.find(
|
||||
({ element }) => (element.textContent ?? "").trim() === target.lineNumber,
|
||||
);
|
||||
if (!gutter) {
|
||||
throw new Error(`Could not find gutter line ${target.lineNumber}`);
|
||||
}
|
||||
const code = codeTexts.find(
|
||||
({ index, element }) =>
|
||||
index === gutter.index && (element.textContent ?? "").includes(target.codeText),
|
||||
);
|
||||
if (!code) {
|
||||
throw new Error(`Could not find code row ${target.codeText}`);
|
||||
}
|
||||
|
||||
const codeRect = code.element.getBoundingClientRect();
|
||||
const gutterRect = gutter.element.getBoundingClientRect();
|
||||
|
||||
return {
|
||||
index: gutter.index,
|
||||
codeText: target.codeText,
|
||||
codeTop: codeRect.top,
|
||||
gutterTop: gutterRect.top,
|
||||
codeCenterY: codeRect.top + codeRect.height / 2,
|
||||
};
|
||||
});
|
||||
}, lines);
|
||||
}
|
||||
Reference in New Issue
Block a user