fix(player): a held key on faded bands stays one blind press; a play asked for mid-restart is the new session's
The repeats of a Space/Enter that landed on faded bands pressed the control once the bands were up; they are swallowed with it. A play or pause while a session restart is out now sets that session's intent instead of playing the outgoing stream. Tests: Skip intro outside the bands keeps its keys, every restore takeover (wheel, finger, scrollbar, key) is covered, and the chip handoff test tells the chip before from the row's first control.
This commit is contained in:
@@ -313,6 +313,54 @@ describe("keyboard focus in the bands", () => {
|
||||
expect(video().paused).toBe(false);
|
||||
});
|
||||
|
||||
it.each([" ", "Enter"])(
|
||||
"a held %j on a control the bands faded from is one blind press, repeats and all",
|
||||
async (key) => {
|
||||
mount({ details: { A: item("A") } });
|
||||
await playingAt("/media/A", 100);
|
||||
vi.useFakeTimers();
|
||||
video()._paused = true;
|
||||
act(() => void video().play());
|
||||
const mute = screen.getByLabelText("plex.player.mute");
|
||||
fireEvent.keyDown(document.body, { key: "Tab" });
|
||||
act(() => mute.focus());
|
||||
fireEvent.keyDown(mute, { key: "ArrowUp" });
|
||||
act(() => vi.advanceTimersByTime(10_500));
|
||||
expect(bandHidden()).toBe(true);
|
||||
expect(fireEvent.keyDown(mute, { key })).toBe(false);
|
||||
const toggles = pause.mock.calls.length + play.mock.calls.length;
|
||||
// the bands are up by now, and the OS repeats the held key: still not the control's
|
||||
expect(fireEvent.keyDown(mute, { key, repeat: true })).toBe(false);
|
||||
expect(fireEvent.keyDown(mute, { key, repeat: true })).toBe(false);
|
||||
expect(pause.mock.calls.length + play.mock.calls.length).toBe(toggles);
|
||||
// a new press on the shown bands is the control's own again
|
||||
fireEvent.keyUp(mute, { key });
|
||||
expect(fireEvent.keyDown(mute, { key })).toBe(true);
|
||||
},
|
||||
);
|
||||
|
||||
it("a Space or Enter on Skip intro, outside the faded bands, stays Skip intro's", async () => {
|
||||
const intro = {
|
||||
...item("A"),
|
||||
markers: [{ type: "intro", start_s: 50, end_s: 200 }],
|
||||
} as unknown as PlexItemDetail;
|
||||
mount({ details: { A: intro } });
|
||||
await playingAt("/media/A", 100);
|
||||
vi.useFakeTimers();
|
||||
video()._paused = true;
|
||||
act(() => void video().play());
|
||||
const skip = screen.getByRole("button", { name: /plex\.player\.skipIntro/ });
|
||||
fireEvent.keyDown(document.body, { key: "Tab" });
|
||||
act(() => skip.focus());
|
||||
act(() => vi.advanceTimersByTime(10_500));
|
||||
expect(bandHidden()).toBe(true);
|
||||
const pauses = pause.mock.calls.length;
|
||||
// not prevented: the button's own activation runs, and Space does not pause the film instead
|
||||
expect(fireEvent.keyDown(skip, { key: " " })).toBe(true);
|
||||
expect(fireEvent.keyDown(skip, { key: "Enter" })).toBe(true);
|
||||
expect(pause.mock.calls.length).toBe(pauses);
|
||||
});
|
||||
|
||||
it("a tap leaving a tapped button for the picture shows the hidden bands", async () => {
|
||||
mount({ details: { A: item("A") } });
|
||||
await playingAt("/media/A", 100);
|
||||
@@ -512,6 +560,55 @@ describe("a Restart of the paused film whose session starts past 0", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("a play asked for while a restart is out", () => {
|
||||
it("is the new session's: the outgoing stream never plays", async () => {
|
||||
const { session } = mount({ details: { A: item("A") } });
|
||||
await playingAt("/media/A", 100);
|
||||
act(() => void video().pause());
|
||||
let answer: (s: Awaited<ReturnType<typeof api.plexSession>>) => void = () => {};
|
||||
session.mockImplementation(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
answer = resolve;
|
||||
}),
|
||||
);
|
||||
const asked = session.mock.calls.length;
|
||||
// paused, and a seek outside the loaded region: a restart that lands paused
|
||||
fireEvent.keyDown(document.body, { key: "ArrowRight" });
|
||||
await waitFor(() => expect(session.mock.calls.length).toBe(asked + 1));
|
||||
const plays = play.mock.calls.length;
|
||||
fireEvent.keyDown(document.body, { key: " " });
|
||||
expect(play.mock.calls.length).toBe(plays); // the old stream stays silent
|
||||
expect(video().paused).toBe(true);
|
||||
await act(async () => answer({ ...direct("A"), start_s: 110 } as never));
|
||||
act(() => void video().dispatchEvent(new Event("loadedmetadata")));
|
||||
expect(play.mock.calls.length).toBe(plays + 1); // and the new one plays, as asked
|
||||
});
|
||||
|
||||
it("a pause asked for while a playing restart is out lands paused", async () => {
|
||||
const { session } = mount({ details: { A: item("A") } });
|
||||
await playingAt("/media/A", 100);
|
||||
act(() => void video().play());
|
||||
let answer: (s: Awaited<ReturnType<typeof api.plexSession>>) => void = () => {};
|
||||
session.mockImplementation(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
answer = resolve;
|
||||
}),
|
||||
);
|
||||
const asked = session.mock.calls.length;
|
||||
fireEvent.keyDown(document.body, { key: "ArrowRight" });
|
||||
await waitFor(() => expect(session.mock.calls.length).toBe(asked + 1));
|
||||
fireEvent.keyDown(document.body, { key: " " });
|
||||
expect(video().paused).toBe(true);
|
||||
const plays = play.mock.calls.length;
|
||||
await act(async () => answer({ ...direct("A"), start_s: 110 } as never));
|
||||
act(() => void video().dispatchEvent(new Event("loadedmetadata")));
|
||||
act(() => void video().dispatchEvent(new Event("canplay")));
|
||||
expect(play.mock.calls.length).toBe(plays);
|
||||
});
|
||||
});
|
||||
|
||||
describe("the C5 row", () => {
|
||||
it("has no Stop, Restart or ±10 s buttons, and keeps the rare tools behind the ⋯", async () => {
|
||||
mount({ details: { A: item("A") } });
|
||||
|
||||
@@ -178,6 +178,13 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false }
|
||||
// a timeupdate AFTER the reset to 0) — sent as the new item's subtitle position and saved as its
|
||||
// progress, or, during a seek-restart, the pre-seek spot overwriting the target.
|
||||
const clockLiveRef = useRef(false);
|
||||
// Whether the session being asked for plays once it can — null while none is out. Set from
|
||||
// loadSession's `resumePlay` and then the viewer's to change: until the answer attaches, the media
|
||||
// is the OUTGOING one, and a play() there sounded the old stream while the new one still attached
|
||||
// paused (E28 review round 4). `said`: the viewer changed it, so it is what they now see.
|
||||
const pendingPlayRef = useRef<{ play: boolean; said?: boolean } | null>(null);
|
||||
// The key of the last press that landed on faded bands, while it is held (see onKey).
|
||||
const blindKeyRef = useRef<string | null>(null);
|
||||
// The item whose session is attached to the <video> right now (see loadSession's failure path).
|
||||
const attachedIdRef = useRef<string | null>(null);
|
||||
// Which encoder the current transcode session ran: true=VAAPI hardware, false=libx264 software,
|
||||
@@ -393,6 +400,12 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false }
|
||||
};
|
||||
};
|
||||
clockLiveRef.current = false;
|
||||
const intent: { play: boolean; said?: boolean } = { play: resumePlay };
|
||||
pendingPlayRef.current = intent;
|
||||
// Answered (or failed): the media from here on is this session's, and a Space is its own.
|
||||
const settled = () => {
|
||||
if (pendingPlayRef.current === intent) pendingPlayRef.current = null;
|
||||
};
|
||||
loadStartRef.current = startAt;
|
||||
setReady(false);
|
||||
setBuffering(false);
|
||||
@@ -409,6 +422,7 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false }
|
||||
sess = await api.plexSession(id, startAt, m ? null : audioRef.current, aoffRef.current, m);
|
||||
} catch (e) {
|
||||
if (!current()) return; // superseded or left: not this request's error to show
|
||||
settled();
|
||||
// A failed seek-restart leaves the old session attached and its clock still true; a failed
|
||||
// move to another item leaves the previous item's media, which is not this item's clock.
|
||||
clockLiveRef.current = attachedIdRef.current === id;
|
||||
@@ -443,19 +457,19 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false }
|
||||
// timeupdate its teardown queues already reads the new start.
|
||||
clockLiveRef.current = true;
|
||||
attachedIdRef.current = id;
|
||||
settled();
|
||||
// Belt-and-suspenders resume: the play() fired on MANIFEST_PARSED / loadedmetadata can be
|
||||
// rejected when the media isn't ready yet (slow first segments over a proxy), which would leave
|
||||
// the video paused after a seek even though the user was watching. Retry once the element can
|
||||
// actually play. Idempotent — only acts if still paused.
|
||||
if (resumePlay) {
|
||||
listen(
|
||||
"canplay",
|
||||
() => {
|
||||
if (videoRef.current?.paused) videoRef.current.play().catch(() => {});
|
||||
},
|
||||
true,
|
||||
);
|
||||
}
|
||||
// actually play. Idempotent — only acts if still paused. The intent is read when it fires: the
|
||||
// viewer may have asked for play or pause since the request went out.
|
||||
listen(
|
||||
"canplay",
|
||||
() => {
|
||||
if (intent.play && videoRef.current?.paused) videoRef.current.play().catch(() => {});
|
||||
},
|
||||
true,
|
||||
);
|
||||
if (hlsRef.current) {
|
||||
hlsRef.current.destroy();
|
||||
hlsRef.current = null;
|
||||
@@ -491,7 +505,7 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false }
|
||||
hls.on(Hls.Events.MANIFEST_PARSED, () => {
|
||||
parsed = true;
|
||||
setReady(true);
|
||||
if (aliveRef.current && resumePlay) video.play().catch(() => {});
|
||||
if (aliveRef.current && intent.play) video.play().catch(() => {});
|
||||
});
|
||||
// Restore the selected audio track ONCE hls.js has parsed the renditions — doing it on
|
||||
// MANIFEST_PARSED was too early (the track list wasn't populated, so the set was dropped and
|
||||
@@ -567,7 +581,7 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false }
|
||||
// detachSessionRef and cancelAutoSkipRef do the same for the same reason.
|
||||
void loadSessionRef.current(
|
||||
playhead(absRef, loadStartRef),
|
||||
parsed ? !video.paused : resumePlay,
|
||||
parsed ? !video.paused : intent.play,
|
||||
);
|
||||
return;
|
||||
}
|
||||
@@ -583,7 +597,7 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false }
|
||||
() => {
|
||||
if (sess.mode === "direct" && startAt > 0) video.currentTime = startAt;
|
||||
setReady(true);
|
||||
if (resumePlay) video.play().catch(() => {});
|
||||
if (intent.play) video.play().catch(() => {});
|
||||
},
|
||||
true,
|
||||
);
|
||||
@@ -826,12 +840,30 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false }
|
||||
);
|
||||
|
||||
// --- controls --------------------------------------------------------------------------------
|
||||
// While a session is being asked for, play and pause are the pending session's intent (see
|
||||
// pendingPlayRef): the outgoing media is never played, only paused if the viewer asks for a pause.
|
||||
const requestPlay = useCallback((want: boolean) => {
|
||||
const v = videoRef.current;
|
||||
if (!v) return;
|
||||
const pending = pendingPlayRef.current;
|
||||
if (pending) {
|
||||
pending.play = want;
|
||||
pending.said = true;
|
||||
if (!want && !v.paused) v.pause();
|
||||
setPlaying(want);
|
||||
return;
|
||||
}
|
||||
if (want) v.play().catch(() => {});
|
||||
else v.pause();
|
||||
}, []);
|
||||
const togglePlay = useCallback(() => {
|
||||
const v = videoRef.current;
|
||||
if (!v) return;
|
||||
if (v.paused) v.play().catch(() => {});
|
||||
else v.pause();
|
||||
}, []);
|
||||
// What the viewer sees: the outgoing media's state, until they have said otherwise.
|
||||
const pending = pendingPlayRef.current;
|
||||
const shown = pending?.said ? pending.play : !v.paused;
|
||||
requestPlay(!shown);
|
||||
}, [requestPlay]);
|
||||
|
||||
// Volume/mute for the HTML5 <video> (0..1, independent mute), persisted + re-applied per session.
|
||||
const { setVol, nudgeVolume, toggleMute, onWheel } = usePlayerVolume({
|
||||
@@ -858,7 +890,7 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false }
|
||||
// intent goes INTO the seek: a paused film whose session starts past 0 restarts the session,
|
||||
// and a play() beside that resumed the outgoing stream while the new one stayed paused.
|
||||
if (fromZero) seekTo(0, true);
|
||||
else videoRef.current?.play().catch(() => {});
|
||||
else requestPlay(true);
|
||||
return;
|
||||
}
|
||||
if (otherId) {
|
||||
@@ -887,7 +919,7 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false }
|
||||
setId(otherId);
|
||||
}
|
||||
},
|
||||
[id, seekTo, settleProgress, finishProgress],
|
||||
[id, seekTo, requestPlay, settleProgress, finishProgress],
|
||||
);
|
||||
|
||||
// Prev/next: from the playlist queue when playing one, else the item's own episode neighbours.
|
||||
@@ -1150,15 +1182,20 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false }
|
||||
isActivationTarget(e) &&
|
||||
!!(e.target as Element | null)?.closest?.("[data-player-band]") &&
|
||||
(e.key === " " || e.key === "Enter");
|
||||
// Held, the key repeats — and by its first repeat the bands are up, so the repeat would press
|
||||
// the control after all (Enter clicks on every repeat, Space re-arms its keyup click). A held
|
||||
// key is one press (keymap.ts): its repeats are as blind as it was.
|
||||
if (!e.repeat) blindKeyRef.current = blind ? e.key : null;
|
||||
const swallow = blind || (e.repeat && e.key === blindKeyRef.current);
|
||||
// Keyboard use also reveals the controls — but typing in a player field is not "using the
|
||||
// remote", and `dispatch` still lets Escape through from inside a field (the only key that
|
||||
// opts in), which is why this is a guard around `wake` rather than an early return.
|
||||
// A slider is not a text field: its arrows are the remote too (E28 review E6).
|
||||
if (!isTypingTarget(e) || (e.target as Element | null)?.matches?.('input[type="range"]'))
|
||||
wake();
|
||||
if (blind) {
|
||||
if (swallow) {
|
||||
e.preventDefault();
|
||||
if (e.key === " ") togglePlay();
|
||||
if (e.key === " " && !e.repeat) togglePlay();
|
||||
return;
|
||||
}
|
||||
// The shared dialog Tab trap. This player paints an opaque full-viewport layer over the app,
|
||||
|
||||
@@ -71,10 +71,10 @@ describe("Chip", () => {
|
||||
});
|
||||
|
||||
// Round 3: the neighbour it hands the focus to can go in the same removal — the "Clear all" that
|
||||
// only shows beside two filters. The focus then goes to the chip before, not to <body>.
|
||||
// only shows beside enough filters. The focus then goes to the chip before, not to <body>.
|
||||
it("a removing chip whose neighbour goes with it hands the focus to the chip before", async () => {
|
||||
function Row() {
|
||||
const [chips, setChips] = useState(["Music", "News"]);
|
||||
const [chips, setChips] = useState(["Music", "News", "Sport"]);
|
||||
return (
|
||||
<div>
|
||||
{chips.map((c) => (
|
||||
@@ -87,17 +87,18 @@ describe("Chip", () => {
|
||||
{c}
|
||||
</Chip>
|
||||
))}
|
||||
{chips.length > 1 && <button onClick={() => setChips([])}>Clear all</button>}
|
||||
{chips.length > 2 && <button onClick={() => setChips([])}>Clear all</button>}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
render(<Row />);
|
||||
const news = screen.getByRole("button", { name: "Remove News" });
|
||||
act(() => news.focus());
|
||||
fireEvent.click(news, { detail: 0 });
|
||||
const sport = screen.getByRole("button", { name: "Remove Sport" });
|
||||
act(() => sport.focus());
|
||||
fireEvent.click(sport, { detail: 0 });
|
||||
expect(screen.queryByRole("button", { name: "Clear all" })).toBeNull();
|
||||
// the chip right before it — not the row's first control, which Music also is
|
||||
await waitFor(() =>
|
||||
expect(document.activeElement).toBe(screen.getByRole("button", { name: "Remove Music" })),
|
||||
expect(document.activeElement).toBe(screen.getByRole("button", { name: "Remove News" })),
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
@@ -155,7 +155,14 @@ describe("PageScroller scroll memory", () => {
|
||||
});
|
||||
|
||||
// Round 3: the give-up put the page at its end 1.5 s in, wherever the reader had scrolled to.
|
||||
it("stops where the reader takes the page over, even for a spot it has not reached", () => {
|
||||
// Every way a reader takes the page: a wheel, a finger, the scrollbar (a pointer), a key.
|
||||
it.each([
|
||||
["a wheel", (main: HTMLElement) => fireEvent.wheel(main)],
|
||||
["a finger", (main: HTMLElement) => fireEvent.touchStart(main)],
|
||||
["the scrollbar", (main: HTMLElement) => fireEvent.pointerDown(main)],
|
||||
["a key", () => fireEvent.keyDown(document.body, { key: "ArrowUp" })],
|
||||
])("stops where %s takes the page over, even for a spot it has not reached", (_, take) => {
|
||||
const removed = vi.spyOn(document, "removeEventListener");
|
||||
let frames: FrameRequestCallback[] = [];
|
||||
vi.stubGlobal("requestAnimationFrame", (cb: FrameRequestCallback) => frames.push(cb));
|
||||
const flush = (n: number) => {
|
||||
@@ -177,7 +184,9 @@ describe("PageScroller scroll memory", () => {
|
||||
setEntry(1);
|
||||
notifyNav();
|
||||
flush(5);
|
||||
fireEvent.wheel(main);
|
||||
take(main);
|
||||
// and it let go of the page-wide key listener with it
|
||||
expect(removed).toHaveBeenCalledWith("keydown", expect.any(Function), true);
|
||||
main.scrollTop = 30; // where the reader scrolled
|
||||
flush(100);
|
||||
expect(main.scrollTop).toBe(30);
|
||||
|
||||
Reference in New Issue
Block a user