fix(ui): size drill screens to the scroll container, not viewport vh (#18) - #26
Merged
Merged
Conversation
…18) After the inner-scroll shell (#6/#15), .app-main is the scroll container and .screen { flex: 1 0 auto } already fills it — so the viewport-relative vh floors on the four drill screens over-measured (viewport > .app-main = viewport − header − nav − safe-area), forcing the screen taller than its container and pushing the bottom-pinned answer buttons below the fold. Replace the vh floors with minHeight="100%" (the SolveScreen prop), matching the pattern LevelsScreen already ships: - QuickDrill 70vh; GuidedSolve / Speedrun / DailyChallenge 62vh → 100% Adds a QuickDrill render guard asserting the SolveScreen root carries minHeight 100% (no vh), so the regression can't silently return. Gates: typecheck, lint, 291 tests, build — all green. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…ry caller (#18) After #18, all six SolveScreen call-sites passed minHeight="100%" — the prop was a constant everywhere, so the vh-vs-container fix belonged in the shared component, not repeated per screen. Default `minHeight = '100%'` (optional, still overridable) and remove the literal from all four drill screens plus the two LevelsScreen runners. A future consumer can no longer forget the prop and re-introduce the #18 regression. Replace the per-screen QuickDrill.test.tsx (which asserted a value the screen merely forwarded, and carried unused IndexedDB-reset boilerplate) with SolveScreen.test.tsx, guarding the default and the override path at the source for all consumers. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This branch was successfully deployed
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
1 participant
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.
Closes #18. Roadmap item 2/7.
After the inner-scroll shell (#6/#15),
.app-mainis the scroll container and.screen { flex: 1 0 auto }already grows to fill it. The four drill screens still pinned a viewportvhfloor, which over-measures (viewport >.app-main= viewport − header − nav − safe-area) and forces the screen taller than its container — pushing the bottom-pinned answer buttons below the fold on a short viewport.Change
Swap the
vhfloors forminHeight="100%"(theSolveScreenprop), exactly asLevelsScreenalready ships:70vh100%62vh100%62vh100%62vh100%base.css's100dvhon.app-shellis the intentional shell height — left untouched.Test
New
QuickDrill.test.tsxrenders the screen and asserts theSolveScreenroot carriesminHeight: 100%(novh), guarding against re-introduction.Severity / verification
Low severity (the PWA pins portrait, where
.app-mainis always taller than 62–70vh; only reachable in a short desktop/in-browser window). typecheck ✓ · lint--max-warnings 0✓ · 291 tests ✓ · build ✓.🤖 Generated with Claude Code