fix: code-review round 1 — 4 findings
- og: guard safe_abs_path's None (missing/gc'd poster) before _jpeg_size, else the public /watch OG render crashed with an uncaught AttributeError - channels: hoist the priority step button to module scope so it no longer remounts on every hold-repeat tick (which dropped the pointer capture and could let a hold run away on pointer drift) - watch: only route audio through Web Audio on iOS (where a backgrounded <video> is suspended); other platforms keep their reliable native audio, avoiding a foreground-silence risk if the AudioContext can't resume - share: drop the unstable `create` mutation object from the auto-create effect deps (it changes every render); the links.data trigger + autoTried ref already fire it exactly once The clipboard-on-focus finding is intended (the user explicitly asked for auto-paste on focus; the read is best-effort and browsers reject it without a gesture, so no real prompt fires).
This commit is contained in:
@@ -125,7 +125,10 @@ def _watch_tags(token: str, db: Session) -> str | None:
|
||||
tags.append(_tag("og:image:secure_url", image))
|
||||
if image.endswith("poster.jpg") and asset.poster_path:
|
||||
tags.append(_tag("og:image:type", "image/jpeg"))
|
||||
dims = _jpeg_size(storage.safe_abs_path(settings.download_root, asset.poster_path))
|
||||
# safe_abs_path returns None if the poster file is missing (gc'd) or escapes the root —
|
||||
# guard it, else _jpeg_size(None) would raise an uncaught AttributeError on this public page.
|
||||
poster_abs = storage.safe_abs_path(settings.download_root, asset.poster_path)
|
||||
dims = _jpeg_size(poster_abs) if poster_abs else None
|
||||
if dims:
|
||||
tags.append(_tag("og:image:width", str(dims[0])))
|
||||
tags.append(_tag("og:image:height", str(dims[1])))
|
||||
|
||||
@@ -819,6 +819,46 @@ function SyncBadge({ ok, label, hint }: { ok: boolean; label: string; hint?: str
|
||||
);
|
||||
}
|
||||
|
||||
// Module-level (NOT defined inside PriorityCell) so its identity is stable across PriorityCell
|
||||
// re-renders — otherwise every hold-repeat tick would remount the button and drop the pointer
|
||||
// capture set on pointerdown, letting a hold run away if the pointer drifts off the arrow.
|
||||
function PriorityStepButton({
|
||||
delta,
|
||||
Icon,
|
||||
label,
|
||||
onHoldStart,
|
||||
onStop,
|
||||
onTap,
|
||||
}: {
|
||||
delta: number;
|
||||
Icon: typeof ArrowUp;
|
||||
label: string;
|
||||
onHoldStart: (delta: number) => void;
|
||||
onStop: () => void;
|
||||
onTap: (delta: number) => void;
|
||||
}) {
|
||||
return (
|
||||
<button
|
||||
onPointerDown={(e) => {
|
||||
// Keep receiving pointer events even if the pointer drifts off the small button mid-hold.
|
||||
try {
|
||||
e.currentTarget.setPointerCapture(e.pointerId);
|
||||
} catch {
|
||||
/* invalid pointer id (e.g. synthetic events) — capture is a nicety, not required */
|
||||
}
|
||||
onHoldStart(delta);
|
||||
}}
|
||||
onPointerUp={onStop}
|
||||
onPointerCancel={onStop}
|
||||
onClick={() => onTap(delta)}
|
||||
className="text-muted hover:text-accent leading-none"
|
||||
aria-label={label}
|
||||
>
|
||||
<Icon className="w-3.5 h-3.5" />
|
||||
</button>
|
||||
);
|
||||
}
|
||||
|
||||
function PriorityCell({
|
||||
c,
|
||||
onPriority,
|
||||
@@ -858,7 +898,7 @@ function PriorityCell({
|
||||
tick();
|
||||
}, 350);
|
||||
};
|
||||
const onClick = (delta: number) => {
|
||||
const onTap = (delta: number) => {
|
||||
if (suppressClick.current) {
|
||||
suppressClick.current = false;
|
||||
return;
|
||||
@@ -866,35 +906,6 @@ function PriorityCell({
|
||||
onPriority(delta);
|
||||
};
|
||||
|
||||
const StepBtn = ({
|
||||
delta,
|
||||
Icon,
|
||||
label,
|
||||
}: {
|
||||
delta: number;
|
||||
Icon: typeof ArrowUp;
|
||||
label: string;
|
||||
}) => (
|
||||
<button
|
||||
onPointerDown={(e) => {
|
||||
// Keep receiving pointer events even if the pointer drifts off the small button mid-hold.
|
||||
try {
|
||||
e.currentTarget.setPointerCapture(e.pointerId);
|
||||
} catch {
|
||||
/* invalid pointer id (e.g. synthetic events) — capture is a nicety, not required */
|
||||
}
|
||||
start(delta);
|
||||
}}
|
||||
onPointerUp={stop}
|
||||
onPointerCancel={stop}
|
||||
onClick={() => onClick(delta)}
|
||||
className="text-muted hover:text-accent leading-none"
|
||||
aria-label={label}
|
||||
>
|
||||
<Icon className="w-3.5 h-3.5" />
|
||||
</button>
|
||||
);
|
||||
|
||||
const commitEdit = () => {
|
||||
const parsed = parseInt(draft, 10);
|
||||
if (!Number.isNaN(parsed) && parsed !== c.priority) onSet(parsed);
|
||||
@@ -941,8 +952,22 @@ function PriorityCell({
|
||||
)}
|
||||
</span>
|
||||
<span className="inline-flex flex-col">
|
||||
<StepBtn delta={1} Icon={ArrowUp} label={t("channels.row.raisePriority")} />
|
||||
<StepBtn delta={-1} Icon={ArrowDown} label={t("channels.row.lowerPriority")} />
|
||||
<PriorityStepButton
|
||||
delta={1}
|
||||
Icon={ArrowUp}
|
||||
label={t("channels.row.raisePriority")}
|
||||
onHoldStart={start}
|
||||
onStop={stop}
|
||||
onTap={onTap}
|
||||
/>
|
||||
<PriorityStepButton
|
||||
delta={-1}
|
||||
Icon={ArrowDown}
|
||||
label={t("channels.row.lowerPriority")}
|
||||
onHoldStart={start}
|
||||
onStop={stop}
|
||||
onTap={onTap}
|
||||
/>
|
||||
</span>
|
||||
</div>
|
||||
</Tooltip>
|
||||
|
||||
@@ -281,7 +281,10 @@ function LinkShare({ job }: { job: DownloadJob }) {
|
||||
autoTried.current = true;
|
||||
create.mutate();
|
||||
}
|
||||
}, [links.data, create]);
|
||||
// Only links.data (empty → auto-create once) should drive this; `create` is a fresh object every
|
||||
// render and would re-run the effect needlessly. The autoTried ref makes it fire exactly once.
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [links.data]);
|
||||
|
||||
return (
|
||||
<div>
|
||||
|
||||
@@ -25,6 +25,14 @@ function useBackgroundAudio(
|
||||
useEffect(() => {
|
||||
const video = videoRef.current;
|
||||
if (!video || !ready) return;
|
||||
// Only iOS suspends a backgrounded <video> — and only there does rerouting the audio through
|
||||
// Web Audio buy anything. Every other platform backgrounds media audio natively, so we leave
|
||||
// their <video> untouched rather than risk the reroute leaving foreground audio silent if the
|
||||
// AudioContext can't resume. (iPadOS reports as MacIntel + touch.)
|
||||
const isIOS =
|
||||
/iP(hone|ad|od)/.test(navigator.userAgent) ||
|
||||
(navigator.platform === "MacIntel" && navigator.maxTouchPoints > 1);
|
||||
if (!isIOS) return;
|
||||
const wire = () => {
|
||||
if (wiredRef.current) {
|
||||
ctxRef.current?.resume().catch(() => {});
|
||||
|
||||
Reference in New Issue
Block a user