fix(core): keep controls visible while tapping controls on touch (#1704)

This commit is contained in:
Renzo Delfino
2026-07-31 11:49:21 -07:00
committed by GitHub
parent c0a43acb0e
commit cfb5632265
4 changed files with 135 additions and 7 deletions
@@ -32,6 +32,20 @@ export class GestureCoordinator {
return () => this.#subscribers.delete(callback);
}
/**
* Whether a registered binding claims this tap for the given action. A claimed tap
* belongs to the gesture layer, so callers should leave it alone. Taps on interactive
* targets (buttons, sliders) are never claimed — the same filtering the pointerup
* listener applies. A disabled binding still claims: disabling a gesture opts out of
* the action, it doesn't hand the tap back to a fallback handler.
*/
claimsTap(event: PointerEvent, action: string): boolean {
if (isInteractiveTarget(event)) return false;
return this.#bindings.some(
(b) => b.type === 'tap' && b.action === action && (!b.pointer || b.pointer === event.pointerType)
);
}
add(binding: GestureBinding): () => void {
const wrapped: GestureBinding = {
...binding,
@@ -122,6 +122,55 @@ describe('GestureCoordinator.subscribe', () => {
});
});
describe('GestureCoordinator.claimsTap', () => {
it('claims a tap when a matching binding is registered', () => {
const container = setup();
createTapGesture(container, vi.fn(), { action: 'toggleControls', pointer: 'touch' });
const event = pointerUp(container, { pointerType: 'touch', clientX: 150 });
expect(getGestureCoordinator(container).claimsTap(event, 'toggleControls')).toBe(true);
});
it('does not claim a tap on an interactive target', () => {
const container = setup();
const button = document.createElement('button');
container.appendChild(button);
createTapGesture(container, vi.fn(), { action: 'toggleControls', pointer: 'touch' });
const event = pointerUp(button, { pointerType: 'touch', clientX: 150 });
expect(getGestureCoordinator(container).claimsTap(event, 'toggleControls')).toBe(false);
});
it('does not claim when no binding matches the action', () => {
const container = setup();
createTapGesture(container, vi.fn(), { action: 'togglePaused', pointer: 'touch' });
const event = pointerUp(container, { pointerType: 'touch', clientX: 150 });
expect(getGestureCoordinator(container).claimsTap(event, 'toggleControls')).toBe(false);
});
it('does not claim when the binding pointer does not match the event', () => {
const container = setup();
createTapGesture(container, vi.fn(), { action: 'toggleControls', pointer: 'mouse' });
const event = pointerUp(container, { pointerType: 'touch', clientX: 150 });
expect(getGestureCoordinator(container).claimsTap(event, 'toggleControls')).toBe(false);
});
it('still claims when the binding is disabled', () => {
const container = setup();
createTapGesture(container, vi.fn(), { action: 'toggleControls', pointer: 'touch', disabled: true });
const event = pointerUp(container, { pointerType: 'touch', clientX: 150 });
expect(getGestureCoordinator(container).claimsTap(event, 'toggleControls')).toBe(true);
});
});
// ---------------------------------------------------------------------------
// Helpers
// ---------------------------------------------------------------------------
@@ -132,10 +181,11 @@ function pointerDown(target: HTMLElement, init: { button?: number } = {}): void
target.dispatchEvent(event);
}
function pointerUp(target: HTMLElement, init: { pointerType: string; clientX: number; button?: number }): void {
function pointerUp(target: HTMLElement, init: { pointerType: string; clientX: number; button?: number }): PointerEvent {
const event = new Event('pointerup', { bubbles: true });
Object.defineProperty(event, 'pointerType', { value: init.pointerType });
Object.defineProperty(event, 'clientX', { value: init.clientX });
Object.defineProperty(event, 'button', { value: init.button ?? 0 });
target.dispatchEvent(event);
return event as PointerEvent;
}
@@ -105,14 +105,13 @@ export const controlsFeature = definePlayerFeature({
}
if (event.pointerType === 'touch' && Date.now() - pointerDownTime < TAP_THRESHOLD) {
// When a toggleControls touch tap gesture is registered, it handles toggle — skip inline handler.
// A claimed tap belongs to the gesture layer, which owns the toggle — nothing
// to do here. An unclaimed tap (e.g. on a control button the coordinator
// ignores) falls through and resets the idle timer below; without that,
// repeatedly tapping a control lets the controls auto-hide mid-interaction.
const coordinator = findGestureCoordinator(container as HTMLElement);
if (
coordinator?.bindings.some(
(b) => b.type === 'tap' && b.action === 'toggleControls' && (!b.pointer || b.pointer === 'touch')
)
) {
if (coordinator?.claimsTap(event, 'toggleControls')) {
return;
}
@@ -1,5 +1,6 @@
import { createStore, flush } from '@videojs/store';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { getGestureCoordinator } from '../../../gesture/coordinator';
import type { PlayerTarget } from '../../../player';
import { createMockVideo } from '../../../tests/test-helpers';
import { controlsFeature } from '../controls';
@@ -369,6 +370,60 @@ describe('controlsFeature', () => {
});
});
describe('touch tap on interactive controls', () => {
it('resets the idle timer when tapping a control button while a toggleControls gesture is registered', () => {
const video = createMockVideo({ paused: false });
const { store, container } = createPlayerStore(video);
addToggleControlsGesture(container!);
// A real control button (e.g. mute, seek ±10s) inside the player.
const button = document.createElement('button');
container!.appendChild(button);
// Advance partway through the idle delay.
vi.advanceTimersByTime(IDLE_DELAY - 500);
// Quick touch tap on the button.
button.dispatchEvent(createPointerEvent('pointerdown', { pointerType: 'touch' }));
vi.advanceTimersByTime(100);
button.dispatchEvent(createPointerEvent('pointerup', { pointerType: 'touch' }));
flush();
expect(store.state.userActive).toBe(true);
expect(store.state.controlsVisible).toBe(true);
// Advance past the original deadline — still active because the tap reset the
// timer. Without the fix a control tap wouldn't count as activity and the
// controls would hide here.
vi.advanceTimersByTime(500);
flush();
expect(store.state.userActive).toBe(true);
expect(store.state.controlsVisible).toBe(true);
});
it('does not reset the idle timer when tapping the video area (gesture owns the toggle)', () => {
const video = createMockVideo({ paused: false });
const { store, container } = createPlayerStore(video);
addToggleControlsGesture(container!);
vi.advanceTimersByTime(IDLE_DELAY - 500);
// Quick touch tap on the bare container (non-interactive). The gesture
// coordinator handles the toggle here — controls.ts must not reset.
container!.dispatchEvent(createPointerEvent('pointerdown', { pointerType: 'touch' }));
vi.advanceTimersByTime(100);
container!.dispatchEvent(createPointerEvent('pointerup', { pointerType: 'touch' }));
flush();
vi.advanceTimersByTime(500);
flush();
expect(store.state.userActive).toBe(false);
expect(store.state.controlsVisible).toBe(false);
});
});
describe('playback state interaction', () => {
it('shows controls when media pauses', () => {
const video = createMockVideo({ paused: false });
@@ -634,6 +689,16 @@ function createPointerEvent(type: string, init?: { pointerType?: string }): Even
return event;
}
function addToggleControlsGesture(container: HTMLElement): () => void {
return getGestureCoordinator(container).add({
type: 'tap',
action: 'toggleControls',
pointer: 'touch',
recognizer: { handleUp() {}, reset() {} },
onActivate() {},
});
}
function createMockRemote(): EventTarget & { state: string; prompt: () => Promise<void> } {
const target = new EventTarget() as EventTarget & { state: string; prompt: () => Promise<void> };
target.state = 'disconnected';