From c489b60c8b037cb0721938d8648604fa10d7417c Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sat, 10 Oct 2026 15:35:21 +0200 Subject: [PATCH] fix(clip): clip mode keeps its range, its focus and its keys through the player's own moves - A mark on the very end seeks a moment short of it; while a clip is marked the end of the item neither closes the player nor steps the queue, and Plex's auto-skip waits. - Plex: Escape and Back peel what is over the picture (info, up-next, the countdown) before the clip; Back under a dialog is the dialog's; no range opens while the length is unknown. - YouTube: the focus goes back to the scissors the user can see, else the player; the panel keeps its name, Loop and focus when the view is maximised or restored under it. - A typed time stays in its field until taken, then shows where it landed; unreadable stays marked invalid; a decimal comma is read (ui/TimecodeField, the editor's fields too). - RangeTrack: a drag is its pointer's, a cancelled drag reports final, an adornment sits beside the slider, a caller-chosen focus ring (amber on the clip strip). CropFrame: a drag is its pointer's. - The editor's remove-cut is a real button at the cut's foot; a segment's keep toggle sits over the cut's hit area. --- docs/DESIGN.md | 15 +++- .../src/components/ClipPanel.dom.test.tsx | 51 +++++++++++- frontend/src/components/ClipPanel.tsx | 56 +++++++------ .../src/components/PlayerModal.dom.test.tsx | 82 ++++++++++++++++++- frontend/src/components/PlayerModal.tsx | 79 +++++++++++++++--- .../PlexPlayer.controls.dom.test.tsx | 76 ++++++++++++++++- frontend/src/components/PlexPlayer.tsx | 48 ++++++----- .../VideoEditor.partial.dom.test.tsx | 38 ++++++++- frontend/src/components/VideoEditor.tsx | 41 +++++----- .../src/components/ui/CropFrame.dom.test.tsx | 14 ++++ frontend/src/components/ui/CropFrame.tsx | 13 ++- frontend/src/components/ui/README.md | 2 +- .../src/components/ui/RangeTrack.dom.test.tsx | 54 ++++++++++++ frontend/src/components/ui/RangeTrack.tsx | 80 +++++++++++++----- frontend/src/components/ui/TimecodeField.tsx | 58 +++++++++++++ frontend/src/lib/plexPlayerHooks.ts | 6 +- frontend/src/lib/timecode.test.ts | 7 +- frontend/src/lib/timecode.ts | 8 +- 18 files changed, 607 insertions(+), 121 deletions(-) create mode 100644 frontend/src/components/ui/TimecodeField.tsx diff --git a/docs/DESIGN.md b/docs/DESIGN.md index d5ceb395..b39d3b5b 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -556,16 +556,23 @@ of a full-width panel and nothing on the right. around the selection with the two ends (`ui/RangeTrack`: each end a slider — arrows step a tenth, Shift a second; drawn 4px, hit 1.5rem), Start / End fields with a tenth back / forward and "to the playhead", the length, Play selection (loops), a name, Cancel / Cut clip. On the always-dark chrome the - ends are white and the playhead amber (a light theme's accent all but vanished there). The strip's window + ends are white, a focused end's ring and the playhead amber (a light theme's accent all but vanished + there). A typed time that cannot be read stays in its field, marked invalid; one past the end shows + where it landed (decimal point or comma). The strip's window follows the selection when it settles, never under a dragging pointer. Plex seeks once a drag is - released (a seek may restart the transcode); YouTube follows the drag. Escape closes the panel before the - player; the scissors gets the focus back. **Crop (owner's picks K1 + a):** a Crop toggle in the panel puts a + released (a seek may restart the transcode); YouTube follows the drag; a mark on the very end seeks a + moment short of it, and while a clip is marked the end of the item never closes the player or steps the + queue, and Plex's auto-skip waits. Escape closes the panel before the player, but after anything over + the picture (Plex: the info, the up-next card, the countdown); ⌫ under a dialog is the dialog's. The + scissors the user can see gets the focus back (maximised: the docked bar's), else the player. The panel + keeps its name, Loop and focus when the view is maximised or restored under it. **Crop (owner's picks K1 + a):** a Crop toggle in the panel puts a frame over the picture (`ui/CropFrame`: on the picture, not its bars; the outside dimmed; drag to move, edges and corners to resize, thirds while dragging; the arrows move it, a focused corner's resize it, Shift further; focus ring amber) with a shape — Free, 16:9, 9:16, 1:1 (the largest of it the picture holds) — and Whole picture; off, the clip is the whole picture. Escape turns the crop off before it closes the panel. A press outside the frame is still the video's. A cut is a toast with a Downloads link and the usual - "Ready to watch" notification. Limits are the server's: 1 s to 10 min (`lib/clipRange`). + "Ready to watch" notification. Limits are the server's: 1 s to 10 min (`lib/clipRange`, held to + `downloads/clip.py` by `clipLimits.contract.test.ts`; the "too long" message names the server's cap). ## Dialogs — _Decided_ (C4, built in S3f) diff --git a/frontend/src/components/ClipPanel.dom.test.tsx b/frontend/src/components/ClipPanel.dom.test.tsx index d37ff282..30154b48 100644 --- a/frontend/src/components/ClipPanel.dom.test.tsx +++ b/frontend/src/components/ClipPanel.dom.test.tsx @@ -49,6 +49,7 @@ function Harness({ onClose = () => {}, onPlayFrom = () => {}, onSeek = () => {}, + duration = 3600, }: { src?: ClipSource; start?: ClipRange; @@ -56,6 +57,7 @@ function Harness({ onClose?: () => void; onPlayFrom?: (t: number) => void; onSeek?: (t: number, final: boolean) => void; + duration?: number; }) { const [clip, setClip] = useState(start); const [crop, setCrop] = useState(null); @@ -67,7 +69,7 @@ function Harness({ src={src} clip={clip} onChange={setClip} - duration={3600} + duration={duration} current={current} picture={16 / 9} crop={crop} @@ -99,6 +101,53 @@ describe("ClipPanel", () => { expect(range()).toBe("140-170"); }); + it("keeps the focus in a time field that took a typed time on Enter", () => { + render(); + const field = () => screen.getAllByRole("textbox", { name: "clip.in" })[0]!; + field().focus(); + fireEvent.change(field(), { target: { value: "2:20" } }); + fireEvent.keyDown(field(), { key: "Enter" }); + expect(range()).toBe("140-170"); + expect(document.activeElement).toBe(field()); + expect((field() as HTMLInputElement).value).toBe("0:02:20.0"); + }); + + it("shows the mark it holds, not a time it could not take", () => { + render(); + const out = () => screen.getAllByRole("textbox", { name: "clip.out" })[0] as HTMLInputElement; + // Past the end: clamped back to the end, and the field says so. + fireEvent.change(out(), { target: { value: "1:15:00" } }); + fireEvent.blur(out()); + expect(range()).toBe("3500-3600"); + expect(out().value).toBe("1:00:00.0"); + expect(out().getAttribute("aria-invalid")).toBeNull(); + // Not a time at all: kept for fixing, and marked as not taken. + fireEvent.change(out(), { target: { value: "abc" } }); + fireEvent.blur(out()); + expect(out().value).toBe("abc"); + expect(out().getAttribute("aria-invalid")).toBe("true"); + expect(range()).toBe("3500-3600"); + }); + + it("takes a decimal comma, as a Hungarian hand types it", () => { + render(); + const field = screen.getAllByRole("textbox", { name: "clip.in" })[0]!; + fireEvent.change(field, { target: { value: "1:42,5" } }); + fireEvent.keyDown(field, { key: "Enter" }); + expect(range()).toBe("102.5-130"); + }); + + it("never seeks the player onto the item's very end (that would end it, and the clip with it)", () => { + const onSeek = vi.fn(); + render(); + fireEvent.keyDown(screen.getByRole("slider", { name: "clip.out" }), { key: "End" }); + expect(range()).toBe("590-600"); + const [time, final] = onSeek.mock.lastCall!; + expect(final).toBe(true); + expect(time).toBeLessThan(600); + expect(time).toBeGreaterThan(599); + }); + it("marks an end at the playhead", () => { render(); fireEvent.click(screen.getByRole("button", { name: "clip.outHere" })); diff --git a/frontend/src/components/ClipPanel.tsx b/frontend/src/components/ClipPanel.tsx index 7479e785..1a10b008 100644 --- a/frontend/src/components/ClipPanel.tsx +++ b/frontend/src/components/ClipPanel.tsx @@ -3,12 +3,13 @@ import { useTranslation } from "react-i18next"; import { useQueryClient } from "@tanstack/react-query"; import { Crop, Crosshair, Maximize, Minus, Plus, Repeat, Scissors, X } from "lucide-react"; import { api, type ClipRequest } from "../lib/api"; +import { DISPLAY_NAME_MAX } from "../lib/api/downloads"; import { invalidateDownloads } from "../lib/downloads"; import { notify } from "../lib/notifications"; import { notifyActionError } from "../lib/actionErrors"; import { navigateTo } from "../lib/nav"; import { log } from "../lib/log"; -import { formatTimecode, parseTimecode } from "../lib/timecode"; +import { formatTimecode } from "../lib/timecode"; import { handleBounds, markIn, markOut, zoomWindow, type ClipRange } from "../lib/clipRange"; import { fitRatio, fracRatio, WHOLE, type Frac } from "../lib/cropGeometry"; import { RangeTrack, RulerTicks, type RangeWindow } from "./ui/RangeTrack"; @@ -17,6 +18,7 @@ import SegmentedToggle from "./ui/SegmentedToggle"; import { Button } from "./ui/Button"; import { IconButton } from "./ui/IconButton"; import { Input } from "./ui/Input"; +import { TimecodeField } from "./ui/TimecodeField"; // The clip panel: marks a range of what is playing and cuts it into a download of its own. It lives // IN the player (the YouTube card under the video, the Plex player's control band), so the video @@ -81,9 +83,15 @@ interface Props { } const NUDGE_S = 0.1; +// How far short of the item's end a seek stops: a mark on the very end moves the player to its last +// moment, not past it — a player put at its end ENDS (a Plex film closes, a YouTube queue steps on) +// and takes the clip with it. +const LAST_MOMENT_S = 0.25; // On the always-dark chrome the ends are white (a light theme's accent is a dark blue there, which // all but vanished on the bar) and the playhead amber, so an end and the playhead are never confused. +// A focused end is ringed in amber too, as the crop frame is: the accent ring was under 3:1 there. const PLAYHEAD = "bg-warn"; +const END_RING = "focus-visible:outline-warn"; export default function ClipPanel({ src, @@ -138,15 +146,19 @@ export default function ClipPanel({ if (looping && current >= clip.end) restart(); }, [looping, current, clip.end]); + const seek = (time: number, final: boolean) => + onSeek(Math.max(0, Math.min(time, duration - LAST_MOMENT_S)), final); const moveIn = (time: number, final: boolean) => { setFrozen(final ? null : (frozen ?? view)); - onChange(markIn(clip, time, duration)); - onSeek(time, final); + const next = markIn(clip, time, duration); + onChange(next); + seek(next.start, final); }; const moveOut = (time: number, final: boolean) => { setFrozen(final ? null : (frozen ?? view)); - onChange(markOut(clip, time, duration)); - onSeek(time, final); + const next = markOut(clip, time, duration); + onChange(next); + seek(next.end, final); }; const body: ClipRequest = { @@ -210,7 +222,7 @@ export default function ClipPanel({ duration={duration} playhead={current} playheadClassName={PLAYHEAD} - onSeek={(time, final) => final && onSeek(time, true)} + onSeek={(time, final) => final && seek(time, true)} className="h-5 flex-1 cursor-pointer rounded-sm bg-white/15" regions={[ { @@ -233,7 +245,8 @@ export default function ClipPanel({ view={view} playhead={current} playheadClassName={PLAYHEAD} - onSeek={onSeek} + onSeek={seek} + handleRingClassName={END_RING} className="h-14 cursor-pointer rounded-md bg-white/10 text-white/45" background={ formatTimecode(s, hours).replace(/\.\d$/, "")} /> @@ -276,7 +289,7 @@ export default function ClipPanel({ moveIn(v, true)} nudge={NUDGE_S} hereLabel={t("clip.inHere")} @@ -285,7 +298,7 @@ export default function ClipPanel({ moveOut(v, true)} nudge={NUDGE_S} hereLabel={t("clip.outHere")} @@ -353,7 +366,7 @@ export default function ClipPanel({ size="sm" className="min-w-48 flex-1" value={name} - maxLength={255} + maxLength={DISPLAY_NAME_MAX} onChange={(e) => setName(e.target.value)} placeholder={t("clip.namePlaceholder")} aria-label={t("clip.name")} @@ -374,7 +387,7 @@ export default function ClipPanel({ function MarkField({ label, value, - text, + hours, onSet, nudge, hereLabel, @@ -382,17 +395,13 @@ function MarkField({ }: { label: string; value: number; - text: string; + hours: boolean; onSet: (v: number) => void; nudge: number; hereLabel: string; onHere: () => void; }) { const { t } = useTranslation(); - const commit = (raw: string) => { - const v = parseTimecode(raw); - if (v != null && Math.abs(v - value) > 1e-3) onSet(v); - }; return (
{label} @@ -404,20 +413,13 @@ function MarkField({ > - commit(e.target.value)} - onKeyDown={(e) => { - if (e.key !== "Enter") return; - e.preventDefault(); - commit(e.currentTarget.value); - }} + onCommit={onSet} /> { const clock = { current: 0, duration: 3000 }; const started: { videoId: string; start: number | undefined }[] = []; // The last player's event handlers, so a test can say "the player is ready" when it wants to. -let playerEvents: { onReady?: () => void } = {}; +let playerEvents: { onReady?: () => void; onStateChange?: (e: { data: number }) => void } = {}; const base = { getCurrentTime: () => clock.current, getDuration: () => clock.duration, @@ -65,7 +65,7 @@ const fakePlayer = new Proxy(base, { window.YT = { Player: function ( _el: unknown, - opts: { videoId: string; playerVars: { start?: number }; events?: { onReady?: () => void } }, + opts: { videoId: string; playerVars: { start?: number }; events?: typeof playerEvents }, ) { started.push({ videoId: opts.videoId, start: opts.playerVars.start }); playerEvents = opts.events ?? {}; @@ -691,6 +691,27 @@ describe("PlayerModal — the side panel on a wide window", () => { expect(row().style.maxWidth).toBe("calc((100dvh - 3rem - 0px) * 16 / 9 + 20rem)"); }); + it("gives the focus to the player when the scissors that opened the clip is not there to take it", async () => { + wideWindow(true); + vi.spyOn(api, "videoDetail").mockReturnValue(new Promise(() => {})); + const A = vid("A", 0); + renderModal( + {}} onState={() => {}} />, + ); + await waitFor(() => expect(lastStart()?.videoId).toBe("A")); + // Shut by the user: closing the clip leaves it shut, and its strip has no scissors. + fireEvent.click(screen.getByRole("button", { name: "player.collapsePanel" })); + act( + () => + void window.dispatchEvent( + new KeyboardEvent("keydown", { key: "c", bubbles: true, cancelable: true }), + ), + ); + fireEvent.click(screen.getByRole("button", { name: "clip.close" })); + await waitFor(() => expect(document.activeElement).not.toBe(document.body)); + expect(screen.getByRole("dialog").contains(document.activeElement)).toBe(true); + }); + it("shuts while a clip is marked and opens again after it, unless the user opened it meanwhile", async () => { wideWindow(true); vi.spyOn(api, "videoDetail").mockReturnValue(new Promise(() => {})); @@ -953,4 +974,61 @@ describe("PlayerModal — clip mode", () => { key("Escape"); expect(screen.queryByTestId("clip-panel")).toBeNull(); }); + + it("gives the focus back to the scissors the user can see: in the maximised view, the bar's", async () => { + open(); + await waitFor(() => expect(lastStart()?.videoId).toBe("A")); + key("f"); + key("c"); + key("Escape"); + const stage = document.querySelector(".player-stage--max")!; + await waitFor(() => + expect(document.activeElement?.getAttribute("aria-label")).toBe("clip.open"), + ); + // The card's own scissors lies under the maximised stage: a Space there reopened the panel. + expect(stage.contains(document.activeElement)).toBe(true); + }); + + it("keeps what was typed in the panel when the view is maximised under it", async () => { + open(); + await waitFor(() => expect(lastStart()?.videoId).toBe("A")); + key("c"); + fireEvent.change(screen.getByRole("textbox", { name: "clip.name" }), { + target: { value: "The bit" }, + }); + const loop = screen.getByRole("button", { name: "clip.loop" }); + fireEvent.click(loop); + act(() => loop.focus()); + const panel = screen.getByTestId("clip-panel"); + key("f"); + expect(document.querySelector(".player-stage--max")!.contains(panel)).toBe(true); + expect(screen.getByTestId("clip-panel")).toBe(panel); + expect((screen.getByRole("textbox", { name: "clip.name" }) as HTMLInputElement).value).toBe( + "The bit", + ); + expect(loop.getAttribute("aria-pressed")).toBe("true"); + expect(document.activeElement).toBe(loop); + // And back: the dock the panel sat in goes, the panel and its focus stay. + key("f"); + expect(document.querySelector(".player-stage--max")).toBeNull(); + expect(screen.getByTestId("clip-panel")).toBe(panel); + expect(document.activeElement).toBe(loop); + }); + + it("does not step on to the next video when one ends while a clip is marked", async () => { + vi.spyOn(api, "videoDetail").mockReturnValue(new Promise(() => {})); + const A = vid("A", 0); + const B = vid("B", 0); + renderModal( + {}} onState={() => {}} />, + ); + await waitFor(() => expect(lastStart()?.videoId).toBe("A")); + // Auto-advance on: an ending video steps on to B. + fireEvent.click(screen.getAllByRole("button", { name: /^player\.autoAdvance\.label/ })[0]!); + key("c"); + act(() => playerEvents.onStateChange?.({ data: 0 })); + await settle(); + expect(lastStart()?.videoId).toBe("A"); + expect(screen.getByTestId("clip-panel")).toBeTruthy(); + }); }); diff --git a/frontend/src/components/PlayerModal.tsx b/frontend/src/components/PlayerModal.tsx index e5eb765f..b45062a6 100644 --- a/frontend/src/components/PlayerModal.tsx +++ b/frontend/src/components/PlayerModal.tsx @@ -52,7 +52,7 @@ import { import { trapTab, useDialogChrome } from "./ui/dialogFocus"; import DownloadButton from "./DownloadButton"; import ClipPanel, { ClipCropFrame, type ClipCrop } from "./ClipPanel"; -import { DEFAULT_CLIP_S, markIn, markOut, openAt, type ClipRange } from "../lib/clipRange"; +import { markInAt, markOutAt, openAt, type ClipRange } from "../lib/clipRange"; import { api, type Video, type VideoStatus } from "../lib/api"; import type { YTPlayer } from "../lib/youtube"; import { @@ -371,6 +371,41 @@ export default function PlayerModal({ const clipDockRef = useRef(null); const [clipDockH, setClipDockH] = useState(0); const clipDocked = clipOpen && (maximized || isFullscreen); + // Its two places are two parents (the dock is IN the stage, the card's slot under it), and a panel + // rendered in the other one is a new panel: the typed name, Loop and the focus were lost on every + // F. So it renders once, into a host node of its own, and the host moves between the two. + const [clipHost] = useState(() => document.createElement("div")); + const clipSlotRef = useRef(null); + // The control focused in the panel. A move is a remove and an insert, and the focus goes with + // neither — leaving the big view, React has already removed the dock (and the host in it) before + // the move below runs, so the focus is on by then and only this still knows where it was. + const clipFocusRef = useRef(null); + useEffect(() => { + const onIn = (e: FocusEvent) => { + clipFocusRef.current = e.target instanceof HTMLElement ? e.target : null; + }; + const onOut = (e: FocusEvent) => { + // Left for somewhere else (not a removal, which fires nothing). + if (e.relatedTarget instanceof Node && !clipHost.contains(e.relatedTarget)) + clipFocusRef.current = null; + }; + clipHost.addEventListener("focusin", onIn); + clipHost.addEventListener("focusout", onOut); + return () => { + clipHost.removeEventListener("focusin", onIn); + clipHost.removeEventListener("focusout", onOut); + }; + }, [clipHost]); + useLayoutEffect(() => { + const slot = clipDocked ? clipDockRef.current : clipSlotRef.current; + if (!clipOpen || !slot || clipHost.parentElement === slot) return; + const active = document.activeElement; + const lost = active == null || active === document.body; + const focused = clipHost.contains(active) ? active : lost ? clipFocusRef.current : null; + slot.appendChild(clipHost); + if (focused instanceof HTMLElement && clipHost.contains(focused)) + focused.focus({ preventScroll: true }); + }, [clipOpen, clipDocked, clipHost]); useLayoutEffect(() => { const el = clipDockRef.current; if (!clipDocked || !el) return; @@ -396,9 +431,14 @@ export default function PlayerModal({ const iv = window.setInterval(read, 200); return () => window.clearInterval(iv); }, [clipOpen]); - // The scissors that opened the panel gets the focus back when it closes. + // The scissors gets the focus back when the panel closes: the one the user can see (the docked + // bar's in the big views — the card's lies under the maximised stage, and a Space on it reopened the + // panel), else the player itself (the card's is not there in a shut wide panel or for a video opened + // from a link). Also when the panel went with its video (a queue step), not only when closed. const clipButtonRef = useRef(null); + const clipDockButtonRef = useRef(null); const refocusClip = useRef(false); + const clipOpenRef = useLatest(clipOpen); const closeClip = () => { refocusClip.current = true; setClip(null); @@ -408,10 +448,18 @@ export default function PlayerModal({ setShutForClip(false); } }; - useEffect(() => { - if (clipOpen || !refocusClip.current) return; + // Read at the moment the panel closes: the view then decides, a later F is not a close. + const refocusAfterClip = useEffectEvent(() => { + const lost = document.activeElement == null || document.activeElement === document.body; + if (!refocusClip.current && !lost) return; refocusClip.current = false; - clipButtonRef.current?.focus(); + const big = maximized || isFullscreen; + const target = big ? clipDockButtonRef.current : clipButtonRef.current; + if (target) target.focus(); + else focusModal(); + }); + useEffect(() => { + if (!clipOpen) refocusAfterClip(); }, [clipOpen]); /** Mark where the player is now (`mark` gets the range so far, or null when the panel is shut). The * panel lives under the video in the card, and in place of the docked bar when maximised; the @@ -588,6 +636,8 @@ export default function PlayerModal({ // advance in the chosen direction, wrapping at the ends only when loop is "all" (a single-item // list repeats). Uses the live queue + settings via refs so it's correct from the bound handler. const advanceOnEnd = () => { + // Marking a clip, the end is a place, not a finish: stepping on would drop the clip. + if (clipOpenRef.current) return; const { autoMode: a, loopMode: l } = modeRef.current; if (l === "one") return replayCurrent(); const { queue: q, index: i } = navRef.current; @@ -698,9 +748,8 @@ export default function PlayerModal({ if (clip) closeClip(); else openClip(); }, - markIn: () => markClip((r, t, d) => markIn(r ?? openAt(t, d), t, d)), - markOut: () => - markClip((r, t, d) => markOut(r ?? openAt(Math.max(0, t - DEFAULT_CLIP_S), d), t, d)), + markIn: () => markClip(markInAt), + markOut: () => markClip(markOutAt), close: (ev) => { // Defer to any layer opened above the player FIRST (a Modal like the download dialog, or an // anchored popover): it takes the Escape and we do nothing — so we don't also step out of @@ -955,9 +1004,7 @@ export default function PlayerModal({ add-to-playlist's popover and download's Modal portal INTO the fullscreen element. */} {isFullscreen && } {clipDocked && ( -
- {clipPanel} -
+
)} {(maximized || isFullscreen) && !clipDocked && (
+ )} @@ -1086,7 +1138,8 @@ export default function PlayerModal({ )}
- {!clipDocked && clipPanel} +
+ {clipPanel && createPortal(clipPanel, clipHost)}
{/* Wide: the side panel. Its content scrolls inside a box the video's height, so a long description never makes the card taller than the video. */} diff --git a/frontend/src/components/PlexPlayer.controls.dom.test.tsx b/frontend/src/components/PlexPlayer.controls.dom.test.tsx index 81e91931..68312d4f 100644 --- a/frontend/src/components/PlexPlayer.controls.dom.test.tsx +++ b/frontend/src/components/PlexPlayer.controls.dom.test.tsx @@ -24,6 +24,7 @@ vi.mock("react-i18next", async (importOriginal) => { import PlexPlayer from "./PlexPlayer"; import { api, type PlexItemDetail } from "../lib/api"; import { ACTIVE_ACCOUNT_KEY } from "../lib/activeAccount"; +import { pushLayer, removeLayer } from "../lib/layerStack"; type Media = HTMLMediaElement & { _t?: number; _paused?: boolean }; @@ -128,6 +129,7 @@ function mount(opts: { queue?: string[]; first?: string; fromStart?: boolean; + onClose?: () => void; }) { vi.spyOn(api, "plexItem").mockImplementation((id: string) => Promise.resolve(opts.details[id]!)); const session = vi @@ -139,7 +141,12 @@ function mount(opts: { const first = opts.first ?? opts.queue?.[0] ?? Object.keys(opts.details)[0]!; render( - {}} queue={opts.queue} fromStart={opts.fromStart} /> + {})} + queue={opts.queue} + fromStart={opts.fromStart} + /> , ); return { session }; @@ -1023,4 +1030,71 @@ describe("PlexPlayer — clip mode", () => { else delete (proto as unknown as Record)[k]; } }); + + const prefs = (p: object) => + localStorage.setItem("siftlode.plexPlayerPrefs.7", JSON.stringify({ subLang: "en", ...p })); + const infoOverlay = () => document.querySelector("[class*='bg-black/70']"); + + it("Escape closes what is over the picture first, then the crop and the panel", async () => { + const full = { ...item("a"), cast: [], genres: [], directors: [], collections: [] }; + mount({ details: { a: full as PlexItemDetail } }); + await playingAt("/media/a", 120); + key("c"); + key("i"); + expect(infoOverlay()).toBeTruthy(); + key("Escape"); + expect(infoOverlay()).toBeNull(); + expect(screen.getByTestId("clip-panel")).toBeTruthy(); + key("Escape"); + expect(screen.queryByTestId("clip-panel")).toBeNull(); + }); + + it("⌫ under a dialog over the player is the dialog's, not the clip's", async () => { + mount({ details: { a: item("a") } }); + await playingAt("/media/a", 120); + key("c"); + const layer = pushLayer(); + try { + key("Backspace"); + expect(screen.getByTestId("clip-panel")).toBeTruthy(); + } finally { + removeLayer(layer); + } + key("Backspace"); + expect(screen.queryByTestId("clip-panel")).toBeNull(); + }); + + it("opens no range while the item's length is unknown", async () => { + mount({ + details: { a: { ...item("a"), duration_seconds: null } as unknown as PlexItemDetail }, + }); + await playingAt("/media/a", 120); + for (const k of ["c", "[", "]"]) key(k); + expect(screen.queryByTestId("clip-panel")).toBeNull(); + }); + + it("an item played to its end while a clip is marked stays, with the clip", async () => { + const onClose = vi.fn(); + mount({ details: { a: item("a") }, onClose }); + await playingAt("/media/a", 2990); + key("c"); + act(() => void video().dispatchEvent(new Event("ended"))); + expect(onClose).not.toHaveBeenCalled(); + expect(screen.getByTestId("clip-panel")).toBeTruthy(); + }); + + it("does not auto-skip while a clip is marked: the skip would carry the player off it", async () => { + prefs({ autoSkipCredits: true, autoSkipDelay: 0 }); + const withCredits = { + ...item("a", 0, "b"), + markers: [{ type: "credits", start_s: 2900, end_s: 2995, final: true }], + } as unknown as PlexItemDetail; + const { session } = mount({ details: { a: withCredits, b: item("b") }, queue: ["a", "b"] }); + await playingAt("/media/a", 2800); + key("c"); + await playingAt("/media/a", 2910); + await act(() => new Promise((r) => setTimeout(r, 50))); + expect(lastStart(session)?.[0]).toBe("a"); + expect(screen.getByTestId("clip-panel")).toBeTruthy(); + }); }); diff --git a/frontend/src/components/PlexPlayer.tsx b/frontend/src/components/PlexPlayer.tsx index 728faacb..3e70e41c 100644 --- a/frontend/src/components/PlexPlayer.tsx +++ b/frontend/src/components/PlexPlayer.tsx @@ -50,7 +50,7 @@ import { } from "../lib/plexPlayerPrefs"; import { Ctrl, PanelHead } from "./plexPlayerControls"; import ClipPanel, { ClipCropFrame, type ClipCrop } from "./ClipPanel"; -import { DEFAULT_CLIP_S, markIn, markOut, openAt, type ClipRange } from "../lib/clipRange"; +import { markInAt, markOutAt, openAt, type ClipRange } from "../lib/clipRange"; import { PlexPlayerSettings } from "./PlexPlayerSettings"; import { PlexStatsOverlay } from "./PlexStatsOverlay"; import { createFragMeter } from "../lib/plexStats"; @@ -833,6 +833,9 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false } seekTo, videoRef, reportWatchedError, + // Marking a clip, the viewer is where they want to be: a skip would carry the player off the + // range (an intro clip skipped past its Out, the credits' leaving for the next item). + suspended: clip != null, }); // Download the ORIGINAL physical file (no re-encode/repackage). One user gesture, direct link. @@ -902,6 +905,8 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false } // Only a genuine play-to-the-end counts. A next episode that failed to load never played // (absRef 0) — its stray 'ended' must not mark anything watched or cascade another advance. if (absRef.current <= 0) return; + // Marking a clip, the end is a place, not a finish: closing or moving on would drop the clip. + if (clipRef.current) return; const watchedWrite = api.plexSetState(id, "watched"); watchedWrite.catch(reportWatchedError); // Whatever follows (the up-next card, the next item, closing) leaves a FINISHED item — unless @@ -918,7 +923,7 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false } }; video.addEventListener("ended", onEnded); return () => video.removeEventListener("ended", onEnded); - }, [id, nextId, go, onClose, finishProgress, reportWatchedError, absRef, upNextPrefRef]); + }, [id, nextId, go, onClose, finishProgress, reportWatchedError, absRef, upNextPrefRef, clipRef]); // Up-next card actions: advance = go to the next item (it was already marked watched on 'ended'); // cancel = dismiss and stay on the finished frame. Loading a new item closes any lingering card. @@ -937,21 +942,25 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false } // The ONE Back cascade for the player's own ways of saying "back": the on-screen arrow, ⌫ and // Escape. It peels off what is layered over the video — the auto-skip countdown, then the open - // panel, then the stats readout — and only then leaves the player. The mouse/browser Back peels - // the layers the viewer OPENED one per press, the one opened last first (each holds its own history - // entry, see BackClose below); the ones that open by themselves have none, so Back leaves past - // them. The shortcuts sheet is not in it: a Modal above us, with its own Escape layer and - // Back-to-close. Leaving closes whatever is still up first (`useHistorySubview`'s back). + // panel, then the stats readout, then the clip being marked (its crop first) — and only then leaves + // the player. The mouse/browser Back peels the layers the viewer OPENED one per press, the one + // opened last first (each holds its own history entry, see BackClose below); the ones that open by + // themselves have none, so Back leaves past them. The shortcuts sheet is not in it: a Modal above + // us, with its own Escape layer and Back-to-close. Leaving closes whatever is still up first + // (`useHistorySubview`'s back). // - The on-screen arrow is a click on "leave", not a cancel, so it does not stop a countdown. // - Escape in fullscreen leaves fullscreen (the browser's own Escape) and not the player. + // - The arrow leaves past a clip as it does past a panel; the keys step out of it. const back = useCallback( (from: "arrow" | "key" | "escape") => { if (from !== "arrow" && skipProgressRef.current != null) cancelAutoSkipRef.current(); else if (panelRef.current !== "none") setPanel("none"); else if (statsOpenRef.current) setStatsOpen(false); - else if (from !== "escape" || !fullscreenElement()) onClose(); + else if (from === "escape" && fullscreenElement()) return; + else if (from !== "arrow" && clipRef.current) stepOutOfClip(); + else onClose(); }, - [onClose, panelRef, statsOpenRef, skipProgressRef, cancelAutoSkipRef], + [onClose, panelRef, statsOpenRef, skipProgressRef, cancelAutoSkipRef, clipRef, stepOutOfClip], ); // The mouse/browser Back closes the layer whose history entry it popped (see BackClose below). const closeStats = useCallback(() => setStatsOpen(false), []); @@ -1072,19 +1081,20 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false } }, info: () => togglePanel("info"), stats: () => setStatsOpen((v) => !v), + // A range needs the item's length (unknown while its detail loads, or never given). clip: () => { if (clipRef.current) closeClip(); - else setClip(openAt(absRef.current, durationRef.current)); + else if (durationRef.current > 0) setClip(openAt(absRef.current, durationRef.current)); }, markIn: () => { const t = absRef.current; const d = durationRef.current; - setClip((c) => markIn(c ?? openAt(t, d), t, d)); + if (d > 0) setClip((c) => markInAt(c, t, d)); }, markOut: () => { const t = absRef.current; const d = durationRef.current; - setClip((c) => markOut(c ?? openAt(Math.max(0, t - DEFAULT_CLIP_S), d), t, d)); + if (d > 0) setClip((c) => markOutAt(c, t, d)); }, back: (ev) => { // HTPC-style "stop & go back" to wherever the player was opened from — mirrors the mouse @@ -1097,15 +1107,14 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false } // ⌫ stopped closing them), then naming just the sheet and palette (the second fix, the third // bug — the app-wide ErrorDialog is neither, so ⌫ tore the player down under it). // The rule that holds: the player's own panels answer first, because `back`'s cascade is - // what closes them; anything else layered above us takes the key. - const ownPanelOpen = - panelRef.current !== "none" || statsOpenRef.current || clipRef.current != null; + // what closes them; anything else layered above us takes the key. The clip panel pushes no + // layer, so with it open a layer above is never the player's own (a refused Cut opens the + // ErrorDialog right over it): the clip is in the cascade, behind that layer. + const ownPanelOpen = panelRef.current !== "none" || statsOpenRef.current; if (!ownPanelOpen && layerIdRef.current != null && anyLayerAbove(layerIdRef.current)) return; ev.preventDefault(); - // The clip panel is the player's own too, and closes before the player does. - if (clipRef.current) stepOutOfClip(); - else back("key"); + back("key"); }, escape: (ev) => { // Defer to any layer opened above the player (Modal or anchored popover) — let it take the @@ -1115,8 +1124,7 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false } // one and its close can pop it off the stack synchronously, so `anyLayerAbove` alone misses it. if (escapeHandled(ev)) return; if (layerIdRef.current != null && anyLayerAbove(layerIdRef.current)) return; - if (clipRef.current) stepOutOfClip(); - else back("escape"); + back("escape"); }, }); }; diff --git a/frontend/src/components/VideoEditor.partial.dom.test.tsx b/frontend/src/components/VideoEditor.partial.dom.test.tsx index 16eab8cb..f5c7abb2 100644 --- a/frontend/src/components/VideoEditor.partial.dom.test.tsx +++ b/frontend/src/components/VideoEditor.partial.dom.test.tsx @@ -6,7 +6,7 @@ // now says how many made it (also when `req` has already shown the refusal in its dialog), deselects // them, holds the cut list still while it queues, and a retry goes on with the same "(i/N)" series. import { describe, it, expect, vi, beforeAll, afterEach } from "vitest"; -import { render, screen, fireEvent, waitFor, cleanup, act } from "@testing-library/react"; +import { render, screen, fireEvent, waitFor, cleanup, act, within } from "@testing-library/react"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; vi.mock("react-i18next", async (importOriginal) => ({ @@ -124,3 +124,39 @@ describe("VideoEditor — a cut on the keyboard", () => { expect(cut.getAttribute("aria-valuenow")).toBe("99.9"); }); }); + +// A cut's remove button and the keep toggle of the segment after it: the button is a button a click +// or a key works (it sat inside the cut's slider, pointer-only, its arrows moving the cut), and it +// sits at the cut's foot, clear of the toggle at the segment's head; the toggle sits over the cut's +// hit area that reaches into its segment, so a press on it is the toggle's. +describe("VideoEditor — a cut's remove button", () => { + const split = () => { + vi.spyOn(api, "downloadStoryboard").mockResolvedValue({ available: false } as never); + render( + + {}} /> + , + ); + const video = document.querySelector("video")!; + Object.defineProperty(video, "currentTime", { value: 50, configurable: true, writable: true }); + fireEvent.timeUpdate(video); + fireEvent.click(screen.getByText("editor.splitHere")); + }; + + it("removes the cut on a click (Enter on a focused button is one), and is not part of the slider", () => { + split(); + const remove = screen.getByRole("button", { name: "editor.removeCut" }); + expect(screen.getByRole("slider", { name: "editor.cutN" }).contains(remove)).toBe(false); + fireEvent.click(remove); + expect(screen.queryByRole("slider", { name: "editor.cutN" })).toBeNull(); + }); + + it("sits at the cut's foot, under the next segment's keep toggle at its head", () => { + split(); + const remove = screen.getByRole("button", { name: "editor.removeCut" }); + expect(remove.className).toMatch(/\bbottom-0\b/); + const track = screen.getByRole("group", { name: "editor.timeline" }); + const toggles = within(track).getAllByRole("button", { pressed: true }); + for (const toggle of toggles) expect(toggle.className).toMatch(/\bz-menu\b/); + }); +}); diff --git a/frontend/src/components/VideoEditor.tsx b/frontend/src/components/VideoEditor.tsx index 32fb3c93..21ec2f07 100644 --- a/frontend/src/components/VideoEditor.tsx +++ b/frontend/src/components/VideoEditor.tsx @@ -15,9 +15,10 @@ import { markReported } from "../lib/api/core"; import { IconButton } from "./ui/IconButton"; import { Pressable } from "./ui/Pressable"; import { Input } from "./ui/Input"; +import { TimecodeField } from "./ui/TimecodeField"; import { SURFACE } from "./ui/surfaces"; import { RangeTrack } from "./ui/RangeTrack"; -import { formatTimecode, parseTimecode } from "../lib/timecode"; +import { formatTimecode } from "../lib/timecode"; type Seg = { id: number; start: number; end: number; keep: boolean }; @@ -385,7 +386,8 @@ export default function VideoEditor({ job, onClose }: { job: DownloadJob; onClos title: s.keep ? t("editor.keptSeg") : t("editor.droppedSeg"), onPointerDown: () => setSelId(s.id), children: ( - // keep/drop toggle + // keep/drop toggle, over the cut's hit area beside it (that reaches 12 px into the + // segment): a press on the eye is the eye's, never a drag of the cut { @@ -393,7 +395,7 @@ export default function VideoEditor({ job, onClose }: { job: DownloadJob; onClos toggleKeep(s.id); }} className={clsx( - "absolute top-1 left-1 p-0.5 rounded-sm bg-black/50 backdrop-blur-xs", + "absolute top-1 left-1 z-menu p-0.5 rounded-sm bg-black/50 backdrop-blur-xs", s.keep ? "text-accent" : "text-muted", )} title={s.keep ? t("editor.drop") : t("editor.keep")} @@ -413,13 +415,14 @@ export default function VideoEditor({ job, onClose }: { job: DownloadJob; onClos step: 0.1, bigStep: 1, onChange: (time: number) => moveBoundary(bi, time), + // At the cut's foot, clear of the next segment's eye at its head. A real button: a + // click or Enter removes the cut; its press is not a seek of the track under it. adornment: ( { - e.stopPropagation(); - deleteBoundary(bi); - }} - className="absolute top-0 -right-0.5 p-0.5 rounded-full bg-black/70 text-muted hover:text-danger" + onPointerDown={(e) => e.stopPropagation()} + onClick={() => deleteBoundary(bi)} + className="pointer-events-auto absolute bottom-0 -right-0.5 p-0.5 rounded-full bg-black/70 text-muted hover:text-danger" + aria-label={t("editor.removeCut")} title={t("editor.removeCut")} > @@ -465,30 +468,24 @@ export default function VideoEditor({ job, onClose }: { job: DownloadJob; onClos
diff --git a/frontend/src/components/ui/CropFrame.dom.test.tsx b/frontend/src/components/ui/CropFrame.dom.test.tsx index 0c26cf32..c7d24d8f 100644 --- a/frontend/src/components/ui/CropFrame.dom.test.tsx +++ b/frontend/src/components/ui/CropFrame.dom.test.tsx @@ -107,6 +107,19 @@ describe("CropFrame", () => { expect(rect()).toBe("0.300 0.300 0.400 0.400"); }); + it("follows the pointer that took it, not a second finger landing on it", () => { + render(); + const frame = screen.getByRole("group", { name: "common.cropFrame.frame" }); + fireEvent.pointerDown(frame, { button: 0, clientX: 500, clientY: 300, pointerId: 1 }); + // A second finger lands 200 px away and stays: it must not become the drag's origin. + fireEvent.pointerDown(frame, { button: 0, clientX: 700, clientY: 300, pointerId: 2 }); + fireEvent.pointerMove(frame, { clientX: 660, clientY: 390, pointerId: 1 }); + expect(rect()).toBe("0.300 0.300 0.400 0.400"); + fireEvent.pointerMove(frame, { clientX: 900, clientY: 300, pointerId: 2 }); + expect(rect()).toBe("0.300 0.300 0.400 0.400"); + fireEvent.pointerUp(frame, { clientX: 660, clientY: 390, pointerId: 1 }); + }); + it("takes the focus on a press, so the arrows go on from where the pointer left it", () => { render(); const frame = screen.getByRole("group", { name: "common.cropFrame.frame" }); @@ -116,6 +129,7 @@ describe("CropFrame", () => { const se = screen.getByRole("group", { name: "common.cropFrame.se" }); fireEvent.pointerDown(se, { button: 0, clientX: 960, clientY: 540 }); expect(document.activeElement).toBe(se); + fireEvent.pointerUp(se, { clientX: 960, clientY: 540 }); // An edge is the pointer's only: its press puts the focus on the frame it belongs to. fireEvent.pointerDown(document.querySelector(".cursor-ew-resize")!, { button: 0, diff --git a/frontend/src/components/ui/CropFrame.tsx b/frontend/src/components/ui/CropFrame.tsx index 0e64182e..d96e867d 100644 --- a/frontend/src/components/ui/CropFrame.tsx +++ b/frontend/src/components/ui/CropFrame.tsx @@ -75,7 +75,9 @@ const SMALL_HANDLE_BOX: Record = { // Below this many px on its shorter side a frame takes the small handles. const SMALL_FRAME_PX = 64; -type Drag = { handle: CropHandle | "move"; x: number; y: number; from: Frac }; +// A drag is its pointer's: a second finger on the frame neither restarts it from where it landed nor +// moves it (two origins at once made the frame jump between them). +type Drag = { handle: CropHandle | "move"; pointer: number; x: number; y: number; from: Frac }; // Not the app's accent ring: over a video it can be any colour, and a light theme's dark-blue accent // all but vanished on a dark picture. Amber, like the players' playhead, reads on both. const FOCUS = "focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-warn"; @@ -106,6 +108,7 @@ export function CropFrame({ aspect, rect, ratio, onChange, className }: Props) { // The press is the frame's: not the video's play / pause, not a scrub of anything under it. e.stopPropagation(); e.preventDefault(); + if (drag) return; // preventDefault also stops the press from focusing: focused by hand, so the arrows go on from // here (an edge is the pointer's only, so its frame takes the focus). e.currentTarget.closest("[tabindex]")?.focus({ preventScroll: true }); @@ -114,10 +117,10 @@ export function CropFrame({ aspect, rect, ratio, onChange, className }: Props) { } catch { // No capture (a synthetic event): the drag still follows while the pointer stays on it. } - setDrag({ handle, x: e.clientX, y: e.clientY, from: rect }); + setDrag({ handle, pointer: e.pointerId, x: e.clientX, y: e.clientY, from: rect }); }; const follow = (e: PointerEvent) => { - if (!drag || !pic) return; + if (!drag || drag.pointer !== e.pointerId || !pic) return; const dx = (e.clientX - drag.x) / pic.width; const dy = (e.clientY - drag.y) / pic.height; onChange( @@ -126,7 +129,9 @@ export function CropFrame({ aspect, rect, ratio, onChange, className }: Props) { : resizeCrop(drag.from, drag.handle, dx, dy, ratio), ); }; - const end = () => setDrag(null); + const end = (e: PointerEvent) => { + if (drag?.pointer === e.pointerId) setDrag(null); + }; const pointer = (handle: Drag["handle"]) => ({ onPointerDown: begin(handle), onPointerMove: follow, diff --git a/frontend/src/components/ui/README.md b/frontend/src/components/ui/README.md index 084d6491..a1024716 100644 --- a/frontend/src/components/ui/README.md +++ b/frontend/src/components/ui/README.md @@ -38,7 +38,7 @@ Counter-examples, deliberately NOT here: `PosterTile` (knows about Plex montage | area | components | | ---------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | overlays & anchoring | `AnchoredPopover` + `anchoredPopoverHooks` (the popover engine: arrows, typeahead, Escape, anchor-flip), `ActionMenu`, `MembershipMenu` (the body of a "which sets is it in" menu: checkbox rows + a field naming a new set), `ContextMenu` + `useContextMenu` (the SAME menu, opened by right-click / long-press / Shift+F10 / a row's `MoreButton` "…" via `buttonProps`; `anchor` for a panel a row opens; `PageMenu` for a page's background menu), `Modal` (+ `ModalFooter`; `initialFocus` by purpose, C4), `Overlay`, `SidePanel`, `Tooltip`, `Toaster` (+ `toastLevels`, `overlayOrder`; `lib/toastDock` docks one control under the stack, in every host), `CommandPalette`, `KeyboardHelp` | -| actions & inputs | `Button`, `IconButton`, `Pressable` (`selectable` when its text is a title or a name), `Chip` + `chipStyles`, `DragHandle` + `OrderButtons` / `useOrderItems` (the non-drag reorder, WCAG 2.5.7) (one per button family, docs/DESIGN.md; a raw ` }); + render(); + const button = screen.getByRole("button", { name: "Remove" }); + expect(screen.getByRole("slider", { name: "In" }).contains(button)).toBe(false); + // The slider's arrows are not the button's. + fireEvent.keyDown(button, { key: "ArrowLeft" }); + expect(h.onChange).not.toHaveBeenCalled(); + }); + + it("rings a focused handle in the caller's colour, for a track on the dark media chrome", () => { + render( + , + ); + expect(screen.getByRole("slider", { name: "In" }).className).toContain( + "focus-visible:outline-warn", + ); + }); + it("seeks on a press of the bare track, following the drag, final on release", () => { const onSeek = vi.fn(); render(); diff --git a/frontend/src/components/ui/RangeTrack.tsx b/frontend/src/components/ui/RangeTrack.tsx index 404e3e19..72d10fb1 100644 --- a/frontend/src/components/ui/RangeTrack.tsx +++ b/frontend/src/components/ui/RangeTrack.tsx @@ -1,4 +1,5 @@ import { + useRef, useState, type CSSProperties, type KeyboardEvent, @@ -18,8 +19,11 @@ import clsx from "clsx"; // Its drawn bar is thin and its hit area 1.5rem wide, at least twice the bar — the long-timeline // editors that drew the hit area as thin as the bar were the ones people missed. // -// Every move reports `final`: false while a pointer drags, true on release or a key. A player whose -// seek is expensive (a transcode restarts) acts on the final one only; a cheap one follows the drag. +// Every move reports `final`: false while a pointer drags, true on release or a key — and when the +// browser takes the pointer away mid-drag (a cancel: a system gesture on a tablet), true where it +// stopped. A player whose seek is expensive (a transcode restarts) acts on the final one only; a cheap +// one follows the drag. A drag is its pointer's: a second finger neither takes it over nor starts +// another, so two fingers on the track never fight over one mark. export interface RangeWindow { from: number; @@ -42,7 +46,9 @@ export interface TrackHandle { onChange: (time: number, final: boolean) => void; /** The drawn bar's look (its colour); the hit area around it is the track's. */ barClassName?: string; - /** Drawn on the handle, e.g. a remove button; it should stop its own pointer events. */ + /** Drawn at the handle, e.g. a remove button; it should stop its own pointer events. Beside the + * slider, not in it: a slider's content is not a control to assistive tech, and its keys are the + * slider's. */ adornment?: ReactNode; } @@ -74,6 +80,9 @@ interface Props { /** Accessible name of the whole track. */ label: string; playheadClassName?: string; + /** A focused handle's ring colour (a `focus-visible:outline-*` class) where the theme accent does + * not read on the track, e.g. on the dark media chrome. */ + handleRingClassName?: string; } const clamp = (v: number, lo: number, hi: number) => Math.max(lo, Math.min(hi, v)); @@ -98,6 +107,7 @@ export function RangeTrack({ className, label, playheadClassName = "bg-white", + handleRingClassName, }: Props) { const win: RangeWindow = view ?? { from: 0, to: duration }; const span = Math.max(win.to - win.from, 1e-6); @@ -105,8 +115,11 @@ export function RangeTrack({ const pct = (f: number) => `${(clamp(f, 0, 1) * 100).toFixed(3)}%`; // The handle a pointer is dragging, and the press on the bare track (a scrub), so a move is only // acted on between its own down and up. - const [dragging, setDragging] = useState(null); - const [scrubbing, setScrubbing] = useState(false); + const [dragging, setDragging] = useState<{ id: string; pointer: number } | null>(null); + const [scrubbing, setScrubbing] = useState(null); + // Where the drag last put its handle, for a cancel to report as final. + const lastRef = useRef(0); + const busy = dragging != null || scrubbing != null; const capture = (e: PointerEvent) => { try { @@ -123,22 +136,25 @@ export function RangeTrack({ aria-label={label} className={clsx("relative touch-none select-none overflow-hidden", className)} onPointerDown={(e) => { - if (!onSeek || e.button !== 0) return; + if (!onSeek || e.button !== 0 || busy) return; capture(e); - setScrubbing(true); + setScrubbing(e.pointerId); onSeek(timeAtPointer(e.currentTarget, e.clientX, win), false); }} onPointerMove={(e) => { const time = timeAtPointer(e.currentTarget, e.clientX, win); onHover?.({ x: e.clientX - e.currentTarget.getBoundingClientRect().left, time }); - if (scrubbing) onSeek?.(time, false); + if (scrubbing === e.pointerId) onSeek?.(time, false); }} onPointerUp={(e) => { - if (!scrubbing) return; - setScrubbing(false); + if (scrubbing !== e.pointerId) return; + setScrubbing(null); onSeek?.(timeAtPointer(e.currentTarget, e.clientX, win), true); }} - onPointerCancel={() => setScrubbing(false)} + // A cancelled scrub seeks no further: where it was taken away is not a place anyone chose. + onPointerCancel={(e) => { + if (scrubbing === e.pointerId) setScrubbing(null); + }} onPointerLeave={() => onHover?.(null)} > {background} @@ -171,7 +187,12 @@ export function RangeTrack({ {handles.map((h) => { const f = frac(h.time); if (f < 0 || f > 1) return null; - const move = (time: number, final: boolean) => h.onChange(clamp(time, h.min, h.max), final); + const move = (time: number, final: boolean) => { + const t = clamp(time, h.min, h.max); + lastRef.current = t; + h.onChange(t, final); + }; + const mine = (e: PointerEvent) => dragging?.id === h.id && dragging.pointer === e.pointerId; const onKeyDown = (e: KeyboardEvent) => { const big = e.shiftKey ? h.bigStep : h.step; const to: Record = { @@ -192,7 +213,7 @@ export function RangeTrack({ e.stopPropagation(); move(next, true); }; - return ( + return [
{ if (e.button !== 0) return; e.stopPropagation(); + if (busy) return; capture(e); - setDragging(h.id); + lastRef.current = h.time; + setDragging({ id: h.id, pointer: e.pointerId }); }} onPointerMove={(e) => { - if (dragging === h.id) move(timeAtPointer(e.currentTarget, e.clientX, win), false); + if (mine(e)) move(timeAtPointer(e.currentTarget, e.clientX, win), false); }} onPointerUp={(e) => { - if (dragging !== h.id) return; + if (!mine(e)) return; e.stopPropagation(); setDragging(null); move(timeAtPointer(e.currentTarget, e.clientX, win), true); }} - onPointerCancel={() => setDragging(null)} + onPointerCancel={(e) => { + if (!mine(e)) return; + setDragging(null); + move(lastRef.current, true); + }} >
- {h.adornment} -
- ); +
, + h.adornment != null && ( +
+ {h.adornment} +
+ ), + ]; })}
); diff --git a/frontend/src/components/ui/TimecodeField.tsx b/frontend/src/components/ui/TimecodeField.tsx new file mode 100644 index 00000000..0b485b51 --- /dev/null +++ b/frontend/src/components/ui/TimecodeField.tsx @@ -0,0 +1,58 @@ +import { useState, type ComponentProps } from "react"; +import { formatTimecode, parseTimecode } from "../../lib/timecode"; +import { Input } from "./Input"; + +// A field a cut's time is typed into (lib/timecode): the editor's segment ends, the clip panel's In / +// Out. A typed draft is the field's until Enter or leaving the field takes it; then the field shows +// the value as it landed — the caller may clamp it (a time past the end) — and a time it cannot read +// stays, marked invalid, for fixing rather than vanishing or seeming taken. One element throughout, +// so Enter leaves the focus in the field (a field re-created per value dropped it on ). + +type Props = Omit< + ComponentProps, + "value" | "defaultValue" | "onChange" | "invalid" +> & { + /** Seconds. */ + value: number; + /** Show hours (pass `duration >= 3600`, so one item's fields agree). */ + hours?: boolean; + /** A readable time was typed (not called when it equals `value`). */ + onCommit: (seconds: number) => void; +}; + +export function TimecodeField({ + value, + hours = false, + onCommit, + onBlur, + onKeyDown, + ...rest +}: Props) { + const [draft, setDraft] = useState(null); + const invalid = draft != null && parseTimecode(draft) == null; + const commit = () => { + if (draft == null) return; + const v = parseTimecode(draft); + if (v == null) return; + setDraft(null); + if (Math.abs(v - value) > 1e-3) onCommit(v); + }; + return ( + setDraft(e.target.value)} + onBlur={(e) => { + commit(); + onBlur?.(e); + }} + onKeyDown={(e) => { + onKeyDown?.(e); + if (e.key !== "Enter" || e.defaultPrevented) return; + e.preventDefault(); + commit(); + }} + /> + ); +} diff --git a/frontend/src/lib/plexPlayerHooks.ts b/frontend/src/lib/plexPlayerHooks.ts index d00096fc..dbc20408 100644 --- a/frontend/src/lib/plexPlayerHooks.ts +++ b/frontend/src/lib/plexPlayerHooks.ts @@ -435,6 +435,8 @@ export function useAutoSkip(opts: { videoRef: RefObject; /** Says a refused watched mark out loud; the translated string belongs to the component layer. */ reportWatchedError: (e: unknown) => void; + /** No countdown while true (a clip being marked); the Skip button still skips. */ + suspended?: boolean; }): { activeMarker: PlexMarker | undefined; skipProgress: number | null; @@ -453,6 +455,7 @@ export function useAutoSkip(opts: { go, seekTo, videoRef, + suspended = false, } = opts; const activeMarker: PlexMarker | undefined = markers?.find( (m) => abs >= m.start_s && abs < m.end_s - 1, @@ -513,7 +516,7 @@ export function useAutoSkip(opts: { if (!m) return setSkipProgress(null); const key = `${m.type}-${m.start_s}`; const enabled = (m.type === "intro" ? autoSkipIntro : autoSkipCredits) && activeAuto; - if (!enabled || cancelledMarkerRef.current === key) return setSkipProgress(null); + if (!enabled || suspended || cancelledMarkerRef.current === key) return setSkipProgress(null); if (autoSkipDelay <= 0) return doSkip(m); // immediate let elapsed = 0; setSkipProgress(0); @@ -541,6 +544,7 @@ export function useAutoSkip(opts: { autoSkipIntro, autoSkipCredits, autoSkipDelay, + suspended, doSkip, videoRef, activeMarkerRef, diff --git a/frontend/src/lib/timecode.test.ts b/frontend/src/lib/timecode.test.ts index 0a745c3a..1b590753 100644 --- a/frontend/src/lib/timecode.test.ts +++ b/frontend/src/lib/timecode.test.ts @@ -29,8 +29,13 @@ describe("parseTimecode", () => { expect(parseTimecode(" 90.5 ")).toBeCloseTo(90.5); }); + it("takes a decimal comma, in the seconds only", () => { + expect(parseTimecode("1:02,5")).toBeCloseTo(62.5); + expect(parseTimecode("12,4")).toBeCloseTo(12.4); + }); + it("is null for what is not a timecode", () => { - for (const v of ["", "abc", "1::2", "1:2:3:4", "-5", "1:-2"]) + for (const v of ["", "abc", "1::2", "1:2:3:4", "-5", "1:-2", "1,5:00", "1:02,5,5"]) expect(parseTimecode(v)).toBeNull(); }); }); diff --git a/frontend/src/lib/timecode.ts b/frontend/src/lib/timecode.ts index 4793f959..1d630ae7 100644 --- a/frontend/src/lib/timecode.ts +++ b/frontend/src/lib/timecode.ts @@ -13,9 +13,13 @@ export function formatTimecode(s: number, hours = false): string { return hours || h > 0 ? `${h}:${String(m).padStart(2, "0")}:${sec}` : `${m}:${sec}`; } -/** A typed timecode back to seconds: `h:mm:ss.d`, `m:ss.d` or bare seconds. Null when it is not one. */ +/** A typed timecode back to seconds: `h:mm:ss.d`, `m:ss.d` or bare seconds, its decimal a point or + * a comma (how a Hungarian hand writes one). Null when it is not one. */ export function parseTimecode(v: string): number | null { - const parts = v.trim().split(":"); + const parts = v + .trim() + .replace(/,(\d*)$/, ".$1") + .split(":"); if (parts.length > 3 || parts.some((p) => p.trim() === "")) return null; let total = 0; for (const p of parts) {