diff --git a/packages/review-tutor/src/page-script.ts b/packages/review-tutor/src/page-script.ts index af80771..f439036 100644 --- a/packages/review-tutor/src/page-script.ts +++ b/packages/review-tutor/src/page-script.ts @@ -88,6 +88,9 @@ export const pageScript = String.raw` const MAX_SELECTED_BYTES = 16 * 1024; const MAX_CONTEXT_BYTES = 32 * 1024; const MOBILE_BREAKPOINT = 860; + const RAIL_TRANSITION_MS = 200; + const COLLAPSED_RAIL_WIDTH = 44; + const ROWS_PER_CHUNK = 120; const encoder = new TextEncoder(); function readQuizEntryIds() { try { @@ -531,6 +534,10 @@ export const pageScript = String.raw` const code = document.createElement("span"); code.className = "evidence-code"; code.textContent = evidence.line + " " + evidence.text; + item.append(code); + evidenceList.append(item); + // Evidence inside an unchanged neighbour has no diff row to land on, so it gets no actions. + if (!files.some((file) => file.path === evidence.path)) return; const open = document.createElement("button"); open.type = "button"; open.className = "ghost open-in-diff"; @@ -544,8 +551,7 @@ export const pageScript = String.raw` const targetIndex = tutorEvidenceIndex(edge, evidenceIndex); jumpToEvidence(edge.evidence[targetIndex], edge.status, targetIndex, edge.evidence, true); }); - item.append(code, askTutor, open); - evidenceList.append(item); + item.append(askTutor, open); }); row.addEventListener("click", () => { const expanded = row.getAttribute("aria-expanded") === "true"; @@ -1559,12 +1565,19 @@ export const pageScript = String.raw` rows.className = "rows"; rows.setAttribute("role", "group"); rows.setAttribute("aria-label", file.path + " selectable lines"); + // Rows are grouped into chunks that skip layout while off-screen, so large diffs stay cheap to resize and scroll. + let chunk = null; file.lines.forEach((data, rowIndex) => { + if (rowIndex % ROWS_PER_CHUNK === 0) { + chunk = document.createElement("div"); + chunk.className = "rows-chunk"; + rows.append(chunk); + } const row = document.createElement("div"); row.className = "diff-row " + data.kind; if (data.kind === "hunk" || data.kind === "meta") { row.textContent = data.text; - rows.append(row); + chunk.append(row); return; } row.dataset.file = String(fileIndex); @@ -1596,7 +1609,7 @@ export const pageScript = String.raw` ), makeSpan("code", data.text), ); - rows.append(row); + chunk.append(row); }); rows.addEventListener("pointerdown", (event) => { const row = event.target.closest(".diff-row[data-row]"); @@ -2330,8 +2343,20 @@ export const pageScript = String.raw` if (innerWidth <= MOBILE_BREAKPOINT) { closeConfig(); return; } railCollapsed = !railCollapsed; sessionStorage.setItem("reviewTutorRailCollapsed", railCollapsed ? "1" : "0"); + holdDiffWidth(); applyRail(); }); + // While the rail width transitions, the diff holds its final width so its rows lay out once, not every frame. + function holdDiffWidth() { + if (typeof matchMedia !== "function" || matchMedia("(prefers-reduced-motion: reduce)").matches) return; + const scroll = element("diff-scroll"); + scroll.style.width = ""; + const width = scroll.getBoundingClientRect().width; + const railWidth = document.querySelector(".rail").getBoundingClientRect().width; + scroll.style.width = (railCollapsed ? width + railWidth - COLLAPSED_RAIL_WIDTH : width) + "px"; + clearTimeout(holdDiffWidth.timer); + holdDiffWidth.timer = setTimeout(() => { scroll.style.width = ""; }, RAIL_TRANSITION_MS + 20); + } element("jump-log").addEventListener("click", () => { if (innerWidth <= MOBILE_BREAKPOINT) closeConfig(false); setView("log", true); diff --git a/packages/review-tutor/src/page-styles.ts b/packages/review-tutor/src/page-styles.ts index 6c1f349..071dd25 100644 --- a/packages/review-tutor/src/page-styles.ts +++ b/packages/review-tutor/src/page-styles.ts @@ -51,11 +51,11 @@ color:var(--rose)} .ghost { background:transparent} .workbench { -height:calc(100vh - var(--topbar-h));display:grid;grid-template-columns:minmax(0,1fr) clamp(320px,30vw,400px)} +height:calc(100vh - var(--topbar-h));display:grid;grid-template-columns:minmax(0,1fr) clamp(320px,30vw,400px);transition:grid-template-columns 200ms ease} .workbench.rail-collapsed { grid-template-columns:minmax(0,1fr) 44px} .diff-pane { -min-width:0;min-height:0;display:flex;flex-direction:column;border-right:1px solid var(--hairline)} +min-width:0;min-height:0;display:flex;flex-direction:column;overflow:hidden;border-right:1px solid var(--hairline)} .source-setup { min-height:0;max-height:100%;overflow:auto;flex:0 1 auto;margin:20px;border:1px solid var(--hairline-strong);border-radius:var(--radius-card);background:var(--surface-1);padding:20px} .source-setup[hidden] { @@ -122,6 +122,8 @@ font:12px var(--font-mono);overflow:hidden;text-overflow:ellipsis;white-space:no margin-left:auto;flex:none;font:12px var(--font-mono);font-variant-numeric:tabular-nums} .rows { min-width:0} +.rows-chunk { +content-visibility:auto;contain-intrinsic-block-size:auto 3000px} .diff-row { display:grid;grid-template-columns:52px 52px 24px minmax(0,1fr);min-height:25px;align-items:stretch;font:12.5px/25px var(--font-mono);position:relative} .diff-row.meta { @@ -165,7 +167,7 @@ color:var(--text)} .code { white-space:pre-wrap;overflow-wrap:anywhere;padding:0 14px} .rail { -min-height:0;overflow-y:auto;background:var(--surface-1)} +position:relative;min-height:0;overflow:hidden auto;background:var(--surface-1)} .tutor { display:none;margin:8px 14px 16px;max-width:760px;border:1px solid var(--hairline-strong);border-radius:var(--radius-card);background:var(--surface-1);padding:16px 18px} .tutor.open { @@ -173,17 +175,17 @@ display:block} .diff-pane>.tutor.open { min-height:0;max-height:100%;overflow:auto;flex:0 1 auto} .rail-head { -display:flex;align-items:center;justify-content:space-between;gap:8px;padding:14px var(--rail-pad) 0} +display:flex;align-items:center;gap:8px;padding:14px 44px 0 var(--rail-pad)} .rail-toggle { -width:28px;min-height:28px;padding:0;border:0;background:transparent;color:var(--muted);font-size:14px} +position:absolute;top:14px;right:8px;width:28px;min-height:28px;padding:0;border:0;background:transparent;color:var(--muted);font-size:14px} .rail-toggle:hover { color:var(--text)} .rail-collapsed .rail-head { -flex-direction:column;justify-content:flex-start;gap:12px;padding:10px 0} +justify-content:center;padding:50px 0 0} .rail-collapsed .rail-eyebrow { writing-mode:vertical-rl} .config-section { -padding:14px var(--rail-pad) 28px} +width:clamp(320px,30vw,400px);padding:14px var(--rail-pad) 28px} .config-section .field,.config-section .two { margin-bottom:var(--field-gap)} .config-actions { @@ -523,9 +525,9 @@ display:block;position:fixed;inset:0;z-index:50;overflow-x:hidden;overflow-y:scr .rail-head { display:flex;padding:0;flex-direction:row} .rail-toggle { -width:auto;min-height:44px;padding:0 14px} +position:static;width:auto;min-height:44px;padding:0 14px} .config-section { -padding:16px 0 28px} +width:auto;padding:16px 0 28px} .config-actions button { min-height:44px} .tutor.open { diff --git a/packages/review-tutor/test/page.test.ts b/packages/review-tutor/test/page.test.ts index f564200..3241f7b 100644 --- a/packages/review-tutor/test/page.test.ts +++ b/packages/review-tutor/test/page.test.ts @@ -290,7 +290,7 @@ describe("Review Tutor composed page", () => { expect(pageHtml).toContain("@media(max-width:860px)"); expect(pageHtml).toContain(".diff-pane {\nmin-width:0;min-height:0;"); expect(pageHtml).toContain(".toolbar select {\nwidth:auto;flex:1 1 120px;min-width:120px;max-width:280px}"); - expect(pageHtml).toContain(".rail {\nmin-height:0;overflow-y:auto;"); + expect(pageHtml).toContain(".rail {\nposition:relative;min-height:0;overflow:hidden auto;"); expect(pageHtml).toContain('role="region" aria-labelledby="tutor-title"'); expect(pageHtml).toContain(".diff-pane>.tutor.open"); expect(pageHtml).toContain("max-height:100%;overflow:auto"); @@ -1181,6 +1181,31 @@ describe("Review Tutor composed page", () => { expect(document.querySelectorAll(".diff-row.structure-landing")).toHaveLength(0); }); + it("offers no diff actions on evidence that lives in an unchanged neighbour", async () => { + const withNeighbour = { + ...structure, + neighbours: { state: "on", count: 1 }, + files: [...structure.files, { path: "src/ctx.ts", status: "context", additions: 0, deletions: 0, analyzed: true }], + edges: [ + ...structure.edges, + { from: "src/ctx.ts", to: "src/a.ts", kind: "import", typeOnly: false, status: "unchanged", specifier: "./a.js", evidence: [{ path: "src/ctx.ts", line: 3, text: "import { a } from './a.js';" }] }, + ], + }; + const { document } = await boot({ structureResponses: [withNeighbour] }); + byId(document, "view-structure").click(); + await flush(); + const rows = [...document.querySelectorAll(".connection-row")] as any[]; + const contextRow = rows.find((row) => row.querySelector(".connection-target")?.textContent === "src/a.ts" && row.closest("[data-path='src/ctx.ts']")) + ?? rows.find((row) => byId(document, row.getAttribute("aria-controls")).textContent?.includes("import { a } from './a.js';")); + expect(contextRow).toBeDefined(); + const evidence = byId(document, contextRow.getAttribute("aria-controls")); + expect(evidence.querySelector(".evidence-code")?.textContent).toBe("3 import { a } from './a.js';"); + expect(evidence.querySelector(".ask-tutor-evidence")).toBeNull(); + expect(evidence.querySelector(".open-in-diff")).toBeNull(); + const changedRow = rows.find((row) => byId(document, row.getAttribute("aria-controls")).textContent?.includes("import type { a }")); + expect(byId(document, changedRow.getAttribute("aria-controls")).querySelector(".ask-tutor-evidence")).not.toBeNull(); + }); + it("asks from graph evidence through the existing list selection path", async () => { const { document } = await boot(); byId(document, "view-structure").click(); @@ -1438,9 +1463,9 @@ describe("Review Tutor composed page", () => { byId(document, "view-structure").click(); rows[1].click(); - (rows[1].nextElementSibling.querySelector(".open-in-diff") as any).click(); - expect(byId(document, "structure-section").hidden).toBe(false); - expect(byId(document, "lifecycle").textContent).toBe("not-rendered.ts is not in the diff view."); + expect(rows[1].nextElementSibling.querySelector(".evidence-code")).not.toBeNull(); + expect(rows[1].nextElementSibling.querySelector(".open-in-diff")).toBeNull(); + expect(rows[1].nextElementSibling.querySelector(".ask-tutor-evidence")).toBeNull(); }); it("uses touch targets and compact chips without making structure heads sticky", async () => { @@ -1627,6 +1652,21 @@ describe("Review Tutor composed page", () => { expect(byId(document, "history-pager").hidden).toBe(true); }); + it("animates the rail width cheaply: off-screen rows skip layout and rail text never rewraps", async () => { + const { document } = await boot(); + const css = document.querySelector("style")?.textContent ?? ""; + expect(css).toContain("transition:grid-template-columns 200ms ease"); + expect(css).toContain(".rows-chunk {\ncontent-visibility:auto;contain-intrinsic-block-size:auto 3000px}"); + expect(document.querySelectorAll(".rows-chunk .diff-row").length).toBe(document.querySelectorAll(".diff-row").length); + expect(css).toContain(".rail {\nposition:relative;min-height:0;overflow:hidden auto;"); + expect(css).toContain(".config-section {\nwidth:clamp(320px,30vw,400px);"); + expect(css).toContain(".rail-toggle {\nposition:absolute;top:14px;right:8px;"); + byId(document, "toggle-rail").click(); + expect(document.querySelector(".workbench")?.classList.contains("rail-collapsed")).toBe(true); + byId(document, "toggle-rail").click(); + expect(document.querySelector(".workbench")?.classList.contains("rail-collapsed")).toBe(false); + }); + it("opens mobile configuration as the sole dialog and restores the desktop collapse preference", async () => { const { window, document } = await boot({ width: 390, railCollapsed: true }); byId(document, "open-config").click();