diff --git a/frontend/js/feed.js b/frontend/js/feed.js index eae199a..03e5937 100644 --- a/frontend/js/feed.js +++ b/frontend/js/feed.js @@ -44,10 +44,21 @@ App.feed = App.feed || {}; // scroll events with positions that no longer map to the active slide; // onResize sets this for the brief realign window so those events don't // flip the active video -- rotating the device must never change which - // slide is playing. Normal swipes (no resize in flight) are unaffected. + // slide is playing. + // + // What it must not ignore is the reader. A re-snap has nothing behind it; + // a swipe has a finger or a wheel. From the scroll event alone the two are + // identical, so the input is recorded and a scroll that follows one closes + // the window instead of being reverted into it. Without that, a swipe on a + // phone was routinely thrown away: the chrome sliding out of the way *is* + // the scroll, so the window was open at exactly the moment it did most + // harm -- the reel moved under the finger, showing the next video coming + // in, and was then pulled back to where it started. let suppressScroll = false; let resizeSettleRaf = null; let resizeSettleTimer = null; + const INPUT_GRACE_MS = 700; + let lastInputAt = 0; // A rotation is not one event. The viewport, the browser's own chrome and // the scroll-snap container settle over many frames and often several // `resize` events, so the realign window is held open until the slide @@ -1319,8 +1330,14 @@ App.feed = App.feed || {}; // in the event, is what keeps that position from being painted -- // waiting for the next frame means showing it first. if (suppressScroll) { - realignToActive(); - return; + // Unless it is the reader scrolling. A swipe closes the window + // instead of being reverted into it -- and closes it without a + // realign, or the close would undo the swipe that caused it. + if (Date.now() - lastInputAt >= INPUT_GRACE_MS) { + realignToActive(); + return; + } + closeSettle(false); } if (scrollRaf) return; scrollRaf = requestAnimationFrame(() => { @@ -1358,6 +1375,12 @@ App.feed = App.feed || {}; const isTurn = function(before, now) { if (Date.now() - turnSignalAt < TURN_SIGNAL_MS) return true; if (!before) return false; + // Swapping the long side moves both of them. Reading the flip on its + // own calls a few pixels of height a rotation whenever the viewport is + // near enough to square for them to cross it -- and then pins the + // slides and turns snapping off for a second over the chrome sliding + // away, which is the one thing this is here to tell a turn apart from. + if (before.w === now.w) return false; return (before.w > before.h) !== (now.w > now.h); }; @@ -1420,8 +1443,11 @@ App.feed = App.feed || {}; } }; - // Ends the realign window: one last realign, then real swipes count again. - const finishSettle = function() { + // Ends the realign window, and `realign` says whether to re-anchor on the + // way out. A window that closes on its own does -- holding the reader's + // video in place is what it was opened for. One cut short by a swipe must + // not, or closing it would undo the very swipe that closed it. + const closeSettle = function(realign) { if (resizeSettleRaf) { cancelAnimationFrame(resizeSettleRaf); resizeSettleRaf = null; @@ -1431,7 +1457,7 @@ App.feed = App.feed || {}; resizeSettleTimer = null; } releaseTurn(); - realignToActive(); + if (realign) realignToActive(); suppressScroll = false; lastShape = viewportShape(); const after = settleDone; @@ -1439,6 +1465,11 @@ App.feed = App.feed || {}; if (after) after(); }; + // Ends the realign window: one last realign, then real swipes count again. + const finishSettle = function() { + closeSettle(true); + }; + // Watches the viewport settle, frame by frame, holding the active video in // place the whole way. const settleStep = function() { @@ -1940,6 +1971,19 @@ App.feed = App.feed || {}; if (!scrollBound) { scroller.addEventListener('scroll', onScroll, { passive: true }); + // How the realign window tells a swipe from the container + // re-anchoring itself (see suppressScroll): a scroll with one of + // these behind it is the reader's, and outranks the window. + // Only input that means to scroll counts -- a finger that has + // moved, not one that has merely landed, since a tap must not + // sign for the re-anchor that follows it. + const noteInput = () => { lastInputAt = Date.now(); }; + ['wheel', 'touchmove'].forEach((type) => { + scroller.addEventListener(type, noteInput, { passive: true }); + }); + // Arrow keys and Page Up/Down scroll the container too, and the + // keystroke lands on whatever has focus rather than the scroller. + window.addEventListener('keydown', noteInput, { passive: true }); window.addEventListener('resize', onResize); // `orientationchange` fires before the new metrics are readable and // the visual viewport moves on its own as the browser's chrome diff --git a/tests/smoke_feed_swipe.py b/tests/smoke_feed_swipe.py new file mode 100644 index 0000000..2722e70 --- /dev/null +++ b/tests/smoke_feed_swipe.py @@ -0,0 +1,216 @@ +#!/usr/bin/env python3 +"""A swipe counts even when the viewport just moved. + +Run against a locally running backend: + + backend/main.py & + .venv/bin/python tests/smoke_feed_swipe.py + +Both the status and the listing are fixtures, so the reel is long enough to +swipe through and the test never waits on a real source. + +A viewport change opens a realign window in which the feed holds the reader's +video in place, because the scroll-snap container re-anchors on positions +measured against the old height and must not be allowed to flip the video (see +smoke_rotate). The window used to revert *every* scroll it saw, and on a phone +that is the worst possible moment to do it: the chrome sliding out of the way +is caused by the swipe, so the window was open exactly when the reader was +swiping. The reel moved under the finger -- far enough to show the next video +coming in at the bottom -- and was then pulled back to where it started. + +So the two halves have to be told apart, and the test asks for both: a scroll +with real input behind it is the reader's and is kept, a bare re-anchor is not +and is still undone. +""" +import json +import os +import re +import sys +from playwright.sync_api import sync_playwright + +CLIP = os.path.join(os.path.dirname(__file__), "fixtures", "clip.mp4") + +BASE = "http://127.0.0.1:5000/" +SERVER = "https://fixture.test" +CHANNEL = "clips" +PORTRAIT = {"width": 430, "height": 930} +# Chrome sliding away: height only, so this is not a turn -- it is the resize +# that used to eat the swipe that caused it. +SHORTER = {"width": 430, "height": 860} +# Near enough to square that a little height crosses it. Reading the `w > h` +# flip on its own called that a rotation, pinned the slides and turned +# snapping off for about a second. +SQUARE = {"width": 800, "height": 810} +SQUARE_SHORT = {"width": 800, "height": 790} + +STATUS = { + "online": True, + "channels": [{ + "id": CHANNEL, + "name": "Fixture Clips", + "description": "A channel that only exists for this test.", + "options": [], + }], +} + +SEED = """([server, channel]) => { + localStorage.setItem('config', JSON.stringify({ servers: [{ [server]: {} }] })); + localStorage.setItem('preferences', JSON.stringify({ [server]: { channelId: channel } })); + localStorage.removeItem('session'); +}""" + +WHERE = """() => { + const scroller = document.getElementById('feed-scroll'); + const root = document.getElementById('feed-view'); + const active = document.querySelector('.feed-slide.is-active'); + const h = scroller.clientHeight; + return { + index: App.state.feedActiveIndex, + videoId: App.state.feedActiveVideoId, + scrollTop: Math.round(scroller.scrollTop), + expected: App.state.feedActiveIndex * h, + height: h, + activeTop: active ? Math.round(active.getBoundingClientRect().top) : None_, + turning: root.classList.contains('is-turning'), + snap: getComputedStyle(scroller).scrollSnapType, + offHeight: Array.from(document.querySelectorAll('.feed-slide')) + .map((s) => s.getBoundingClientRect().height) + .filter((sh) => Math.abs(sh - h) > 1), + }; +}""".replace("None_", "null") + +GO = """(step) => { + const scroller = document.getElementById('feed-scroll'); + scroller.scrollTop = step * scroller.clientHeight; + return App.state.feedActiveIndex; +}""" + +# The container's own re-anchor: a scroll position and nothing behind it. +RESNAP = """(top) => { + document.getElementById('feed-scroll').scrollTop = top; +}""" + + +class Checks: + def __init__(self): + self.failed = 0 + + def ok(self, label, condition, detail=""): + mark = "PASS" if condition else "FAIL" + if not condition: + self.failed += 1 + print(f" [{mark}] {label}" + (f" -- {detail}" if detail and not condition else "")) + + +def items(): + return [{"id": f"vid-{i}", "title": f"clip {i}", + "url": f"https://fixture.test/v{i}.mp4", "thumb": "", "channel": CHANNEL, + "duration": 30, "isLive": False, "tags": []} for i in range(36)] + + +def swipe_to(page, step): + for n in range(1, step + 1): + page.evaluate(GO, n) + page.wait_for_timeout(250) + page.wait_for_timeout(400) + + +def main(): + c = Checks() + + with sync_playwright() as p: + browser = p.chromium.launch(args=[ + "--no-sandbox", "--disable-dev-shm-usage", "--disable-gpu", + "--autoplay-policy=no-user-gesture-required", + "--renderer-process-limit=1", "--js-flags=--max-old-space-size=512", + ]) + page = browser.new_page(viewport=PORTRAIT) + crashes = [] + page.on("pageerror", lambda e: crashes.append(str(e))) + + page.route("**/api/status", lambda route: route.fulfill( + status=200, content_type="application/json", body=json.dumps(STATUS))) + page.route("**/api/videos", lambda route: route.fulfill( + status=200, content_type="application/json", + body=json.dumps({"items": items(), "pageInfo": {"hasNextPage": False}}))) + page.route(re.compile(r"\.mp4"), lambda route: route.fulfill( + status=200, content_type="video/mp4", body=open(CLIP, "rb").read())) + + page.goto(BASE, wait_until="domcontentloaded") + page.evaluate(SEED, [SERVER, CHANNEL]) + page.goto(BASE, wait_until="load") + page.wait_for_selector(".video-card", timeout=30000) + page.wait_for_timeout(500) + page.evaluate("() => App.feed.open()") + page.wait_for_selector(".feed-slide", timeout=20000) + page.wait_for_timeout(800) + + print("\nseven videos into the reel") + swipe_to(page, 7) + start = page.evaluate(WHERE) + c.ok("that is where the reel is", start["index"] == 7, str(start)) + + print("\na swipe while the chrome is still sliding away") + page.mouse.move(215, 500) + page.set_viewport_size(SHORTER) + page.wait_for_timeout(50) # well inside the realign window + page.mouse.wheel(0, SHORTER["height"]) + page.wait_for_timeout(2200) + swiped = page.evaluate(WHERE) + c.ok("the swipe moved the reel on", + swiped["index"] == start["index"] + 1, + f"step {start['index']} -> {swiped['index']}") + c.ok("onto the next video", + swiped["videoId"] != start["videoId"], + f"{start['videoId']} -> {swiped['videoId']}") + c.ok("and it is sitting on that step, not between two", + abs(swiped["scrollTop"] - swiped["expected"]) <= 1, str(swiped)) + c.ok("so no second video shows at the bottom of the screen", + swiped["activeTop"] == 0, str(swiped["activeTop"])) + + print("\na bare re-anchor in the same window is still undone") + page.wait_for_timeout(900) # let the input grace lapse + held = page.evaluate(WHERE) + page.set_viewport_size(PORTRAIT) + page.wait_for_timeout(50) + # A position measured against the height from before the resize, which + # is what the container itself hands back mid-rotation. + page.evaluate(RESNAP, held["index"] * held["height"]) + page.wait_for_timeout(1800) + resnapped = page.evaluate(WHERE) + c.ok("the same video is still playing", + resnapped["videoId"] == held["videoId"], + f"{held['videoId']} -> {resnapped['videoId']}") + c.ok("on the same step", + resnapped["index"] == held["index"], + f"{held['index']} -> {resnapped['index']}") + c.ok("re-anchored onto its own snap point", + abs(resnapped["scrollTop"] - resnapped["expected"]) <= 1, str(resnapped)) + + print("\na little height on a near-square viewport is not a rotation") + page.set_viewport_size(SQUARE) + page.wait_for_timeout(1800) + page.set_viewport_size(SQUARE_SHORT) + page.wait_for_timeout(120) # mid-window, when the pin would be on + pinned = page.evaluate(WHERE) + c.ok("the slides are not pinned for it", not pinned["turning"], str(pinned["turning"])) + c.ok("so snapping stays on", pinned["snap"] != "none", str(pinned["snap"])) + page.wait_for_timeout(1800) + settled = page.evaluate(WHERE) + c.ok("every slide is exactly a screenful", + settled["offHeight"] == [], str(settled["offHeight"])) + 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']}") + + c.ok("nothing threw along the way", not crashes, str(crashes[:2])) + browser.close() + + print(f"\n{'FAILED' if c.failed else 'OK'}: {c.failed} check(s) failed") + return 1 if c.failed else 0 + + +if __name__ == "__main__": + sys.exit(main())