Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 54 additions & 17 deletions docs/features/foreign-order-resilience.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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`).
Expand All @@ -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).
6 changes: 6 additions & 0 deletions frontend/src/features/display/DisplayPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}),
Expand Down
3 changes: 3 additions & 0 deletions frontend/src/features/pdf/PDFGenerator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
});

Expand Down
65 changes: 65 additions & 0 deletions frontend/src/features/pdf/PagePreview.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<PagePreview
pageWidthMM={A4_WIDTH_MM}
pageHeightMM={A4_HEIGHT_MM}
bleedEdgeMM={0}
margins={zeroMargins}
spacing={zeroSpacing}
slots={[
{
imageUrl: "https://example.com/1.png",
name: "Card 1",
orphanLabel: "Your file",
},
]}
showCutLines={false}
maxWidthPx={400}
/>
);

expect(screen.getByTestId("orphan-badge")).toHaveTextContent("Your file");
});

it("renders no orphan badge when orphanLabel is omitted (a non-orphan card)", () => {
render(
<PagePreview
pageWidthMM={A4_WIDTH_MM}
pageHeightMM={A4_HEIGHT_MM}
bleedEdgeMM={0}
margins={zeroMargins}
spacing={zeroSpacing}
slots={[{ imageUrl: "https://example.com/1.png", name: "Card 1" }]}
showCutLines={false}
maxWidthPx={400}
/>
);

expect(screen.queryByTestId("orphan-badge")).not.toBeInTheDocument();
});

it("renders no orphan badge on a slot with no resolved imageUrl, even with orphanLabel set", () => {
render(
<PagePreview
pageWidthMM={A4_WIDTH_MM}
pageHeightMM={A4_HEIGHT_MM}
bleedEdgeMM={0}
margins={zeroMargins}
spacing={zeroSpacing}
slots={[
{
imageUrl: undefined,
name: "Card 1",
orphanLabel: "Your file",
},
]}
showCutLines={false}
maxWidthPx={400}
/>
);

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", () => {
Expand Down
43 changes: 43 additions & 0 deletions frontend/src/features/pdf/PagePreview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
<div
data-testid="orphan-badge"
style={{
position: "absolute",
right: "1mm",
top: "1mm",
padding: "0.5mm 1.5mm",
fontSize: "2.2mm",
fontWeight: 700,
lineHeight: 1.2,
textTransform: "uppercase",
letterSpacing: "0.03em",
color: "white",
background: "rgba(13, 110, 253, 0.85)",
borderRadius: "1mm",
pointerEvents: "none",
whiteSpace: "nowrap",
}}
>
{content.orphanLabel}
</div>
)}
{content != null && content.imageUrl == null && (
<div
data-testid="page-preview-empty-slot-label"
Expand Down
51 changes: 45 additions & 6 deletions frontend/tests/OrphanRendering.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,13 @@
* listenerMiddleware.ts) are unaffected by which page mounts them - orphan rendering on /display
* needed NO code change (it already reads `cardDocument.mediumThumbnailUrl`/`isOrphan` off the
* same shared `cardDocuments` store slice PagePreview.tsx's own sheet-cell renderer uses), so
* this file is proof, not a fix. Note this is a DIFFERENT rendering path from Card.tsx (the
* classic editor's own card component, which additionally shows an "orphan-badge" corner label -
* see Card.test.tsx for that unit-level coverage): PagePreview.tsx (the /display sheet's own
* renderer) has no badge equivalent today, so no badge assertion appears below - that's a
* pre-existing, page-scoped gap in the sheet's own visual language, not a regression this file
* introduces or hides.
* this file is proof, not a fix. This is a DIFFERENT rendering path from Card.tsx (the classic
* editor's own card component, which also shows an "orphan-badge" corner label - see
* Card.test.tsx for that unit-level coverage): PagePreview.tsx (the /display sheet's own
* renderer) previously had no badge equivalent - a recorded, page-scoped gap in
* docs/features/foreign-order-resilience.md - now closed by porting the same `orphan-badge`
* testid/treatment onto PagePreview's own slot renderer (PagePreview.tsx's `orphanLabel` prop,
* wired from DisplayPage.tsx), asserted below on both faces.
*
* The classic editor's own "Common Cardback" panel (CommonCardback.tsx, /editor only - /display
* has no equivalent persistent tile, only the CardbackToolbarButton picker) is covered
Expand Down Expand Up @@ -77,6 +78,9 @@ test.describe("orphan rendering (issue #324) - unified /display page", () => {
"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({
Expand Down Expand Up @@ -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({
Expand All @@ -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",
});
});
});
Loading