diff --git a/packages/core/src/core/ui/slider/tests/time-slider-core.test.ts b/packages/core/src/core/ui/slider/tests/time-slider-core.test.ts index e1d8fa6b..730f5df6 100644 --- a/packages/core/src/core/ui/slider/tests/time-slider-core.test.ts +++ b/packages/core/src/core/ui/slider/tests/time-slider-core.test.ts @@ -184,5 +184,34 @@ describe('TimeSliderCore', () => { expect(attrs['aria-label']).toBe('Progress'); }); + + it('preserves disabled across getTimeState calls', () => { + const core = new TimeSliderCore({ disabled: true }); + + // getTimeState overrides min/max via super.setProps — disabled must survive. + const state = core.getTimeState(createMediaState({ duration: 300 }), createInteraction()); + + expect(state.disabled).toBe(true); + + const attrs = core.getAttrs(state); + expect(attrs['aria-disabled']).toBe('true'); + expect(attrs.tabindex).toBe(-1); + }); + + it('preserves thumbAlignment across getTimeState calls', () => { + const core = new TimeSliderCore({ thumbAlignment: 'edge' }); + + const state = core.getTimeState(createMediaState({ duration: 300 }), createInteraction()); + + expect(state.thumbAlignment).toBe('edge'); + }); + + it('preserves orientation across getTimeState calls', () => { + const core = new TimeSliderCore({ orientation: 'vertical' }); + + const state = core.getTimeState(createMediaState({ duration: 300 }), createInteraction()); + + expect(state.orientation).toBe('vertical'); + }); }); }); diff --git a/packages/core/src/core/ui/slider/tests/volume-slider-core.test.ts b/packages/core/src/core/ui/slider/tests/volume-slider-core.test.ts index 76b359bf..fa81aff3 100644 --- a/packages/core/src/core/ui/slider/tests/volume-slider-core.test.ts +++ b/packages/core/src/core/ui/slider/tests/volume-slider-core.test.ts @@ -144,5 +144,16 @@ describe('VolumeSliderCore', () => { expect(attrs['aria-label']).toBe('Sound'); }); + + it('respects disabled prop', () => { + const core = new VolumeSliderCore({ disabled: true }); + const state = core.getVolumeState(createMediaState({ volume: 0.5 }), createInteraction()); + + expect(state.disabled).toBe(true); + + const attrs = core.getAttrs(state); + expect(attrs['aria-disabled']).toBe('true'); + expect(attrs.tabindex).toBe(-1); + }); }); }); diff --git a/packages/core/src/core/ui/slider/time-slider-core.ts b/packages/core/src/core/ui/slider/time-slider-core.ts index 40565191..32b400af 100644 --- a/packages/core/src/core/ui/slider/time-slider-core.ts +++ b/packages/core/src/core/ui/slider/time-slider-core.ts @@ -37,8 +37,8 @@ export class TimeSliderCore extends SliderCore { getTimeState(media: MediaTimeState & MediaBufferState, interaction: SliderInteraction): TimeSliderState { const { duration, currentTime, seeking, buffered } = media; - // Override min/max for time domain - super.setProps({ min: 0, max: duration, step: this.#props.step, largeStep: this.#props.largeStep }); + // Override min/max for time domain, forwarding all user props so disabled/thumbAlignment aren't lost. + super.setProps({ ...this.#props, min: 0, max: duration }); // Raw precision during drag for smooth scrubbing — step snapping only applies to keyboard. const value = interaction.dragging ? clamp((interaction.dragPercent / 100) * duration, 0, duration) : currentTime; diff --git a/packages/core/src/dom/ui/event.ts b/packages/core/src/dom/ui/event.ts index df6b57ed..9b6a2a5f 100644 --- a/packages/core/src/dom/ui/event.ts +++ b/packages/core/src/dom/ui/event.ts @@ -5,6 +5,8 @@ export interface UIEvent { export interface UIKeyboardEvent extends UIEvent { key: string; shiftKey: boolean; + ctrlKey: boolean; + altKey: boolean; metaKey: boolean; target: Node; currentTarget: Node; diff --git a/packages/core/src/dom/ui/slider.ts b/packages/core/src/dom/ui/slider.ts index 7f491c97..a0a4636f 100644 --- a/packages/core/src/dom/ui/slider.ts +++ b/packages/core/src/dom/ui/slider.ts @@ -1,6 +1,7 @@ import { createState, type State } from '@videojs/store'; +import { listen } from '@videojs/utils/dom'; import { clamp, roundToStep } from '@videojs/utils/number'; - +import { isNull } from '@videojs/utils/predicate'; import type { SliderInteraction } from '../../core/ui/slider/slider-core'; import { getPercentFromPointerEvent } from '../utils/pointer'; import type { UIKeyboardEvent, UIPointerEvent } from './event'; @@ -61,11 +62,25 @@ export function createSlider(options: SliderOptions): SliderHandle { }); const abort = new AbortController(); - let isDragging = false; - let moveCount = 0; - let cachedRTL = false; - let cachedRect: DOMRect | null = null; - let documentCleanup: (() => void) | null = null; + let isDragging = false, + moveCount = 0, + cachedRTL = false, + cachedRect: DOMRect | null = null, + documentCleanup: (() => void) | null = null, + capturedPointerId: number | null = null; + + function releaseCapture(): void { + if (isNull(capturedPointerId)) return; + + const id = capturedPointerId; + capturedPointerId = null; + + try { + options.getElement().releasePointerCapture(id); + } catch { + // Pointer may already have been released by the browser (e.g., after pointerup). + } + } function endDrag(): void { if (!isDragging) { @@ -76,6 +91,11 @@ export function createSlider(options: SliderOptions): SliderHandle { options.onDragEnd?.(); } + cleanup(); + } + + function cleanup() { + releaseCapture(); documentCleanup?.(); documentCleanup = null; cachedRect = null; @@ -114,15 +134,15 @@ export function createSlider(options: SliderOptions): SliderHandle { } function addDocumentListeners(): void { - const docAc = new AbortController(); - const signal = docAc.signal; + const abort = new AbortController(); + const signal = abort.signal; - document.addEventListener('pointermove', onDocumentPointerMove, { passive: true, signal }); - document.addEventListener('pointerup', onDocumentPointerUp, { signal }); - document.addEventListener('pointercancel', endDrag, { signal }); - document.addEventListener('touchmove', (e) => e.preventDefault(), { passive: false, signal }); + listen(document, 'pointermove', onDocumentPointerMove, { passive: true, signal }); + listen(document, 'pointerup', onDocumentPointerUp, { signal }); + listen(document, 'pointercancel', endDrag, { signal }); + listen(document, 'touchmove', (e) => e.preventDefault(), { passive: false, signal }); - documentCleanup = () => docAc.abort(); + documentCleanup = () => abort.abort(); } // --- Root props --- @@ -136,6 +156,8 @@ export function createSlider(options: SliderOptions): SliderHandle { cachedRTL = options.isRTL(); moveCount = 0; + releaseCapture(); + capturedPointerId = event.pointerId; el.setPointerCapture(event.pointerId); const percent = getPercentFromPointerEvent(event, cachedRect, options.getOrientation(), cachedRTL); @@ -146,7 +168,6 @@ export function createSlider(options: SliderOptions): SliderHandle { // Focus the thumb for keyboard follow-up and screen reader tracking. options.getThumbElement?.()?.focus(); - // Clean up any stale document listeners before adding new ones. documentCleanup?.(); addDocumentListeners(); }, @@ -187,7 +208,6 @@ export function createSlider(options: SliderOptions): SliderHandle { // Horizontal arrows flip for RTL. Vertical arrows are unaffected. const horizontalSign = rtl ? -1 : 1; - // Shift upgrades arrow keys to large step. const step = event.shiftKey ? largeStepPercent : stepPercent; let newPercent: number | null = null; @@ -218,8 +238,8 @@ export function createSlider(options: SliderOptions): SliderHandle { newPercent = 100; break; default: - // Numeric keys 0-9: jump to N * 10%. - if (!event.metaKey && event.key >= '0' && event.key <= '9') { + // Suppress when any modifier is held to avoid hijacking browser/OS shortcuts. + if (!event.metaKey && !event.ctrlKey && !event.altKey && event.key >= '0' && event.key <= '9') { newPercent = Number(event.key) * 10; } break; @@ -243,11 +263,7 @@ export function createSlider(options: SliderOptions): SliderHandle { }, }; - // Abort all document listeners on destroy. - abort.signal.addEventListener('abort', () => { - documentCleanup?.(); - documentCleanup = null; - }); + listen(abort.signal, 'abort', cleanup, { once: true }); return { interaction: state, diff --git a/packages/core/src/dom/ui/tests/slider.test.ts b/packages/core/src/dom/ui/tests/slider.test.ts index fefad589..b3f38e0d 100644 --- a/packages/core/src/dom/ui/tests/slider.test.ts +++ b/packages/core/src/dom/ui/tests/slider.test.ts @@ -63,6 +63,8 @@ function keyboardEvent(key: string, overrides: Partial = {}): U return { key, shiftKey: false, + ctrlKey: false, + altKey: false, metaKey: false, target: node, currentTarget: node, @@ -271,6 +273,18 @@ describe('createSlider', () => { slider.destroy(); }); + it('releases pointer capture on pointerup', () => { + const el = createMockElement({ left: 0, width: 200 }); + const slider = createSlider(createOptions({ getElement: () => el })); + + slider.rootProps.onPointerDown(pointerEvent({ pointerId: 42, clientX: 50 })); + fireDocumentPointerUp({ clientX: 100 }); + + expect(el.releasePointerCapture).toHaveBeenCalledWith(42); + + slider.destroy(); + }); + it('calls onValueCommit on pointerup even without drag', () => { const onValueCommit = vi.fn(); const el = createMockElement({ left: 0, width: 200 }); @@ -302,6 +316,18 @@ describe('createSlider', () => { slider.destroy(); }); + + it('releases pointer capture on pointercancel', () => { + const el = createMockElement({ left: 0, width: 200 }); + const slider = createSlider(createOptions({ getElement: () => el })); + + slider.rootProps.onPointerDown(pointerEvent({ pointerId: 42, clientX: 50 })); + fireDocumentPointerCancel(); + + expect(el.releasePointerCapture).toHaveBeenCalledWith(42); + + slider.destroy(); + }); }); describe('pointer: stale drag safety', () => { @@ -325,6 +351,40 @@ describe('createSlider', () => { slider.destroy(); }); + + it('releases pointer capture on stale drag', () => { + const el = createMockElement({ left: 0, width: 200 }); + const slider = createSlider(createOptions({ getElement: () => el })); + + slider.rootProps.onPointerDown(pointerEvent({ pointerId: 42, clientX: 50 })); + fireDocumentPointerMove({ clientX: 60, buttons: 0, pointerType: 'mouse' }); + + expect(el.releasePointerCapture).toHaveBeenCalledWith(42); + + slider.destroy(); + }); + + it('does not end drag for touch pointer with buttons 0', () => { + const onDragEnd = vi.fn(); + const onValueChange = vi.fn(); + const el = createMockElement({ left: 0, width: 200 }); + const slider = createSlider(createOptions({ getElement: () => el, onDragEnd, onValueChange })); + + slider.rootProps.onPointerDown(pointerEvent({ clientX: 50 })); + fireDocumentPointerMove({ clientX: 60 }); + fireDocumentPointerMove({ clientX: 80 }); + flush(); + expect(slider.interaction.current.dragging).toBe(true); + + // Touch with buttons=0 should NOT trigger stale drag detection + fireDocumentPointerMove({ clientX: 100, buttons: 0, pointerType: 'touch' }); + flush(); + + expect(slider.interaction.current.dragging).toBe(true); + expect(onDragEnd).not.toHaveBeenCalled(); + + slider.destroy(); + }); }); describe('pointer: hover (no drag)', () => { @@ -525,6 +585,28 @@ describe('createSlider', () => { slider.destroy(); }); + it('numeric keys do not fire when ctrlKey is held', () => { + const onValueChange = vi.fn(); + const slider = createSlider(createOptions({ onValueChange })); + + slider.thumbProps.onKeyDown(keyboardEvent('5', { ctrlKey: true })); + + expect(onValueChange).not.toHaveBeenCalled(); + + slider.destroy(); + }); + + it('numeric keys do not fire when altKey is held', () => { + const onValueChange = vi.fn(); + const slider = createSlider(createOptions({ onValueChange })); + + slider.thumbProps.onKeyDown(keyboardEvent('5', { altKey: true })); + + expect(onValueChange).not.toHaveBeenCalled(); + + slider.destroy(); + }); + it('clamps to 0-100 range', () => { const onValueChange = vi.fn(); const slider = createSlider(createOptions({ getPercent: () => 99, getStepPercent: () => 5, onValueChange })); @@ -767,6 +849,16 @@ describe('createSlider', () => { expect(() => slider.destroy()).not.toThrow(); }); + it('releases pointer capture on destroy', () => { + const el = createMockElement({ left: 0, width: 200 }); + const slider = createSlider(createOptions({ getElement: () => el })); + + slider.rootProps.onPointerDown(pointerEvent({ pointerId: 42, clientX: 50 })); + slider.destroy(); + + expect(el.releasePointerCapture).toHaveBeenCalledWith(42); + }); + it('destroy cleans up active document listeners', () => { const onDragEnd = vi.fn(); const el = createMockElement({ left: 0, width: 200 }); diff --git a/packages/core/src/dom/utils/pointer.ts b/packages/core/src/dom/utils/pointer.ts index d2dd991b..052e779a 100644 --- a/packages/core/src/dom/utils/pointer.ts +++ b/packages/core/src/dom/utils/pointer.ts @@ -17,5 +17,8 @@ export function getPercentFromPointerEvent( ratio = (event.clientX - rect.left) / rect.width; } + // Guard against zero-sized rects (e.g., hidden/collapsed element) producing NaN. + if (!Number.isFinite(ratio)) return 0; + return clamp(ratio * 100, 0, 100); } diff --git a/packages/core/src/dom/utils/tests/pointer.test.ts b/packages/core/src/dom/utils/tests/pointer.test.ts new file mode 100644 index 00000000..50d489fe --- /dev/null +++ b/packages/core/src/dom/utils/tests/pointer.test.ts @@ -0,0 +1,88 @@ +import { describe, expect, it } from 'vitest'; + +import { getPercentFromPointerEvent } from '../pointer'; + +function rect(overrides: Partial = {}): DOMRect { + return { + x: 0, + y: 0, + width: 200, + height: 100, + top: 0, + right: 200, + bottom: 100, + left: 0, + toJSON() {}, + ...overrides, + }; +} + +function pointer(clientX = 0, clientY = 0) { + return { clientX, clientY }; +} + +describe('getPercentFromPointerEvent', () => { + describe('horizontal LTR', () => { + it('returns 0 at the left edge', () => { + expect(getPercentFromPointerEvent(pointer(0), rect(), 'horizontal', false)).toBe(0); + }); + + it('returns 100 at the right edge', () => { + expect(getPercentFromPointerEvent(pointer(200), rect(), 'horizontal', false)).toBe(100); + }); + + it('returns 50 at the midpoint', () => { + expect(getPercentFromPointerEvent(pointer(100), rect(), 'horizontal', false)).toBe(50); + }); + + it('clamps below 0', () => { + expect(getPercentFromPointerEvent(pointer(-10), rect(), 'horizontal', false)).toBe(0); + }); + + it('clamps above 100', () => { + expect(getPercentFromPointerEvent(pointer(250), rect(), 'horizontal', false)).toBe(100); + }); + }); + + describe('horizontal RTL', () => { + it('returns 0 at the right edge', () => { + expect(getPercentFromPointerEvent(pointer(200), rect(), 'horizontal', true)).toBe(0); + }); + + it('returns 100 at the left edge', () => { + expect(getPercentFromPointerEvent(pointer(0), rect(), 'horizontal', true)).toBe(100); + }); + + it('returns 50 at the midpoint', () => { + expect(getPercentFromPointerEvent(pointer(100), rect(), 'horizontal', true)).toBe(50); + }); + }); + + describe('vertical', () => { + it('returns 100 at the top edge', () => { + expect(getPercentFromPointerEvent(pointer(0, 0), rect(), 'vertical', false)).toBe(100); + }); + + it('returns 0 at the bottom edge', () => { + expect(getPercentFromPointerEvent(pointer(0, 100), rect(), 'vertical', false)).toBe(0); + }); + + it('returns 50 at the midpoint', () => { + expect(getPercentFromPointerEvent(pointer(0, 50), rect(), 'vertical', false)).toBe(50); + }); + }); + + describe('zero-sized rect', () => { + it('returns 0 for zero-width horizontal rect', () => { + expect(getPercentFromPointerEvent(pointer(50), rect({ width: 0 }), 'horizontal', false)).toBe(0); + }); + + it('returns 0 for zero-height vertical rect', () => { + expect(getPercentFromPointerEvent(pointer(0, 50), rect({ height: 0 }), 'vertical', false)).toBe(0); + }); + + it('returns 0 for zero-width RTL rect', () => { + expect(getPercentFromPointerEvent(pointer(50), rect({ width: 0 }), 'horizontal', true)).toBe(0); + }); + }); +});