Skip to content

Read the margin layout the resize scheduled before measuring the sidebar page #153

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.

Sign up for GitHub

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

Merged
max-sixty merged 1 commit into from
Aug 31, 2026
Merged

Read the margin layout the resize scheduled before measuring the sidebar page #153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions tests/render_harness.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1340,6 +1340,27 @@ def new_page(**kwargs):
return SimpleNamespace(new_page=new_page)


def margins_laid_out(page):
"""Run the margin layout the page has scheduled, so a geometry read follows it.

The margin's own geometry owner answers a width change through a ResizeObserver on
the body and a `requestAnimationFrame`, which is one whole rendering turn later than
the resize event `resized` waits for. A read taken in between is a read of the rows
at the width they have just left: on a page narrowed to exactly what its residents
need, the widest margin row was still at its roomy width and hung 24px past the
window — but only on the runs where the frame had not landed yet, which is why the
same probe condensed on one run and not the next.

The pending frame is not a fact to wait a frame for (`tests/CLAUDE.md`, "a fixed
number of animation frames only guesses"), so the work is run instead of guessed at.
Whether the observer schedules it at all is `test_render_margin.py`'s subject, not
that of a test reading the layout it produces."""
page.evaluate(
"() => import('/runtime/margin-layout.js')"
".then(({layoutMarginRows}) => layoutMarginRows())"
)
Comment on lines +1358 to +1361

@leaf-agent leaf-agent Aug 31, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import is awaited inside page.evaluate, so nothing bounds it: if /runtime/margin-layout.js stalls on the way out of the server, this doesn't fail in thirty seconds naming its test — it holds the worker for the rest of the job step. wait_for_function puts the deadline on the driver side, which is the shape tests/CLAUDE.md prefers ("stating synchronous readiness inside the page and polling it with wait_for_probe, whose driver-side wait carries SERVED_TIMEOUT_MS"), and render_checks_model is already imported here.

Suggested change
page.evaluate(
"() => import('/runtime/margin-layout.js')"
".then(({layoutMarginRows}) => layoutMarginRows())"
)
page.wait_for_function(
"() => import('/runtime/margin-layout.js')"
".then(({layoutMarginRows}) => (layoutMarginRows(), true))",
timeout=render_checks_model.SERVED_TIMEOUT_MS,
)

@leaf-agent leaf-agent Aug 31, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actioned in #154, against main — this merged about three minutes after the review, so the unbounded await is on the default branch rather than on a branch that can still absorb the change.

The suggestion is taken as written: wait_for_function with render_checks.SERVED_TIMEOUT_MS, one round trip, layout still inside a frame. The docstring now carries the reason the bound is driver-side, since the helper is exported and the next caller will not have this thread. #154 is green on the everyday suite and on the sidebar test at --run-nightly -n0.



def panel_settled(page, open=True):
"""Wait for the panel to reach `open` and the page to finish making room for it.

Expand Down
2 changes: 2 additions & 0 deletions tests/render_support.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@
hold_selection,
key_line,
leaf_page,
margins_laid_out,
navigate,
nudge,
open_page,
Expand Down Expand Up @@ -600,6 +601,7 @@
"live_leaf",
"live_url",
"live_watcher",
"margins_laid_out",
"mark_edges",
"mark_point",
"mark_shows_beside_composer",
Expand Down
5 changes: 2 additions & 3 deletions tests/test_render_margin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
compare_with,
leaf_page,
live_url,
margins_laid_out,
open_page,
panel_settled,
resized,
Expand Down Expand Up @@ -1028,9 +1029,7 @@ def offset():
page.evaluate(
"() => document.scrollingElement.scrollBy({top: 320, behavior: 'instant'})"
)
page.evaluate(
"() => import('/runtime/margin-layout.js').then(({layoutMarginRows}) => layoutMarginRows())"
)
margins_laid_out(page)
assert offset() == pytest.approx(before, abs=1)

assert errors == []
Expand Down
3 changes: 3 additions & 0 deletions tests/test_render_pages.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
composer_quote,
leaf_page,
live_url,
margins_laid_out,
nudge,
open_page,
page_registry,
Expand Down Expand Up @@ -2036,6 +2037,7 @@ def test_a_left_sidebar_uses_the_margin_until_the_page_needs_it_back(browser, se
}"""

resized(page, 1400, 900)
margins_laid_out(page)
roomy = page.evaluate(reading)
assert roomy["strip"] == 264
assert roomy["float"] == "left" and roomy["position"] == "sticky"
Expand Down Expand Up @@ -2069,6 +2071,7 @@ def test_a_left_sidebar_uses_the_margin_until_the_page_needs_it_back(browser, se
# root scrollport holds outside the container query's own width.
exact = roomy["strip"] + 720 + roomy["rail"]
resized(page, math.ceil(exact + roomy["viewportWidth"] - roomy["pageWidth"]), 900)
margins_laid_out(page)
tighter = page.evaluate(reading)
assert exact <= tighter["pageWidth"] <= exact + 1, (
f"the narrowed page is not the width the residents and column need: {tighter}"
Expand Down
Loading