The app is not at fault. Playwright scrolls a target into view before
clicking it, and the test clicked the first card's button after paging the
row to the end. So Playwright scrolled the container back to the start, and the
assertion measured Playwright's own scrolling rather than anything the page did. 2 is the resting position of the scroller — its 2px focus-ring padding is the
first snap point — which is the tell.
How it was diagnosed
Scroll-snap was the obvious suspect and was ruled out first: on a static
reproduction, a button mutating in place inside the scroller moves it not at all
under mandatory, proximity or no snap.
The actual mechanism was then reproduced directly, on a page with no React on
it: scroller at 615, Playwright clicks the off-screen first card's button, scrollLeft becomes 2. Same numbers, same shape, no application code involved.
The fix
Click the last card's button, which is visible at the end of the travel, so
no scroll-into-view is needed. The comparison allows a few pixels for snap and
sub-pixel adjustment while still failing loudly on a reset to the start, which
is the property actually worth pinning.
Worth saying plainly: that property was never under test before this change. It
passes locally in the sense that the suite compiles and registers, but the only
real evidence is the next post-merge staging run.
The other four nearby-schools journeys passed, including all three mobile
reference widths.
Fixes the one failure on the post-merge staging run for #151.
```
Expected: 537
Received: 2
> expect(await scroller.evaluate(node => node.scrollLeft)).toBe(offsetBefore);
```
**The app is not at fault.** Playwright scrolls a target into view before
clicking it, and the test clicked the **first** card's button after paging the
row to the end. So Playwright scrolled the container back to the start, and the
assertion measured Playwright's own scrolling rather than anything the page did.
`2` is the resting position of the scroller — its 2px focus-ring padding is the
first snap point — which is the tell.
## How it was diagnosed
Scroll-snap was the obvious suspect and was ruled out first: on a static
reproduction, a button mutating in place inside the scroller moves it not at all
under `mandatory`, `proximity` or no snap.
The actual mechanism was then reproduced directly, on a page with no React on
it: scroller at 615, Playwright clicks the off-screen first card's button,
`scrollLeft` becomes 2. Same numbers, same shape, no application code involved.
## The fix
Click the **last** card's button, which is visible at the end of the travel, so
no scroll-into-view is needed. The comparison allows a few pixels for snap and
sub-pixel adjustment while still failing loudly on a reset to the start, which
is the property actually worth pinning.
Worth saying plainly: that property was never under test before this change. It
passes locally in the sense that the suite compiles and registers, but the only
real evidence is the next post-merge staging run.
The other four nearby-schools journeys passed, including all three mobile
reference widths.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
The assertion that adding to compare does not reset the carousel failed on
staging: 537 → 2. The app was not at fault. Playwright scrolls a target into
view before clicking, and the test clicked the FIRST card's button after
paging the row to the end — so Playwright scrolled the container back to the
start, and the assertion measured that.
Reproduced on a static page with no React on it: a snap scroller at 615,
Playwright clicks the off-screen first card, scrollLeft becomes 2. Scroll-snap
was ruled out first — mandatory, proximity and no-snap all behave identically
when the button mutates in place.
Now clicks the last card's button, which is visible at the end of the travel,
and allows a few pixels for snap and sub-pixel adjustment while still failing
on a reset to the start. The property was never actually under test before;
it is now.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This is a test-only fix to a flaky e2e assertion in the nearby-schools journey: it now clicks the last visible 'Add to compare' card instead of the first (avoiding Playwright's implicit scroll-into-view resetting the row), and relaxes the scroll-position assertion to an 8px tolerance instead of exact equality. The change is well-reasoned and scoped to test code only, with no production impact.
✅ No issues found.
## 🤖 AI Code Review (Claude Code)
This is a test-only fix to a flaky e2e assertion in the nearby-schools journey: it now clicks the last visible 'Add to compare' card instead of the first (avoiding Playwright's implicit scroll-into-view resetting the row), and relaxes the scroll-position assertion to an 8px tolerance instead of exact equality. The change is well-reasoned and scoped to test code only, with no production impact.
✅ No issues found.
tudor
merged commit 9dba5ff1ff into main2026-09-22 13:50:26 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes the one failure on the post-merge staging run for #151.
The app is not at fault. Playwright scrolls a target into view before
clicking it, and the test clicked the first card's button after paging the
row to the end. So Playwright scrolled the container back to the start, and the
assertion measured Playwright's own scrolling rather than anything the page did.
2is the resting position of the scroller — its 2px focus-ring padding is thefirst snap point — which is the tell.
How it was diagnosed
Scroll-snap was the obvious suspect and was ruled out first: on a static
reproduction, a button mutating in place inside the scroller moves it not at all
under
mandatory,proximityor no snap.The actual mechanism was then reproduced directly, on a page with no React on
it: scroller at 615, Playwright clicks the off-screen first card's button,
scrollLeftbecomes 2. Same numbers, same shape, no application code involved.The fix
Click the last card's button, which is visible at the end of the travel, so
no scroll-into-view is needed. The comparison allows a few pixels for snap and
sub-pixel adjustment while still failing loudly on a reset to the start, which
is the property actually worth pinning.
Worth saying plainly: that property was never under test before this change. It
passes locally in the sense that the suite compiles and registers, but the only
real evidence is the next post-merge staging run.
The other four nearby-schools journeys passed, including all three mobile
reference widths.
🤖 Generated with Claude Code
🤖 AI Code Review (Claude Code)
This is a test-only fix to a flaky e2e assertion in the nearby-schools journey: it now clicks the last visible 'Add to compare' card instead of the first (avoiding Playwright's implicit scroll-into-view resetting the row), and relaxes the scroll-position assertion to an 8px tolerance instead of exact equality. The change is well-reasoned and scoped to test code only, with no production impact.
✅ No issues found.