From 5bdc9b1941ec1478a0576984dfc5bb800dda2e4d Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Wed, 22 Jul 2026 01:18:42 +0000 Subject: [PATCH] Fix round: /whatsthat quiz-reveal hero owner-review issues - title-under-navbar + /display toolbar clash: add useNavbarHeight() (ResizeObserver-measured, issue #250) for ContentContainer and the hero's own height calc, scoped to these two confirmed-broken consumers only - pinning broken live: replace HeroGrid's incomplete max-height calc (never accounted for StarburstBackground's real padding or Footer's height) with a flex chain (PageColumn -> StarburstContent -> FeedRoot -> HeroGrid) so it can't drift out of sync again; strengthen the Playwright pinning assertion with a real mouse-wheel scroll instead of only scrolling the inner box - remove the old intro/headline/subcounts/flavor text from the main flow, replaced by one small stats line at the bottom of the question column - flatten the blue radial vignette to a subtler two-stop gradient - fix hover-zoom/hover-burst clipping at the candidate grid's edges (HeroQuestionsArea bleed + an edge-column HoverBurst variant), with a new regression test Co-Authored-By: Claude Fable 5 --- docs/features/printing-tags.md | 124 +++++++++++---- docs/troubleshooting.md | 22 ++- frontend/src/common/useNavbarHeight.ts | 72 +++++++++ .../src/features/printingTags/cardPanel.tsx | 16 +- .../questionFeed/QuestionFeed.test.tsx | 6 +- .../features/questionFeed/QuestionFeed.tsx | 142 +++++++++++------- frontend/src/features/ui/Layout.tsx | 38 ++++- frontend/src/pages/whatsthat.tsx | 70 +++++++-- frontend/tests/QuestionFeed.spec.ts | 3 - frontend/tests/QuestionFeedResponsive.spec.ts | 93 ++++++++++++ 10 files changed, 476 insertions(+), 110 deletions(-) create mode 100644 frontend/src/common/useNavbarHeight.ts diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index ca398bd4d..a00d4c0f8 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -165,9 +165,13 @@ printings, artists, tags, and moderation from one screen. defaults to `UNRESOLVED` on both `printing_tag_status` and `artist_vote_status` simultaneously, which is why a flat sum of the three category counts (the pre-fix implementation) over-counted every - such card 2-3x. `QuestionFeed.tsx`'s headline leads with `confirmable` - ("N quick confirmations ready") when non-zero, falling back to `total` - once nothing quick remains. + such card 2-3x. `QuestionFeed.tsx` surfaces all three via a single small, + muted stats line ("N ready · N in catalog · N contested", + `question-feed-stats`) tucked at the bottom of the question column - a + fix round on the quiz-reveal hero (PR #305/#308's owner review) retired + the old standalone headline/subcounts text that used to sit above the + question and eat into the vertical space needed to keep the answer + buttons above the fold. ## Frontend architecture @@ -578,17 +582,22 @@ printings, artists, tags, and moderation from one screen. - **Palette** (W6/W7): the page's old `#ff4719` orange full-bleed field — kept as "the page's deliberate identity" by a 2026-07-18 decision — is **superseded**: `StarburstBackground` (`whatsthat.tsx`) is now a - deep-blue radial vignette (`#1a4f8a` → `#123a6b` → body `#0f2537`), - reconciling with issue #302's sitewide orange retheme instead of - clashing with it (two similar oranges were _less_ distinguishable - than the old blue-on-orange pairing this page shipped with - originally). The page-scoped `ACCENT_NAVY` override (buttons/links/ - pills recolored so they'd clear AA against the orange field) is + deep-blue field, reconciling with issue #302's sitewide orange retheme + instead of clashing with it (two similar oranges were _less_ + distinguishable than the old blue-on-orange pairing this page shipped + with originally). The page-scoped `ACCENT_NAVY` override (buttons/ + links/pills recolored so they'd clear AA against the orange field) is removed entirely — off that field and onto the standard dark body, the sitewide accent `#df6919` already clears AA (4.61:1) with no override needed. The starburst's own two identity colors (outer blue `#4d8ddf`, inner white `#ffffff`, `starburstShape.ts`) are unchanged — - only the field _behind_ it moved. + only the field _behind_ it moved. A fix round on the owner's live + review (PR #305/#308) flattened the original three-stop radial + (`#1a4f8a` → `#123a6b` → body `#0f2537`, a pronounced vignette that + "felt unnatural") to a two-stop `#1d4d82` → `#123a6b` gradient with no + third, darker stop at all — a small highlight around the starburst + that settles into a flat deep blue well before the edges, instead of + fading further toward near-black. - **Enlarged hero starburst**: `BurstSvg` (`cardPanel.tsx`) gained an additive, default-off `$hero` prop (`width: 230%` of the card box vs. the existing `55%`, at `md`+) so it dominates the hero's left column @@ -603,24 +612,83 @@ printings, artists, tags, and moderation from one screen. field, a real legibility problem the desktop case doesn't have. - **Reference-card pinning** (owner addendum): the card must stay fully visible while the user works through the questions. At `md`+, the - whole `HeroGrid` is bounded to one viewport-height row - (`calc(100dvh - NavbarHeight - 2rem)`) and only `HeroQuestionsArea` - scrolls internally (`overflow-y: auto`, a subtle themed scrollbar via - `scrollbar-color`/`::-webkit-scrollbar-thumb`, not default browser - chrome) — the card's own grid cell never scrolls, so the old sticky- - plus-negative-z-index mechanism (see the superseded `cardPanel.tsx` - bullet above) has nothing left to do and was removed. Below `md` - (where a bounded-height scroll box would read as a cramped cage on a - small screen), `HeroCardArea` instead becomes a `position: sticky` - compact bar (shrunk via `max-width`, same `cardNode` markup, not a - separate rendering) that pins to the viewport top while the questions - scroll beneath it — the phone-shaped interpretation of the same - "keep the reference comparable" intent, not a literal port of the - desktop mechanism. Verified via `QuestionFeedResponsive.spec.ts`'s - scroll-then-reread-`boundingBox()` assertion (full equality, not just - visibility) on desktop; the phone interpretation was visually - verified via real Playwright screenshots (scrolled state), not - covered by its own dedicated assertion. + whole `HeroGrid` is bounded to one viewport-height row and only + `HeroQuestionsArea` scrolls internally (`overflow-y: auto`, a subtle + themed scrollbar via `scrollbar-color`/`::-webkit-scrollbar-thumb`, + not default browser chrome) — the card's own grid cell never scrolls, + so the old sticky-plus-negative-z-index mechanism (see the superseded + `cardPanel.tsx` bullet above) has nothing left to do and was removed. + Below `md` (where a bounded-height scroll box would read as a cramped + cage on a small screen), `HeroCardArea` instead becomes a + `position: sticky` compact bar (shrunk via `max-width`, same + `cardNode` markup, not a separate rendering) that pins to the + viewport top while the questions scroll beneath it — the phone-shaped + interpretation of the same "keep the reference comparable" intent, + not a literal port of the desktop mechanism. Verified via + `QuestionFeedResponsive.spec.ts`'s scroll-then-reread-`boundingBox()` + assertion (full equality, not just visibility) on desktop; the phone + interpretation was visually verified via real Playwright screenshots + (scrolled state), not covered by its own dedicated assertion. + - **Fix round (PR #305/#308 owner review)**: the original + `HeroGrid { max-height: calc(100dvh - NavbarHeight - 2rem) }` + passed CI but let the whole page scroll live — the flat `2rem` + guess never accounted for `StarburstBackground`'s real + padding/margin (4.5rem, not 2rem) or `Footer`'s entire height below + it, so total page content routinely exceeded + `Layout.tsx`'s `ContentContainer` and forced its own outer + scrollbar to activate, moving the "pinned" card along with + everything else. Replaced with pure flex sizing instead of a + hand-maintained calc: `whatsthat.tsx`'s `PageColumn` (flex column, + height locked to `calc(100dvh - navbarHeight)` at `md`+ via the new + `useNavbarHeight()` hook — see below) wraps `StarburstBackground` + + `Footer`, `StarburstBackground` takes `flex: 1 1 auto; min-height: 0` (whatever's left after `Footer`'s natural size), and + that flex chain propagates down through `StarburstContent` → + `QuestionFeed.tsx`'s own `FeedRoot` → `HeroGrid` (now + `flex: 1; min-height: 0` instead of its old `max-height` calc) — it + structurally can't drift out of sync with `Footer`'s real height + again. Deliberately NOT extended through the moderator + `Tab.Container`/`Tab.Content`/`Tab.Pane` switcher (a small, + privileged audience) — that branch keeps its previous natural/auto + height, unchanged, rather than wiring three more react-bootstrap + wrappers into the flex chain. `useNavbarHeight()` + (`frontend/src/common/useNavbarHeight.ts`) replaces the hardcoded + `NavbarHeight` constant (issue #250 — confirmed 64-88px real vs the + constant's 50px in some auth/nav-link states, see + `docs/troubleshooting.md`) with a `ResizeObserver`-measured value, + for `Layout.tsx`'s `ContentContainer` (sitewide — this is what was + hiding the first several px of top-of-page content, including this + page's hero title and `/display`'s own toolbar, behind the real + navbar in a taller-navbar state) and this page's `PageColumn`. + Scoped, not a blanket swap — every other `NavbarHeight` consumer + (`Explore.tsx`, `ProjectEditor.tsx`, `FinishedMyProject.tsx`) is + unchanged, and #250 stays open for that broader decision (the hook + also doesn't yet handle the navbar's own crowded-state wrapping to + a second, taller row — only the single-row height mismatch). + Strengthened `QuestionFeedResponsive.spec.ts`'s own pinning + assertion with a real `page.mouse.wheel()` scroll (not just + `el.scrollTop` on the inner questions box) plus a + `content-container` testid check — that gap (never exercising the + outer container) is exactly what let the original bug pass CI. + - **Hover-zoom/hover-burst edge clipping** (fix round, PR #305/#308 + owner review): `ZoomableThumbnail`'s hover-zoom and `HoverBurst`'s + glow (`cardPanel.tsx`) were both deliberately built with no + `overflow: hidden` of their own so the enlarged art/glow could pop out + uncropped — the pinning fix above's `overflow-y: auto` on + `HeroQuestionsArea` forces `overflow-x: auto` too (CSS's own "visible + computes to auto once the other axis isn't visible" rule), silently + re-clipping both right at that box's left/right edges, worst on the + left where the first column in every row sits flush against it with + no buffer. Two-part fix: `HeroQuestionsArea` itself gets `margin: 0 -2.5rem` + matching `padding` (bleeds its own clip boundary 2.5rem + into the real empty space already there — the grid's own column gap + on the left, the page's outer margin on the right — with zero visible + resting-layout shift, verified via `boundingBox()` diff); `HoverBurst` + gained an additive `$edge` prop that shrinks its 331.2% bloom to 150% + for the first/last column specifically (`index % 4 === 0 || index % 4 === 3`, the two columns still short on room even with the added + bleed) — interior columns keep the full-size, unmodified glow. + Regression coverage: `QuestionFeedResponsive.spec.ts`'s new + horizontal-only containment check (vertical clipping at the top/ + bottom of this box is its intended scroll behaviour; only left/right + clipping is the bug). - **One hero card slot per item type, not per stage**: Level 3 dropped its old inline 48px thumbnail+name row (the shared hero card already shows the same art, one persistent slot across every stage per the diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index b35d1f7ae..8959ddaa0 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -882,11 +882,23 @@ new always-visible link is needed — put it in the right-hand icon button + the Sources button, with real spare width). This keeps the navbar's real rendered height identical to `origin/master`'s (confirmed via the same `boundingBox()` check) rather than papering over the -symptom with a longer Playwright timeout or a spec edit. The deeper fix -— replacing the hardcoded `NavbarHeight` constant with a real -`ResizeObserver`-driven measurement — is a legitimate follow-up (this -whole class of bug recurs the next time a link is added to that row) but -is out of scope for a single-link addition. +symptom with a longer Playwright timeout or a spec edit. + +**Partial deeper fix landed** (fix round on PR #305/#308's `/whatsthat` +quiz-reveal hero, owner review) — +`frontend/src/common/useNavbarHeight.ts` replaces the constant with a +real `ResizeObserver`-measured value, but only for the two consumers a +live report actually confirmed broken: `Layout.tsx`'s `ContentContainer` +(sitewide — every page's own top-of-content offset) and +`/whatsthat`'s own `PageColumn` height calc. Every other consumer +(`Explore.tsx`, `ProjectEditor.tsx`, `FinishedMyProject.tsx`) still uses +the static constant directly — this issue stays open for that broader +swap. The hook also only measures the navbar's _current_ height; it +doesn't change anything about the underlying wrapping behaviour above +(a crowded, every-link-visible state can still jump from 64px to 88px +when one more link tips it to a second line) — the hook picks that jump +up correctly via its own `ResizeObserver`, but the wrapping itself is +still exactly the layout fragility this entry describes. ## A `reparse_collector_evidence`/Stage D retraction pass silently never routes its own newly-touched cards to slow-path review diff --git a/frontend/src/common/useNavbarHeight.ts b/frontend/src/common/useNavbarHeight.ts new file mode 100644 index 000000000..308bec7b4 --- /dev/null +++ b/frontend/src/common/useNavbarHeight.ts @@ -0,0 +1,72 @@ +import { useEffect, useState } from "react"; + +import { NavbarHeight } from "@/common/constants"; + +/** + * Real, measured height of the fixed navbar (`nav.navbar`, rendered by Navbar.tsx), replacing + * the hardcoded `NavbarHeight` constant for consumers whose own positioning math breaks when the + * real navbar is taller than that guess (issue #250 - confirmed via + * `docs/troubleshooting.md`'s own `boundingBox()` measurement: 64px real vs the constant's 50px + * in the fully-authenticated, every-backend-feature-enabled state, and up to 88px once the + * crowded left-hand `Nav` wraps to a second line). `NavbarHeight` itself is left untouched here + * (still the SSR/pre-mount fallback below, and still what every OTHER heightDelta consumer - + * Explore.tsx, ProjectEditor.tsx, FinishedMyProject.tsx - uses directly) - this hook is an + * additive, opt-in replacement for the specific call sites confirmed broken by a real navbar/ + * content collision (Layout.tsx's `ContentContainer`, QuestionFeed.tsx's hero grid), not a + * blanket swap-out of the constant everywhere; #250 stays open for that broader decision. + * + * A plain `ResizeObserver` on the actual DOM node (found by the same `nav.navbar` selector + * `docs/troubleshooting.md` already uses to confirm this bug) rather than global state - the + * navbar is a single, always-mounted element and every consumer just needs its current height, + * so there's no real state to coordinate between components. `Navbar.tsx` itself renders inside + * `DisableSSR` (client-only), so this hook has nothing to observe until after that first client + * paint - a `MutationObserver` on `document.body` catches the node appearing, then hands off to + * `ResizeObserver` for every height change after that (nav wrapping to a second line on + * window resize, or a conditionally-rendered link appearing once an async `whoami`/backend- + * config query resolves). + */ +export function useNavbarHeight(): number { + const [height, setHeight] = useState(NavbarHeight); + + useEffect(() => { + let resizeObserver: ResizeObserver | null = null; + + const observeNavbar = (nav: Element) => { + resizeObserver = new ResizeObserver((entries) => { + const entry = entries[0]; + if (entry != null) { + setHeight(entry.contentRect.height); + } + }); + resizeObserver.observe(nav); + // Capture the current height immediately too - ResizeObserver's callback fires async on + // the next frame, and we'd rather start with a real measurement than the fallback for + // however long that takes. + setHeight(nav.getBoundingClientRect().height); + }; + + const existingNavbar = document.querySelector("nav.navbar"); + if (existingNavbar != null) { + observeNavbar(existingNavbar); + return () => resizeObserver?.disconnect(); + } + + // DisableSSR mounts the navbar a tick after this component's own first client render - + // watch for it to appear, then switch to ResizeObserver as above. + const mutationObserver = new MutationObserver(() => { + const nav = document.querySelector("nav.navbar"); + if (nav != null) { + mutationObserver.disconnect(); + observeNavbar(nav); + } + }); + mutationObserver.observe(document.body, { childList: true, subtree: true }); + + return () => { + mutationObserver.disconnect(); + resizeObserver?.disconnect(); + }; + }, []); + + return height; +} diff --git a/frontend/src/features/printingTags/cardPanel.tsx b/frontend/src/features/printingTags/cardPanel.tsx index 2f3720194..c32333a8c 100644 --- a/frontend/src/features/printingTags/cardPanel.tsx +++ b/frontend/src/features/printingTags/cardPanel.tsx @@ -319,7 +319,17 @@ export const CandidateButton = styled(Button)` // cheap. Centred on and scaled up from the button's own box, the same way the page-level // burst is centred on the subject card. Faded/scaled in via CSS on CandidateButton's // `:hover` above rather than JS state, so nothing needs to track which card is hovered. -export const HoverBurst = styled.svg` +// `$edge` (fix round, PR #305/#308) - the candidate grid's scroll box (HeroQuestionsArea, +// QuestionFeed.tsx) genuinely clips this burst's full 331.2% bloom for the leftmost/rightmost +// column in every row (confirmed via a real boundingBox()-vs-container overlap check, not just +// a visual read): even with that box's own added bleed room (2.5rem each side), a burst this +// oversized still overhangs past it for an edge column specifically (a middle column's bloom +// safely overlaps its neighbours instead, which is the existing, accepted "on-aesthetic bleed" +// look). Shrinking ONLY the edge columns' burst - not every candidate's - keeps the approved, +// full-size glow everywhere it geometrically fits, trading a uniformly smaller effect +// (which would look identical everywhere but weaker) for a fully unclipped one that's only +// slightly reduced right at the two edges where there's genuinely no more room to give it. +export const HoverBurst = styled.svg<{ $edge?: boolean }>` position: absolute; top: 50%; left: 50%; @@ -330,4 +340,8 @@ export const HoverBurst = styled.svg` transition: opacity 0.18s ease-out, transform 0.18s ease-out; pointer-events: none; z-index: -1; + + @media (min-width: 768px) { + width: ${(props) => (props.$edge ? "150%" : "331.2%")}; + } `; diff --git a/frontend/src/features/questionFeed/QuestionFeed.test.tsx b/frontend/src/features/questionFeed/QuestionFeed.test.tsx index 85fce2425..aa6623269 100644 --- a/frontend/src/features/questionFeed/QuestionFeed.test.tsx +++ b/frontend/src/features/questionFeed/QuestionFeed.test.tsx @@ -473,9 +473,9 @@ describe("QuestionFeed", () => { renderFeed(); await revealCard(); - const headline = await screen.findByTestId("question-feed-headline"); - expect(headline.textContent).toBe("Still need help with: 3 cards"); - expect(headline.textContent).not.toMatch(/undefined/); + const stats = await screen.findByTestId("question-feed-stats"); + expect(stats.textContent).toBe("0 ready · 3 in catalog · 0 contested"); + expect(stats.textContent).not.toMatch(/undefined/); }); it("shows the rate-limit banner (not a toast) when a printing vote is rejected with 429", async () => { diff --git a/frontend/src/features/questionFeed/QuestionFeed.tsx b/frontend/src/features/questionFeed/QuestionFeed.tsx index e74bd1d2d..6f3a17e08 100644 --- a/frontend/src/features/questionFeed/QuestionFeed.tsx +++ b/frontend/src/features/questionFeed/QuestionFeed.tsx @@ -44,7 +44,6 @@ import Row from "react-bootstrap/Row"; import { errorToNotification, isRateLimited } from "@/common/apiErrors"; import { getPrintingCandidateDataAttributes } from "@/common/cardDom"; -import { NavbarHeight } from "@/common/constants"; import { getOrCreateAnonymousId } from "@/common/cookies"; import { PrintingCandidate, @@ -160,10 +159,20 @@ const ThumbChip = styled(Button)` // awkward locked cage on a small screen instead - HeroCardArea's own mobile override (below) // takes the "keep the card comparable while scrolling" intent in a phone-shaped direction: // a condensed sticky bar, not this bounded-box mechanism. +// +// Fix round (PR #305/#308 owner review): this used to bound itself via its own +// `max-height: calc(100dvh - NavbarHeight - 2rem)` - wrong on two independent counts. (1) the +// static NavbarHeight constant regularly undercounts the navbar's real rendered height (issue +// #250), and (2) even with an accurate navbar height, the flat "2rem" guess ignored +// StarburstBackground's own real padding/margin (4.5rem, not 2rem) AND Footer's entire height +// below it - so the true total page content routinely exceeded the space actually available, +// forcing Layout.tsx's ContentContainer to scroll as a whole and breaking the "hero stays +// pinned" invariant live despite passing CI (a scrollTop-only assertion on the inner questions +// box never exercised that outer container). Replaced with `flex: 1; min-height: 0` below - +// FeedRoot/StarburstContent (whatsthat.tsx) now do this arithmetic structurally instead of via +// a hand-maintained calc, so this can't drift out of sync with either figure again. // --------------------------------------------------------------------------------------- -const HERO_MAX_HEIGHT = `calc(100dvh - ${NavbarHeight}px - 2rem)`; - const HeroGrid = styled.div` display: grid; gap: 1.25rem; @@ -175,7 +184,26 @@ const HeroGrid = styled.div` grid-template-columns: minmax(0, 42%) minmax(0, 1fr); grid-template-rows: auto minmax(0, 1fr); grid-template-areas: "card words" "card questions"; - max-height: ${HERO_MAX_HEIGHT}; + flex: 1; + min-height: 0; + } +`; + +// QuestionFeed's own root - `flex: 1; min-height: 0` at >= md opts into StarburstContent's +// `display: flex; flex-direction: column; height: 100%` (whatsthat.tsx) so HeroGrid's own +// `flex: 1` above has a real, resolvable height to consume. Only meaningful when this is a +// direct flex child of a flex parent with a definite height - true for the common, +// non-moderator render path (StarburstContent renders this directly), but NOT for the +// moderator Tab.Container/Tab.Content/Tab.Pane switcher (whatsthat.tsx), which isn't part of +// that flex chain - this deliberately falls back to auto/natural height there instead +// (unchanged from before this fix round), rather than extending the flex chain through three +// more react-bootstrap wrapper components for a small, privileged audience. +const FeedRoot = styled.div` + @media (min-width: 768px) { + display: flex; + flex-direction: column; + flex: 1; + min-height: 0; } `; @@ -221,6 +249,28 @@ const HeroWordsArea = styled.div` // Subtle themed scrollbar (owner addendum) - not the browser's default chrome, but not // hidden either: a hidden scrollbar on a genuinely-scrollable region is its own usability // trap, since "scrolling locks to the box" should still visibly look scrollable. +// Owner review (fix round, PR #305/#308): the candidate grid's hover-zoom (ZoomableThumbnail) +// and hover-burst (HoverBurst, cardPanel.tsx) were both deliberately built with no +// `overflow: hidden` of their own, specifically so the enlarged art/glow could pop out +// uncropped (see cardPanel.tsx's own comments on both, and docs/lessons.md's "a new wrapper +// placed around an existing effect can silently fight that effect's own CSS" entry for the +// exact prior incident this repeats) - this box's overflow-y: auto (needed for the pinning fix +// above) forces overflow-x: auto too per the CSS spec's own "visible computes to auto once the +// other axis isn't visible" rule (confirmed via getComputedStyle in this task's own debug +// pass), re-clipping both hover effects right at this box's left/right edges - worst on the +// left, where the first column sits flush with zero buffer at all. +// +// `margin: 0 -2.5rem` + matching `padding` bleeds this box's own clip boundary (its border/ +// padding edge - where overflow: auto actually clips, not wherever its children happen to be +// positioned) 2.5rem past its grid-assigned track on each side, into the real empty space +// already there (the grid's own 2.5rem column gap on the left; the page's own outer margin - +// StarburstContent's max-width cap plus the full-bleed background beyond it - on the right, +// measured at >= 130px in this task's own debug pass, comfortably more than needed). The +// padding exactly cancels the bleed for layout purposes (content still starts/ends at the +// exact same x-position as before - verified via boundingBox() diff), so this is purely +// additional clip headroom, not a visible resting-layout change. HoverBurst's own edge-column +// variant (cardPanel.tsx's $edge prop) targets the remaining gap this alone doesn't cover - +// its 331.2%-wide glow needs more room than 2.5rem gives on either side. const HeroQuestionsArea = styled.div` grid-area: questions; min-width: 0; @@ -228,7 +278,8 @@ const HeroQuestionsArea = styled.div` @media (min-width: 768px) { overflow-y: auto; min-height: 0; - padding-right: 0.75rem; + margin: 0 -2.5rem; + padding: 0 calc(2.5rem + 0.75rem) 0 2.5rem; scrollbar-width: thin; scrollbar-color: rgba(255, 255, 255, 0.25) transparent; @@ -268,15 +319,15 @@ function normalizeQuestionFeedCounts( } if (typeof raw === "number") { // legacy shape - no tier breakdown available, so confirmable/contested fall back to 0 - // (never show a false "quick confirmations ready" headline) and fresh mirrors total. + // (never show a false "N ready" count in the stats line below) and fresh mirrors total. return { total: raw, confirmable: 0, contested: 0, fresh: raw }; } // `total === fresh` is expected for the legacy number shape above (fresh is forced to mirror // total there), but for a genuine object-shaped response it would mean every card in the // catalog is still "fresh" - vanishingly unlikely in practice, and far more likely a sign that // this build is talking to a backend that hasn't finished rolling out the fresh/total split. - // Never shown to the user (the subcounts line dropped `fresh` entirely - see the audit note - // above the render) - this is purely a version-skew signal for whoever reads the console. + // Never shown to the user (the stats line below never renders `fresh` at all) - this is + // purely a version-skew signal for whoever reads the console. if (raw.total === raw.fresh) { console.warn( "QuestionFeed: counts.total === counts.fresh on a non-legacy response - possible backend/frontend version skew." @@ -480,8 +531,7 @@ export function QuestionFeed() { // The pre-classified exit for "this is real art, just not an official printing" - one tap // instead of "None of these" -> the reason strip, since the tap already told us why (see - // reason_tags.py's existing seeded "custom-art" tag - no new endpoint). Shares the funnel's - // usual flavor-text slot for a brief, specific confirmation instead of the generic copy. + // reason_tags.py's existing seeded "custom-art" tag - no new endpoint). const classifyAsCustomArt = () => { if (backendURL == null || item == null) { return; @@ -507,9 +557,6 @@ export function QuestionFeed() { "same-origin", "question-feed" ).catch(() => undefined); - setFlavorText( - "Logged as custom / alternate art - thanks! Moving on..." - ); fetchNext(); }) .catch(reportVoteFailed) @@ -974,7 +1021,12 @@ export function QuestionFeed() {

)} - {visibleCandidates.map((candidate) => ( + {/* Row is 4-wide at >= md (the only breakpoint HeroQuestionsArea's overflow-y: auto, + and therefore its hover-clip risk, applies at - see that component's own + comment) - every 1st/4th candidate in a row sits flush against the scroll box's + own left/right edge, where even the added bleed room isn't enough for + HoverBurst's full-size bloom (see that component's own $edge comment). */} + {visibleCandidates.map((candidate, index) => ( + {/* Keyed on the card identifier so both the pop-in-sync-with-THAT pulse (below) and @@ -1160,44 +1213,6 @@ export function QuestionFeed() { -

- Test your Magic: the Gathering knowledge! One card at a time, help - identify which real-world printing, artist, or descriptor tag each - card image depicts - contested and machine-suggested cards come - first, since they need your eyes the most. -

- {counts != null && ( - <> - {/* Headline leads with quick confirmations (tier 1 - an unresolved machine- - suggested printing awaiting a one-tap human yes/no) since that's the easiest, - fastest-to-clear category - falls back to the overall total once there's - nothing quick left, rather than always showing the same undifferentiated - "cards remaining" copy. */} -

- {counts.confirmable > 0 - ? `${counts.confirmable} quick confirmation${ - counts.confirmable !== 1 ? "s" : "" - } ready` - : `Still need help with: ${counts.total} card${ - counts.total !== 1 ? "s" : "" - }`} -

-

- {counts.total} in catalog · {counts.contested} contested -

- - )} - {flavorText != null && ( -

- {flavorText} -

- )} {rateLimited && ( // Persistent (not a self-dismissing toast) and dismissible - a rate-limit pause is // an expected, honest condition in a one-tap funnel, not a failure, so it gets its @@ -1217,8 +1232,27 @@ export function QuestionFeed() { )} {questionsNode} + {/* Fix round (PR #305/#308 owner review, "Maybe the old text should go?") - the + intro paragraph, "N quick confirmations ready" headline, and "N in catalog · N + contested" subcounts line used to sit ABOVE the question, eating into the vertical + budget that pulls the suggested-match card + answer buttons up beside the + reference card at eye level (the L1 case must fit entirely above the fold at + 1400x900 - see this task's own screenshots). The underlying counts are still + useful, just not at the cost of that space - a single small, muted line tucked + at the bottom of this column (after the question itself, never part of the + scroll budget a user has to clear before answering) keeps the information without + the vertical cost. */} + {counts != null && ( +

+ {counts.confirmable} ready · {counts.total} in catalog + · {counts.contested} contested +

+ )} - + ); } diff --git a/frontend/src/features/ui/Layout.tsx b/frontend/src/features/ui/Layout.tsx index b05c85366..5467aea9c 100644 --- a/frontend/src/features/ui/Layout.tsx +++ b/frontend/src/features/ui/Layout.tsx @@ -5,13 +5,14 @@ import { PropsWithChildren } from "react"; import Container from "react-bootstrap/Container"; import { Provider } from "react-redux"; -import { ContentMaxWidth, NavbarHeight } from "@/common/constants"; +import { ContentMaxWidth } from "@/common/constants"; import { getLocalStorageFavorites, getLocalStorageManualOverrides, } from "@/common/cookies"; import { useAppDispatch } from "@/common/types"; import { useChunkErrorRecovery } from "@/common/useChunkErrorRecovery"; +import { useNavbarHeight } from "@/common/useNavbarHeight"; import { useBackendSetter } from "@/features/backend/useBackendSetter"; import { ClientSearchContextProvider } from "@/features/clientSearch/clientSearchContext"; import { clientSearchService } from "@/features/clientSearch/clientSearchService"; @@ -34,15 +35,34 @@ const OverscrollProvider = styled(Provider)` overflow-y: hidden; // https://stackoverflow.com/a/69589919/13021511 `; -const ContentContainer = styled(Container)` +// `top`/`height` are driven by useNavbarHeight() (issue #250) rather than the static +// NavbarHeight constant - a real, measured value rather than a guess that regularly +// undercounts the navbar's actual rendered height (confirmed 64-88px vs the constant's 50px +// once enough nav links are visible/wrapped - see docs/troubleshooting.md). Getting this wrong +// isn't just a cosmetic few-px overlap: it's this container's own top-of-content offset, so an +// undercount hides that many pixels of every page's own top content behind the real fixed +// navbar (confirmed live on both /whatsthat's hero title and /display's toolbar - see +// docs/features/printing-tags.md's questionFeed section). Emotion's `css`-prop-style dynamic +// interpolation (a function of props) re-renders this rule whenever the measured height +// changes, same as any other prop-driven styled-component. +const ContentContainer = styled(Container, { + // Container is react-bootstrap's own component, not a native element - emotion only auto- + // filters non-DOM props for native `styled.div`-style tags (via @emotion/is-prop-valid), so a + // custom prop on a wrapped third-party component gets forwarded all the way down to the + // underlying
unless told not to (the same "React does not recognize the X prop" fix + // MaxWidthContainer below already needs for its own `fullWidth`). + shouldForwardProp: (prop) => prop !== "$navbarHeight", +})<{ $navbarHeight: number }>` overflow-y: scroll; overflow-x: hidden; - top: ${NavbarHeight}px; + top: ${(props) => props.$navbarHeight}px; position: fixed; height: calc( - 100vh - ${NavbarHeight}px + 100vh - ${(props) => props.$navbarHeight}px ); // for compatibility with older browsers - height: calc(100dvh - ${NavbarHeight}px); // handles the ios address bar + height: calc( + 100dvh - ${(props) => props.$navbarHeight}px + ); // handles the ios address bar `; interface MaxWidthContainerProps { @@ -77,8 +97,14 @@ export function ProjectContainer({ fullWidth = false, children, }: PropsWithChildren) { + const navbarHeight = useNavbarHeight(); return ( - + = md (`useNavbarHeight` below, not the static NavbarHeight +// constant - see that hook's own comment for why) - below md the page still scrolls normally +// end to end, matching HeroCardArea's own mobile sticky-bar design intent. +const PageColumn = styled.div<{ $navbarHeight: number }>` + display: flex; + flex-direction: column; + + @media (min-width: 768px) { + height: calc(100dvh - ${(props) => props.$navbarHeight}px); + } +`; + const StarburstBackground = styled.div` position: relative; /* Deliberately clip-path, not overflow: hidden - QuestionFeed.tsx's mobile HeroCardArea @@ -41,20 +67,29 @@ const StarburstBackground = styled.div` element's nearest scrolling ancestor resolves to). clip-path clips the same way visually without establishing a scroll container, so it doesn't have that side effect. */ clip-path: inset(0); - /* Deep-blue radial vignette (wtc-redesign-spec.md W6, issue #305) - retires the page's old - loud #ff4719 orange full-bleed field, reconciling with the new sitewide orange accent - (#302's retheme) rather than clashing with it. Sells the "blue and white starbursts" - hero (#305) and matches the quiz-reveal reference. */ + /* Deep-blue field (wtc-redesign-spec.md W6, issue #305) - retires the page's old loud + #ff4719 orange full-bleed field, reconciling with the new sitewide orange accent (#302's + retheme) rather than clashing with it. Sells the "blue and white starbursts" hero (#305) + and matches the quiz-reveal reference. + Fix round (PR #305/#308 owner review, "the blue fade feels unnatural") - the original + three-stop radial (a lighter highlight fading through the deep blue down to a much darker + near-black edge, 78% away) read as a much stronger vignette than intended. Flattened to + a small, subtle highlight around the starburst's own center that settles into a flat + deep blue well before the edges (no third, darker stop at all - the + gradient simply has nothing left to fade toward past its one real transition), so the + field reads as near-flat deep blue rather than a spotlight/vignette effect. The starburst + itself (BurstSvg/HoverBurst, unchanged) is still blue/white as approved. */ background: radial-gradient( - 120% 130% at 30% 42%, - #1a4f8a 0%, - ${HERO_FIELD_BLUE_DEEP} 46%, - #0f2537 78% + 120% 120% at 30% 40%, + #1d4d82 0%, + ${HERO_FIELD_BLUE_DEEP} 55% ); width: 100vw; margin-left: calc(50% - 50vw); padding: 1.5rem 0 2rem; margin-bottom: 1rem; + flex: 1 1 auto; + min-height: 0; /* The questions region simply inherits the sitewide theme now (wtc-redesign-spec.md W7) - the ACCENT_NAVY override this page used to carry existed only because buttons/links/pills @@ -66,12 +101,26 @@ const StarburstBackground = styled.div` // Sits above CardPanel's own local stacking context (see cardPanel.tsx) so the burst bleeding // out from behind the card doesn't cover the words/questions columns rendered alongside it. +// +// `display: flex; flex-direction: column; height: 100%` (PageColumn/StarburstBackground fix +// round above) hands QuestionFeed.tsx's own root a real, resolvable height to flex against - +// QuestionFeed.tsx's own FeedRoot opts into `flex: 1; min-height: 0` to consume it (replacing +// HeroGrid's old NavbarHeight-derived `max-height` calc), so the hero's own height ultimately +// traces back to PageColumn's real measured navbar height with no separate guess of its own. +// Safe for the moderator Tab.Container branch too (see PrintingQueueOrDefault below) - that +// branch simply doesn't opt into `flex: 1`, so it keeps its previous natural/auto height, +// unchanged. Below md, PageColumn itself has no explicit height (see its own comment), so this +// `height: 100%` resolves against nothing and is silently inert - today's mobile behaviour +// (whole page scrolls normally) is unaffected. const StarburstContent = styled.div` position: relative; z-index: 1; max-width: ${ContentMaxWidth}px; margin: 0 auto; padding: 0 1.5rem; + display: flex; + flex-direction: column; + height: 100%; `; // The wordmark's sliced-word teaser (WhatsThatWords, rendered inside QuestionFeed.tsx's hero @@ -97,9 +146,10 @@ function PrintingQueueOrDefault() { // regardless (see docs/features/moderation.md) const whoami = useGetWhoamiQuery(); const isModerator = whoami.data?.moderator === true; + const navbarHeight = useNavbarHeight(); return remoteBackendConfigured ? ( - <> + What's That Card? @@ -137,7 +187,7 @@ function PrintingQueueOrDefault() {