diff --git a/src/review/unified-comment-bridge.ts b/src/review/unified-comment-bridge.ts index 426da542c6..e3ffcc9222 100644 --- a/src/review/unified-comment-bridge.ts +++ b/src/review/unified-comment-bridge.ts @@ -458,8 +458,13 @@ export function buildBeforeAfterCollapsible(routes: CaptureRoute[]): UnifiedColl .replace(/`/g, "\\`") .replace(/\|/g, "\\|") .replace(/[<>]/g, (char) => (char === "<" ? "<" : ">"))}\``; + // #6324: the same one-line caption UNDER the thumbnail that the screenshot-table contract itself requires + // of contributors (see e.g. .claude/skills/metagraphed/SKILL.md's Phase B2 in JSONbored/metagraphed) -- + // previously only present as the invisible `alt` attribute, never rendered as visible text.
(not a + // literal newline, which would break the GFM table row) keeps the caption inside the same cell; is + // the same de-emphasized styling this table already uses for its own footer legend line below. const cell = (url: string | undefined, label: string): string => - url ? `${attr(label)}` : "—"; + url ? `${attr(label)}
${attr(label)}` : "—"; const rows: string[] = []; let hasAnyDiff = false; for (const route of routes) { @@ -506,8 +511,9 @@ export function buildScrollPreviewCollapsible(routes: CaptureRoute[]): UnifiedCo .replace(/`/g, "\\`") .replace(/\|/g, "\\|") .replace(/[<>]/g, (char) => (char === "<" ? "<" : ">"))}\``; + // #6324: same visible one-line caption as buildBeforeAfterCollapsible's own cell() -- see its doc comment. const cell = (url: string | undefined, label: string): string => - url ? `${attr(label)}` : "—"; + url ? `${attr(label)}
${attr(label)}` : "—"; const rows: string[] = []; for (const route of routes) { if (!route.beforeGifUrl && !route.afterGifUrl) continue; diff --git a/test/unit/visual-collapsible.test.ts b/test/unit/visual-collapsible.test.ts index 2effea8005..5ad3955434 100644 --- a/test/unit/visual-collapsible.test.ts +++ b/test/unit/visual-collapsible.test.ts @@ -50,6 +50,19 @@ describe("buildBeforeAfterCollapsible", () => { expect(c?.body).toContain("| `/` | desktop | — | { + const c = buildBeforeAfterCollapsible(routes); + // The caption text is the SAME string already used as the (invisible) alt attribute -- now also visible. + expect(c?.body).toContain('before /app/analytics
before /app/analytics'); + expect(c?.body).toContain('after /app/analytics
after /app/analytics'); + }); + + it("#6324: a dash cell has no caption to escape (no
emitted for a missing slot)", () => { + const c = buildBeforeAfterCollapsible([{ path: "/", afterUrl: "https://api.example.dev/gittensory/shot?key=gittensory/shots/x.png" }]); + expect(c?.body).toContain("| `/` | desktop | — | "); + }); + it("returns null when no route has any shot URL (no empty table)", () => { expect(buildBeforeAfterCollapsible([])).toBeNull(); expect(buildBeforeAfterCollapsible([{ path: "/" }])).toBeNull(); @@ -169,6 +182,9 @@ describe("buildScrollPreviewCollapsible (#3612)", () => { expect(c?.body).toContain('before /app/analytics (scroll)"); + expect(c?.body).toContain("
after /app/analytics (scroll)"); }); it("returns null when no route has a scroll GIF — byte-identical to pre-#3612 for every non-opted-in repo", () => {