test: add Playwright coverage for book-level back-to-top-navigation - #14889
Merged
Merged
Conversation
PR #14881 fixed book projects silently dropping website-level tool options (e.g. back-to-top-navigation) set under the book key. The existing smoke-all fixture only checks the control is injected into the HTML, not that it behaves correctly in a browser.
Collaborator
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
The scroll-down step and the following scroll-up step both used page.evaluate(scrollTo) back-to-back with no wait between them. The nav script's show/hide logic only updates its internal scroll-position tracker inside the scroll event handler, so if that handler hadn't run yet before the reversal, the up-scroll direction check compared against a stale position and the button never showed. Poll scrollY to confirm the down-scroll's event has been processed before scrolling back up.
Captures the fix from PR #14889 (book-back-to-top-navigation flaky test) as a reusable pattern: reversed scrollTo calls can race a scroll-direction-tracking event handler, and asserting on the already-hidden UI state doesn't prove the handler ran. Polling scrollY forces the event-loop turn needed.
…ollY
The prior fix polled window.scrollY between the down- and up-scroll, but
scrollTo({behavior: "instant"}) updates scrollY synchronously, so the poll's
first check passed before the queued "scroll" event even reached the page's
own listener. The race it was meant to close was still there: repeated local
runs (--repeat-each=15, 3 browsers) hit it ~9/45 times.
Switching to an explicit one-shot scroll listener, awaited via a Promise
before reversing direction, closes it for real (45/45, then another 20/20
firefox-only run with zero failures; the earlier single firefox failure was
an unrelated browserContext.close teardown error). Listeners for the same
event fire in registration order, so a listener registered here always runs
after the page's already-registered one has updated its tracked state.
Also drops the now-redundant scrollY poll after the click-to-top step, since
no further scroll follows it there, and updates the best-practices doc that
had documented the ineffective poll pattern.
…ting it The pattern section repeated the same await-scroll-event Promise block twice inline. Point at scrollToAndSettle in book-back-to-top.spec.ts instead, kept local there since it's a single-caller helper (see finishing-a-development discussion): no second spec needs the same scroll-direction-race handling yet, so a shared utils.ts helper would be premature.
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
2 participants
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.
Companion to #14881, which fixed book projects silently dropping website-level tool options (e.g.
back-to-top-navigation) set under thebookkey. The existing smoke-all fixture only checks that the control is injected into the HTML, not that it behaves correctly in a browser.Adds a Playwright fixture (
tests/docs/playwright/book/back-to-top/) and spec asserting the#quarto-back-to-topcontrol's scroll show/hide behavior and click-to-top reset in a book project.