Stay on the same video when the phone turns
Rotating mid-reel jumped the reel forward a few videos. Two reasons, both about the same arithmetic: every scroll position in the feed is index * scroller.clientHeight, with a spacer standing in for the slides above. A slide was 100dvh, which is not the scroller's height on a phone whose chrome comes and goes, so that mapping addressed the wrong slide by a growing margin; it is now the scroller's own height, which it cannot disagree with. And the realign window lasted two frames. A rotation is many frames and several resizes, with the container's re-anchor on pre-rotation pixels landing in the middle of them -- so the window now stays open until the slide height has held still for a stretch of frames (with a clock to close it in a tab that isn't drawing), and orientationchange and the visual viewport open it too. Moving the feed into a picture-in-picture window settles the same way. tests/smoke_rotate.py replays a rotation as the browser delivers it, on a fixture reel so nothing waits on a real source; against the old code the reel goes from step 7 to step 12. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MPZFnNdHbPGDTqQUNiE4ZN
This commit is contained in:
@@ -2246,12 +2246,17 @@ body.feed-mode-open .mode-toggle-btn .icon-svg {
|
|||||||
|
|
||||||
/* One screenful. It holds the pane tree rather than a video directly: with
|
/* One screenful. It holds the pane tree rather than a video directly: with
|
||||||
several panes, a step shows that many videos at once and one swipe advances
|
several panes, a step shows that many videos at once and one swipe advances
|
||||||
the whole set. */
|
the whole set.
|
||||||
|
|
||||||
|
The height is the scroller's own, not 100dvh: every scroll position in the
|
||||||
|
feed is computed as `index * scroller.clientHeight` (the top spacer stands
|
||||||
|
in for the slides above), so a slide that is even a few pixels off that
|
||||||
|
height makes the mapping drift by a slide's worth after a handful of them --
|
||||||
|
which is what used to make rotating the phone land on a different video. */
|
||||||
.feed-slide {
|
.feed-slide {
|
||||||
position: relative;
|
position: relative;
|
||||||
width: 100%;
|
width: 100%;
|
||||||
height: 100vh;
|
height: 100%;
|
||||||
height: 100dvh;
|
|
||||||
scroll-snap-align: start;
|
scroll-snap-align: start;
|
||||||
scroll-snap-stop: always;
|
scroll-snap-stop: always;
|
||||||
overflow: hidden;
|
overflow: hidden;
|
||||||
|
|||||||
@@ -47,6 +47,19 @@ App.feed = App.feed || {};
|
|||||||
// slide is playing. Normal swipes (no resize in flight) are unaffected.
|
// slide is playing. Normal swipes (no resize in flight) are unaffected.
|
||||||
let suppressScroll = false;
|
let suppressScroll = false;
|
||||||
let resizeSettleRaf = null;
|
let resizeSettleRaf = null;
|
||||||
|
let resizeSettleTimer = null;
|
||||||
|
// 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
|
||||||
|
// height has held still for a stretch of frames -- not for a fixed two,
|
||||||
|
// which ran out while the viewport was still moving and let the stale
|
||||||
|
// scroll positions through, skipping the reel forward a few videos.
|
||||||
|
const SETTLE_STABLE_FRAMES = 10;
|
||||||
|
const SETTLE_MAX_MS = 1500;
|
||||||
|
let settleStartedAt = 0;
|
||||||
|
let settleStableFrames = 0;
|
||||||
|
let settleHeight = -1;
|
||||||
|
let settleDone = null; // run once the window closes, if anything asked
|
||||||
|
|
||||||
// HUD auto-hide: the reels HUD fades out after this much inactivity and
|
// HUD auto-hide: the reels HUD fades out after this much inactivity and
|
||||||
// reappears on any pointer movement / tap / scroll. Buttons keep their
|
// reappears on any pointer movement / tap / scroll. Buttons keep their
|
||||||
@@ -1313,23 +1326,63 @@ App.feed = App.feed || {};
|
|||||||
if (activeSlide) measureFeedTitle(activeSlide);
|
if (activeSlide) measureFeedTitle(activeSlide);
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// Ends the realign window: one last realign, then real swipes count again.
|
||||||
|
const finishSettle = function() {
|
||||||
|
if (resizeSettleRaf) {
|
||||||
|
cancelAnimationFrame(resizeSettleRaf);
|
||||||
|
resizeSettleRaf = null;
|
||||||
|
}
|
||||||
|
if (resizeSettleTimer) {
|
||||||
|
clearTimeout(resizeSettleTimer);
|
||||||
|
resizeSettleTimer = null;
|
||||||
|
}
|
||||||
|
realignToActive();
|
||||||
|
suppressScroll = false;
|
||||||
|
const after = settleDone;
|
||||||
|
settleDone = null;
|
||||||
|
if (after) after();
|
||||||
|
};
|
||||||
|
|
||||||
|
// Watches the viewport settle, frame by frame, holding the active video in
|
||||||
|
// place the whole way.
|
||||||
|
const settleStep = function() {
|
||||||
|
resizeSettleRaf = null;
|
||||||
|
if (!state.feedOpen || state.feedActiveIndex < 0) {
|
||||||
|
suppressScroll = false;
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
const height = slideHeight();
|
||||||
|
if (height === settleHeight) settleStableFrames += 1;
|
||||||
|
else {
|
||||||
|
settleHeight = height;
|
||||||
|
settleStableFrames = 0;
|
||||||
|
}
|
||||||
|
realignToActive();
|
||||||
|
if (settleStableFrames < SETTLE_STABLE_FRAMES &&
|
||||||
|
(Date.now() - settleStartedAt) < SETTLE_MAX_MS) {
|
||||||
|
resizeSettleRaf = requestAnimationFrame(settleStep);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
// One more frame, so a re-snap the container has already scheduled
|
||||||
|
// lands inside the window rather than just after it.
|
||||||
|
resizeSettleRaf = requestAnimationFrame(finishSettle);
|
||||||
|
};
|
||||||
|
|
||||||
const onResize = function() {
|
const onResize = function() {
|
||||||
if (!state.feedOpen || state.feedActiveIndex < 0) return;
|
if (!state.feedOpen || state.feedActiveIndex < 0) return;
|
||||||
// Suppress scroll handling while we realign so the container's re-snap
|
// Suppress scroll handling while we realign so the container's re-snap
|
||||||
// doesn't flip the active video, then re-enable it once layout settles.
|
// doesn't flip the active video, then re-enable it once layout settles.
|
||||||
suppressScroll = true;
|
suppressScroll = true;
|
||||||
|
settleStartedAt = Date.now();
|
||||||
|
settleStableFrames = 0;
|
||||||
|
settleHeight = -1;
|
||||||
realignToActive();
|
realignToActive();
|
||||||
// Orientation changes can settle over more than one frame (the visual
|
|
||||||
// viewport and the scroll-snap re-anchor in stages); realign again once
|
|
||||||
// layout has settled, then stop suppressing real swipes.
|
|
||||||
if (resizeSettleRaf) cancelAnimationFrame(resizeSettleRaf);
|
if (resizeSettleRaf) cancelAnimationFrame(resizeSettleRaf);
|
||||||
resizeSettleRaf = requestAnimationFrame(() => {
|
resizeSettleRaf = requestAnimationFrame(settleStep);
|
||||||
realignToActive();
|
// Frames stop in a hidden tab, which would leave the window open for
|
||||||
resizeSettleRaf = requestAnimationFrame(() => {
|
// as long as the tab stays away; the clock closes it regardless.
|
||||||
resizeSettleRaf = null;
|
if (resizeSettleTimer) clearTimeout(resizeSettleTimer);
|
||||||
suppressScroll = false;
|
resizeSettleTimer = setTimeout(finishSettle, SETTLE_MAX_MS + 500);
|
||||||
});
|
|
||||||
});
|
|
||||||
};
|
};
|
||||||
|
|
||||||
// ------------------------------------------------------------------
|
// ------------------------------------------------------------------
|
||||||
@@ -1559,21 +1612,21 @@ App.feed = App.feed || {};
|
|||||||
// recomputes them from the active video's id -- the same thing an
|
// recomputes them from the active video's id -- the same thing an
|
||||||
// orientation change does.
|
// orientation change does.
|
||||||
const settleAfterMove = function() {
|
const settleAfterMove = function() {
|
||||||
suppressScroll = true;
|
const resume = function() {
|
||||||
realignToActive();
|
const slide = slidesByIndex.get(state.feedActiveIndex);
|
||||||
requestAnimationFrame(() => {
|
if (slide) panesOf(slide).forEach((pane) => {
|
||||||
realignToActive();
|
const video = pane.querySelector('.feed-video');
|
||||||
requestAnimationFrame(() => {
|
if (video && video.paused && pane.classList.contains('is-loaded')) {
|
||||||
suppressScroll = false;
|
video.play().catch(() => {});
|
||||||
const slide = slidesByIndex.get(state.feedActiveIndex);
|
}
|
||||||
if (slide) panesOf(slide).forEach((pane) => {
|
|
||||||
const video = pane.querySelector('.feed-video');
|
|
||||||
if (video && video.paused && pane.classList.contains('is-loaded')) {
|
|
||||||
video.play().catch(() => {});
|
|
||||||
}
|
|
||||||
});
|
|
||||||
});
|
});
|
||||||
});
|
};
|
||||||
|
if (!state.feedOpen || state.feedActiveIndex < 0) {
|
||||||
|
resume();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
settleDone = resume;
|
||||||
|
onResize();
|
||||||
};
|
};
|
||||||
|
|
||||||
// Puts the feed back in the page. Called when the window closes, however it
|
// Puts the feed back in the page. Called when the window closes, however it
|
||||||
@@ -1720,6 +1773,10 @@ App.feed = App.feed || {};
|
|||||||
cancelAnimationFrame(resizeSettleRaf);
|
cancelAnimationFrame(resizeSettleRaf);
|
||||||
resizeSettleRaf = null;
|
resizeSettleRaf = null;
|
||||||
}
|
}
|
||||||
|
if (resizeSettleTimer) {
|
||||||
|
clearTimeout(resizeSettleTimer);
|
||||||
|
resizeSettleTimer = null;
|
||||||
|
}
|
||||||
const spacer = getTopSpacer();
|
const spacer = getTopSpacer();
|
||||||
if (spacer) spacer.style.height = '0px';
|
if (spacer) spacer.style.height = '0px';
|
||||||
const scroller = getScroller();
|
const scroller = getScroller();
|
||||||
@@ -1749,6 +1806,14 @@ App.feed = App.feed || {};
|
|||||||
if (!scrollBound) {
|
if (!scrollBound) {
|
||||||
scroller.addEventListener('scroll', onScroll, { passive: true });
|
scroller.addEventListener('scroll', onScroll, { passive: true });
|
||||||
window.addEventListener('resize', onResize);
|
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
|
||||||
|
// slides away, so both open the realign window too -- the watcher
|
||||||
|
// below waits for the height to stop moving either way.
|
||||||
|
window.addEventListener('orientationchange', onResize);
|
||||||
|
if (window.visualViewport) {
|
||||||
|
window.visualViewport.addEventListener('resize', onResize);
|
||||||
|
}
|
||||||
scrollBound = true;
|
scrollBound = true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
BIN
tests/fixtures/clip.mp4
vendored
Normal file
BIN
tests/fixtures/clip.mp4
vendored
Normal file
Binary file not shown.
208
tests/smoke_rotate.py
Normal file
208
tests/smoke_rotate.py
Normal file
@@ -0,0 +1,208 @@
|
|||||||
|
#!/usr/bin/env python3
|
||||||
|
"""Turning the phone keeps the same video on screen.
|
||||||
|
|
||||||
|
Run against a locally running backend:
|
||||||
|
|
||||||
|
backend/main.py &
|
||||||
|
.venv/bin/python tests/smoke_rotate.py
|
||||||
|
|
||||||
|
Both the status and the listing are fixtures, so the reel is long enough to
|
||||||
|
skip forward in and the test never waits on a real source.
|
||||||
|
|
||||||
|
A rotation is not one event: the browser fires `resize` several times as its
|
||||||
|
chrome and the viewport settle, and the scroll-snap container re-anchors on
|
||||||
|
pixel positions measured against the old height somewhere in the middle of
|
||||||
|
that. The test replays exactly that -- a second `resize` and then a stale
|
||||||
|
scroll position -- and asks that the reel still be on the video it was on.
|
||||||
|
"""
|
||||||
|
import json
|
||||||
|
import os
|
||||||
|
import re
|
||||||
|
import sys
|
||||||
|
from playwright.sync_api import sync_playwright
|
||||||
|
|
||||||
|
# A real (tiny, navy, silent) clip: the feed drops a video whose source fails,
|
||||||
|
# and a reel that empties itself has no scroll positions to get wrong.
|
||||||
|
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}
|
||||||
|
LANDSCAPE = {"width": 930, "height": 430}
|
||||||
|
|
||||||
|
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 active = document.querySelector('.feed-slide.is-active');
|
||||||
|
const h = scroller.clientHeight;
|
||||||
|
return {
|
||||||
|
index: App.state.feedActiveIndex,
|
||||||
|
videoId: App.state.feedActiveVideoId,
|
||||||
|
scrollTop: scroller.scrollTop,
|
||||||
|
expected: App.state.feedActiveIndex * h,
|
||||||
|
height: h,
|
||||||
|
// Where the active slide actually sits: the one thing the reader sees.
|
||||||
|
activeTop: active ? Math.round(active.getBoundingClientRect().top) : null,
|
||||||
|
activeId: active ? active.dataset.step : null,
|
||||||
|
// Every rendered slide must be exactly a scroller tall, or
|
||||||
|
// `index * clientHeight` stops addressing the slide it names.
|
||||||
|
offHeight: Array.from(document.querySelectorAll('.feed-slide'))
|
||||||
|
.map((s) => s.getBoundingClientRect().height)
|
||||||
|
.filter((sh) => Math.abs(sh - h) > 1)
|
||||||
|
};
|
||||||
|
}"""
|
||||||
|
|
||||||
|
# A rotation as the browser actually delivers it: another resize while things
|
||||||
|
# are still moving, then the container's own re-anchor on a position measured
|
||||||
|
# against the height from before.
|
||||||
|
ROTATION_NOISE = """(oldHeight) => new Promise((resolve) => {
|
||||||
|
const scroller = document.getElementById('feed-scroll');
|
||||||
|
const index = App.state.feedActiveIndex;
|
||||||
|
setTimeout(() => window.dispatchEvent(new Event('resize')), 60);
|
||||||
|
setTimeout(() => {
|
||||||
|
scroller.scrollTop = index * oldHeight;
|
||||||
|
resolve(index * oldHeight);
|
||||||
|
}, 140);
|
||||||
|
})"""
|
||||||
|
|
||||||
|
GO = """(step) => {
|
||||||
|
const scroller = document.getElementById('feed-scroll');
|
||||||
|
scroller.scrollTop = step * scroller.clientHeight;
|
||||||
|
return App.state.feedActiveIndex;
|
||||||
|
}"""
|
||||||
|
|
||||||
|
|
||||||
|
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 swipe_to(page, step):
|
||||||
|
"""One step at a time, the way a thumb gets there: only the slides around
|
||||||
|
the active one exist, so the scroll range doesn't reach step 7 in one go."""
|
||||||
|
for n in range(1, step + 1):
|
||||||
|
page.evaluate(GO, n)
|
||||||
|
page.wait_for_timeout(250)
|
||||||
|
page.wait_for_timeout(400)
|
||||||
|
|
||||||
|
|
||||||
|
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 rotate(page, c, size, label):
|
||||||
|
before = page.evaluate(WHERE)
|
||||||
|
page.set_viewport_size(size)
|
||||||
|
stale = page.evaluate(ROTATION_NOISE, before["height"])
|
||||||
|
page.wait_for_timeout(1400)
|
||||||
|
after = page.evaluate(WHERE)
|
||||||
|
print(f" (was step {before['index']} at {before['scrollTop']}px of {before['height']};"
|
||||||
|
f" the stale re-anchor asked for {stale}px)")
|
||||||
|
c.ok(f"{label}: the same video is still the one playing",
|
||||||
|
after["videoId"] == before["videoId"], f"{before['videoId']} -> {after['videoId']}")
|
||||||
|
c.ok(f"{label}: and the same step",
|
||||||
|
after["index"] == before["index"], f"{before['index']} -> {after['index']}")
|
||||||
|
c.ok(f"{label}: the scroll position is the one that step sits at",
|
||||||
|
abs(after["scrollTop"] - after["expected"]) <= 1, str(after))
|
||||||
|
c.ok(f"{label}: so the step fills the screen rather than straddling two",
|
||||||
|
after["activeTop"] == 0, str(after["activeTop"]))
|
||||||
|
c.ok(f"{label}: every slide is exactly a screenful",
|
||||||
|
after["offHeight"] == [], str(after["offHeight"]))
|
||||||
|
return after
|
||||||
|
|
||||||
|
|
||||||
|
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))
|
||||||
|
c.ok("and the slides are a screenful each to begin with",
|
||||||
|
start["offHeight"] == [], str(start["offHeight"]))
|
||||||
|
|
||||||
|
print("\nturning it sideways")
|
||||||
|
land = rotate(page, c, LANDSCAPE, "landscape")
|
||||||
|
c.ok("the slides took the new height", land["height"] == 430, str(land["height"]))
|
||||||
|
|
||||||
|
print("\nand back upright")
|
||||||
|
port = rotate(page, c, PORTRAIT, "portrait")
|
||||||
|
c.ok("the slides took the height back", port["height"] == 930, str(port["height"]))
|
||||||
|
|
||||||
|
print("\nswiping still works once it has settled")
|
||||||
|
page.evaluate(GO, port["index"] + 1)
|
||||||
|
page.wait_for_timeout(700)
|
||||||
|
moved = page.evaluate(WHERE)
|
||||||
|
c.ok("a real swipe after a rotation moves the reel on",
|
||||||
|
moved["index"] == port["index"] + 1, str(moved["index"]))
|
||||||
|
|
||||||
|
print("\nrotating twice in a row, as a wrist does")
|
||||||
|
page.set_viewport_size(LANDSCAPE)
|
||||||
|
page.wait_for_timeout(90)
|
||||||
|
page.set_viewport_size(PORTRAIT)
|
||||||
|
page.wait_for_timeout(1500)
|
||||||
|
twice = page.evaluate(WHERE)
|
||||||
|
c.ok("the reel is where it was left", twice["index"] == moved["index"] and
|
||||||
|
twice["videoId"] == moved["videoId"], str(twice))
|
||||||
|
c.ok("and sitting on its snap point",
|
||||||
|
abs(twice["scrollTop"] - twice["expected"]) <= 1, str(twice))
|
||||||
|
|
||||||
|
c.ok("nothing threw along the way", not crashes, str(crashes[:2]))
|
||||||
|
browser.close()
|
||||||
|
|
||||||
|
print(f"\n{'OK' if not c.failed else 'FAILED'}: {c.failed} check(s) failed")
|
||||||
|
return 1 if c.failed else 0
|
||||||
|
|
||||||
|
|
||||||
|
sys.exit(main())
|
||||||
Reference in New Issue
Block a user