From 0f4a9b4127d3fbb59b11e278af70c23f83f51c6f Mon Sep 17 00:00:00 2001 From: Sam Potts Date: Fri, 31 Jul 2026 00:37:20 +1000 Subject: [PATCH] fix(packages): prevent controls click triggering interactions (#1885) --- apps/e2e/tests/gestures.spec.ts | 14 ++++++++++++++ .../core/src/dom/gesture/tests/gesture.test.ts | 18 ++++++++++++++++++ .../html/src/ui/controls/controls-element.ts | 2 ++ .../ui/controls/tests/controls-element.test.ts | 12 ++++++++++++ .../react/src/ui/controls/controls-root.tsx | 2 +- .../src/ui/controls/tests/controls.test.tsx | 17 +++++++++++++++++ 6 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 packages/react/src/ui/controls/tests/controls.test.tsx diff --git a/apps/e2e/tests/gestures.spec.ts b/apps/e2e/tests/gestures.spec.ts index 7eb938ba..b4451924 100644 --- a/apps/e2e/tests/gestures.spec.ts +++ b/apps/e2e/tests/gestures.spec.ts @@ -53,6 +53,13 @@ test.describe('Mouse Gestures', () => { await expect(player.playButton).not.toHaveAttribute(DATA_ATTRS.paused, { timeout: 5_000 }); }); + test('click on controls container does not trigger container gesture', async () => { + await expect(player.playButton).toHaveAttribute(DATA_ATTRS.paused, ''); + await player.controls.dispatchEvent('pointerdown', { button: 0, pointerType: 'mouse' }); + await player.controls.dispatchEvent('pointerup', { button: 0, pointerType: 'mouse' }); + await expect(player.playButton).toHaveAttribute(DATA_ATTRS.paused, ''); + }); + test('click on slider does not trigger container gesture', async ({ page }) => { // Start playback so the slider has a seekable range await player.play(); @@ -91,6 +98,13 @@ test.describe('React Mouse Gestures', () => { await expect(player.playButton).not.toHaveAttribute(DATA_ATTRS.paused, { timeout: 5_000 }); }); + test('click on controls container does not trigger container gesture', async () => { + await expect(player.playButton).toHaveAttribute(DATA_ATTRS.paused, ''); + await player.controls.dispatchEvent('pointerdown', { button: 0, pointerType: 'mouse' }); + await player.controls.dispatchEvent('pointerup', { button: 0, pointerType: 'mouse' }); + await expect(player.playButton).toHaveAttribute(DATA_ATTRS.paused, ''); + }); + test('click on slider does not trigger container gesture', async ({ page }) => { await player.play(); await page.waitForTimeout(500); diff --git a/packages/core/src/dom/gesture/tests/gesture.test.ts b/packages/core/src/dom/gesture/tests/gesture.test.ts index 4ab2ddee..38229dc7 100644 --- a/packages/core/src/dom/gesture/tests/gesture.test.ts +++ b/packages/core/src/dom/gesture/tests/gesture.test.ts @@ -464,6 +464,24 @@ describe('interactive child filtering', () => { expect(handler).not.toHaveBeenCalled(); }); + it('does not fire from any child inside a marked controls surface', () => { + const container = setup(); + const controls = document.createElement('div'); + const label = document.createElement('span'); + controls.setAttribute('data-interactive', ''); + controls.appendChild(label); + container.appendChild(controls); + + const handler = vi.fn(); + createTapGesture(container, handler); + + pointerDown(label); + vi.advanceTimersByTime(50); + pointerUp(label, { pointerType: 'mouse', clientX: 150 }); + + expect(handler).not.toHaveBeenCalled(); + }); + it('fires when event originates from a non-interactive child', () => { const container = setup(); const overlay = document.createElement('div'); diff --git a/packages/html/src/ui/controls/controls-element.ts b/packages/html/src/ui/controls/controls-element.ts index 8cec5a91..e7ec914d 100644 --- a/packages/html/src/ui/controls/controls-element.ts +++ b/packages/html/src/ui/controls/controls-element.ts @@ -20,6 +20,8 @@ export class ControlsElement extends MediaElement { override connectedCallback(): void { super.connectedCallback(); + this.setAttribute('data-interactive', ''); + if (__DEV__ && !this.#mediaState.value && this.#mediaState.displayName) { logMissingFeature(this.localName, this.#mediaState.displayName); } diff --git a/packages/html/src/ui/controls/tests/controls-element.test.ts b/packages/html/src/ui/controls/tests/controls-element.test.ts index 5974de0a..6b3531ec 100644 --- a/packages/html/src/ui/controls/tests/controls-element.test.ts +++ b/packages/html/src/ui/controls/tests/controls-element.test.ts @@ -98,6 +98,18 @@ afterEach(() => { }); describe('ControlsElement', () => { + it('marks the controls surface as interactive', async () => { + const provider = document.createElement('test-controls-player-provider') as TestPlayerProviderElement; + const controls = createDefinedElement(ControlsElement); + + document.body.append(provider); + provider.append(controls); + + await controls.updateComplete; + + expect(controls.hasAttribute('data-interactive')).toBe(true); + }); + it('closes owned popovers, menus, and tooltips when controls hide', async () => { const provider = document.createElement('test-controls-player-provider') as TestPlayerProviderElement; const controls = createDefinedElement(ControlsElement); diff --git a/packages/react/src/ui/controls/controls-root.tsx b/packages/react/src/ui/controls/controls-root.tsx index 632f4d97..331308b5 100644 --- a/packages/react/src/ui/controls/controls-root.tsx +++ b/packages/react/src/ui/controls/controls-root.tsx @@ -42,7 +42,7 @@ export const ControlsRoot = forwardRef(function ControlsRoot( state, stateAttrMap: ControlsDataAttrs, ref: [forwardedRef], - props: [{ children }, elementProps], + props: [{ children }, elementProps, { 'data-interactive': '' }], } )} diff --git a/packages/react/src/ui/controls/tests/controls.test.tsx b/packages/react/src/ui/controls/tests/controls.test.tsx new file mode 100644 index 00000000..a542938a --- /dev/null +++ b/packages/react/src/ui/controls/tests/controls.test.tsx @@ -0,0 +1,17 @@ +import { render } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; + +import { createPlayerWrapper } from '../../../testing/mocks'; +import { ControlsRoot } from '../controls-root'; + +describe('ControlsRoot', () => { + it('marks the controls surface as interactive', () => { + const { Wrapper } = createPlayerWrapper({ + controlsVisible: true, + userActive: true, + }); + const { getByTestId } = render(, { wrapper: Wrapper }); + + expect(getByTestId('controls').hasAttribute('data-interactive')).toBe(true); + }); +});