fix(player): controls bar taps are not player gestures (DR-098)
The bottom play/pause button did nothing. The gesture listener lives on the outer container and touch events bubble, so tapping the button ran handleTouchStart (toggle #1) and then the button's own onclick (toggle #2). The two cancelled out, leaving the control apparently dead. Ignore container-level gestures for touches that land on an interactive control: buttons, links, inputs (the seek bar), or anything inside the controls bar, now marked `data-player-controls`. The rule itself is a pure function over the ancestor chain (isControlSurfaceTouch), so it is unit tested without a DOM. Same root shape as the play-overlay bug in the previous commit: a second click target over the video that the gesture layer did not account for.
This commit is contained in:
@@ -26,6 +26,7 @@
|
||||
resolveSeekTarget,
|
||||
clampSeekTarget,
|
||||
isSynthesizedTouchClick,
|
||||
isControlSurfaceTouch,
|
||||
SEEK_FORWARD_SECONDS,
|
||||
SEEK_BACKWARD_SECONDS,
|
||||
type TapFeedback,
|
||||
@@ -1442,8 +1443,33 @@
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Walk up from the touch target collecting the tag/attribute pairs
|
||||
* `isControlSurfaceTouch` needs, so the rule itself stays DOM-free and testable.
|
||||
*/
|
||||
function ancestorChain(target: EventTarget | null) {
|
||||
const chain: Array<{ tag: string; isPlayerControls?: boolean }> = [];
|
||||
let node = target as HTMLElement | null;
|
||||
// Bounded walk: controls live a few levels below the player root, and
|
||||
// stopping at <body> keeps this cheap and avoids depending on a bound ref.
|
||||
while (node && node.tagName !== "BODY") {
|
||||
chain.push({
|
||||
tag: node.tagName ?? "",
|
||||
isPlayerControls: node.dataset?.playerControls !== undefined,
|
||||
});
|
||||
node = node.parentElement;
|
||||
}
|
||||
return chain;
|
||||
}
|
||||
|
||||
// Touch gesture handlers
|
||||
function handleTouchStart(e: TouchEvent) {
|
||||
// Taps on the controls belong to those controls. This listener is on the
|
||||
// container and touch events bubble, so without this a tap on the bottom
|
||||
// play button would toggle here AND again via the button's own click — the
|
||||
// two cancelling out and leaving the control apparently dead (DR-098).
|
||||
if (isControlSurfaceTouch(ancestorChain(e.target))) return;
|
||||
|
||||
const touch = e.touches[0];
|
||||
touchStartX = touch.clientX;
|
||||
touchStartY = touch.clientY;
|
||||
@@ -1834,8 +1860,10 @@
|
||||
{/if}
|
||||
</div>
|
||||
|
||||
<!-- Controls -->
|
||||
<!-- Controls. `data-player-controls` marks this subtree as interactive so
|
||||
container-level tap gestures ignore touches here (see DR-098). -->
|
||||
<div
|
||||
data-player-controls
|
||||
class="absolute bottom-0 left-0 right-0 bg-gradient-to-t from-black/80 to-transparent p-4 transition-opacity duration-300"
|
||||
class:opacity-0={!showControls}
|
||||
class:pointer-events-none={!showControls}
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
clampSeekTarget,
|
||||
END_SEEK_MARGIN_SECONDS,
|
||||
isSynthesizedTouchClick,
|
||||
isControlSurfaceTouch,
|
||||
TOUCH_CLICK_SUPPRESS_MS,
|
||||
} from "./tapGestures";
|
||||
|
||||
@@ -199,6 +200,34 @@ describe("seek target resolution", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("control-surface touches are not gestures", () => {
|
||||
// Regression: the gesture listener is on the outer container and touch events
|
||||
// bubble, so tapping the bottom play/pause button ran the gesture handler
|
||||
// (toggle #1) AND the button's own click handler (toggle #2). The two
|
||||
// cancelled out and the control appeared dead.
|
||||
it("treats a tap on a button as a control, not a gesture", () => {
|
||||
expect(isControlSurfaceTouch([{ tag: "svg" }, { tag: "button" }, { tag: "div" }])).toBe(true);
|
||||
});
|
||||
|
||||
it("treats the seek bar input as a control", () => {
|
||||
expect(isControlSurfaceTouch([{ tag: "input" }, { tag: "div" }])).toBe(true);
|
||||
});
|
||||
|
||||
it("treats anything inside the controls bar as a control", () => {
|
||||
expect(
|
||||
isControlSurfaceTouch([{ tag: "span" }, { tag: "div", isPlayerControls: true }])
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("lets a tap on the bare video surface through as a gesture", () => {
|
||||
expect(isControlSurfaceTouch([{ tag: "video" }, { tag: "div" }, { tag: "div" }])).toBe(false);
|
||||
});
|
||||
|
||||
it("is case-insensitive about tag names", () => {
|
||||
expect(isControlSurfaceTouch([{ tag: "BUTTON" }])).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("synthesized touch-click suppression", () => {
|
||||
// Regression: pausing renders a full-screen play-overlay button over the
|
||||
// video, so the compatibility click Android synthesizes from the tap lands on
|
||||
|
||||
@@ -31,6 +31,28 @@ export const DOUBLE_TAP_WINDOW_MS = 300;
|
||||
*/
|
||||
export const TOUCH_CLICK_SUPPRESS_MS = 700;
|
||||
|
||||
/**
|
||||
* Whether a touch landed on an interactive control rather than the bare video
|
||||
* surface, and so must NOT be interpreted as a play/pause or seek gesture.
|
||||
*
|
||||
* The gesture listener sits on the outer container, and touch events bubble, so
|
||||
* without this a tap on the bottom control bar runs the gesture handler (toggle
|
||||
* #1) *and* the button's own click handler (toggle #2) — the two cancel out and
|
||||
* the button appears dead. Buttons, links, inputs (the seek bar), and anything
|
||||
* inside an element marked `data-player-controls` are treated as controls.
|
||||
*
|
||||
* Takes the ancestor chain as plain tag/attribute pairs so the rule is unit
|
||||
* testable without a DOM.
|
||||
*/
|
||||
export function isControlSurfaceTouch(
|
||||
ancestors: Array<{ tag: string; isPlayerControls?: boolean }>
|
||||
): boolean {
|
||||
const INTERACTIVE = new Set(["button", "a", "input", "select", "textarea", "label"]);
|
||||
return ancestors.some(
|
||||
(node) => node.isPlayerControls === true || INTERACTIVE.has(node.tag.toLowerCase())
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a `click` should be ignored because a touch tap already handled it.
|
||||
*
|
||||
|
||||
Reference in New Issue
Block a user