From c60f5e01dc475fce87be5a34aab1259f808f8058 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 27 Oct 2026 22:26:39 +0000 Subject: [PATCH] Re-anchor a moving reel on where it is, not what it was playing A viewport change re-anchors the reel on the video that is playing, resolved by id rather than by a scroll position the change has already invalidated. That is what keeps a rotation on the same video. Mid-swipe it names the wrong one. setActive runs from onScroll's next frame, so a resize landing before that frame still reads the video being swiped *away from* -- and realignToActive writes its position straight to scrollTop, hauling the reel back to it. A phone slides its chrome on every scroll, so the resize lands mid-swipe on nearly every swipe: the reel went where the thumb put it, far enough to show the top of the next video at the bottom of the screen, and was then written back a beat later. The input guard in onScroll could not catch this. There is no scroll event to guard -- the write happens inside the resize. So while the reader is demonstrably driving, take the reel's own position as the authority and re-anchor on the step it is nearest at the new height. It still lands on a snap point, so nothing is left straddling two screens, but the swipe stands. Only a step's worth of disagreement counts as theirs: a scroller just re-parented into or out of the picture-in-picture window reads 0, and following that would land them on the first video instead of their own. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MPZFnNdHbPGDTqQUNiE4ZN --- frontend/js/feed.js | 27 +++++++++++++++++++++++++-- tests/smoke_feed_swipe.py | 36 ++++++++++++++++++++++++++++++++++-- 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/frontend/js/feed.js b/frontend/js/feed.js index b6d6f85..8d3946e 100644 --- a/frontend/js/feed.js +++ b/frontend/js/feed.js @@ -1413,16 +1413,40 @@ App.feed = App.feed || {}; // video playing/focused rather than snapping to a neighbour. const realignToActive = function() { if (stepCount() === 0) return; + const h = slideHeight(); + const scroller = getScroller(); + // Anchoring by id is what keeps a rotation on the same video, because + // the scroll position it would otherwise read has been invalidated by + // the very change being answered. But mid-swipe that id names the video + // being *left*: setActive runs from onScroll's next frame, so a resize + // landing before it still reads the old one -- and writing its position + // hauls the reel back to it. A phone slides its chrome on every scroll, + // so that happened on nearly every swipe. + // + // While the reader is demonstrably driving, the reel's own position is + // the better authority: re-anchor on the step it is nearest at the new + // height. That still puts it on a snap point -- no slide left straddling + // two screens -- without undoing the swipe. let index = state.feedActiveIndex; if (state.feedActiveVideoId != null) { const found = (state.loadedVideos || []) .findIndex((v) => String(v.id) === String(state.feedActiveVideoId)); if (found >= 0) index = Math.floor(found / paneCount()); } + const driving = Date.now() - lastInputAt < INPUT_GRACE_MS; + if (driving && scroller && h) { + // A swipe moves by a step, so only a step's worth of disagreement + // is the reader's doing. Anything further apart is the position + // being meaningless rather than ahead -- a scroller that has just + // been re-parented into (or out of) the picture-in-picture window + // reads 0, and following that would land the reel on the first + // video instead of theirs. + const atScroll = clampIndex(Math.round(scroller.scrollTop / h)); + if (atScroll >= 0 && Math.abs(atScroll - index) <= 1) index = atScroll; + } index = clampIndex(index); if (index < 0) return; state.feedActiveIndex = index; - const h = slideHeight(); const start = windowBounds(index).start; // Only write what actually differs. This runs on every frame of a // turn, and a spacer height or a scrollTop written back unchanged @@ -1431,7 +1455,6 @@ App.feed = App.feed || {}; const spacer = getTopSpacer(); const spacerHeight = `${start * h}px`; if (spacer && spacer.style.height !== spacerHeight) spacer.style.height = spacerHeight; - const scroller = getScroller(); const top = index * h; if (scroller && Math.abs(scroller.scrollTop - top) > 0.5) scroller.scrollTop = top; const activeSlide = slidesByIndex.get(index); diff --git a/tests/smoke_feed_swipe.py b/tests/smoke_feed_swipe.py index 2722e70..40eefd2 100644 --- a/tests/smoke_feed_swipe.py +++ b/tests/smoke_feed_swipe.py @@ -90,6 +90,22 @@ RESNAP = """(top) => { document.getElementById('feed-scroll').scrollTop = top; }""" +# A swipe, and the chrome reporting itself before the feed has caught up. +# setActive runs from onScroll's next frame, so doing both in one block -- +# no frame boundary in between -- leaves feedActiveVideoId still naming the +# video being left, which is the state a phone resize routinely lands in. +# onResize then re-anchors synchronously, with no scroll event for the +# input guard in onScroll to see. +SWIPE_THEN_RESIZE = """() => { + const s = document.getElementById('feed-scroll'); + const before = App.state.feedActiveIndex; + s.dispatchEvent(new WheelEvent('wheel', { deltaY: 120, bubbles: true })); + s.scrollTop = s.scrollTop + s.clientHeight; + const moved = s.scrollTop; + window.dispatchEvent(new Event('resize')); + return { before: before, moved: moved, after: s.scrollTop, h: s.clientHeight }; +}""" + class Checks: def __init__(self): @@ -187,6 +203,22 @@ def main(): c.ok("re-anchored onto its own snap point", abs(resnapped["scrollTop"] - resnapped["expected"]) <= 1, str(resnapped)) + print("\nthe chrome reporting itself before the feed has caught up") + page.wait_for_timeout(900) + pre = page.evaluate(WHERE) + got = page.evaluate(SWIPE_THEN_RESIZE) + page.wait_for_timeout(2200) + kept = page.evaluate(WHERE) + print(f" (swiped to {got['moved']}px; the resize left it at {got['after']}px)") + c.ok("the resize did not haul the reel back to the video being left", + got["after"] >= got["moved"] - 1, + f"{got['moved']} -> {got['after']}") + c.ok("the swipe still landed on the next step", + kept["index"] == pre["index"] + 1, + f"step {pre['index']} -> {kept['index']}") + c.ok("and on that step's snap point", + abs(kept["scrollTop"] - kept["expected"]) <= 1, str(kept)) + print("\na little height on a near-square viewport is not a rotation") page.set_viewport_size(SQUARE) page.wait_for_timeout(1800) @@ -202,8 +234,8 @@ def main(): c.ok("and the reel rests on its snap point", abs(settled["scrollTop"] - settled["expected"]) <= 1, str(settled)) c.ok("still on the same video", - settled["videoId"] == resnapped["videoId"], - f"{resnapped['videoId']} -> {settled['videoId']}") + settled["videoId"] == kept["videoId"], + f"{kept['videoId']} -> {settled['videoId']}") c.ok("nothing threw along the way", not crashes, str(crashes[:2])) browser.close()