From 48a984aecd39e08c39e786b79c17f64244a983fe Mon Sep 17 00:00:00 2001 From: Renzo Delfino <75499398+R-Delfino95@users.noreply.github.com> Date: Wed, 10 Jun 2026 14:39:39 -0300 Subject: [PATCH] fix(core): prevent mobile controls flash on first tap after auto-hide (#1556) --- .../core/src/dom/store/features/controls.ts | 60 +++++++- .../dom/store/features/tests/controls.test.ts | 142 ++++++++++++++++++ 2 files changed, 197 insertions(+), 5 deletions(-) diff --git a/packages/core/src/dom/store/features/controls.ts b/packages/core/src/dom/store/features/controls.ts index 3712c482..af1a6a46 100644 --- a/packages/core/src/dom/store/features/controls.ts +++ b/packages/core/src/dom/store/features/controls.ts @@ -9,6 +9,7 @@ import { isRemotePlaybackConnected, isRemotePlaybackConnecting } from '../../pre const IDLE_DELAY = 2000; const TAP_THRESHOLD = 250; +const TOUCH_SETTLE_DELAY = 500; export const controlsFeature = definePlayerFeature({ name: 'controls', @@ -74,14 +75,36 @@ export const controlsFeature = definePlayerFeature({ }, }); - // Touch tap-to-toggle + // Touch tap-to-toggle. + // + // When the skin registers `tap action="toggleControls"` alongside + // `doubletap` gestures, the tap recognizer defers its callback by 200 ms + // (doubletap window) and re-reads live state at fire time. Any synthetic + // event that flips visibility during that window inverts the toggle — + // Android first-tap flash. The guards below short-circuit such events + // inside a touch interaction. + // + // `lastTouchAt` is recorded on pointerdown as well as pointerup: the + // container's own pointerup listener calls this.focus() synchronously + // before ours runs, firing focusin while lastTouchAt would otherwise + // still be 0. let pointerDownTime = 0; + let lastTouchAt = 0; - function onPointerDown() { + const isRecentTouch = () => lastTouchAt > 0 && Date.now() - lastTouchAt < TOUCH_SETTLE_DELAY; + + function onPointerDown(event: PointerEvent) { pointerDownTime = Date.now(); + if (event.pointerType === 'touch') { + lastTouchAt = pointerDownTime; + } } function onPointerUp(event: PointerEvent) { + if (event.pointerType === 'touch') { + lastTouchAt = Date.now(); + } + if (event.pointerType === 'touch' && Date.now() - pointerDownTime < TAP_THRESHOLD) { // When a toggleControls touch tap gesture is registered, it handles toggle — skip inline handler. const coordinator = findGestureCoordinator(container as HTMLElement); @@ -117,15 +140,42 @@ export const controlsFeature = definePlayerFeature({ } }; + function onPointerMove(event: PointerEvent): void { + // On touch, don't flip visibility mid-gesture — just keep the idle timer alive. + if (event.pointerType === 'touch') { + if (get().userActive) scheduleIdle(); + return; + } + setActive(); + } + // Container event listeners - listen(container, 'pointermove', setActive, { signal }); + listen(container, 'pointermove', onPointerMove, { signal }); listen(container, 'pointerdown', onPointerDown, { signal }); listen(container, 'pointerup', onPointerUp, { signal }); listen(container, 'keyup', setActive, { signal }); - listen(container, 'focusin', setActive, { signal }); + listen( + container, + 'focusin', + () => { + // Ignore focusin from the container's own pointerup focus grab. + if (isRecentTouch()) return; + setActive(); + }, + { signal } + ); // On touch devices pointerleave would fire after a pointerup event which hides the controls. // https://w3c.github.io/pointerevents/#dfn-pointerup - listen(container, 'mouseleave', setInactive, { signal }); + listen( + container, + 'mouseleave', + () => { + // Ignore synthetic mouseleave that Android Chrome dispatches after touchend. + if (isRecentTouch()) return; + setInactive(); + }, + { signal } + ); // Media event listeners for playback state changes. listen(media, 'play', onPlaybackChange, { signal }); diff --git a/packages/core/src/dom/store/features/tests/controls.test.ts b/packages/core/src/dom/store/features/tests/controls.test.ts index d0525f3e..04bc3625 100644 --- a/packages/core/src/dom/store/features/tests/controls.test.ts +++ b/packages/core/src/dom/store/features/tests/controls.test.ts @@ -225,6 +225,148 @@ describe('controlsFeature', () => { expect(store.state.userActive).toBe(true); expect(store.state.controlsVisible).toBe(true); }); + + it('touch pointermove while controls are hidden does not show controls', () => { + const video = createMockVideo({ paused: false }); + const { store, container } = createPlayerStore(video); + + // Let controls auto-hide + vi.advanceTimersByTime(IDLE_DELAY); + flush(); + + expect(store.state.controlsVisible).toBe(false); + + // Touch pointermove should not flip controlsVisible + container!.dispatchEvent(createPointerEvent('pointermove', { pointerType: 'touch' })); + flush(); + + expect(store.state.controlsVisible).toBe(false); + expect(store.state.userActive).toBe(false); + }); + + it('touch pointermove while controls are visible keeps idle timer alive without re-patching state', () => { + const video = createMockVideo({ paused: false }); + const { store, container } = createPlayerStore(video); + + // Advance partway through idle delay + vi.advanceTimersByTime(IDLE_DELAY - 500); + + // Touch pointermove should keep the timer alive without forcing a state change + container!.dispatchEvent(createPointerEvent('pointermove', { pointerType: 'touch' })); + flush(); + + expect(store.state.userActive).toBe(true); + expect(store.state.controlsVisible).toBe(true); + + // Advance past the original deadline — still active because timer was reset + vi.advanceTimersByTime(500); + flush(); + + expect(store.state.userActive).toBe(true); + + // Wait the full idle delay from the pointermove — now it should go inactive + vi.advanceTimersByTime(IDLE_DELAY - 500); + flush(); + + expect(store.state.userActive).toBe(false); + }); + + it('synthetic focusin fired between touch pointerdown and pointerup is ignored', () => { + // Mirrors Android Chrome: the container's own pointerup listener calls + // this.focus() before the controls feature's pointerup handler runs, + // so focusin fires while lastTouchAt was only set by pointerdown. + const video = createMockVideo({ paused: false }); + const { store, container } = createPlayerStore(video); + + vi.advanceTimersByTime(IDLE_DELAY); + flush(); + + expect(store.state.controlsVisible).toBe(false); + + // Touch pointerdown starts the tap window. + container!.dispatchEvent(createPointerEvent('pointerdown', { pointerType: 'touch' })); + vi.advanceTimersByTime(50); + + // focusin fires before our pointerup handler runs (synchronous focus grab). + container!.dispatchEvent(new Event('focusin')); + flush(); + + expect(store.state.userActive).toBe(false); + expect(store.state.controlsVisible).toBe(false); + }); + + it('synthetic focusin shortly after touch pointerup does not re-activate hidden controls', () => { + const video = createMockVideo({ paused: false }); + const { store, container } = createPlayerStore(video); + + // Tap to hide controls (starts visible). This records lastTouchUpAt + // and sets controlsVisible=false via the inline tap-toggle. + container!.dispatchEvent(new Event('pointerdown')); + vi.advanceTimersByTime(100); + container!.dispatchEvent(createPointerEvent('pointerup', { pointerType: 'touch' })); + flush(); + + expect(store.state.userActive).toBe(false); + expect(store.state.controlsVisible).toBe(false); + + // Synthetic focusin within 500 ms of touch pointerup (from the container's + // own focus() call) should be ignored. + vi.advanceTimersByTime(100); + container!.dispatchEvent(new Event('focusin')); + flush(); + + expect(store.state.userActive).toBe(false); + expect(store.state.controlsVisible).toBe(false); + }); + + it('focusin after a touch tap window has elapsed still activates controls', () => { + const video = createMockVideo({ paused: false }); + const { store, container } = createPlayerStore(video); + + // Tap to hide + container!.dispatchEvent(new Event('pointerdown')); + vi.advanceTimersByTime(100); + container!.dispatchEvent(createPointerEvent('pointerup', { pointerType: 'touch' })); + flush(); + + expect(store.state.controlsVisible).toBe(false); + + // After the 500 ms guard expires, focusin should still re-activate + // (e.g., keyboard navigation focusing the container). + vi.advanceTimersByTime(600); + container!.dispatchEvent(new Event('focusin')); + flush(); + + expect(store.state.userActive).toBe(true); + expect(store.state.controlsVisible).toBe(true); + }); + + it('synthetic mouseleave shortly after touch pointerup does not call setInactive', () => { + const video = createMockVideo({ paused: false }); + const { store, container } = createPlayerStore(video); + + // Let controls auto-hide first + vi.advanceTimersByTime(IDLE_DELAY); + flush(); + + expect(store.state.controlsVisible).toBe(false); + + // Tap shows controls (controlsVisible=false → setActive path) + container!.dispatchEvent(new Event('pointerdown')); + vi.advanceTimersByTime(100); + container!.dispatchEvent(createPointerEvent('pointerup', { pointerType: 'touch' })); + flush(); + + expect(store.state.controlsVisible).toBe(true); + + // Synthetic mouseleave within 500 ms of touchend should be ignored + vi.advanceTimersByTime(100); + container!.dispatchEvent(new Event('mouseleave')); + flush(); + + expect(store.state.userActive).toBe(true); + expect(store.state.controlsVisible).toBe(true); + }); }); describe('playback state interaction', () => {