From f87224611db18c42f0abb4540afa42104f710624 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elberte=20Pl=C3=ADnio?= Date: Tue, 25 Aug 2026 11:19:03 -0300 Subject: [PATCH 1/7] feat(review-tutor): animate the configuration rail and drop diff actions on neighbour evidence --- packages/review-tutor/src/page-script.ts | 7 ++++-- packages/review-tutor/src/page-styles.ts | 2 +- packages/review-tutor/test/page.test.ts | 31 +++++++++++++++++++++--- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/packages/review-tutor/src/page-script.ts b/packages/review-tutor/src/page-script.ts index af80771..c3189d3 100644 --- a/packages/review-tutor/src/page-script.ts +++ b/packages/review-tutor/src/page-script.ts @@ -531,6 +531,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 +548,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"; diff --git a/packages/review-tutor/src/page-styles.ts b/packages/review-tutor/src/page-styles.ts index 6c1f349..ebf7017 100644 --- a/packages/review-tutor/src/page-styles.ts +++ b/packages/review-tutor/src/page-styles.ts @@ -51,7 +51,7 @@ 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 { diff --git a/packages/review-tutor/test/page.test.ts b/packages/review-tutor/test/page.test.ts index f564200..790eb88 100644 --- a/packages/review-tutor/test/page.test.ts +++ b/packages/review-tutor/test/page.test.ts @@ -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 () => { From a028e4abd238bede1ca68d701d8dfb54b6f44453 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elberte=20Pl=C3=ADnio?= Date: Tue, 25 Aug 2026 11:44:47 -0300 Subject: [PATCH 2/7] fix(review-tutor): slide the rail with transforms and keep its toggle in place --- packages/review-tutor/src/page-script.ts | 10 +++++++ packages/review-tutor/src/page-styles.ts | 12 ++++----- packages/review-tutor/test/page.test.ts | 34 +++++++++++++++++++++++- 3 files changed, 49 insertions(+), 7 deletions(-) diff --git a/packages/review-tutor/src/page-script.ts b/packages/review-tutor/src/page-script.ts index c3189d3..7f1a740 100644 --- a/packages/review-tutor/src/page-script.ts +++ b/packages/review-tutor/src/page-script.ts @@ -2333,8 +2333,18 @@ export const pageScript = String.raw` if (innerWidth <= MOBILE_BREAKPOINT) { closeConfig(); return; } railCollapsed = !railCollapsed; sessionStorage.setItem("reviewTutorRailCollapsed", railCollapsed ? "1" : "0"); + const rail = document.querySelector(".rail"); + const before = rail.getBoundingClientRect().left; applyRail(); + slideRail(rail, before - rail.getBoundingClientRect().left); }); + // One layout pass, then transforms only: the rail slides into place and the diff is unclipped in step with it. + function slideRail(rail, offset) { + if (!offset || typeof rail.animate !== "function" || typeof matchMedia !== "function" || matchMedia("(prefers-reduced-motion: reduce)").matches) return; + const timing = { duration: 200, easing: "ease" }; + rail.animate([{ transform: "translateX(" + offset + "px)" }, { transform: "none" }], timing); + if (offset < 0) element("diff-pane").animate([{ clipPath: "inset(0 " + -offset + "px 0 0)" }, { clipPath: "inset(0)" }], timing); + } 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 ebf7017..dd78c0d 100644 --- a/packages/review-tutor/src/page-styles.ts +++ b/packages/review-tutor/src/page-styles.ts @@ -51,7 +51,7 @@ 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);transition:grid-template-columns 200ms ease} +height:calc(100vh - var(--topbar-h));display:grid;grid-template-columns:minmax(0,1fr) clamp(320px,30vw,400px)} .workbench.rail-collapsed { grid-template-columns:minmax(0,1fr) 44px} .diff-pane { @@ -165,7 +165,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-y: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,13 +173,13 @@ 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 { @@ -523,7 +523,7 @@ 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} .config-actions button { diff --git a/packages/review-tutor/test/page.test.ts b/packages/review-tutor/test/page.test.ts index 790eb88..7fbc52a 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-y: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"); @@ -1652,6 +1652,38 @@ describe("Review Tutor composed page", () => { expect(byId(document, "history-pager").hidden).toBe(true); }); + it("collapses the rail with one layout pass and transform-only animation", async () => { + const { window, document } = await boot(); + const rail = document.querySelector(".rail") as any; + const diffPane = byId(document, "diff-pane") as any; + const calls: Array<{ target: string; keyframes: any[] }> = []; + for (const [target, node] of [["rail", rail], ["diff", diffPane]] as const) { + node.animate = (keyframes: any[]) => { calls.push({ target, keyframes }); return {}; }; + } + const original = document.querySelector(".workbench")!; + rail.getBoundingClientRect = () => ({ left: original.classList.contains("rail-collapsed") ? 1300 : 1000 }); + let reduced = false; + (window as any).matchMedia = () => ({ matches: reduced }); + + byId(document, "toggle-rail").click(); + expect(original.classList.contains("rail-collapsed")).toBe(true); + expect(calls.map((call) => call.target)).toEqual(["rail", "diff"]); + expect(calls[0]?.keyframes).toEqual([{ transform: "translateX(-300px)" }, { transform: "none" }]); + expect(calls[1]?.keyframes).toEqual([{ clipPath: "inset(0 300px 0 0)" }, { clipPath: "inset(0)" }]); + + calls.length = 0; + byId(document, "toggle-rail").click(); + expect(original.classList.contains("rail-collapsed")).toBe(false); + expect(calls.map((call) => call.target)).toEqual(["rail"]); + expect(calls[0]?.keyframes[0]).toEqual({ transform: "translateX(300px)" }); + + calls.length = 0; + reduced = true; + byId(document, "toggle-rail").click(); + expect(original.classList.contains("rail-collapsed")).toBe(true); + expect(calls).toEqual([]); + }); + 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(); From d6bc5642d62c00f86b1cc97008256b72df7a9dc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elberte=20Pl=C3=ADnio?= Date: Tue, 25 Aug 2026 12:04:17 -0300 Subject: [PATCH 3/7] fix(review-tutor): slide the expanded rail itself so the collapse is visible --- packages/review-tutor/src/page-script.ts | 25 +++++++++++++------- packages/review-tutor/test/page.test.ts | 30 +++++++++++------------- 2 files changed, 30 insertions(+), 25 deletions(-) diff --git a/packages/review-tutor/src/page-script.ts b/packages/review-tutor/src/page-script.ts index 7f1a740..91bfcce 100644 --- a/packages/review-tutor/src/page-script.ts +++ b/packages/review-tutor/src/page-script.ts @@ -88,6 +88,7 @@ export const pageScript = String.raw` const MAX_SELECTED_BYTES = 16 * 1024; const MAX_CONTEXT_BYTES = 32 * 1024; const MOBILE_BREAKPOINT = 860; + const COLLAPSED_RAIL_WIDTH = 44; const encoder = new TextEncoder(); function readQuizEntryIds() { try { @@ -2334,16 +2335,22 @@ export const pageScript = String.raw` railCollapsed = !railCollapsed; sessionStorage.setItem("reviewTutorRailCollapsed", railCollapsed ? "1" : "0"); const rail = document.querySelector(".rail"); - const before = rail.getBoundingClientRect().left; - applyRail(); - slideRail(rail, before - rail.getBoundingClientRect().left); + if (railCollapsed) { + // Slide the panel out first, then relayout once; expanding relayouts first and slides the panel in. + const slide = slideRail(rail, "out"); + if (slide) slide.onfinish = slide.oncancel = applyRail; + else applyRail(); + } else { + applyRail(); + slideRail(rail, "in"); + } }); - // One layout pass, then transforms only: the rail slides into place and the diff is unclipped in step with it. - function slideRail(rail, offset) { - if (!offset || typeof rail.animate !== "function" || typeof matchMedia !== "function" || matchMedia("(prefers-reduced-motion: reduce)").matches) return; - const timing = { duration: 200, easing: "ease" }; - rail.animate([{ transform: "translateX(" + offset + "px)" }, { transform: "none" }], timing); - if (offset < 0) element("diff-pane").animate([{ clipPath: "inset(0 " + -offset + "px 0 0)" }, { clipPath: "inset(0)" }], timing); + function slideRail(rail, direction) { + if (typeof rail.animate !== "function" || typeof matchMedia !== "function" || matchMedia("(prefers-reduced-motion: reduce)").matches) return null; + const distance = rail.getBoundingClientRect().width - COLLAPSED_RAIL_WIDTH; + if (distance <= 0) return null; + const away = { transform: "translateX(" + distance + "px)" }, home = { transform: "none" }; + return rail.animate(direction === "out" ? [home, away] : [away, home], { duration: 200, easing: "ease" }); } element("jump-log").addEventListener("click", () => { if (innerWidth <= MOBILE_BREAKPOINT) closeConfig(false); diff --git a/packages/review-tutor/test/page.test.ts b/packages/review-tutor/test/page.test.ts index 7fbc52a..b2a9a14 100644 --- a/packages/review-tutor/test/page.test.ts +++ b/packages/review-tutor/test/page.test.ts @@ -1652,36 +1652,34 @@ describe("Review Tutor composed page", () => { expect(byId(document, "history-pager").hidden).toBe(true); }); - it("collapses the rail with one layout pass and transform-only animation", async () => { + it("slides the expanded rail with transforms around a single layout pass", async () => { const { window, document } = await boot(); const rail = document.querySelector(".rail") as any; - const diffPane = byId(document, "diff-pane") as any; - const calls: Array<{ target: string; keyframes: any[] }> = []; - for (const [target, node] of [["rail", rail], ["diff", diffPane]] as const) { - node.animate = (keyframes: any[]) => { calls.push({ target, keyframes }); return {}; }; - } - const original = document.querySelector(".workbench")!; - rail.getBoundingClientRect = () => ({ left: original.classList.contains("rail-collapsed") ? 1300 : 1000 }); + const workbench = document.querySelector(".workbench")!; + const calls: any[][] = []; + let animation: any; + rail.animate = (keyframes: any[]) => { calls.push(keyframes); animation = {}; return animation; }; + rail.getBoundingClientRect = () => ({ width: 400 }); let reduced = false; (window as any).matchMedia = () => ({ matches: reduced }); byId(document, "toggle-rail").click(); - expect(original.classList.contains("rail-collapsed")).toBe(true); - expect(calls.map((call) => call.target)).toEqual(["rail", "diff"]); - expect(calls[0]?.keyframes).toEqual([{ transform: "translateX(-300px)" }, { transform: "none" }]); - expect(calls[1]?.keyframes).toEqual([{ clipPath: "inset(0 300px 0 0)" }, { clipPath: "inset(0)" }]); + expect(calls).toEqual([[{ transform: "none" }, { transform: "translateX(356px)" }]]); + expect(workbench.classList.contains("rail-collapsed")).toBe(false); + animation.onfinish(); + expect(workbench.classList.contains("rail-collapsed")).toBe(true); + expect(byId(document, "config-section").hidden).toBe(true); calls.length = 0; byId(document, "toggle-rail").click(); - expect(original.classList.contains("rail-collapsed")).toBe(false); - expect(calls.map((call) => call.target)).toEqual(["rail"]); - expect(calls[0]?.keyframes[0]).toEqual({ transform: "translateX(300px)" }); + expect(workbench.classList.contains("rail-collapsed")).toBe(false); + expect(calls).toEqual([[{ transform: "translateX(356px)" }, { transform: "none" }]]); calls.length = 0; reduced = true; byId(document, "toggle-rail").click(); - expect(original.classList.contains("rail-collapsed")).toBe(true); expect(calls).toEqual([]); + expect(workbench.classList.contains("rail-collapsed")).toBe(true); }); it("opens mobile configuration as the sole dialog and restores the desktop collapse preference", async () => { From d180382f68c3c47def4a36cd0261a020edf0bf61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elberte=20Pl=C3=ADnio?= Date: Tue, 25 Aug 2026 12:12:29 -0300 Subject: [PATCH 4/7] fix(review-tutor): animate the rail width without relaying out the diff every frame --- packages/review-tutor/src/page-script.ts | 28 ++++++++--------- packages/review-tutor/src/page-styles.ts | 12 ++++---- packages/review-tutor/test/page.test.ts | 38 +++++++----------------- 3 files changed, 29 insertions(+), 49 deletions(-) diff --git a/packages/review-tutor/src/page-script.ts b/packages/review-tutor/src/page-script.ts index 91bfcce..6fb4631 100644 --- a/packages/review-tutor/src/page-script.ts +++ b/packages/review-tutor/src/page-script.ts @@ -89,6 +89,7 @@ export const pageScript = String.raw` const MAX_CONTEXT_BYTES = 32 * 1024; const MOBILE_BREAKPOINT = 860; const COLLAPSED_RAIL_WIDTH = 44; + const RAIL_TRANSITION_MS = 200; const encoder = new TextEncoder(); function readQuizEntryIds() { try { @@ -2334,23 +2335,18 @@ export const pageScript = String.raw` if (innerWidth <= MOBILE_BREAKPOINT) { closeConfig(); return; } railCollapsed = !railCollapsed; sessionStorage.setItem("reviewTutorRailCollapsed", railCollapsed ? "1" : "0"); - const rail = document.querySelector(".rail"); - if (railCollapsed) { - // Slide the panel out first, then relayout once; expanding relayouts first and slides the panel in. - const slide = slideRail(rail, "out"); - if (slide) slide.onfinish = slide.oncancel = applyRail; - else applyRail(); - } else { - applyRail(); - slideRail(rail, "in"); - } + holdDiffWidth(); + applyRail(); }); - function slideRail(rail, direction) { - if (typeof rail.animate !== "function" || typeof matchMedia !== "function" || matchMedia("(prefers-reduced-motion: reduce)").matches) return null; - const distance = rail.getBoundingClientRect().width - COLLAPSED_RAIL_WIDTH; - if (distance <= 0) return null; - const away = { transform: "translateX(" + distance + "px)" }, home = { transform: "none" }; - return rail.animate(direction === "out" ? [home, away] : [away, home], { duration: 200, easing: "ease" }); + // While the rail width transitions, the diff keeps 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"); + const width = element("diff-pane").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); diff --git a/packages/review-tutor/src/page-styles.ts b/packages/review-tutor/src/page-styles.ts index dd78c0d..5c65bb5 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] { @@ -123,7 +123,7 @@ margin-left:auto;flex:none;font:12px var(--font-mono);font-variant-numeric:tabul .rows { min-width:0} .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} +content-visibility:auto;contain-intrinsic-block-size:auto 25px;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 { display:block;padding:3px 14px;min-height:0;color:var(--muted);background:transparent;font:10.5px/1.5 var(--font-mono);white-space:pre-wrap} .diff-row.hunk { @@ -165,7 +165,7 @@ color:var(--text)} .code { white-space:pre-wrap;overflow-wrap:anywhere;padding:0 14px} .rail { -position:relative;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 { @@ -183,7 +183,7 @@ 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 { @@ -525,7 +525,7 @@ display:flex;padding:0;flex-direction:row} .rail-toggle { 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 b2a9a14..8e6d244 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 {\nposition:relative;min-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"); @@ -1652,34 +1652,18 @@ describe("Review Tutor composed page", () => { expect(byId(document, "history-pager").hidden).toBe(true); }); - it("slides the expanded rail with transforms around a single layout pass", async () => { - const { window, document } = await boot(); - const rail = document.querySelector(".rail") as any; - const workbench = document.querySelector(".workbench")!; - const calls: any[][] = []; - let animation: any; - rail.animate = (keyframes: any[]) => { calls.push(keyframes); animation = {}; return animation; }; - rail.getBoundingClientRect = () => ({ width: 400 }); - let reduced = false; - (window as any).matchMedia = () => ({ matches: reduced }); - - byId(document, "toggle-rail").click(); - expect(calls).toEqual([[{ transform: "none" }, { transform: "translateX(356px)" }]]); - expect(workbench.classList.contains("rail-collapsed")).toBe(false); - animation.onfinish(); - expect(workbench.classList.contains("rail-collapsed")).toBe(true); - expect(byId(document, "config-section").hidden).toBe(true); - - calls.length = 0; + 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(".diff-row {\ncontent-visibility:auto;contain-intrinsic-block-size:auto 25px;"); + 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(workbench.classList.contains("rail-collapsed")).toBe(false); - expect(calls).toEqual([[{ transform: "translateX(356px)" }, { transform: "none" }]]); - - calls.length = 0; - reduced = true; + expect(document.querySelector(".workbench")?.classList.contains("rail-collapsed")).toBe(true); byId(document, "toggle-rail").click(); - expect(calls).toEqual([]); - expect(workbench.classList.contains("rail-collapsed")).toBe(true); + 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 () => { From d0f8eabef1834deb7998e06b9425943557051f14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elberte=20Pl=C3=ADnio?= Date: Tue, 25 Aug 2026 12:16:40 -0300 Subject: [PATCH 5/7] perf(review-tutor): chunk diff rows so off-screen rows skip layout --- packages/review-tutor/src/page-script.ts | 19 ++++++++++++------- packages/review-tutor/src/page-styles.ts | 4 +++- packages/review-tutor/test/page.test.ts | 3 ++- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/packages/review-tutor/src/page-script.ts b/packages/review-tutor/src/page-script.ts index 6fb4631..b9c0024 100644 --- a/packages/review-tutor/src/page-script.ts +++ b/packages/review-tutor/src/page-script.ts @@ -88,8 +88,8 @@ export const pageScript = String.raw` const MAX_SELECTED_BYTES = 16 * 1024; const MAX_CONTEXT_BYTES = 32 * 1024; const MOBILE_BREAKPOINT = 860; - const COLLAPSED_RAIL_WIDTH = 44; const RAIL_TRANSITION_MS = 200; + const ROWS_PER_CHUNK = 120; const encoder = new TextEncoder(); function readQuizEntryIds() { try { @@ -1564,12 +1564,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); @@ -1601,7 +1608,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]"); @@ -2338,13 +2345,11 @@ export const pageScript = String.raw` holdDiffWidth(); applyRail(); }); - // While the rail width transitions, the diff keeps its final width so its rows lay out once, not every frame. + // While the rail width transitions, the diff keeps its current width so its rows lay out once at the end, not every frame. function holdDiffWidth() { if (typeof matchMedia !== "function" || matchMedia("(prefers-reduced-motion: reduce)").matches) return; const scroll = element("diff-scroll"); - const width = element("diff-pane").getBoundingClientRect().width; - const railWidth = document.querySelector(".rail").getBoundingClientRect().width; - scroll.style.width = (railCollapsed ? width + railWidth - COLLAPSED_RAIL_WIDTH : width) + "px"; + scroll.style.width = scroll.getBoundingClientRect().width + "px"; clearTimeout(holdDiffWidth.timer); holdDiffWidth.timer = setTimeout(() => { scroll.style.width = ""; }, RAIL_TRANSITION_MS + 20); } diff --git a/packages/review-tutor/src/page-styles.ts b/packages/review-tutor/src/page-styles.ts index 5c65bb5..071dd25 100644 --- a/packages/review-tutor/src/page-styles.ts +++ b/packages/review-tutor/src/page-styles.ts @@ -122,8 +122,10 @@ 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 { -content-visibility:auto;contain-intrinsic-block-size:auto 25px;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} +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 { display:block;padding:3px 14px;min-height:0;color:var(--muted);background:transparent;font:10.5px/1.5 var(--font-mono);white-space:pre-wrap} .diff-row.hunk { diff --git a/packages/review-tutor/test/page.test.ts b/packages/review-tutor/test/page.test.ts index 8e6d244..3241f7b 100644 --- a/packages/review-tutor/test/page.test.ts +++ b/packages/review-tutor/test/page.test.ts @@ -1656,7 +1656,8 @@ describe("Review Tutor composed page", () => { const { document } = await boot(); const css = document.querySelector("style")?.textContent ?? ""; expect(css).toContain("transition:grid-template-columns 200ms ease"); - expect(css).toContain(".diff-row {\ncontent-visibility:auto;contain-intrinsic-block-size:auto 25px;"); + 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;"); From babe43bfbcfa02427b2c98d1b7853e9c8101be43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elberte=20Pl=C3=ADnio?= Date: Tue, 25 Aug 2026 12:18:57 -0300 Subject: [PATCH 6/7] fix(review-tutor): let the diff grow in step with the collapsing rail --- packages/review-tutor/src/page-script.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/review-tutor/src/page-script.ts b/packages/review-tutor/src/page-script.ts index b9c0024..82a2dd0 100644 --- a/packages/review-tutor/src/page-script.ts +++ b/packages/review-tutor/src/page-script.ts @@ -89,6 +89,7 @@ export const pageScript = String.raw` 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() { @@ -2345,11 +2346,13 @@ export const pageScript = String.raw` holdDiffWidth(); applyRail(); }); - // While the rail width transitions, the diff keeps its current width so its rows lay out once at the end, not every frame. + // 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 = scroll.getBoundingClientRect().width + "px"; + 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); } From 253704be2833d962ecf16105a46d7b7b4f1763a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elberte=20Pl=C3=ADnio?= Date: Tue, 25 Aug 2026 12:20:47 -0300 Subject: [PATCH 7/7] fix(review-tutor): measure the diff width from a released hold --- packages/review-tutor/src/page-script.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/review-tutor/src/page-script.ts b/packages/review-tutor/src/page-script.ts index 82a2dd0..f439036 100644 --- a/packages/review-tutor/src/page-script.ts +++ b/packages/review-tutor/src/page-script.ts @@ -2350,6 +2350,7 @@ export const pageScript = String.raw` 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";