Repository navigation
test(learn): real zero-scroll sidebar for the mid-size course in lesson-shell - #60
Merged
Merged
Conversation
…assert it as a precondition At 1280x800 the sidebar is about 44px plus 63px per lesson in a 752px box, so 13 lessons scrolled about 108px inside the box. Ten lessons fit the box yet reach below the window, so only the window can reveal the last lesson. The test now asserts maxScrollTop is 0 and that the box reaches below the window before it checks the reveal.
Contributor
Author
|
Coordinator review: approved. This is a test-only change. 10 lessons give about 44 + 630 = 674px, which fits the 752px box (maxScrollTop 0). At scrollY=0 the bottom is at about 847px, below the 800px window, so this exercises the zero-scroll clamp the round-4 NIT asked for. Landing on green. |
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.
Why
A review NIT on #53 after merge: the 13-lesson case in
lesson-shell.spec.tswas meant to cover a sidebar whose box cannot scroll, where only the window can reveal the last lesson. At 1280x800 the sidebar is about 44px plus 63px per lesson in a 752px box, so 13 lessons gave about 108px of scroll inside the box. The case passed, but it tested something other than what its name said.What users will see
Nothing. Test-only change.
What changed
apps/web/e2e/lesson-shell.spec.ts: the mid-size course has 10 lessons, which fit the box with no scroll of their own while still reaching below the window at the top of the page.maxScrollTop === 0(scrollHeight - clientHeightof[data-outline-scroll]) and that the box reaches below the window.Validation
pnpm check: passed at 6e0fccb (1325 unit tests, build compiled).supabase test db: not applicable, no migration or policy change.e2e/lesson-shell.spec.ts25 passed. The production-build run was not done for this test-only change; CI covers it.outline-sheet.tsxfrom cf3483b (before the round-3 fix) on "ring above the window bottom", after its new set-up assertions pass. It passes on main.Review receipt
Merge danger
Two-way door. Blast radius: one e2e test.
Generated by Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.