Fix round: /whatsthat quiz-reveal hero owner-review issues - #310
Merged
Conversation
- 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 <noreply@anthropic.com>
4 tasks
WilfordGrimley
added a commit
that referenced
this pull request
Jul 22, 2026
) * Fix /whatsthat question-box overflow and desync'd reveal animation Word stack rendered ~1.4x the mockup's proportion and, since #310 bounded the hero to one viewport row, ate directly into the question box's budget - even Level 1 didn't fit without an internal scroll. Shrinks the word stack via a viewport-relative clamp(), trims chrome-only spacing (HeroGrid row-gap, StarburstBackground padding/margin), and trims the Level 1 reference thumbnail, with a new hard Playwright assertion (no overflow + all four L1 controls fully on-screen at 1400x900). Also syncs the reveal fade / word pop / card pulse to the subject card's actual image-load event (via a shared imageLoaded gate + each animation's own animation-play-state), instead of firing independently at mount time. Along the way, fixed a real dependency-array bug this surfaced: an identifier-keyed catch-up effect silently skipped re-running on a duplicate fetch resolution (legitimate per the existing chipStates comment, and routine under dev-mode Strict Mode), permanently stranding the UI on "Loading..." - fixed by keying it on a generation counter bumped unconditionally in the reset block instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix /display sheet-position pill under-reporting last sheet (D17 CI failure) Boundary-clamp the scroll-position IntersectionObserver in DisplayPage.tsx so it can correctly report the first/last sheet at true scroll extremes; the center-band heuristic alone could never do so once a boundary sheet was too short to reach the band. Real, reproducible bug (not flaky), pre-existing since #313, unrelated to this PR's own /whatsthat changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fix round on the live
/whatsthatquiz-reveal hero (#305, merged as #308) from the owner's live review with screenshots. Not closing any issue - this is a follow-up implementation-fix pass on #305/#308's own design, reviewed live post-merge. Frame given for all five items: "consider what the user actually needs to do on this page" - see the reference card, see the question, answer, without scrolling in the common case. Style/placement/colors from #305 stayed approved; this is mechanics.1. Title sliced words rendering under the navbar
Root cause, stated plainly: this is issue #250 (
NavbarHeighthardcoded to 50px infrontend/src/common/constants.ts, but the real navbar can render taller - confirmed 64-88px indocs/troubleshooting.md's ownboundingBox()measurement once enough nav links are visible/wrapped).Layout.tsx'sContentContaineruses that same wrong constant for its owntop/heightoffset, so in a taller-navbar state its content starts painting under the real navbar's bottom edge - on/whatsthatthat'sWhatsThatWords(mobile stack: "words" is the very first grid area); the coordinator separately confirmed the same symptom family on/display's toolbar (also a plain block sitting right atContentContainer's own top offset).I could not reproduce a live navbar/content collision in this sandbox's own mocked test state (navbar measures exactly 50px there, matching the constant, since fewer conditional nav links render) - the bug is real and well-documented for the taller-navbar auth/link-count states, just not directly screenshot-reproducible here.
Fix: added
frontend/src/common/useNavbarHeight.ts- a plain hook (no Redux; aResizeObserveronnav.navbar,MutationObserverfallback for theDisableSSR-delayed first mount) returning the real measured navbar height, falling back to theNavbarHeightconstant pre-mount. Wired intoLayout.tsx'sContentContainer(sitewide - fixes both/whatsthatand/display) and/whatsthat's ownPageColumnheight calc.Scope/sitewide implications: deliberately NOT a blanket swap of the constant everywhere -
Explore.tsx,ProjectEditor.tsx, andFinishedMyProject.tsxstill use the staticNavbarHeightdirectly, unchanged. #250 stays open for that broader decision. The hook also only reacts to the navbar's current height - it doesn't fix the underlying crowded-state wrapping behaviour (a nav that jumps from 64px to 88px when one more link tips it to a second line); it just measures whatever the real height is at any given moment, correctly, via its ownResizeObserver.docs/troubleshooting.md's existing NavbarHeight entry is updated in place with this partial-fix note.2. Card/scroll-box pinning broken live despite passing CI
Root cause (found empirically, not by inspection):
HeroGrid's oldmax-height: calc(100dvh - NavbarHeight - 2rem)was wrong on two independent counts. (a) the same NavbarHeight mismatch as above, and (b) even with an accurate navbar height, the flat2remguess never accounted forStarburstBackground's real padding/margin (4.5rem, not 2rem) orFooter's entire height below it. A debug Playwright pass (measuring the fixedContentContainerancestor directly) confirmed this: even in a test state where the navbar height matched the constant exactly (50px = 50px),ContentContainerstill overflowed by 153px, and a realpage.mouse.wheel()scroll moved the "pinned" card by exactly that amount - proof the pinning invariant fails independent of the navbar question, and that ascrollTop-only assertion on the inner questions box (the pre-existing test) never exercises the outer container that actually broke.Fix: replaced the hand-maintained calc with real flex sizing.
whatsthat.tsx's newPageColumn(flex column, height-locked tocalc(100dvh - navbarHeight)at >= md viauseNavbarHeight()) wrapsStarburstBackground+Footer;StarburstBackgroundtakesflex: 1 1 auto; min-height: 0(whatever's left after Footer's natural size); that flex chain propagates down throughStarburstContent->QuestionFeed.tsx's newFeedRoot->HeroGrid(nowflex: 1; min-height: 0instead of its own calc). Structurally can't drift out of sync with Footer's real height again.Deviation: deliberately did NOT extend this flex chain through the moderator
Tab.Container/Tab.Content/Tab.Paneswitcher (a small, privileged audience) - that branch keeps its previous natural/auto height, unchanged, rather than wiring three more react-bootstrap wrapper components into the flex chain for a rare case.Test strengthened per the task:
QuestionFeedResponsive.spec.tsnow has a new test using a realpage.mouse.wheel()over the hero card (notel.scrollTopon the inner box), asserting the card'sboundingBox()is unchanged AND the newcontent-containertestid's ownscrollTopstays 0 - this is exactly the gap that let the original bug pass CI.3. Old text removed from the main flow
Removed the intro paragraph ("Test your Magic..."), the "N quick confirmations ready" headline, the "N in catalog - N contested" subcounts line, and the flavor line, all from the per-question render. The underlying counts are preserved as a single small, muted stats line ("N ready - N in catalog - N contested",
question-feed-stats) tucked at the very bottom of the question column, after the answer controls - never part of the vertical budget a user has to clear before answering.The reclaimed space pulls the suggested-match card + answer buttons up beside the reference card. Verified via screenshot at 1400x900 (Level 1 case): all four answer buttons (Yes/Not Sure/No/Skip) have
boundingBox().y + height <= 900- fully above the fold with room to spare, screenshot at/tmp/final-l1-1400x900.pngin the session (not committed - description here per the task's own instruction).Also quietly retired
classifyAsCustomArt's own deadsetFlavorText(...)call (its only visible effect would have been leaking a "Logged as custom / alternate art" message onto the unrelated "you're all caught up" terminal screen, now that the main-flow flavor-text render is gone) - the caught-up screen's own generic flavor text (fromadvance()) is untouched.4. Blue vignette flattened
StarburstBackground's radial-gradient was a 3-stop vignette (#1a4f8aat 0% -> deep blue at 46% -> near-black#0f2537at 78%). Flattened to a 2-stop gradient (#1d4d82at 0% -> deep blue at 55%, no third stop at all) - a small highlight around the starburst that settles into a flat deep blue well before the edges, instead of continuing to fade toward near-black. Starburst itself (blue/white,BurstSvg/HoverBurst) is unchanged.5. Hover-zoom/hover-burst clipped at the candidate grid's edges (owner live report, added mid-task)
ZoomableThumbnail's hover-zoom andHoverBurst's glow (cardPanel.tsx) were both deliberately built with nooverflow: hiddenof their own (seedocs/lessons.md's "a new wrapper placed around an existing effect can silently fight that effect's own CSS" entry - this is the same class of bug recurring). Fix #2'soverflow-y: autoonHeroQuestionsAreaforcesoverflow-x: autotoo (confirmed viagetComputedStyle- CSS's own "visible computes to auto once the other axis isn't visible" rule), silently re-clipping both right at that box's edges - worst on the left, where the first column in every row sits flush against it with zero buffer.Chose the inner-padding + edge-column-reduction hybrid (of the three options offered):
HeroQuestionsAreagetsmargin: 0 -2.5rem+ matchingpadding(bleeds its own clip boundary 2.5rem into real, already-empty space - the grid's own column gap on the left, the page's own outer margin on the right - with zero visible resting-layout shift, verified viaboundingBox()diff). This alone is enough for the image zoom (needs ~16px each side) but not enough forHoverBurst's much larger 331.2% bloom (needs ~170px each side) at the true edge columns specifically - soHoverBurstalso gained an additive$edgeprop shrinking it to 150% for the first/last column in each row (index % 4 === 0 || index % 4 === 3), while every interior column keeps the full-size, unmodified glow. Verified empirically (not just visually) via a horizontal-only containment check (100% of both the image and burst inside the container's own box, for edge AND middle columns, before landing on this) - screenshots at/tmp/hover-edge-first-after-fix.png,/tmp/hover-edge-last-after-fix.png,/tmp/hover-middle-after-fix.pngin the session.Added a regression test (
QuestionFeedResponsive.spec.ts) hovering the leftmost candidate and asserting both the image and burst are horizontally contained within the scroll container (vertical clipping at the top/bottom of this box is its intended scroll behaviour, so the check is horizontal-only).Screenshots (described - not committed to the repo; all captured this session)
/tmp/final-l1-1400x900.png- Level 1 (confirm_suggestion), 1400x900, all old text gone, flattened blue field, suggested-match card + Yes/Not Sure buttons visible above the fold beside the reference card./tmp/final-l2-1400x900.png- Level 2 (identify_printing), 1400x900, candidate grid + "None of these" visible, further buttons scroll within the box (expected/accepted for L2)./tmp/final-l2-1400-after-scroll.png- 1400x900, after a realpage.mouse.wheel()scroll over the hero card - pixel-identical to the rest-state screenshot (card, title, footer all unchanged), confirming the pinning fix at the width where it matters most (>= md)./tmp/final-l2-390-rest.png//tmp/final-l2-390-after-scroll.png- mobile 390px, rest and after a real wheel-scroll: sticky condensed card bar, candidates, answer buttons, and the new stats line all visible without scrolling; scrolling doesn't change anything since it all already fits./tmp/hover-edge-first-after-fix.png,/tmp/hover-edge-last-after-fix.png,/tmp/hover-middle-after-fix.png- hover-clip fix, edge vs. middle candidate comparison.Checklist
pre-commitand installed the hooks withpre-commit installbefore creating any commits.QuestionFeed.spec.ts,QuestionFeedResponsive.spec.ts,WhatsThatWordsAnimation.spec.ts,WhatsThatPWA.spec.ts) - 34/34 pass.DisplayPage.spec.ts/DisplayFinishFooter.spec.ts/SavedDecks.spec.tsas a sitewide sanity check on theLayout.tsx/ContentContainerchange - 40/41 pass; the 1 failure (the floating sheet-position pill updates live while scrolling at phone width (D17)) reproduces identically on stockorigin/masterwith my changes stashed out, confirmed pre-existing and unrelated.npx jest- 504/504 pass.npx tsc --noEmit- clean.npx prettier@2.7.1 --checkon every touched file - clean.npx eslinton every touched file - 0 errors (6 pre-existing warnings, unrelated).page.mouse.wheel()scroll (not justel.scrollTop) to confirm the pinning fix and the hover-clip fix, per the task's own instruction.docs/features/printing-tags.md's quiz-reveal hero section updated in place (palette, pinning, hover-clip, stats-line sub-bullets).docs/troubleshooting.md's existing NavbarHeight entry updated in place with the partial-fix note and remaining scope.