diff --git a/docs/features/foreign-order-resilience.md b/docs/features/foreign-order-resilience.md index 91219f6f5..aea013e45 100644 --- a/docs/features/foreign-order-resilience.md +++ b/docs/features/foreign-order-resilience.md @@ -126,22 +126,48 @@ regression-guarded addition, not a loosening of that existing behaviour - **Acceptance surface**: the classic `/editor` page is a legacy route held behind the route-swap PR #389 — owner ruling, this review round: the UNIFIED `/display` page (nav "Editor") is the only acceptance surface for - frontend rendering work. `OrphanRendering.spec.ts`'s two Playwright cases + frontend rendering work. `OrphanRendering.spec.ts`'s Playwright cases (originally verified against `/editor`) were moved to run against `/display` instead, screenshotting `test-results/orphan-text-import-desktop.png`/ - `orphan-xml-import-desktop.png`/`orphan-xml-import-backs-desktop.png`. -- **The `/display` sheet needed no code change**: `PagePreview.tsx` (the - unified page's own sheet-cell renderer, a DIFFERENT component from - `Card.tsx`) reads `cardDocument.mediumThumbnailUrl`/`isOrphan` straight off - the same shared `cardDocuments` store slice — since `synthesizeOrphanCardDocument` - already sets both, an orphan's image renders correctly there for free, for - both fronts (text import) and backs (the XML `b:null` case, once the page's - own Fronts/Backs toggle is switched — the "cardback corner" from the - owner's report; see the next bullet for the surface that PagePreview.tsx - is NOT). One gap this surfaces: `PagePreview.tsx` has no `OrphanBadge` - equivalent (`Card.tsx`'s corner label) — a pre-existing, page-scoped visual - gap the sheet already had for every other card attribute it doesn't badge, - not a regression from this pass. + `orphan-xml-import-desktop.png`/`orphan-xml-import-backs-desktop.png`/ + `orphan-text-import-narrow-390.png`. +- **The `/display` sheet needed no code change to render the image itself**: + `PagePreview.tsx` (the unified page's own sheet-cell renderer, a DIFFERENT + component from `Card.tsx`) reads `cardDocument.mediumThumbnailUrl`/ + `isOrphan` straight off the same shared `cardDocuments` store slice — since + `synthesizeOrphanCardDocument` already sets both, an orphan's image renders + correctly there for free, for both fronts (text import) and backs (the XML + `b:null` case, once the page's own Fronts/Backs toggle is switched — the + "cardback corner" from the owner's report; see the next bullet for the + surface that PagePreview.tsx is NOT). +- **Badge gap CLOSED (2026-07-23 follow-up)**: `PagePreview.tsx` previously + had no `OrphanBadge` equivalent (`Card.tsx`'s corner label) — a + page-scoped visual gap that also left a parity Playwright test + (`ImportXML.spec.ts`'s orphan-cardback case, `parity-wave1` branch) red + with nothing to assert against. Closed by adding a `orphanLabel?: string` + prop to `PagePreviewSlotContent` (`PagePreview.tsx`) — `undefined` renders + no badge (every non-orphan slot, and any slot with no resolved `imageUrl` + yet); set, it renders a `data-testid="orphan-badge"` pill in the slot's + top-right corner (same background/text-transform/weight as Card.tsx's + `OrphanBadge`, reimplemented in this component's own mm-unit idiom rather + than px, since every other PagePreview overlay is sized in mm so it stays + legible after the outer `transform: scale()` — a raw px badge would nearly + vanish on a heavily letterboxed phone sheet, the same reasoning already + written up for this component's screen-only border/radius above). Top-right + rather than Card.tsx's top-left, clear of the existing bleed badge's + top-left corner (the two never co-occur in practice — an orphan has no + `sourceType`, so PDF.tsx's bleed-normalization eligibility check never + fires for one — but kept visually separable regardless). Wired from both + callers that already resolve a `CardDocument`: `DisplayPage.tsx` (the + `/display` sheet, `cardDocument.sourceName` when `cardDocument.isOrphan`) + and `PDFGenerator.tsx`'s fast preview (`doc.sourceName` when + `doc.isOrphan`, free since `doc` was already resolved for the bleed + badge). Same testid as Card.tsx's own badge, by design, so a spec can + target either surface uniformly. Coverage: `PagePreview.test.tsx`'s new + "orphan badge" describe block (label shown/omitted/gated on a resolved + `imageUrl`), and `OrphanRendering.spec.ts`'s badge assertions on both the + text-import and XML-import (front + "cardback corner" back) cases, plus a + dedicated 390px-narrow-viewport case. - **REAL BUG, fixed**: the classic editor's "Common Cardback" panel (`CommonCardback.tsx`'s right-panel mount, `/editor` only — `/display` has no equivalent persistent tile, only the `CardbackToolbarButton` picker) @@ -269,6 +295,14 @@ already offered exactly as it always was for any slot. consensus-surface gate. - `frontend/src/features/pdf/pdfImage.ts` — `getOrphanPDFImageURL`, the full-resolution PDF-export path. +- `frontend/src/features/pdf/PagePreview.tsx` — (2026-07-23 follow-up) + `PagePreviewSlotContent`'s `orphanLabel` prop and its `orphan-badge` + render, the `/display` sheet's own port of Card.tsx's `OrphanBadge`. +- `frontend/src/features/display/DisplayPage.tsx` — wires `orphanLabel` + from `cardDocument.isOrphan`/`sourceName` into each sheet slot's content. +- `frontend/src/features/pdf/PDFGenerator.tsx` — wires the same `orphanLabel` + into its fast-preview slots (`fastPreviewSlots`), alongside the existing + bleed badge. - `frontend/src/common/types.ts` — the `isOrphan?: boolean` marker on the frontend's own `Card`/`CardDocument` type (never present in the quicktype-generated `schema_types.ts`). @@ -282,6 +316,9 @@ already offered exactly as it always was for any slot. back-face case, plus the new `cardback` field cases), `downloadXML.test.ts` (round-trip), `Card.test.tsx` (badge/click-suppression/error-degrade), `ImportXML.spec.ts`'s "brand new project" Playwright case (the Common - Cardback fix), and the Playwright `tests/OrphanRendering.spec.ts` (both - reported symptoms, end to end, on the unified `/display` page as of the - 2026-07-23 acceptance-surface correction, with screenshots). + Cardback fix), `PagePreview.test.tsx`'s "orphan badge" describe block + (2026-07-23 follow-up), and the Playwright `tests/OrphanRendering.spec.ts` + (both reported symptoms, end to end, on the unified `/display` page as of + the 2026-07-23 acceptance-surface correction, with screenshots, plus the + sheet's own `orphan-badge` assertions and a dedicated narrow-viewport case + added in the same follow-up). diff --git a/frontend/src/features/display/DisplayPage.tsx b/frontend/src/features/display/DisplayPage.tsx index 5307413ea..54a357689 100644 --- a/frontend/src/features/display/DisplayPage.tsx +++ b/frontend/src/features/display/DisplayPage.tsx @@ -1558,6 +1558,12 @@ export function DisplayPage() { loadState === "failed" ? buildScryfallReferenceUrl(query) : undefined, + // Foreign-order resilience Phase 1 follow-up (issue #324) - PagePreview's own + // OrphanBadge equivalent, same "sourceName" text Card.tsx already shows for this + // identifier on the classic editor surface. + orphanLabel: cardDocument?.isOrphan + ? cardDocument.sourceName + : undefined, }; return content; }), diff --git a/frontend/src/features/pdf/PDFGenerator.tsx b/frontend/src/features/pdf/PDFGenerator.tsx index 75e5b22d6..0c0c0b2b0 100644 --- a/frontend/src/features/pdf/PDFGenerator.tsx +++ b/frontend/src/features/pdf/PDFGenerator.tsx @@ -1402,6 +1402,9 @@ export const PDFGenerator = ({ heightDelta = 0 }: { heightDelta?: number }) => { imageUrl: doc?.smallThumbnailUrl, name: doc?.name ?? "", willGenerateBleed, + // Foreign-order resilience Phase 1 follow-up (issue #324) - same badge PagePreview's own + // /display caller wires up; free here since `doc` is already the resolved CardDocument. + orphanLabel: doc?.isOrphan ? doc.sourceName : undefined, }; }); diff --git a/frontend/src/features/pdf/PagePreview.test.tsx b/frontend/src/features/pdf/PagePreview.test.tsx index 015fd5dcb..b05dd0079 100644 --- a/frontend/src/features/pdf/PagePreview.test.tsx +++ b/frontend/src/features/pdf/PagePreview.test.tsx @@ -360,6 +360,71 @@ describe("PagePreview - bleed badge (Proposal B PR-3)", () => { }); }); +describe("PagePreview - orphan badge (issue #324 follow-up)", () => { + it("renders the orphan badge with the given label when orphanLabel is set", () => { + render( + + ); + + expect(screen.getByTestId("orphan-badge")).toHaveTextContent("Your file"); + }); + + it("renders no orphan badge when orphanLabel is omitted (a non-orphan card)", () => { + render( + + ); + + expect(screen.queryByTestId("orphan-badge")).not.toBeInTheDocument(); + }); + + it("renders no orphan badge on a slot with no resolved imageUrl, even with orphanLabel set", () => { + render( + + ); + + expect(screen.queryByTestId("orphan-badge")).not.toBeInTheDocument(); + }); +}); + // Sanity: the card constants this component relies on (via computeLayout) are the same ones // PDF.tsx itself uses, so a preview slot's box size always matches a generated PDF's. describe("PagePreview - card constants", () => { diff --git a/frontend/src/features/pdf/PagePreview.tsx b/frontend/src/features/pdf/PagePreview.tsx index 82bf612d0..f327f7bc7 100644 --- a/frontend/src/features/pdf/PagePreview.tsx +++ b/frontend/src/features/pdf/PagePreview.tsx @@ -199,6 +199,16 @@ export interface PagePreviewSlotContent { * (scryfallReference.ts), shown only alongside `loadState === "failed"`. `undefined` renders no * link (there was nothing in the query to build one from). */ findCardUrl?: string; + /** Foreign-order resilience Phase 1 follow-up (issue #324) - porting Card.tsx's own + * `OrphanBadge` corner label to this, the /display sheet's own renderer, which previously had + * no orphan visual treatment at all (a recorded gap in + * docs/features/foreign-order-resilience.md). Set to the synthesized CardDocument's own + * `sourceName` ("Your file" on this - the author/editor - surface, same text Card.tsx already + * shows) whenever `cardDocument.isOrphan` is true; `undefined` (every non-orphan slot, and any + * slot with no resolved image yet) renders no badge. Only shown alongside a resolved + * `imageUrl` - same gating Card.tsx's own badge uses (never drawn over the empty/loading/failed + * placeholder states above, which already carry their own distinct visual language). */ + orphanLabel?: string; } export interface PagePreviewProps { @@ -443,6 +453,39 @@ function PagePreviewSlotEl({ }} /> )} + {content?.imageUrl != null && content.orphanLabel != null && ( + // Foreign-order resilience Phase 1 follow-up (issue #324) - same visual treatment as + // Card.tsx's own `OrphanBadge` (blue pill, white uppercase text), ported to this + // component's own mm-unit idiom (matching the bleed badge just below rather than + // Card.tsx's px-based styled-component) so it stays legible at any `maxWidthPx` + // letterboxing after the outer `transform: scale()` - see this file's own module comment + // on why every overlay here is sized in mm, not px. Top-right, deliberately not the + // bleed badge's top-left corner: the two are mutually exclusive in practice (an orphan + // has no sourceType, so PDF.tsx's bleed-normalization eligibility check never fires for + // one - see the docs' "Bleed normalization for orphans" deferred note), but keeping them + // visually separable costs nothing. +
+ {content.orphanLabel} +
+ )} {content != null && content.imageUrl == null && (
{ "src", `https://lh4.googleusercontent.com/d/${orphanId}=h800` ); + // The sheet's own corner badge (PagePreview.tsx's `orphanLabel`, ported from Card.tsx's + // `OrphanBadge` - same testid, same "sourceName" text, on the /display sheet surface). + await expect(slot.getByTestId("orphan-badge")).toHaveText("Your file"); await page.waitForTimeout(500); await page.screenshot({ @@ -133,6 +137,7 @@ test.describe("orphan rendering (issue #324) - unified /display page", () => { "src", `https://lh4.googleusercontent.com/d/${orphanId}=h800` ); + await expect(slot.getByTestId("orphan-badge")).toHaveText("Your file"); await page.waitForTimeout(500); await page.screenshot({ @@ -152,10 +157,44 @@ test.describe("orphan rendering (issue #324) - unified /display page", () => { "src", `https://lh4.googleusercontent.com/d/${orphanBackId}=h800` ); + await expect(slot.getByTestId("orphan-badge")).toHaveText("Your file"); await page.waitForTimeout(500); await page.screenshot({ path: "test-results/orphan-xml-import-backs-desktop.png", }); }); + + test("the sheet's orphan badge renders at a narrow phone viewport too", async ({ + page, + network, + }) => { + network.use(...defaultHandlers); + // Same 390x844 phone viewport DisplayPage.spec.ts's own "phone viewport (issue #266)" block + // uses - set before navigation so the /display route's own responsive layout (left-rail + // drawer, sheet scaled to fit) is already in its narrow-viewport shape from first paint. + await page.setViewportSize({ width: 390, height: 844 }); + await loadPageWithDefaultBackend(page, "display"); + await expect(page.getByTestId("display-empty-state")).toBeVisible({ + timeout: 20_000, + }); + + await page + .getByRole("textbox", { name: "import-text" }) + .fill(`1x Kharn [mpc:${orphanId}]`); + await page.getByRole("button", { name: "import-text-submit" }).click(); + + await expect(page.getByTestId("display-page")).toBeVisible(); + await expect(page.getByText("Review Invalid Cards")).not.toBeVisible(); + + const slot = page.getByTestId("page-preview-slot").first(); + const image = slot.locator("img"); + await expect(image).toHaveCount(1, { timeout: 45_000 }); + await expect(slot.getByTestId("orphan-badge")).toHaveText("Your file"); + + await page.waitForTimeout(500); + await page.screenshot({ + path: "test-results/orphan-text-import-narrow-390.png", + }); + }); });