From 291ac3d009d00b3337c46154bfac0c55dc979b3c Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Thu, 6 Aug 2026 11:03:24 +0000 Subject: [PATCH] questionFeed: fix subject title overlay, illustration-tile frame, Yes button width - SubjectArtTitle no longer overlays the artwork's bottom edge; SubjectArt splits into SubjectArtImage (owns the art's own sizing) + a normal-flow title below it (#741). - Illustration-group candidate tiles get a landscape frame (IllustrationArtPlaceholder) sized from the actual Scryfall art-crop region instead of the full card's 63/88 ratio; ungrouped (full-scan) tiles keep the card-ratio frame. CandidateGrid's tile clamp raised from 78-116px to 120-190px (#746). - Root cause for #746's frame not reliably honoring its own aspect-ratio: ZoomableThumbnail's img used height:100% against an indeterminate parent, which per the CSS box model falls back to the image's own intrinsic ratio, silently overriding the frame's declared ratio. ZoomableThumbnail is now position:absolute;inset:0 so it never participates in that computation. - Level 1 Yes button drops .block; ActionStack's default flex stretch (not just the class) was independently forcing full width, so its first child now opts out via align-self:flex-start (#740). - SPEC-wtc-rebuild.md: corrects amendment A3 (wrongly concluded the title overlay wasn't a defect), supersedes A13 (kept .block), and updates the affected binding-table rows. --- .../mockups/wtc-rebuild/SPEC-wtc-rebuild.md | 171 +++++++++++++----- .../src/features/printingTags/cardPanel.tsx | 46 ++++- .../features/questionFeed/QuestionFeed.tsx | 104 +++++++---- frontend/tests/QuestionFeed.spec.ts | 88 +++++++++ frontend/tests/QuestionFeedResponsive.spec.ts | 30 ++- 5 files changed, 351 insertions(+), 88 deletions(-) diff --git a/docs/proposals/mockups/wtc-rebuild/SPEC-wtc-rebuild.md b/docs/proposals/mockups/wtc-rebuild/SPEC-wtc-rebuild.md index 4614e89d2..d7241f936 100644 --- a/docs/proposals/mockups/wtc-rebuild/SPEC-wtc-rebuild.md +++ b/docs/proposals/mockups/wtc-rebuild/SPEC-wtc-rebuild.md @@ -177,6 +177,77 @@ above rather than proposing anything new):** its ActionGrid siblings now — hierarchy comes from position (its own row, above the grid) and colour (`.primary`, orange) rather than from being a larger control. +--- + +**AMENDMENTS — 2026-08-06, closing #740/#741/#746 (subject-card presentation defects live on +this page after the 2026-08-06 layout pass above):** + +14. **A14: A3 corrected — the title overlay was a real, separate defect (#741).** A3 concluded + the gradient overlay was not the defect and attributed the reported symptom entirely to + #705's hover-grow clipping. That conclusion doesn't hold: `SubjectArtTitle` was + `position: absolute; left: 0; right: 0; bottom: 0` inside `SubjectArt`'s own box — its + containing block is the art's own box, so it sat over the art's bottom edge unconditionally, + with no hover state involved and no dependency on the clipping/overflow mechanism #705 + fixed. Live measurement confirmed the overlap directly (title top at the art box's own + interior, not below it). `SubjectArtTitle` is now a normal-flow sibling below the art + (`SubjectArtImage`, a new box that owns the art's own sizing/aspect-ratio so the title can + never share its space) rather than an absolutely-positioned child of it — the "subject art + title" row's colour cell changes from a gradient overlay to a solid bar (below, not on top + of, the art) for the same reason a gradient fade only reads correctly against content it's + laid over: `#fff` on `rgba(0, 0, 0, 0.72)` (previously `linear-gradient(transparent, rgba(0, 0, 0, .72))`) — the darkest stop of the old gradient, kept as a flat colour since + there's no longer art underneath it to fade over. Size/spacing (font-size clamp, weight) + is unchanged. + +15. **A15: A13 superseded — `.block` dropped too (#740).** A13 kept `.block` on the Level 1 Yes + button on the reasoning that font-size parity plus position/colour was enough for + hierarchy. Live measurement after A13 shipped showed the button still ~2x its "Not sure" + sibling's width (494px vs 243px at an 800px viewport) — `.block`'s `width: 100%` was still + forcing it to span the full row. `.block` is now dropped; `.primary` (colour) is kept, since + it's legitimate affordance for the primary action and the complaint was about size, not + about the button being identifiable. Dropping the class alone wasn't sufficient either: the + button's parent (`ActionStack`) is a `flex-direction: column` container, whose default + `align-items: stretch` was independently forcing the button to the row's full width + regardless of `.block`. `ActionStack` now opts its first child (the Yes button) out of that + stretch via `align-self: flex-start`, while its second child (`ActionGrid`, needing the full + row width to lay out its own three siblings) keeps the default. The button now sizes to its + label like its siblings; hierarchy is position (its own row, above the grid) and colour only. + +16. **A16: Candidate grid size + a second, non-card art frame (#746).** Two coupled changes to + the "candidate grid"/"candidate tile" §1c rows, both scoped to Level 2's candidate tiles + (not the subject art, which was already sized to the `.subject` column's own `clamp(240px, 30cqi, 340px)`): + - **Size.** `CandidateGrid`'s `grid-template-columns` minmax floor/ceiling moves from + `clamp(78px, 15cqi, 116px)` to `clamp(120px, 24cqi, 190px)` — the artwork is the subject + of the question being asked, and a 78-116px tile didn't read that way. Fewer columns per + row at a given container width is the accepted trade for legible art. + - **Frame.** `ArtPlaceholder` (63/88, i.e. the full card's own ratio) was the one shared + frame for every candidate tile, including illustration-group representative tiles, whose + image is a Scryfall art crop — not card-shaped. A new `IllustrationArtPlaceholder` + (identical in every other respect: same `MysteryCard`/`ZoomableThumbnail` composition, + same `object-fit: cover` contract) uses a landscape ratio instead, measured from the + actual crop region rather than assumed: MPCAutofill's `local_phash.ART_CROP_BOX` (the + fractional box every art crop is harvested from) resolves to pixel regions of + width:height ~= 584:444 across the golden-set fixtures + (`MPCAutofill/cardpicker/golden_set.py`'s `art_crop_px` entries). Illustration-group + tiles (`QuestionFeed.tsx`'s `renderCandidateTile`, called from the illustration-cluster + branch) use this frame when the representative candidate carries an `artCropUrl`; every + other candidate tile (ungrouped, or a cluster representative with no crop, falling back to + its full scan) keeps `ArtPlaceholder`. + - **Root cause note (applies to both frames, not just the new one).** Neither frame's own + `aspect-ratio` was actually reaching the screen: `ZoomableThumbnail`, the `img`'s + immediate wrapper, sized itself from its content (`position: relative`, no explicit + size), and the `img` inside used `height: 100%` — a percentage height against an + indeterminate ancestor computes to `auto` per the CSS box model, and an `auto`-sized + replaced element with a known width falls back to its own intrinsic ratio. That + intrinsic-ratio height then became `ZoomableThumbnail`'s own content height, which grew + the frame past whatever `aspect-ratio` it declared, since the frame is deliberately + `overflow: visible` (needed so `ZoomableThumbnail`'s hover-zoom, A12, isn't clipped). + In effect, every candidate tile's rendered shape always followed whatever the loaded + image's own ratio happened to be, silently overriding both frames' declared ratios — a + real full-card scan already being close to 63/88 is why this was never visible for + `ArtPlaceholder`. `ZoomableThumbnail` is now `position: absolute; inset: 0` instead, which + removes it from the frame's in-flow content entirely, so the frame's own `aspect-ratio` is + what actually renders regardless of the loaded image's own ratio. + D-number scope note: D-numbers are per-proposal in this repo (proposal-h owns its own D1–D19; the old WTC round used W4–W7). The decisions below are the **WTC-rebuild round's** ledger, numbered WD1.. to avoid collision with either. @@ -239,56 +310,56 @@ retired bespoke elements. Every visible element on the affected page. Values are binding. -| element | color | size / spacing | radius | -| -------------------- | --------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------- | ----------- | -| page field wrapper | bg `--wtc-field` | pad `14px 16px 22px` | — | -| wordmark `h1` | `--wtc-wordmark`; `?` span = `--primary` | `font-size:clamp(26px,4.4cqi,44px)`; weight 900; text-shadow `0 0 22px color-mix(accent 26%)` | — | -| wordmark sub | `--muted` | `.34em` of wordmark; weight 600; mt 3px | — | -| solved pill | text `--muted`, count `--success` | pad `5px 12px`; font 12px | `--r-pill` | -| solved dots | filled `--success`, empty `--divider` | 6px circles, gap 3px | 50% | -| hero container | — | `max-width:1180px`; flex-wrap; gap `clamp(12px,2.2cqi,22px)` | — | -| subject `.subject` | — | `flex:1 1 300px; max-width:clamp(240px,30cqi,340px)` | — | -| subject card | bg `--raised`, border `--divider` | — | `--r-card` | -| subject art | bright card-art (real image in app) | `aspect-ratio:63/88` | — | -| subject art title | `#fff` on `linear-gradient(transparent,rgba(0,0,0,.72))` | `font-size:clamp(13px,3.4cqi,17px)`; weight 700 | — | -| subject caption | `--muted` on `--conf`; glyph `--accent` | pad `8px 11px`; font 12px | — | -| qhead prompt | `--text` | `font-size:clamp(17px,3.4cqi,22px)`; weight 800 | — | -| shapepill.easy | ink `--btn-ink` on `--success` | pad `3px 10px`; 11px; upper | `--r-pill` | -| shapepill.pick | ink `--btn-ink` on `--accent` | " | `--r-pill` | -| shapepill.neg | ink `--btn-ink` on `--danger` | " | `--r-pill` | -| shapepill.hard | `--accent` on transparent, 1px dashed `--accent` | " | `--r-pill` | -| qhint | `--muted` | 13px | — | -| `.btn` (base) | — | `min-height:44px`; font 15px/600; pad `6px 16px`; 1px border | `--r-btn` | -| `.btn.big` | — | font 17px/800; pad `10px 20px` | `--r-btn` | -| `.btn-primary` | `--btn-ink` on `--primary` | — | `--r-btn` | -| `.btn-secondary` | `--text` on `--raised`, border `--divider` | — | `--r-btn` | -| `.btn-accent` | `--btn-ink` on `--accent`; 800 | — | `--r-btn` | -| `.btn-ghost` (Skip) | `--muted`, transparent | — | `--r-btn` | -| `.btn-danger` | `--danger`, 1px `--danger`, transparent | — | `--r-btn` | -| action stack/grid | — | stack gap 9px; grid `repeat(auto-fit,minmax(clamp(120px,34cqi,180px),1fr))` gap 9px | — | -| suggested card | bg `--conf`, border `--divider` | pad 11px; gap 13px | `--r-card` | -| suggested thumb | card-art | `flex:0 0 clamp(70px,20cqi,104px)`; `aspect 63/88` | 6px | -| suggested name | `--text` | `clamp(16px,3.2cqi,19px)`; 800 | — | -| suggested set | `--muted` monospace | 13px | — | -| confidence pill | `--accent`, 1px `--accent` | pad `2px 9px`; 11px/700; dot 7px | `--r-pill` | -| landed feedback | `--success` on `color-mix(success 12%)`, 1px `color-mix(success 45%)` | pad `5px 12px`; 13px; mt 12px | `--r-pill` | -| candidate grid | — | `grid; gap clamp(7px,1.6cqi,11px); grid-template-columns:repeat(auto-fill,minmax(clamp(78px,15cqi,116px),1fr))` | — | -| candidate tile | bg `--raised`, border `--divider`; `.sel` outline `2px --accent` | art `aspect 63/88` | `--r-card` | -| candidate caption | name `--text` 700, set `--muted` monospace 10px | pad `5px 7px 6px`; 11px | — | -| negative wrapper | 1px `--danger` on `color-mix(danger 8%,--conf)` | pad 13px | `--r-card` | -| negative header | `--danger` | `clamp(15px,3cqi,18px)`; 800 | — | -| reason chip | `--text`, 1px `--danger`; `×` mark `--danger` | `min-height:44px`; pad `6px 12px`; 14px | `--r-btn` | -| reason-chip grid | — | `repeat(auto-fill,minmax(clamp(130px,40cqi,190px),1fr))` gap 8px | — | -| open wrapper | 1px dashed `--accent` on `color-mix(accent 6%,--conf)` | pad 14px | `--r-card` | -| search field | bg `--raised`, 1px `--divider`; input `--text` | height 46px; pad `0 12px`; gap 9px; font 15px | `--r-input` | -| open help links | `--accent`, underline `color-mix(accent 45%)` | 13px; mt 11px | — | -| picker button | `--text` on `--raised`, 1px `--divider`; `.unknown` `--muted` | `min-height:44px`; 14px | `--r-btn` | -| picker grid | — | `repeat(auto-fill,minmax(clamp(120px,34cqi,180px),1fr))` gap 8px | — | -| tag `h6` | `--text`, `` = `--accent` | `clamp(17px,3.4cqi,21px)`; 800 | — | -| group label | `--muted` upper | 10px/700; mt 12px mb 5px | — | -| chip (tri-state) | default `--text` 1px `--muted`; `.pos` `--accent` on `color-mix(accent 20%)`; `.neg` `--danger` on `color-mix(danger 16%)` line-through | `min-height:38px`; pad `4px 12px`; 13px | `--r-btn` | -| toggle (independent) | default `--text` 1px `--muted`; `.on` `--accent` on `color-mix(accent 20%)` | `min-height:38px`; box 14px | `--r-btn` | -| mystery-mini face | glyph `--wtc-mystery-glyph` on `--wtc-mystery-face`; glow `--wtc-reveal-glow` | 44×61px | 6px | +| element | color | size / spacing | radius | +| -------------------- | --------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------- | +| page field wrapper | bg `--wtc-field` | pad `14px 16px 22px` | — | +| wordmark `h1` | `--wtc-wordmark`; `?` span = `--primary` | `font-size:clamp(26px,4.4cqi,44px)`; weight 900; text-shadow `0 0 22px color-mix(accent 26%)` | — | +| wordmark sub | `--muted` | `.34em` of wordmark; weight 600; mt 3px | — | +| solved pill | text `--muted`, count `--success` | pad `5px 12px`; font 12px | `--r-pill` | +| solved dots | filled `--success`, empty `--divider` | 6px circles, gap 3px | 50% | +| hero container | — | `max-width:1180px`; flex-wrap; gap `clamp(12px,2.2cqi,22px)` | — | +| subject `.subject` | — | `flex:1 1 300px; max-width:clamp(240px,30cqi,340px)` | — | +| subject card | bg `--raised`, border `--divider` | — | `--r-card` | +| subject art | bright card-art (real image in app) | `aspect-ratio:63/88` | — | +| subject art title | `#fff` on `rgba(0,0,0,.72)` (A14 — flat bar below the art, superseding the overlay gradient) | `font-size:clamp(13px,3.4cqi,17px)`; weight 700 | — | +| subject caption | `--muted` on `--conf`; glyph `--accent` | pad `8px 11px`; font 12px | — | +| qhead prompt | `--text` | `font-size:clamp(17px,3.4cqi,22px)`; weight 800 | — | +| shapepill.easy | ink `--btn-ink` on `--success` | pad `3px 10px`; 11px; upper | `--r-pill` | +| shapepill.pick | ink `--btn-ink` on `--accent` | " | `--r-pill` | +| shapepill.neg | ink `--btn-ink` on `--danger` | " | `--r-pill` | +| shapepill.hard | `--accent` on transparent, 1px dashed `--accent` | " | `--r-pill` | +| qhint | `--muted` | 13px | — | +| `.btn` (base) | — | `min-height:44px`; font 15px/600; pad `6px 16px`; 1px border | `--r-btn` | +| `.btn.big` | — | font 17px/800; pad `10px 20px` | `--r-btn` | +| `.btn-primary` | `--btn-ink` on `--primary` | — | `--r-btn` | +| `.btn-secondary` | `--text` on `--raised`, border `--divider` | — | `--r-btn` | +| `.btn-accent` | `--btn-ink` on `--accent`; 800 | — | `--r-btn` | +| `.btn-ghost` (Skip) | `--muted`, transparent | — | `--r-btn` | +| `.btn-danger` | `--danger`, 1px `--danger`, transparent | — | `--r-btn` | +| action stack/grid | — | stack gap 9px; grid `repeat(auto-fit,minmax(clamp(120px,34cqi,180px),1fr))` gap 9px | — | +| suggested card | bg `--conf`, border `--divider` | pad 11px; gap 13px | `--r-card` | +| suggested thumb | card-art | `flex:0 0 clamp(70px,20cqi,104px)`; `aspect 63/88` | 6px | +| suggested name | `--text` | `clamp(16px,3.2cqi,19px)`; 800 | — | +| suggested set | `--muted` monospace | 13px | — | +| confidence pill | `--accent`, 1px `--accent` | pad `2px 9px`; 11px/700; dot 7px | `--r-pill` | +| landed feedback | `--success` on `color-mix(success 12%)`, 1px `color-mix(success 45%)` | pad `5px 12px`; 13px; mt 12px | `--r-pill` | +| candidate grid | — | `grid; gap clamp(7px,1.6cqi,11px); grid-template-columns:repeat(auto-fill,minmax(clamp(120px,24cqi,190px),1fr))` (A16, superseding `clamp(78px,15cqi,116px)`) | — | +| candidate tile | bg `--raised`, border `--divider`; `.sel` outline `2px --accent` | art `aspect 63/88`; illustration-group representative tile (has an `artCropUrl`) uses `aspect 584/444` instead (A16) | `--r-card` | +| candidate caption | name `--text` 700, set `--muted` monospace 10px | pad `5px 7px 6px`; 11px | — | +| negative wrapper | 1px `--danger` on `color-mix(danger 8%,--conf)` | pad 13px | `--r-card` | +| negative header | `--danger` | `clamp(15px,3cqi,18px)`; 800 | — | +| reason chip | `--text`, 1px `--danger`; `×` mark `--danger` | `min-height:44px`; pad `6px 12px`; 14px | `--r-btn` | +| reason-chip grid | — | `repeat(auto-fill,minmax(clamp(130px,40cqi,190px),1fr))` gap 8px | — | +| open wrapper | 1px dashed `--accent` on `color-mix(accent 6%,--conf)` | pad 14px | `--r-card` | +| search field | bg `--raised`, 1px `--divider`; input `--text` | height 46px; pad `0 12px`; gap 9px; font 15px | `--r-input` | +| open help links | `--accent`, underline `color-mix(accent 45%)` | 13px; mt 11px | — | +| picker button | `--text` on `--raised`, 1px `--divider`; `.unknown` `--muted` | `min-height:44px`; 14px | `--r-btn` | +| picker grid | — | `repeat(auto-fill,minmax(clamp(120px,34cqi,180px),1fr))` gap 8px | — | +| tag `h6` | `--text`, `` = `--accent` | `clamp(17px,3.4cqi,21px)`; 800 | — | +| group label | `--muted` upper | 10px/700; mt 12px mb 5px | — | +| chip (tri-state) | default `--text` 1px `--muted`; `.pos` `--accent` on `color-mix(accent 20%)`; `.neg` `--danger` on `color-mix(danger 16%)` line-through | `min-height:38px`; pad `4px 12px`; 13px | `--r-btn` | +| toggle (independent) | default `--text` 1px `--muted`; `.on` `--accent` on `color-mix(accent 20%)` | `min-height:38px`; box 14px | `--r-btn` | +| mystery-mini face | glyph `--wtc-mystery-glyph` on `--wtc-mystery-face`; glow `--wtc-reveal-glow` | 44×61px | 6px | --- diff --git a/frontend/src/features/printingTags/cardPanel.tsx b/frontend/src/features/printingTags/cardPanel.tsx index b00a45d32..5ee0921a1 100644 --- a/frontend/src/features/printingTags/cardPanel.tsx +++ b/frontend/src/features/printingTags/cardPanel.tsx @@ -43,6 +43,8 @@ export const revealAnimation = keyframes` export const RevealWrapper = styled.div` position: relative; overflow: hidden; + width: 100%; + height: 100%; `; // The one "mystery card" backdrop every blue/purple-tinted placeholder on the page renders - @@ -146,8 +148,18 @@ export const StaticCardPanel = styled.div` // so the enlarged art is fully visible rather than cut off at the original box edge - // raised above its siblings on hover so it doesn't render underneath the neighbouring grid // cells it now overlaps. +// +// Issue #746 - `position: absolute; inset: 0` (rather than a normal-flow box the img sizes +// via a `height: 100%` percentage) so this never counts as in-flow content for its +// aspect-ratio'd ArtPlaceholder/IllustrationArtPlaceholder parent to size itself around: a +// percentage height on a replaced element (the inside) resolves against an +// indeterminate ancestor by falling back to the image's own intrinsic ratio, which was +// silently overriding the parent's declared aspect-ratio with whatever ratio the loaded +// image happened to have. Taking this box out of flow removes that override path, so the +// parent's aspect-ratio is what actually renders, matching what its own token table says. export const ZoomableThumbnail = styled.div` - position: relative; + position: absolute; + inset: 0; z-index: 0; img { @@ -218,6 +230,31 @@ export const ArtPlaceholder = styled.div` } `; +// Issue #746 - the harvested Scryfall illustration crop (candidate.artCropUrl) is not +// card-shaped, so forcing it through ArtPlaceholder's 63/88 card frame either crops its edges +// or letterboxes it. This frame's ratio is measured from the actual crop region instead of +// assumed: MPCAutofill's local_phash.ART_CROP_BOX (the fractional box every art crop is +// harvested from) resolves to pixel regions of width:height ~= 584:444 across the golden-set +// fixtures (MPCAutofill/cardpicker/golden_set.py `art_crop_px` entries, e.g. +// `[48, 92, 632, 536]`) - a landscape ratio, nothing like the full card's portrait 63/88. Same +// object-fit/hover-zoom contract as ArtPlaceholder, just sized to what an illustration crop +// actually looks like. +export const ILLUSTRATION_CROP_ASPECT_RATIO = "584 / 444"; + +export const IllustrationArtPlaceholder = styled.div` + position: relative; + width: 100%; + aspect-ratio: ${ILLUSTRATION_CROP_ASPECT_RATIO}; + + img { + position: relative; + z-index: 1; + width: 100%; + height: 100%; + object-fit: cover; + } +`; + // The spec's ".ctile" candidate tile (SPEC-wtc-rebuild.md section 1c "candidate tile" row) - // `bg raised, border divider; .sel outline 2px --accent`. "highlighted" (this component's own // className, kept unchanged at every call site) marks the suggested printing from a @@ -288,12 +325,17 @@ export const CandidateCaption = styled.div` // "continuous fold points" table) - an intrinsic auto-fill grid in container units, replacing // the old `MobileCandidateScroller` horizontal-scroll wrapper (retired, WD8) with a grid that // folds continuously (6 -> 4 -> 3 -> 2 columns) as the hero container narrows, no breakpoint. +// Issue #746 - the harvested artwork IS the subject of the question being asked, so a +// 78-116px tile read as "far too small by default" against that role. Raised to +// 120-190px (still an intrinsic auto-fill clamp, no viewport breakpoint - section 3's +// container-first policy) so a candidate's art is legible enough to actually compare on +// sight, at the cost of fewer columns per row on a narrow container. export const CandidateGrid = styled.div` display: grid; gap: clamp(7px, 1.6cqi, 11px); margin-top: 4px; grid-template-columns: repeat( auto-fill, - minmax(clamp(78px, 15cqi, 116px), 1fr) + minmax(clamp(120px, 24cqi, 190px), 1fr) ); `; diff --git a/frontend/src/features/questionFeed/QuestionFeed.tsx b/frontend/src/features/questionFeed/QuestionFeed.tsx index 6ff5838ca..fe280ade0 100644 --- a/frontend/src/features/questionFeed/QuestionFeed.tsx +++ b/frontend/src/features/questionFeed/QuestionFeed.tsx @@ -78,6 +78,7 @@ import { CandidateGrid, CARD_ASPECT_RATIO, CardPanel, + IllustrationArtPlaceholder, MysteryCard, randomFlavorText, RevealWrapper, @@ -217,29 +218,39 @@ const SubjectCardBox = styled.div` } `; +// Issue #741 - the art frame and the title bar are now two stacked boxes rather than one +// box with the title absolutely positioned over the art's own bottom edge (SubjectArtImage +// owns the sizing/aspect-ratio, SubjectArtTitle is a normal-flow sibling below it), so the +// title can never cover the artwork it labels. SubjectArt itself is just the flex column +// that holds both - at the hero's compact fold it becomes the 132px-wide row item WD3 +// specifies, stretched to the row's height by SubjectCardBox's `align-items: stretch`. const SubjectArt = styled.div` - aspect-ratio: ${CARD_ASPECT_RATIO}; position: relative; @container hero (max-width: 560px) { + display: flex; + flex-direction: column; flex: 0 0 132px; width: 132px; + } +`; + +const SubjectArtImage = styled.div` + position: relative; + aspect-ratio: ${CARD_ASPECT_RATIO}; + + @container hero (max-width: 560px) { + flex: 1; aspect-ratio: auto; } `; const SubjectArtTitle = styled.div` - position: absolute; - left: 0; - right: 0; - bottom: 0; - z-index: 2; - background: linear-gradient(transparent, rgba(0, 0, 0, 0.72)); + background: rgba(0, 0, 0, 0.72); color: #fff; font-weight: 700; font-size: clamp(13px, 3.4cqi, 17px); - padding: 22px 10px 8px; - text-shadow: 0 1px 2px #000; + padding: 6px 10px; `; const SubjectCap = styled.div` @@ -445,6 +456,14 @@ const ActionStack = styled.div` flex-direction: column; gap: 9px; margin-top: 4px; + + /* Issue #740 - dropping .block from the Yes button (this stack's first child) only sizes + it to its content if it also opts out of the flex column's default align-items: stretch; + ActionGrid (the second child) keeps stretching, since its own grid needs the full row + width to lay out its three siblings. */ + > button:first-child { + align-self: flex-start; + } `; const ActionGrid = styled.div` @@ -1280,13 +1299,18 @@ export function QuestionFeed() { ); // The full subject card composition (SPEC-wtc-rebuild.md's "subject card"/"subject art"/ - // "subject art title"/"subject caption" rows) - art with the card name overlaid at its own - // bottom edge, plus a caption strip below explaining what the subject IS. + // "subject art title"/"subject caption" rows) - art, the card name below it in its own row + // (issue #741 - previously overlaid on the art's own bottom edge), plus a caption strip + // explaining what the subject IS. const subjectCard = ( - {cardArt} - {item.card.name} + + {cardArt} + + + {item.card.name} + ? @@ -1361,7 +1385,7 @@ export function QuestionFeed() { item.suggestedPrinting != null && @@ -1493,7 +1517,11 @@ export function QuestionFeed() { // Illustration clusters (below) pass candidate.artCropUrl, falling back to the full // scan when a candidate's metadata sidecar has none - see this function's own comment // on showArtistCaption for why grouped tiles diverge from the ungrouped default here. - imageUrl: string = candidate.mediumThumbnailUrl + imageUrl: string = candidate.mediumThumbnailUrl, + // Issue #746 - an illustration crop isn't card-shaped, so illustration clusters (below) + // pass IllustrationArtPlaceholder instead of the card-ratio ArtPlaceholder every + // ungrouped (full-scan) tile keeps by default. + Frame: typeof ArtPlaceholder = ArtPlaceholder ) => ( - + )} - +
{" "} @@ -1703,7 +1731,13 @@ export function QuestionFeed() { ), false, representative.artCropUrl || - representative.mediumThumbnailUrl + representative.mediumThumbnailUrl, + // Only the illustration-crop image is landscape-shaped - the + // mediumThumbnailUrl fallback above is still a full card scan, so it + // keeps the card-ratio frame. + representative.artCropUrl + ? IllustrationArtPlaceholder + : ArtPlaceholder )} @@ -1792,21 +1826,25 @@ export function QuestionFeed() { // substitute the Scryfall art crop when the item carries one (see subjectImageSrc). cardNode = ( - - {item.card.name} onCardImageSettled(false)} - onError={() => - onCardImageSettled(item.card.mediumThumbnailUrl !== "") - } - /> - {item.card.name} + + + {item.card.name} onCardImageSettled(false)} + onError={() => + onCardImageSettled(item.card.mediumThumbnailUrl !== "") + } + /> + + + {item.card.name} + ); diff --git a/frontend/tests/QuestionFeed.spec.ts b/frontend/tests/QuestionFeed.spec.ts index 26656d0c5..2e3258420 100644 --- a/frontend/tests/QuestionFeed.spec.ts +++ b/frontend/tests/QuestionFeed.spec.ts @@ -341,6 +341,52 @@ test.describe("question feed - Level 2 illustration grouping", () => { ).toHaveAttribute("src", illustrationGroupCandidateC.mediumThumbnailUrl); }); + // Issue #746 - the illustration crop isn't card-shaped, so its tile's frame must not force + // the full-card 63/88 ratio the way an ungrouped (full-scan) tile's frame still does. + test("the illustration group's tile uses a landscape frame; an ungrouped tile keeps the card-ratio frame", async ({ + page, + network, + }) => { + network.use( + questionFeedIdentifyPrintingGroupedByIllustration, + ...defaultHandlers + ); + await loadPageWithDefaultBackend(page, "whatsthat"); + + // Measured on the art frame itself (question-feed-candidate-art-frame), not the whole + // tile - the tile's total height also includes the caption strip below the frame, whose + // own fixed text height would dilute the frame's actual aspect ratio at narrow tile + // widths where the caption is a larger fraction of the total. + const group = page.getByTestId("question-feed-illustration-group"); + const groupFrameBox = await group + .locator( + `[data-card-identifier="${illustrationGroupCandidateA.identifier}"]` + ) + .getByTestId("question-feed-candidate-art-frame") + .boundingBox(); + expect(groupFrameBox).not.toBeNull(); + const groupFrameRatio = groupFrameBox!.width / groupFrameBox!.height; + // Landscape (584/444 ~= 1.315) - wider than tall. + expect(groupFrameRatio).toBeGreaterThan(1.2); + expect(groupFrameRatio).toBeLessThan(1.4); + + const ungroupedGrid = page.getByTestId( + "question-feed-candidate-grid-ungrouped" + ); + const ungroupedFrameBox = await ungroupedGrid + .locator( + `[data-card-identifier="${illustrationGroupCandidateC.identifier}"]` + ) + .getByTestId("question-feed-candidate-art-frame") + .boundingBox(); + expect(ungroupedFrameBox).not.toBeNull(); + const ungroupedFrameRatio = + ungroupedFrameBox!.width / ungroupedFrameBox!.height; + // Portrait card ratio (63/88 ~= 0.716) - taller than wide. + expect(ungroupedFrameRatio).toBeGreaterThan(0.65); + expect(ungroupedFrameRatio).toBeLessThan(0.78); + }); + // Issue #709 - the illustration-credit ArtistSupportLink used to always stack the page-link // button, up to five commerce buttons, a badge, and a credit line next to the question - up to // ~8 rows. It now defaults to one collapsed line and expands on demand; the expansion must @@ -698,6 +744,48 @@ test.describe("question feed - confirm_suggestion question type", () => { }); }); +// Issue #741 - the subject art title used to sit absolutely-positioned inside the artwork's +// own box, covering its bottom edge; it now renders below the art in normal document flow. +test.describe("question feed - subject art title placement (issue #741)", () => { + test("at Level 1, the title never overlaps the artwork, and sits below it", async ({ + page, + network, + }) => { + network.use(questionFeedConfirmSuggestion, ...defaultHandlers); + await loadPageWithDefaultBackend(page, "whatsthat"); + + const artBox = await page + .getByTestId("question-feed-subject-art-image") + .boundingBox(); + const titleBox = await page + .getByTestId("question-feed-subject-art-title") + .boundingBox(); + expect(artBox).not.toBeNull(); + expect(titleBox).not.toBeNull(); + expect(boxesIntersect(artBox!, titleBox!)).toBe(false); + expect(titleBox!.y).toBeGreaterThanOrEqual(artBox!.y + artBox!.height); + }); + + test("at Level 2 (pinned subject sidebar), the title never overlaps the artwork, and sits below it", async ({ + page, + network, + }) => { + network.use(questionFeedIdentifyPrinting, ...defaultHandlers); + await loadPageWithDefaultBackend(page, "whatsthat"); + + const artBox = await page + .getByTestId("question-feed-subject-art-image") + .boundingBox(); + const titleBox = await page + .getByTestId("question-feed-subject-art-title") + .boundingBox(); + expect(artBox).not.toBeNull(); + expect(titleBox).not.toBeNull(); + expect(boxesIntersect(artBox!, titleBox!)).toBe(false); + expect(titleBox!.y).toBeGreaterThanOrEqual(artBox!.y + artBox!.height); + }); +}); + // One Playwright flow per question type, per the queue-redesign task spec's TESTS // requirement - artist and tag types reuse ArtistVotePicker/QueueTagQuestion directly (no // forks), so these assert the unified feed renders them correctly, not the pickers' diff --git a/frontend/tests/QuestionFeedResponsive.spec.ts b/frontend/tests/QuestionFeedResponsive.spec.ts index 639a1efa4..8ed5b8213 100644 --- a/frontend/tests/QuestionFeedResponsive.spec.ts +++ b/frontend/tests/QuestionFeedResponsive.spec.ts @@ -709,9 +709,10 @@ test.describe("question feed - hover-zoom is not clipped by its frame (issue #70 }); }); -// NEW coverage (layout pass, issue #711) - the Level 1 Yes button no longer carries the -// oversized `.big` modifier, so it now reads at the same font size as its ActionGrid siblings -// (hierarchy by position/colour, not by disproportionate size). +// Issue #711 / #740 - the Level 1 Yes button carries neither the oversized `.big` modifier +// nor the full-width `.block` modifier, so it reads at the same font size and a +// content-sized width, like its ActionGrid siblings. Hierarchy comes from position (its own +// row, above the grid) and its `.primary` colour, not from disproportionate size. test.describe("question feed - Level 1 answer-row hierarchy (issue #711)", () => { test("the Yes button reads at the same font size as its 'Not sure' sibling", async ({ page, @@ -733,4 +734,27 @@ test.describe("question feed - Level 1 answer-row hierarchy (issue #711)", () => ); expect(yesFontSize).toBe(notSureFontSize); }); + + test("the Yes button sizes to its content instead of spanning full width (issue #740)", async ({ + page, + network, + }) => { + network.use(questionFeedConfirmSuggestion, ...defaultHandlers); + await page.setViewportSize({ width: 800, height: 900 }); + await loadPageWithDefaultBackend(page, "whatsthat"); + + const yesButton = page.getByTestId("question-feed-level1-yes"); + const notSureButton = page.getByTestId("question-feed-level1-not-sure"); + await expect(yesButton).toBeVisible(); + await expect(notSureButton).toBeVisible(); + + const yesBox = await yesButton.boundingBox(); + const notSureBox = await notSureButton.boundingBox(); + expect(yesBox).not.toBeNull(); + expect(notSureBox).not.toBeNull(); + // A content-sized button's width tracks its label length, not a fixed ratio - 1.5x is a + // generous tolerance for "Yes — that's the one" being a longer label than "Not sure", + // while still catching a full-width regression (which measures ~2x at this viewport). + expect(yesBox!.width).toBeLessThan(notSureBox!.width * 1.5); + }); });