From 2e5332cef5e1e34a21352198d22fbf55648c9503 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Thu, 6 Aug 2026 03:48:43 +0000 Subject: [PATCH] whatsthat: layout pass reclaiming desktop/mobile space (#705, #707, #710, #711) - Reference card (Subject) is pinned within the hero container via position: sticky, not the viewport, so it stays visible while scrolling at every width, including WD3's compacted phone strip. - WtcHero/WtcHead's own max-width raised from 1180px to 1600px, and whatsthat.tsx now passes fullWidth to ProjectContainer - the sitewide 1200px ContentMaxWidth cap was the actual constraint keeping desktop narrow regardless of the hero's own value. - The attribute-chip panel shows automatically for identify_printing/ confirm_suggestion questions instead of sitting behind a manual toggle, and no longer shares a box with the reference card (it renders in the questions column now, not a ring around the card) - guaranteeing it can never occlude the pinned card. - An implied-negative chip collapses its label instead of only dimming, reclaiming the row space several dimmed siblings used to reserve. - SuggestedThumb/CandidateButton release their overflow:hidden clip for exactly the hover duration ZoomableThumbnail scales art up, so the zoom is no longer cut flush at the tile edge. - The Level 1 Yes button drops its oversized .big modifier, reading at the same size as its ActionGrid siblings. Tests: Jest 73/73 suites, 676/676 passing. Playwright 351 total, 344 passed + 1 pre-existing parallel-load flake (confirmed passing standalone, unrelated file) + 6 skipped. npm run build compiles clean. coverage-delta gate clean (2 acks for a default-visibility rename and a retired ring-layout test, both in .github/coverage-acks.txt). --- .github/coverage-acks.txt | 17 +++ .../mockups/wtc-rebuild/SPEC-wtc-rebuild.md | 57 ++++++++ .../attributeChips/AttributeChipPanel.tsx | 90 ++++++++---- .../attributeChips/attributeChipRender.tsx | 21 ++- .../src/features/printingTags/cardPanel.tsx | 7 + .../questionFeed/QuestionFeed.test.tsx | 22 +-- .../features/questionFeed/QuestionFeed.tsx | 89 ++++++++---- frontend/src/pages/whatsthat.tsx | 8 +- frontend/tests/QuestionFeed.spec.ts | 11 +- frontend/tests/QuestionFeedResponsive.spec.ts | 137 ++++++++++++++++-- 10 files changed, 367 insertions(+), 92 deletions(-) diff --git a/.github/coverage-acks.txt b/.github/coverage-acks.txt index f0187627d..6c3f0028d 100644 --- a/.github/coverage-acks.txt +++ b/.github/coverage-acks.txt @@ -144,3 +144,20 @@ coverage-ack: frontend/tests/QuestionFeed.spec.ts::question feed - Level 2 illus # describe both halves of the contract, and extended with a mocked handler + payload assertion # for the new POST - same surface (the NOT SURE button), same "no printing vote" claim kept. coverage-ack: frontend/tests/QuestionFeed.spec.ts::question feed - confirm_suggestion question type > NOT SURE drops to Level 2's candidate grid without casting a vote — PR #731 (issue #712): renamed to "NOT SURE drops to Level 2's candidate grid without casting a printing vote, but does POST an abstention" now that NOT SURE fires a fire-and-forget POST to the new /2/submitQuestionAbstention/ endpoint; the "no printing vote" assertion is kept, extended with coverage of the new abstention POST. + +# Layout pass (2026-08-06, issues #705/#707/#710/#711) - A4 amendment (SPEC-wtc-rebuild.md, +# 2026-08-05 owner ruling) supersedes the attribute-chip panel's collapsed-by-default behavior: +# it's now shown automatically for identify_printing/confirm_suggestion questions. The old +# title's factual claim ("collapsed by default") is false under the new default, so it was +# renamed rather than left stale; the underlying toggle-still-works coverage is kept, not +# dropped - see the new title's own body. +coverage-ack: frontend/tests/QuestionFeed.spec.ts::question feed - Level 2 (candidate grid) > the attribute-chip filter is collapsed by default and expands on tap — renamed to "the attribute-chip filter is shown automatically for identify_printing questions, and can be hidden" (A4 amendment, SPEC-wtc-rebuild.md): the panel now defaults to shown for this question type instead of collapsed; the toggle-still-works assertion is kept, just inverted to match the new default. + +# Layout pass (2026-08-06, issue #707) - AttributeChipPanel's ring-around-card composition +# (CardArea, used only by this page's Level 2 call site) is retired: the chip panel now renders +# in QPanel, never sharing a box with the reference card, so the "ring squeezes the card at a +# narrow width" failure mode this test guarded against can no longer occur structurally. Dropped +# in favour of a direct "chips never overlap the pinned card" check under a new title, covering +# the same underlying hard constraint (chips must never occlude the reference card) against the +# new mechanism instead of the retired one. +coverage-ack: frontend/tests/QuestionFeedResponsive.spec.ts::question feed - Level 2 layout containment (real-device regression guard) > at 360px with the attribute-chip filter expanded, the ring collapses to a stack instead of squeezing the card — dropped: the ring-around-card composition (CardArea) it guarded is retired by issue #707 (chips render in QPanel now, never sharing a box with the card); replaced by "at 360px with the attribute-chip filter shown, the chips never overlap the pinned reference card" covering the same never-occlude constraint against the new mechanism. diff --git a/docs/proposals/mockups/wtc-rebuild/SPEC-wtc-rebuild.md b/docs/proposals/mockups/wtc-rebuild/SPEC-wtc-rebuild.md index e162748f2..4614e89d2 100644 --- a/docs/proposals/mockups/wtc-rebuild/SPEC-wtc-rebuild.md +++ b/docs/proposals/mockups/wtc-rebuild/SPEC-wtc-rebuild.md @@ -120,6 +120,63 @@ PR #446 shipped:** button size, and reference-card visibility). Refer to the epic for current status rather than duplicating its contents here. +--- + +**AMENDMENTS — 2026-08-06, closing #705/#707/#710/#711 as one layout pass (implements A1/A2/A4 +above rather than proposing anything new):** + +8. **A8: A2's sticky mechanism, concretely.** `Subject` (QuestionFeed.tsx) carries + `position: sticky; top: 16px` unconditionally — including inside the WD3-compacted + horizontal strip below the hero's 560px fold, so "visible at every viewport" is genuinely + universal, not a wide-container-only affordance. No `useViewportTier`/media-query gate: the + rule is never conditional on viewport width, satisfying A1's container-first policy. The + actual scrolling ancestor sticky resolves against is Layout.tsx's `ContentContainer` + (`position: fixed; overflow-y: scroll`, top-offset by the real navbar height already) — not + `document` — so no additional navbar-height accounting was needed in QuestionFeed.tsx itself. + +9. **A9: Desktop horizontal space — the actual constraint was one level up.** WtcHero/WtcHead's + own `max-width` (1180px, §1c) was never the binding constraint on desktop: the sitewide + `ContentMaxWidth` (1200px, `Layout.tsx`) wraps every route in a `MaxWidthContainer` that + capped this page just as tightly regardless of WtcHero's own value. `whatsthat.tsx` now + passes `fullWidth` to `ProjectContainer` — the same escape hatch `/display` already uses for + an identical problem (`Layout.tsx`'s own comment on `MaxWidthContainer`) — and WtcHero/WtcHead's + `max-width` moves from 1180px to 1600px (superseding that one §1c row; every other binding + value in the table is unchanged). This is a static value, not a viewport breakpoint or `vw` + unit, so A1 stands: the hero still doesn't size itself against the viewport, it simply has + more room to fold its existing `clamp()`/`auto-fill` primitives into on a wide screen. + +10. **A10: WD6's ring-around-card composition retired for this page (supersedes the + §4/§5 "shortlist container" and "AttributeChipPanel" rows' original composition, not + listed as a change row there).** A4 makes the panel auto-visible far more often than its + old opt-in toggle ever did, and `AttributeChipPanel`'s `ChipRing` reflows via `@media (min-width: 576px)` — a viewport query, not a container query — which inside the narrow + (`clamp(240px,30cqi,340px)`) Subject column it used to render in would squeeze three chip + groups into a box that never has room for them, at exactly the widths A4 now shows it by + default. Resolution: the panel no longer shares a box with the card at all. It renders in + QPanel instead (its own `cardSlot` prop is now optional; omitting it swaps `ChipRing` for a + plain `FlatChipStack` — three stacked `ChipRow`s, no ring), which trivially satisfies A4's + "never occlude the reference card" hard constraint by construction (different flex column + from the now-pinned Subject, A2) rather than by geometry that has to keep working as chip + counts change. + +11. **A11: Chip collapse (#707b).** An implied-negative chip (§ "chip (tri-state)" row, + `attributeChipRender.tsx`) collapses its label away instead of only dimming — the reserved + "dead space" the owner reported. Its two state buttons (and their full 44px tap targets, + including the existing tap-to-override-the-implied-state affordance) are unchanged; only + the label text's box is removed. + +12. **A12: Hover-zoom clipping (#705) — root cause was one level higher than the binding + table's own rows.** `ZoomableThumbnail`/`ArtPlaceholder` (both unchanged) were never the + defect; their two immediate ancestors on this page (`SuggestedThumb`, `CandidateButton`) + each clip with `overflow: hidden` for the resting state's own rounded-corner look, which + also clips the hover-scale. Both now add `&:hover { overflow: visible; }` — clipping stays + for the resting state, releases for exactly the hover duration the zoom needs it to. No + §1c row changes (radius/size/colour are untouched). + +13. **A13: Yes button (#711).** Drops `.big` (17px/800-weight/oversized padding) from the + Level 1 Yes button; keeps `.block` (full width). Reads at the same 15px/600-weight text as + 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. + 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. diff --git a/frontend/src/features/attributeChips/AttributeChipPanel.tsx b/frontend/src/features/attributeChips/AttributeChipPanel.tsx index a28b04afe..a49d4d63d 100644 --- a/frontend/src/features/attributeChips/AttributeChipPanel.tsx +++ b/frontend/src/features/attributeChips/AttributeChipPanel.tsx @@ -104,6 +104,15 @@ const CardArea = styled.div` position: relative; `; +// A caller with no card to center (QuestionFeed.tsx's Level 2, since the reference card lives +// in its own pinned Subject column now - issue #707) gets a plain vertical stack of the same +// three ChipRow groups instead of the ring - there's no card slot for a ring to form around. +const FlatChipStack = styled.div` + display: flex; + flex-direction: column; + gap: 0.6rem; +`; + interface AttributeChipPanelProps { backendURL: string; cardIdentifier: string; @@ -115,8 +124,9 @@ interface AttributeChipPanelProps { onChipStatesChange: (next: Record) => void; /** The card image/reveal-overlay/caption, rendered dead center with chips forming a ring * around it - passed in rather than owned here so QuestionFeed.tsx keeps sole ownership of - * the reveal-animation state machine (revealed/onAnimationEnd) that slot's contents depend on. */ - cardSlot: React.ReactNode; + * the reveal-animation state machine (revealed/onAnimationEnd) that slot's contents depend + * on. Omitted entirely by a caller with nothing to center (FlatChipStack above). */ + cardSlot?: React.ReactNode; /** Called instead of the usual error toast when a submission is rejected with 429 - this * component has only one caller (QuestionFeed.tsx), so this is effectively always provided, * but stays optional to match the same safe-default convention as the other funnel @@ -155,38 +165,58 @@ export function AttributeChipPanel({ // arbitrary but fixed assignment, not a semantic left/right meaning for either group. const [leftGroup, rightGroup] = EXCLUSION_GROUPS; + const legend = hasAttributeLean(confidence) && ( +

+ Chip color shows how community + machine votes lean - not a confirmed + fact. +

+ ); + const topArea = ( + + {STANDALONE_CHIPS.map((chip) => + renderAttributeChip(chipArgs, chip.tagName, chip.label) + )} + + ); + const leftArea = leftGroup != null && ( + + {leftGroup.chips.map((chip) => + renderAttributeChip(chipArgs, chip.tagName, chip.label) + )} + + ); + const rightArea = rightGroup != null && ( + + {rightGroup.chips.map((chip) => + renderAttributeChip(chipArgs, chip.tagName, chip.label) + )} + + ); + + if (cardSlot == null) { + return ( + <> + {legend} + + {topArea} + {leftArea} + {rightArea} + + + ); + } + return ( <> - {hasAttributeLean(confidence) && ( -

- Chip color shows how community + machine votes lean - not a confirmed - fact. -

- )} + {legend} - - {STANDALONE_CHIPS.map((chip) => - renderAttributeChip(chipArgs, chip.tagName, chip.label) - )} - - {leftGroup != null && ( - - {leftGroup.chips.map((chip) => - renderAttributeChip(chipArgs, chip.tagName, chip.label) - )} - - )} + {topArea} + {leftArea} {cardSlot} - {rightGroup != null && ( - - {rightGroup.chips.map((chip) => - renderAttributeChip(chipArgs, chip.tagName, chip.label) - )} - - )} + {rightArea} ); diff --git a/frontend/src/features/attributeChips/attributeChipRender.tsx b/frontend/src/features/attributeChips/attributeChipRender.tsx index 187d57c68..b16cfa0cd 100644 --- a/frontend/src/features/attributeChips/attributeChipRender.tsx +++ b/frontend/src/features/attributeChips/attributeChipRender.tsx @@ -33,10 +33,17 @@ import { // Size, AA) call for, on the ring's own answer controls. min-height guarantees the real hit // area of the group as a whole regardless of label length; flex centering keeps the label // vertically centered against the taller Yes/No buttons beside it. +// Issue #707b - an implied-negative chip (a sibling of an explicitly-positive exclusion-group +// chip) used to stay full width/height at 0.45 opacity, reserving exactly as much row space as +// before the selection that dimmed it. It now collapses its LABEL away (below) - narrowing to +// just its two state buttons - rather than only dimming, so several dimmed siblings actually +// reclaim the row space the owner reported as dead. Height/tap-targets are unchanged (still the +// full 44px buttons, still directly tappable to override the implied state - see this file's +// header comment). export const ChipGroup = styled.div<{ impliedNegative: boolean }>` border: 2px solid rgba(0, 0, 0, 0.25); border-radius: 0.5rem; - opacity: ${(props) => (props.impliedNegative ? 0.45 : 1)}; + opacity: ${(props) => (props.impliedNegative ? 0.6 : 1)}; color: inherit; font-size: 0.85rem; white-space: nowrap; @@ -46,10 +53,11 @@ export const ChipGroup = styled.div<{ impliedNegative: boolean }>` overflow: hidden; `; -export const ChipLabel = styled.span<{ fill: string }>` +export const ChipLabel = styled.span<{ fill: string; collapsed?: boolean }>` background-color: ${(props) => props.fill}; - padding: 0.35rem 0.5rem; - display: inline-flex; + padding: ${(props) => (props.collapsed ? "0" : "0.35rem 0.5rem")}; + width: ${(props) => (props.collapsed ? "0" : "auto")}; + display: ${(props) => (props.collapsed ? "none" : "inline-flex")}; align-items: center; `; @@ -168,7 +176,10 @@ export function renderAttributeChip( data-chip-state={explicitState} title={lean ?? undefined} > - + {getTagDisplayName(label)} up, + so the resting look is unchanged but the zoom is no longer cut flush at this edge. */ + &:hover { + overflow: visible; + } + &:disabled { opacity: 0.6; cursor: default; diff --git a/frontend/src/features/questionFeed/QuestionFeed.test.tsx b/frontend/src/features/questionFeed/QuestionFeed.test.tsx index 80e94625c..bbef4bc35 100644 --- a/frontend/src/features/questionFeed/QuestionFeed.test.tsx +++ b/frontend/src/features/questionFeed/QuestionFeed.test.tsx @@ -117,21 +117,21 @@ function questionFeedOnce() { } describe("QuestionFeed", () => { - it("the attribute-chip filter is collapsed by default, and 'None of these' works without touching it", async () => { + it("the attribute-chip filter is shown automatically for identify_printing questions, and 'None of these' works without touching it", async () => { server.use(questionFeedOnce()); renderFeed(); await revealCard(); expect( - screen.queryByTestId("attribute-chip-panel") - ).not.toBeInTheDocument(); + await screen.findByTestId("attribute-chip-Full Art") + ).toBeInTheDocument(); const noMatchButton = await screen.findByTestId("question-feed-no-match"); expect(noMatchButton).not.toBeDisabled(); fireEvent.click(screen.getByTestId("question-feed-filter-toggle")); expect( - await screen.findByTestId("attribute-chip-Full Art") - ).toBeInTheDocument(); + screen.queryByTestId("attribute-chip-panel") + ).not.toBeInTheDocument(); }); it("clicking 'None of these' submits a no-match printing vote", async () => { @@ -726,8 +726,8 @@ describe("QuestionFeed", () => { renderFeed(); await revealCard(); - // expand the filter and set "Full Art" positive - narrows item 1's grid to printing-2 only - fireEvent.click(screen.getByTestId("question-feed-filter-toggle")); + // identify_printing questions show the filter automatically now - set "Full Art" positive + // directly, narrowing item 1's grid to printing-2 only. fireEvent.click(await screen.findByTestId("attribute-chip-Full Art-yes")); await waitFor(() => expect(screen.queryByTestId("attribute-chip-Full Art")).toHaveAttribute( @@ -747,10 +747,12 @@ describe("QuestionFeed", () => { // survived, neither would render, reproducing the reported empty-grid symptom. expect(await screen.findByAltText("def 3")).toBeInTheDocument(); expect(screen.getByAltText("ghi 4")).toBeInTheDocument(); - // the filter panel itself resets closed too, same as any other fresh item + // the chip states themselves reset too, same as any other fresh item - the "Full Art" + // chip (still auto-shown, item 2 is identify_printing too) is untouched again, not + // carrying item 1's stale "positive" state forward. expect( - screen.queryByTestId("attribute-chip-panel") - ).not.toBeInTheDocument(); + await screen.findByTestId("attribute-chip-Full Art") + ).toHaveAttribute("data-chip-state", "untouched"); }); // Issue #503 (WTC phase C2) / #524 - wiring the illustration-grouped grid (C1) to diff --git a/frontend/src/features/questionFeed/QuestionFeed.tsx b/frontend/src/features/questionFeed/QuestionFeed.tsx index 207f95400..6ff5838ca 100644 --- a/frontend/src/features/questionFeed/QuestionFeed.tsx +++ b/frontend/src/features/questionFeed/QuestionFeed.tsx @@ -116,7 +116,7 @@ const WtcHead = styled.div` justify-content: space-between; gap: 14px; flex-wrap: wrap; - max-width: 1180px; + max-width: 1600px; margin: 0 auto 12px; @media (max-width: 520px) { @@ -168,7 +168,7 @@ const SolvedDots = styled.span` const WtcHero = styled.div` container-type: inline-size; container-name: hero; - max-width: 1180px; + max-width: 1600px; margin: 0 auto; display: flex; flex-wrap: wrap; @@ -176,10 +176,20 @@ const WtcHero = styled.div` align-items: start; `; +// Reference-card visibility (issue #710, A2 amendment) - pinned WITHIN the hero container via +// `position: sticky`, not the page viewport (WD4's rejection of viewport-COUPLED positioning +// stands; this is scoped to WtcHero's own box, since a flex item's sticky containing block is +// its flex container - it stays pinned only as long as WtcHero itself, whose height spans the +// taller QPanel column too, hasn't scrolled past). Applies unconditionally at every hero +// width, including the WD3-compacted horizontal strip below the 560px fold - there is no +// separate mobile-only rule to keep the "always visible" guarantee genuinely universal. const Subject = styled.div` flex: 1 1 300px; min-width: 0; max-width: clamp(240px, 30cqi, 340px); + position: sticky; + top: 16px; + z-index: 1; /* Continuous fold point (section 3's table): the subject compacts to horizontal (WD3) on a narrow CONTAINER, not a narrow viewport - keeps the confirm hero reachable near the top @@ -348,6 +358,18 @@ const IllustrationCredit = styled.div` margin-bottom: 8px; `; +// Issue #707 - the attribute-chip panel's home now that it no longer replaces the subject +// card slot (see plainCardPanel's own comment). Framed like the page's other secondary +// panels (SuggestedCard/NegWrap/OpenWrap) rather than left bare, so it reads as a distinct, +// dismissible section of QPanel instead of loose content between the prompt and the grid. +const FilterPanelWrap = styled.div` + background: var(--conf); + border: 1px solid var(--divider); + border-radius: var(--r-card); + padding: 10px 12px; + margin: 10px 0; +`; + // The spec's `.btn` base + variants (section 1c) - min 44px thumb targets (mobile funnel // pass, WCAG 2.5.5/Apple HIG), replacing the old `ThumbButton`/`FilterToggleButton` gold // overrides with plain token-derived variants. A native