-
Notifications
You must be signed in to change notification settings - Fork 1
Read the note-set axis against the page's box, not the window #36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
roomnow sits besidepageBoxanswering the same question two different ways.span()takes its right edge asgetBoundingClientRect().right - paddingRight, and that rect is body's border box, which contains the reserved gutter — so wherever a scrollbar takes room,room.rightis 15px past where content can actually go. That is the reading this PR is removing from the note test, still in place one line above it.Three assertions use
room.rightas the page's right edge:"past the page, right"intest_a_widget_that_declares_width_takes_the_room_and_the_column_stays_putand intest_a_copy_reads_the_room_from_its_own_window, and"the exhibit hangs over the panel that displaced it"intest_a_wide_widget_gives_the_panel_its_strip. The copy one is honest —:where(html:not(.lf-copy))withholds the scroll arrangement, so body isn't the scroll container there and its border box already excludes the viewport's scrollbar. The other two are 15px looser on the runner than they read.Nothing is red today:
sideways == 0stands beside both of them and catches overflow that reachesscrollWidth. But the bound still states something weaker than its message claims, and after this diff the next test wanting "the page's right edge" gets the window's fromroomand the page's frompageBox, with only the comment onpage()to say which is which. Takingspan's edges offclientWidth— the padding box less any gutter — rather than off the rect would let one answer serve both, andpage()would then be the same reading without the paddings subtracted.Not for this PR: it tightens assertions in two other tests and would want its own bug-back to show the direction of the failure moved as intended (
tests/CLAUDE.md, "Bounds and geometry tests are especially prone to staying green after the fault moves to another edge"). Worth not leaving implicit, though, right beside a comment that spends six lines explaining the gutter.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Measured it on this runner rather than leaving it as arithmetic. On
WIDE_AND_NARROW_PAGEat a 1200px window:room.rightis 1200,pageBox.rightis 1185, and body'soffsetWidth - clientWidthis that same 15. So the two readings sitting next to each other differ by exactly the gutter, on every assertion that treatsroom.rightas the page's right edge. The runtime already takes the gutter that way —leaf.js#L4526computes it asoffsetWidth - clientWidth— which is the direction takingspan's edges offclientWidthpoints in, and it would letpage()be the same reading with the paddings left in.Agreed it stays out of this PR. It moves a bound in two tests whose subject is width rather than the axis, so it is revertable independently of this change, and
tests/CLAUDE.md's "bounds and geometry tests are especially prone to staying green after the fault moves to another edge" is exactly the case — it wants its own bug-back showing the direction of the failure moved as intended, which is a different piece of evidence from the two in the description here.Not opening it as a follow-up right now either: eleven bot PRs are open against one maintainer, so this is recorded here rather than added to that queue.