[pawank925]

Summary

Nothing in the author carousels reads or writes scrollLeft any more, so nothing assumes what it means in RTL. #98 named two possible remedies — feature-detect the convention, or compare positions with getBoundingClientRect() — and this uses both, because each covers a part the other can't.

State (hasPrevious / hasNext) comes from measured geometry: the track's rect against the outer edges of its children gives the content hanging out past each edge, and the writing direction says which of those two is "ahead". scrollLeft is not read at all, so the negative-range, reversed-positive and legacy-positive readings all produce the same state.

Movement has to be expressed as a scroll offset, since scrollIntoView scrolls every scrollable ancestor — that is what pulled the whole page down to the carousel when an arrow was pressed, and it is why the movement can't be delegated to the layout engine. So the sign is measured instead of assumed: before the first arrow press on a carousel, the composable writes the delta it intends to apply, reads back how far the engine actually went, measures which way the cards moved, and restores the position. That yields one bit — "does a positive delta advance the content or retreat it" — which is cached per carousel.

The probe is a whole card, not a pixel, and that detail is measured rather than guessed: both carousels set scroll-snap-type: x mandatory, and Chrome refuses a scroll position that is off a snap point — a ±1px write reads back unchanged, so a pixel-sized probe detects nothing on exactly the components that need it (accepted: 0 at every position). A card-width write lands on a snap point and is accepted (accepted: 200, reduced: 200, restored: 0).

If both directions are refused — a track with nothing to scroll, or an engine that accepts neither — the composable falls back to the spec convention rather than doing nothing. That fallback is not the primary path and is tested as a distinct case.

The "scroll to current author" watch was the third place with a baked-in convention: scrollTo({ left: -offsetFromStart }) with an RTL branch reconstructing the offset from scrollWidth - (offsetLeft + clientWidth). It now says what it means — align this card to the inline start — through alignCell, which measures the card's distance from the start edge and applies it with the resolved sign. The mobile/desktop choice of which card to align is unchanged, and it stays an instant jump rather than a smooth scroll, as before.

#162 merged more convention assumptions into the same component while this PR was open — the progressive-loading thresholds (Math.abs(track.scrollLeft) against scrollWidth - clientWidth, in checkAndLoadMore and both arrow handlers) and the compensating write in loadPrevious (track.scrollLeft = isRtl ? prevScrollLeft - diff : prevScrollLeft + diff, where diff is the scrollWidth growth from prepending a batch). Rebasing meant reconciling with them rather than reverting them, so they are converted too, and that is what makes this the whole fix rather than half of it:

• checkAndLoadMore and the two arrow handlers ask distanceToEnd() / distanceToStart() — the same overflow measurements the arrows' enabled state already uses — instead of translating scrollLeft into "distance from the start" by hand.
• loadPrevious no longer infers how much content appeared below the reader. It remembers the distance of the card at the inline start, prepends the batch, and re-aligns that same card to the distance it had, through alignCell(anchor, anchorDistance). That reaches the same goal as the scrollWidth arithmetic — the reader keeps looking at the same card — but it is measured on the card the reader is actually on, so it needs no convention to pick its sign and no assumption that the only thing that changed was a uniform batch prepended at the start.

CollectionCarousel.vue:42-43 still translates Math.abs(scrollLeft) by hand for its mobile load-more. It is outside the two files #98 names and outside the carousel this PR is about, so I left it alone rather than widen the diff — say the word and it can use useScrollTrack's distanceToEnd() the same way, either here or in a follow-up.

Tests

• The spec models a real scrollable track instead of mocking scrollWidth / clientWidth / scrollLeft: stubScrollEngine installs a scrollLeft accessor plus a scrollBy mock over one internal number — how far the content has advanced from the logical start — maps that to a scrollLeft reading under the convention under test, clamps writes, and re-stubs the card rects from the new position so measurements and writes stay consistent. One option makes the engine refuse any position that is not a card boundary, reproducing what mandatory scroll-snap does in Chrome; the RTL cases run with it on.
• Arrow cases assert the observable outcome — which card ends up at the start — rather than the arguments passed to scrollBy, under each of the three conventions: shiftNext advances exactly one card and stops at the last one, shiftPrevious returns, and the RTL state sequence matches LTR's.
• One case pins the measured deltas (RTL: -100 under negative-range and reversed, +100 under legacy) so the resolution can't silently collapse back onto the writing direction, and one asserts the probe leaves the position exactly where it found it.
• alignCell is covered per convention, at the inline start and at an arbitrary distance from it (what loadPrevious needs), plus the already-aligned case that must not touch the track. distanceOf / distanceToStart / distanceToEnd are covered with a lying scrollLeft present — the reads are geometry-only, so a track that reports 0 in both directions still reports the right distances.
• The carousel's own spec (which UI: implement progressive loading with infinite scroll in carousels #162 added, and which stubbed scrollWidth / scrollLeft) now stubs card geometry and a scroll engine, so the windowing tests measure what they claim: scrolling near the inline end loads a later batch and near the inline start loads an earlier one, each asserted by which author ends up where rather than by "the cell count went up". The RTL case runs the same two triggers against a mirrored layout. A separate case pins loadPrevious's anchor: after the batch is prepended, the card the reader was looking at is at the same distance from the inline start as before, and the scroll position moved by exactly the width of the batch.
• Full UI suite locally: 42 files, 422 tests passed; prettier --check, eslint src/ and vue-tsc --build --force clean.
• Verified in a real engine, not only jsdom: a standalone page mirroring the carousel CSS (flex track, overflow-x: auto, overflow-anchor: none, scroll-snap-type: x mandatory, scroll-snap-align: start, 50%-wide cards) inside a document tall enough to scroll, driven through headless Chrome 156, starting at scrollTop 700. The arrow walk moved scrollLeft 200 → 400 → 600 → 800 → 804 (the last one clamps at the far edge, 4px past 800 because of the track's border) with state P,N P,N P,N P,N P,-, then back to 600 and 400; RTL mirrored it exactly at -200 → … → -804 → -600 → -400 with the same state sequence. On top of that, the windowing path was driven the same way, identically in both directions: the initial scroll-to-author put the current card 402px from the inline start (2 cards, the extra 2px being snap rounding); reaching the inline start and prepending a batch kept author 39 at the start with its distance unchanged (2px before, 2px after) while the position advanced by the 3000px the new cards occupy (scrollLeft ±3000, distanceToStart 2998 after snap rounding), state P,N; one arrow step then moved it to author 40 at 3198px; and appending a batch at the far end moved nothing at all. The document's scrollTop delta was 0 at every step of both runs, in both directions, and each probe recorded restored equal to the position it started from.
• Limits, stated plainly: only the negative-range convention exists in the browsers I can drive here, so the reversed and legacy paths rest on the fake engine, and the fallback is exercised only in jsdom. The papers UI itself was not run against a loaded ZIM (no scraped data in this checkout), so the Chrome result is the mechanism, not the app.

fixes #98

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.83%. Comparing base (244bfcd) to head (d2a5764).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #163   +/-   ##
=======================================
  Coverage   77.83%   77.83%           
=======================================
  Files          68       68           
  Lines        4502     4502           
  Branches      725      725           
=======================================
  Hits         3504     3504           
  Misses        793      793           
  Partials      205      205           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[benoit74]

Using scrollIntoView is problematic because it causes the page to focus on that element instead of staying where it was. On author details page, it causes the page to scroll down to show the authors caroussel everytime we change author, even if this is done with arrows on the top of the page.

[pawank925]

Confirmed, and thanks for catching it — scrollIntoView scrolls every scrollable ancestor, so an arrow press on the author page pulled the whole page down to the carousel. Reverted and force-pushed (9dc78f5 → 6526687).

What changed in the rework:

• The arrows go back to scrollBy on the track only, exactly as on main.
• The "scroll to current author" watch in AuthorDetailCarousel is back to main untouched as well.
• Kept: the geometry-based hasPrevious / hasNext. That is the part Carousel RTL scrollLeft assumes the modern negative-range convention #98 is actually about — reading scrollLeft to decide which arrow to disable is what inverted the arrows in RTL, because the value's meaning differs per engine — and it is fixed without any scrolling side-effect, since it only measures card positions.
• Kept: the cellSelector fix (.carousel-cell--others never matched, so the arrows were using the clientWidth / 5 guess rather than a real card width).

To stop this from creeping back, the composable's tests now assert scrollIntoView is never called.

Re-checked in headless Chrome with the same page that mirrors the carousel CSS: LTR scrollLeft 0 → 200 → 400 → 600 → 800 (one card per click) with state (-,N) (P,N) (P,N) (P,N) (P,-), and RTL mirrored 0 → -200 → -400 → -600 → -800 with the same sequence. Page scroll position stayed fixed throughout; only the track moved. Locally: 397 UI tests pass, prettier/eslint/vue-tsc clean.

One thing I want to be explicit about rather than bury: the sign assumption is still there in scrollBy/scrollTo (isRtl ? -1 : 1, unchanged from main), so the arrows' movement and the initial scroll-to-author still presume the negative-range convention. Removing it needs either scrollIntoView — which is the behaviour you flagged — or a convention probe, which is ambiguous at the logical start where 0 is a valid reading under two of the three conventions. Happy to leave that as a separate issue if you agree it is out of scope here; #98 is about the arrow state, not the movement.

[benoit74]

I'm a bit lost. What does with state (-,N) (P,N) (P,N) (P,N) (P,-) means?

the arrows' movement and the initial scroll-to-author still presume the negative-range convention

Either we solve the whole "negative-range convention" or we just don't care. I'm in favor of not caring because all browsers have converged, but I'm open to merge a PR which solves the issue. But not something "in-the-middle" since it brings too little added value for the added complexity.

[pawank925]

@benoit74 First, the notation you asked about: those pairs are the arrow state the harness prints after each step, P meaning "a card is still hidden before the inline start, so the previous arrow is enabled", N meaning "a card is hidden past the inline end, so the next arrow is enabled", and - meaning nothing is hidden on that side. So (-,N) (P,N) (P,N) (P,N) (P,-) reads: at the far start you can only go forward, the three middle positions have both arrows, and at the far end the next arrow switches off.

On "the whole thing or nothing" — agreed, and the rebase onto #162 forced exactly that question, because that PR merged three more hand-translations of scrollLeft into the same component. They are converted too, so there is no scrollLeft arithmetic left in the author carousels:

• arrow state: card geometry;
• arrow steps and the initial scroll-to-author: card geometry plus one throwaway probe that learns the engine's sign and caches it per carousel;
• checkAndLoadMore and both arrow handlers: distanceToEnd() / distanceToStart() instead of Math.abs(scrollLeft) measured against scrollWidth - clientWidth;
• loadPrevious: remembers the distance of the card the reader is looking at, prepends the batch, and re-aligns that same card to the distance it had — instead of inferring the shift from the scrollWidth delta, which needs the convention to pick its sign.

To be honest about what that buys a reader today: nothing visible. Every evergreen engine implements the negative-range convention, so Math.abs was already right and the old loadPrevious arithmetic was already right. What changes is that the component stops encoding a guess about the engine, and does it through one mechanism instead of four separate translations — which is also why the middle ground you were objecting to is gone: the PR no longer fixes the state while leaving the movement to assume.

Driven through headless Chrome 156 against the carousel CSS (mandatory snap, overflow-anchor: none), both writing directions behave identically: the initial scroll-to-author puts the current card 402px from the inline start (two cards, the extra 2px being snap rounding); reaching the inline start and prepending a batch keeps author 39 at the start with its distance unchanged while the position advances by the 3000px the new cards occupy (scrollLeft ±3000, distanceToStart 2998); one arrow step then moves to author 40; appending a batch at the far end moves nothing. The document's scrollTop delta was 0 at every step, which is the regression from the previous round. 422 UI tests pass, prettier/eslint/vue-tsc clean; #162's own carousel spec now asserts which author ends up where rather than "the cell count grew", plus an RTL trigger case and an anchor-preservation case.

One site still uses the old pattern: CollectionCarousel.vue:42-43 translates Math.abs(scrollLeft) by hand for its mobile load-more. #98 names AuthorDetailCarousel and SelectedAuthorsCarousel only, so I left it rather than widen the diff — a few lines with useScrollTrack's distanceToEnd(), so tell me whether @benoit want it here or in a follow-up.

[benoit74]

I'm sorry but this is just way above my concentration bandwidth ATM ; since this is only about chasing a potential bug we've never seen in the wild, I'll park this PR for the time being until I have a better bandwidth or more capabilities.

Sorry to waste your time by not merging it, but here I can't accept to merge something I do not really understand and is maybe not driving value by solving a bug we've really encountered in the wild.

[pawank925]

Completely understood, and no apology needed — you're right on the cost/benefit. Every engine we can actually test implements the negative-range convention, so this defends against something nobody has hit; and a change you can't follow isn't a change you can maintain, whatever its tests say. I've converted this to draft so it's out of your review queue.

If a real engine ever regresses here, everything needed to finish it — the two candidate remedies, the measured-convention approach, and the harness results in both writing directions — is written down in this PR and on this branch, so this isn't lost work. Thanks for the patient review across the rounds.

[pawank925]

@benoit74 I've brought this back out of draft — the review request from the earlier round is still standing, so it's in your queue again whenever you have bandwidth, with no urgency at all. If the answer when you get there is still "not worth owning", closing #98 as won't-fix is a fine outcome too; the analysis stays here either way.

Two things changed since your last look, both mechanical, and neither adds anything new to understand: it's rebased onto current main (the only conflict was one CHANGELOG.md line), and that head is green — 42 UI test files / 422 tests pass locally, prettier/eslint/vue-tsc clean, and every CI check passes on it.

If it helps to know where to start: ui/src/composables/useScrollTrack.ts is 133 lines, and the only part that needs a model in your head is resolveScrollSign (lines 46-62, 17 lines) — it writes the delta it intends to apply, reads back where the engine actually went, and caches one bit per carousel. Everything else in that file is plain measured geometry (edges, distanceToStart / distanceToEnd, alignCell), and the diff at the call sites is those measurements replacing four hand-translations of scrollLeft. The tests assert which author ends up where rather than which numbers were passed around, so reading the six component-spec cases first may be the cheaper way in.