From 56996694b9df8bc294ab9b2d8fe5811db91e6ced Mon Sep 17 00:00:00 2001 From: Ivan Cheung Date: Thu, 6 Aug 2026 03:18:00 +0000 Subject: [PATCH] =?UTF-8?q?viewer/framing-lint:=20places-single-image=20ru?= =?UTF-8?q?le=20=E2=80=94=20enforce=20multi-image=20carousels?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Place cards rendering ONE bare Image (or a 1-image ImageCarousel) now warn at push time: Places photos come 4-at-a-time, so a single image means the fetch stopped early. Complements places-no-image; same non-blocking {warnings,findings} contract. 3 tests. --- packages/viewer/src/flow-geometry.ts | 2 +- packages/viewer/src/framing-lint.ts | 66 +++++++++++++++++++++++ packages/viewer/test/framing-lint.test.ts | 31 +++++++++++ 3 files changed, 98 insertions(+), 1 deletion(-) diff --git a/packages/viewer/src/flow-geometry.ts b/packages/viewer/src/flow-geometry.ts index 7b7b967..012a81f 100644 --- a/packages/viewer/src/flow-geometry.ts +++ b/packages/viewer/src/flow-geometry.ts @@ -65,7 +65,7 @@ export interface Finding { // Framing codes (framing-lint.ts): does the board tell a first-time reader WHAT it is and HOW to use it? | "framing-no-title" | "framing-no-lede" | "framing-interactive-unexplained" | "framing-data-no-legend" | "framing-skeletal" - | "places-no-image" + | "places-no-image" | "places-single-image" // Density codes (element-density.ts): UI cells rendered below the readable-width floor. | "narrow-nested-grid" | "narrow-table-cols" | "narrow-standalone-grid"; message: string; diff --git a/packages/viewer/src/framing-lint.ts b/packages/viewer/src/framing-lint.ts index 0533526..edfb5f9 100644 --- a/packages/viewer/src/framing-lint.ts +++ b/packages/viewer/src/framing-lint.ts @@ -222,6 +222,59 @@ function placesItemHasImage(item: unknown): boolean { return false; } +/** Classify an item's imagery: "carousel" (ImageCarousel with 2+ images), "single" + * (bare Image, or a 1-image ImageCarousel), or "none". Bounded like subtreeHasImage. */ +function placesItemImagery(item: unknown): "carousel" | "single" | "none" { + if (!item || typeof item !== "object") return "none"; + const it = item as { images?: unknown; node?: unknown }; + if (Array.isArray(it.images) && it.images.length >= 2) return "carousel"; + let single = false; + const walk = (node: unknown, cap = { n: 0 }): "carousel" | undefined => { + if (cap.n > MAX_TEXT_NODES || !node || typeof node !== "object") return undefined; + cap.n++; + const n = node as { type?: unknown; props?: Record; children?: unknown }; + if (n.type === "ImageCarousel") { + const imgs = (n.props as { images?: unknown } | undefined)?.images; + if (Array.isArray(imgs) && imgs.length >= 2) return "carousel"; + if (Array.isArray(imgs) && imgs.length === 1) single = true; + } + if (n.type === "Image") { + const src = (n.props as { src?: unknown } | undefined)?.src; + if (typeof src === "string" && src.trim()) single = true; + } + const ch = n.children; + if (Array.isArray(ch)) { for (const c of ch) { const r = walk(c, cap); if (r) return r; } } + else if (ch && typeof ch === "object") { const r = walk(ch, cap); if (r) return r; } + return undefined; + }; + if (walk(it.node) === "carousel") return "carousel"; + return single ? "single" : "none"; +} + +/** Walk the tree; for every PlacesExplorer with items, report how many render only a + * SINGLE image (bare Image or 1-image carousel) instead of a multi-image ImageCarousel. */ +function placesSingleImageGaps(node: unknown, out: { group: string; single: number; total: number }[] = [], cap = { n: 0 }): { group: string; single: number; total: number }[] { + if (cap.n > MAX_TEXT_NODES) return out; + cap.n++; + if (!node || typeof node !== "object") return out; + const n = node as { type?: unknown; props?: Record; children?: unknown }; + if (n.type === "PlacesExplorer") { + const items = Array.isArray((n.props as { items?: unknown } | undefined)?.items) + ? ((n.props as { items: unknown[] }).items) : []; + if (items.length > 0) { + const single = items.filter((it) => placesItemImagery(it) === "single").length; + const group = typeof (n.props as { group?: unknown } | undefined)?.group === "string" + ? (n.props as { group: string }).group + : (typeof (n.props as { id?: unknown } | undefined)?.id === "string" ? (n.props as { id: string }).id : "?"); + out.push({ group, single, total: items.length }); + } + } + const ch = n.children; + if (Array.isArray(ch)) for (const c of ch) placesSingleImageGaps(c, out, cap); + else if (ch && typeof ch === "object") placesSingleImageGaps(ch, out, cap); + return out; +} + /** Walk the tree; for every PlacesExplorer with items, report how many lack a photo. * Correct-by-construction guard: place cards are expected to carry image carousels. */ function placesImageGaps(node: unknown, out: { group: string; missing: number; total: number }[] = [], cap = { n: 0 }): { group: string; missing: number; total: number }[] { @@ -323,6 +376,19 @@ export function framingReport(type: string, content: string): { warnings: string }); } } + // Rule 7 (correct-by-construction): a place card that renders ONE bare image reads as + // unfinished next to carousel cards — places photos come 4-at-a-time from the Places + // API, so a single image means the fetch stopped early. Warn toward ImageCarousel. + for (const gap of placesSingleImageGaps(root)) { + if (gap.single > 0) { + findings.push({ + severity: "warning", code: "places-single-image", count: gap.single, + message: `PlacesExplorer "${gap.group}": ${gap.single}/${gap.total} place card(s) render a single image — ` + + `use a multi-image ImageCarousel instead (fetch 3-4 Places photos per venue; ` + + `{type:"ImageCarousel",props:{images:[{src,alt},…]}}). Single Image nodes are deprecated for place cards.`, + }); + } + } allText = textOf(root); nodeCount = totalNodeCount(root); // BoardHeader's structured legend/howToUse satisfy the interactive-hint and legend rules — diff --git a/packages/viewer/test/framing-lint.test.ts b/packages/viewer/test/framing-lint.test.ts index 4191cff..4113884 100644 --- a/packages/viewer/test/framing-lint.test.ts +++ b/packages/viewer/test/framing-lint.test.ts @@ -356,3 +356,34 @@ describe("framingReport — PlacesExplorer image carousels (places-no-image)", ( expect(r.findings.map((f) => f.code)).not.toContain("places-no-image"); }); }); + +describe("places-single-image", () => { + const card = (imgs: number) => ({ + type: "Card", children: imgs === 0 ? [{ type: "Text", children: ["x"] }] : [ + imgs === -1 + ? { type: "Image", props: { src: "https://x/p.jpg" } } + : { type: "ImageCarousel", props: { images: Array.from({ length: imgs }, (_, i) => ({ src: `https://x/${i}.jpg` })) } }, + ], + }); + const board = (items: unknown[]) => JSON.stringify({ + type: "Stack", children: [ + { type: "Title", children: ["Places board with plenty of words in the title"] }, + { type: "Text", children: ["This lede explains what the board shows and how to click, filter and search the cards below properly."] }, + { type: "PlacesExplorer", props: { id: "pe-t", group: "t", items } }, + ], + }); + it("warns on bare single Image cards", () => { + const { findings } = framingReport("component", board([{ id: "a", node: card(-1) }, { id: "b", node: card(3) }])); + const f = findings.find((x) => x.code === "places-single-image"); + expect(f).toBeTruthy(); + expect(f?.count).toBe(1); + }); + it("warns on 1-image carousels", () => { + const { findings } = framingReport("component", board([{ id: "a", node: card(1) }])); + expect(findings.some((x) => x.code === "places-single-image")).toBe(true); + }); + it("passes multi-image carousels and imageless cards", () => { + const { findings } = framingReport("component", board([{ id: "a", node: card(3) }, { id: "b", node: card(0) }])); + expect(findings.some((x) => x.code === "places-single-image")).toBe(false); + }); +});