From 0c95ab4109fe20bab22ecf5dcd2c04a19ec2b4a5 Mon Sep 17 00:00:00 2001 From: rahim Date: Sun, 5 Apr 2026 22:19:11 -0700 Subject: [PATCH] refactor(packages): move slider throttle from commit to change (#1219) --- .../tests/time-slider-core.test.ts | 2 +- .../core/ui/time-slider/time-slider-core.ts | 6 +- packages/core/src/dom/ui/slider.ts | 56 +++- packages/core/src/dom/ui/tests/slider.test.ts | 287 +++++++++++------- .../tests/time-slider-element.test.ts | 2 +- .../src/ui/time-slider/time-slider-element.ts | 10 +- packages/react/src/ui/hooks/use-slider.ts | 4 +- .../src/ui/time-slider/time-slider-root.tsx | 8 +- .../utils/src/function/tests/throttle.test.ts | 132 ++++++++ packages/utils/src/function/throttle.ts | 66 +++- 10 files changed, 423 insertions(+), 150 deletions(-) diff --git a/packages/core/src/core/ui/time-slider/tests/time-slider-core.test.ts b/packages/core/src/core/ui/time-slider/tests/time-slider-core.test.ts index 44f37479..acbae1a4 100644 --- a/packages/core/src/core/ui/time-slider/tests/time-slider-core.test.ts +++ b/packages/core/src/core/ui/time-slider/tests/time-slider-core.test.ts @@ -42,7 +42,7 @@ describe('TimeSliderCore', () => { value: 0, min: 0, max: 100, - commitThrottle: 100, + changeThrottle: 100, }); }); }); diff --git a/packages/core/src/core/ui/time-slider/time-slider-core.ts b/packages/core/src/core/ui/time-slider/time-slider-core.ts index a0be9ab2..3a9bca49 100644 --- a/packages/core/src/core/ui/time-slider/time-slider-core.ts +++ b/packages/core/src/core/ui/time-slider/time-slider-core.ts @@ -13,8 +13,8 @@ export interface TimeSliderProps extends SliderProps { min?: number | undefined; /** @internal Derived from `duration` — not user-settable. */ max?: number | undefined; - /** Trailing-edge throttle (ms) for seek requests during drag. */ - commitThrottle?: number | undefined; + /** Leading+trailing throttle (ms) for `onValueChange` during drag. */ + changeThrottle?: number | undefined; } export interface TimeSliderState extends SliderState, Pick { @@ -27,7 +27,7 @@ export class TimeSliderCore extends SliderCore { static override readonly defaultProps: NonNullableObject = { ...SliderCore.defaultProps, label: 'Seek', - commitThrottle: 100, + changeThrottle: 100, }; #props = { ...TimeSliderCore.defaultProps }; diff --git a/packages/core/src/dom/ui/slider.ts b/packages/core/src/dom/ui/slider.ts index 0d4109c0..e10cb9e4 100644 --- a/packages/core/src/dom/ui/slider.ts +++ b/packages/core/src/dom/ui/slider.ts @@ -25,14 +25,17 @@ export interface SliderOptions { getLargeStepPercent: () => number; /** - * Trailing-edge throttle (ms) for `onValueCommit` during drag. When `> 0`, - * `onValueCommit` fires periodically while dragging, then a final unthrottled - * commit fires on pointer release. `0` (default) disables — commits only on release. + * Leading+trailing throttle (ms) for `onValueChange` during drag. When + * `> 0`, `onValueChange` fires immediately on the first drag move (leading + * edge), then at most once per window during subsequent moves. `0` (default) + * disables throttling — `onValueChange` fires on every pointermove. */ - commitThrottle?: number | undefined; + changeThrottle?: number | undefined; /** Adjust a raw 0–100 percent for thumb alignment. Enables `adjustForAlignment()`. */ adjustPercent?: ((rawPercent: number, thumbSize: number, trackSize: number) => number) | undefined; + /** Fires continuously as the value changes (every pointermove during drag, keyboard steps). */ onValueChange?: ((percent: number) => void) | undefined; + /** Fires once when the user commits the value (pointer release, keyboard step). */ onValueCommit?: ((percent: number) => void) | undefined; onDragStart?: (() => void) | undefined; onDragEnd?: (() => void) | undefined; @@ -86,16 +89,29 @@ export function createSlider(options: SliderOptions): SliderApi { }); const abort = new AbortController(); - const commitThrottleMs = options.commitThrottle ?? 0; + const changeThrottleMs = options.changeThrottle ?? 0; let isDragging = false, moveCount = 0, cachedRTL = false, cachedRect: DOMRect | null = null, - capturedPointerId: number | null = null; + capturedPointerId: number | null = null, + lastDragPercent = 0, + committedOnRelease = false; - const throttledCommit = - commitThrottleMs > 0 ? throttle((percent: number) => options.onValueCommit?.(percent), commitThrottleMs) : null; + const throttledChange = + changeThrottleMs > 0 + ? throttle((percent: number) => options.onValueChange?.(percent), changeThrottleMs, { leading: true }) + : null; + + /** Fire `onValueChange` — throttled during drag when `changeThrottle > 0`. */ + function fireChange(percent: number, duringDrag: boolean): void { + if (duringDrag && throttledChange) { + throttledChange(percent); + } else { + options.onValueChange?.(percent); + } + } function releaseCapture(): void { if (isNull(capturedPointerId)) return; @@ -114,16 +130,22 @@ export function createSlider(options: SliderOptions): SliderApi { if (!isDragging) { input.patch({ pointing: false }); } else { + // Fire a final commit if pointerup didn't already handle it. + if (!committedOnRelease) { + options.onValueCommit?.(lastDragPercent); + } + isDragging = false; input.patch({ dragging: false, pointing: false }); options.onDragEnd?.(); } + committedOnRelease = false; cleanup(); } function cleanup() { - throttledCommit?.cancel(); + throttledChange?.cancel(); capturedPointerId = null; cachedRect = null; } @@ -144,6 +166,7 @@ export function createSlider(options: SliderOptions): SliderApi { cachedRect = el.getBoundingClientRect(); cachedRTL = options.isRTL(); moveCount = 0; + committedOnRelease = false; releaseCapture(); capturedPointerId = event.pointerId; @@ -151,6 +174,7 @@ export function createSlider(options: SliderOptions): SliderApi { const percent = getPercentFromPointerEvent(event, cachedRect, options.getOrientation(), cachedRTL); + lastDragPercent = percent; input.patch({ pointing: true, pointerPercent: percent, dragPercent: percent }); options.onValueChange?.(percent); @@ -178,14 +202,14 @@ export function createSlider(options: SliderOptions): SliderApi { if (!isDragging && moveCount >= DRAG_THRESHOLD) { isDragging = true; + lastDragPercent = percent; input.patch({ dragging: true, dragPercent: percent, pointerPercent: percent }); options.onDragStart?.(); - options.onValueChange?.(percent); - throttledCommit?.(percent); + fireChange(percent, true); } else if (isDragging) { + lastDragPercent = percent; input.patch({ dragPercent: percent, pointerPercent: percent }); - options.onValueChange?.(percent); - throttledCommit?.(percent); + fireChange(percent, true); } else { // Below drag threshold — update hover preview only. input.patch({ pointerPercent: percent }); @@ -207,9 +231,11 @@ export function createSlider(options: SliderOptions): SliderApi { const percent = getPercentFromPointerEvent(event, cachedRect!, options.getOrientation(), cachedRTL); - // Cancel pending throttled commit before the final unthrottled one. - throttledCommit?.cancel(); + // Cancel any pending throttled change before the final unthrottled pair. + throttledChange?.cancel(); + options.onValueChange?.(percent); options.onValueCommit?.(percent); + committedOnRelease = true; }, onPointerLeave() { diff --git a/packages/core/src/dom/ui/tests/slider.test.ts b/packages/core/src/dom/ui/tests/slider.test.ts index a50e537d..d2e5d962 100644 --- a/packages/core/src/dom/ui/tests/slider.test.ts +++ b/packages/core/src/dom/ui/tests/slider.test.ts @@ -873,31 +873,11 @@ describe('createSlider', () => { }); }); - describe('commitThrottle', () => { - it('does not fire onValueCommit during drag when commitThrottle is 0', () => { + describe('commit semantics', () => { + it('does not fire onValueCommit during drag', () => { const onValueCommit = vi.fn(); const el = createMockElement({ left: 0, width: 200 }); - const slider = createSlider(createOptions({ getElement: () => el, onValueCommit, commitThrottle: 0 })); - - slider.rootProps.onPointerDown(pointerEvent({ clientX: 50 })); - onValueCommit.mockClear(); - - // Pass drag threshold - firePointerMove(slider, { clientX: 60 }); - firePointerMove(slider, { clientX: 80 }); - firePointerMove(slider, { clientX: 100 }); - - expect(onValueCommit).not.toHaveBeenCalled(); - - slider.destroy(); - }); - - it('fires throttled onValueCommit during drag when commitThrottle > 0', () => { - vi.useFakeTimers(); - - const onValueCommit = vi.fn(); - const el = createMockElement({ left: 0, width: 200 }); - const slider = createSlider(createOptions({ getElement: () => el, onValueCommit, commitThrottle: 100 })); + const slider = createSlider(createOptions({ getElement: () => el, onValueCommit })); slider.rootProps.onPointerDown(pointerEvent({ clientX: 50 })); onValueCommit.mockClear(); @@ -906,103 +886,211 @@ describe('createSlider', () => { firePointerMove(slider, { clientX: 60 }); firePointerMove(slider, { clientX: 80 }); firePointerMove(slider, { clientX: 100 }); - firePointerMove(slider, { clientX: 120 }); - // Not yet — throttle hasn't fired + // Commit must not fire during drag — only on release. expect(onValueCommit).not.toHaveBeenCalled(); - // Advance timer past throttle - vi.advanceTimersByTime(100); + slider.destroy(); + }); + + it('fires onValueChange on pointerup before onValueCommit', () => { + const order: string[] = []; + const onValueChange = vi.fn(() => order.push('change')); + const onValueCommit = vi.fn(() => order.push('commit')); + const el = createMockElement({ left: 0, width: 200 }); + const slider = createSlider(createOptions({ getElement: () => el, onValueChange, onValueCommit })); + + slider.rootProps.onPointerDown(pointerEvent({ clientX: 50 })); + firePointerMove(slider, { clientX: 60 }); + firePointerMove(slider, { clientX: 80 }); + + onValueChange.mockClear(); + onValueCommit.mockClear(); + order.length = 0; + + firePointerUp(slider, { clientX: 100 }); + + expect(onValueChange).toHaveBeenCalledWith(50); + expect(onValueCommit).toHaveBeenCalledWith(50); + expect(order).toEqual(['change', 'commit']); + + slider.destroy(); + }); + + it('fires onValueCommit on stale drag exit with last drag percent', () => { + const onValueCommit = vi.fn(); + const el = createMockElement({ left: 0, width: 200 }); + const slider = createSlider(createOptions({ getElement: () => el, onValueCommit })); + + slider.rootProps.onPointerDown(pointerEvent({ clientX: 50 })); + firePointerMove(slider, { clientX: 60 }); + firePointerMove(slider, { clientX: 80 }); + onValueCommit.mockClear(); + + // Stale drag: buttons = 0, mouse pointer + firePointerMove(slider, { clientX: 100, buttons: 0, pointerType: 'mouse' }); expect(onValueCommit).toHaveBeenCalledOnce(); - // Should have the latest drag percent (120/200 = 60%) - expect(onValueCommit).toHaveBeenCalledWith(60); + // Last drag percent was 40% (80/200) — the stale move doesn't update it. + expect(onValueCommit).toHaveBeenCalledWith(40); + + slider.destroy(); + }); + + it('fires onValueCommit on lostpointercapture without pointerup', () => { + const onValueCommit = vi.fn(); + const el = createMockElement({ left: 0, width: 200 }); + const slider = createSlider(createOptions({ getElement: () => el, onValueCommit })); + + slider.rootProps.onPointerDown(pointerEvent({ clientX: 50 })); + firePointerMove(slider, { clientX: 60 }); + firePointerMove(slider, { clientX: 80 }); + onValueCommit.mockClear(); + + // Lost capture without pointerup (e.g., tab switch, pointercancel). + fireLostPointerCapture(slider); + + expect(onValueCommit).toHaveBeenCalledOnce(); + expect(onValueCommit).toHaveBeenCalledWith(40); + + slider.destroy(); + }); + + it('does not double-commit on pointerup followed by lostpointercapture', () => { + const onValueCommit = vi.fn(); + const el = createMockElement({ left: 0, width: 200 }); + const slider = createSlider(createOptions({ getElement: () => el, onValueCommit })); + + slider.rootProps.onPointerDown(pointerEvent({ clientX: 50 })); + firePointerMove(slider, { clientX: 60 }); + firePointerMove(slider, { clientX: 80 }); + onValueCommit.mockClear(); + + // Normal flow: pointerup commits, then lostpointercapture cleans up. + firePointerUp(slider, { clientX: 100 }); + fireLostPointerCapture(slider); + + // Only one commit from pointerup. + expect(onValueCommit).toHaveBeenCalledOnce(); + expect(onValueCommit).toHaveBeenCalledWith(50); + + slider.destroy(); + }); + + it('does not fire onValueCommit on lostpointercapture without drag', () => { + const onValueCommit = vi.fn(); + const el = createMockElement({ left: 0, width: 200 }); + const slider = createSlider(createOptions({ getElement: () => el, onValueCommit })); + + slider.rootProps.onPointerDown(pointerEvent({ clientX: 50 })); + onValueCommit.mockClear(); + + // Lost capture before drag threshold was reached. + fireLostPointerCapture(slider); + + expect(onValueCommit).not.toHaveBeenCalled(); + + slider.destroy(); + }); + }); + + describe('changeThrottle', () => { + it('fires onValueChange immediately on first drag move (leading edge)', () => { + vi.useFakeTimers(); + + const onValueChange = vi.fn(); + const el = createMockElement({ left: 0, width: 200 }); + const slider = createSlider(createOptions({ getElement: () => el, onValueChange, changeThrottle: 100 })); + + slider.rootProps.onPointerDown(pointerEvent({ clientX: 50 })); + onValueChange.mockClear(); + + // Pass drag threshold — first drag move fires immediately (leading edge). + firePointerMove(slider, { clientX: 60 }); + firePointerMove(slider, { clientX: 80 }); + + expect(onValueChange).toHaveBeenCalledOnce(); + expect(onValueChange).toHaveBeenCalledWith(40); slider.destroy(); vi.useRealTimers(); }); - it('uses trailing-edge: batches rapid moves into one commit', () => { + it('coalesces rapid moves during cooldown to trailing edge', () => { vi.useFakeTimers(); - const onValueCommit = vi.fn(); + const onValueChange = vi.fn(); const el = createMockElement({ left: 0, width: 200 }); - const slider = createSlider(createOptions({ getElement: () => el, onValueCommit, commitThrottle: 100 })); + const slider = createSlider(createOptions({ getElement: () => el, onValueChange, changeThrottle: 100 })); slider.rootProps.onPointerDown(pointerEvent({ clientX: 50 })); - onValueCommit.mockClear(); + onValueChange.mockClear(); - // Pass threshold + // Pass threshold — leading fires. firePointerMove(slider, { clientX: 60 }); firePointerMove(slider, { clientX: 80 }); + expect(onValueChange).toHaveBeenCalledOnce(); - // Multiple rapid moves during drag + // More moves during cooldown. firePointerMove(slider, { clientX: 100 }); firePointerMove(slider, { clientX: 120 }); firePointerMove(slider, { clientX: 140 }); - vi.advanceTimersByTime(100); + // Still only the leading call. + expect(onValueChange).toHaveBeenCalledOnce(); - // Only one commit with the latest value (140/200 = 70%) - expect(onValueCommit).toHaveBeenCalledOnce(); - expect(onValueCommit).toHaveBeenCalledWith(70); + // Trailing fires on cooldown expiry with latest value (140/200 = 70%). + vi.advanceTimersByTime(100); + expect(onValueChange).toHaveBeenCalledTimes(2); + expect(onValueChange).toHaveBeenLastCalledWith(70); slider.destroy(); vi.useRealTimers(); }); - it('fires another throttled commit after the first one completes', () => { - vi.useFakeTimers(); - - const onValueCommit = vi.fn(); + it('does not throttle onValueChange when changeThrottle is 0', () => { + const onValueChange = vi.fn(); const el = createMockElement({ left: 0, width: 200 }); - const slider = createSlider(createOptions({ getElement: () => el, onValueCommit, commitThrottle: 100 })); + const slider = createSlider(createOptions({ getElement: () => el, onValueChange, changeThrottle: 0 })); slider.rootProps.onPointerDown(pointerEvent({ clientX: 50 })); - onValueCommit.mockClear(); + onValueChange.mockClear(); - // Pass threshold and drag + // Every drag move fires immediately. firePointerMove(slider, { clientX: 60 }); firePointerMove(slider, { clientX: 80 }); - vi.advanceTimersByTime(100); - expect(onValueCommit).toHaveBeenCalledOnce(); + firePointerMove(slider, { clientX: 100 }); - // Continue dragging — should schedule another throttle - firePointerMove(slider, { clientX: 140 }); - vi.advanceTimersByTime(100); - - expect(onValueCommit).toHaveBeenCalledTimes(2); - expect(onValueCommit).toHaveBeenLastCalledWith(70); + expect(onValueChange).toHaveBeenCalledTimes(2); slider.destroy(); - vi.useRealTimers(); }); - it('cancels throttle and fires final unthrottled commit on pointerup', () => { + it('cancels throttled change and fires unthrottled on pointerup', () => { vi.useFakeTimers(); - const onValueCommit = vi.fn(); + const onValueChange = vi.fn(); const el = createMockElement({ left: 0, width: 200 }); - const slider = createSlider(createOptions({ getElement: () => el, onValueCommit, commitThrottle: 100 })); + const slider = createSlider(createOptions({ getElement: () => el, onValueChange, changeThrottle: 100 })); slider.rootProps.onPointerDown(pointerEvent({ clientX: 50 })); - onValueCommit.mockClear(); + onValueChange.mockClear(); - // Pass threshold and drag + // Pass threshold (leading fires) and more moves. firePointerMove(slider, { clientX: 60 }); firePointerMove(slider, { clientX: 80 }); firePointerMove(slider, { clientX: 120 }); - // Release before throttle fires + // Release before trailing fires. firePointerUp(slider, { clientX: 150 }); - // Final commit with release position (150/200 = 75%) - expect(onValueCommit).toHaveBeenCalledOnce(); - expect(onValueCommit).toHaveBeenCalledWith(75); + // Leading (threshold) + unthrottled pointerup. The coalesced moves were cancelled. + expect(onValueChange).toHaveBeenCalledTimes(2); + expect(onValueChange).toHaveBeenLastCalledWith(75); - // Advancing timer should NOT fire a stale throttled commit + // Advancing timer should NOT fire a stale trailing change. vi.advanceTimersByTime(200); - expect(onValueCommit).toHaveBeenCalledOnce(); + expect(onValueChange).toHaveBeenCalledTimes(2); slider.destroy(); vi.useRealTimers(); @@ -1011,85 +1099,56 @@ describe('createSlider', () => { it('cancels throttle on destroy', () => { vi.useFakeTimers(); - const onValueCommit = vi.fn(); + const onValueChange = vi.fn(); const el = createMockElement({ left: 0, width: 200 }); - const slider = createSlider(createOptions({ getElement: () => el, onValueCommit, commitThrottle: 100 })); + const slider = createSlider(createOptions({ getElement: () => el, onValueChange, changeThrottle: 100 })); slider.rootProps.onPointerDown(pointerEvent({ clientX: 50 })); - onValueCommit.mockClear(); + onValueChange.mockClear(); - // Pass threshold + // Pass threshold (leading fires). firePointerMove(slider, { clientX: 60 }); firePointerMove(slider, { clientX: 80 }); + // More moves pending. + firePointerMove(slider, { clientX: 120 }); slider.destroy(); - // Advancing timer should NOT fire + // Advancing timer should NOT fire the pending trailing change. vi.advanceTimersByTime(200); - expect(onValueCommit).not.toHaveBeenCalled(); + expect(onValueChange).toHaveBeenCalledOnce(); vi.useRealTimers(); }); - it('cancels throttle on lostpointercapture', () => { - vi.useFakeTimers(); - - const onValueCommit = vi.fn(); - const el = createMockElement({ left: 0, width: 200 }); - const slider = createSlider(createOptions({ getElement: () => el, onValueCommit, commitThrottle: 100 })); - - slider.rootProps.onPointerDown(pointerEvent({ clientX: 50 })); - onValueCommit.mockClear(); - - // Pass threshold - firePointerMove(slider, { clientX: 60 }); - firePointerMove(slider, { clientX: 80 }); - - fireLostPointerCapture(slider); - - // Advancing timer should NOT fire - vi.advanceTimersByTime(200); - expect(onValueCommit).not.toHaveBeenCalled(); - - slider.destroy(); - vi.useRealTimers(); - }); - - it('does not throttle keyboard commits', () => { - const onValueCommit = vi.fn(); - const slider = createSlider(createOptions({ getPercent: () => 50, onValueCommit, commitThrottle: 100 })); + it('does not throttle keyboard changes', () => { + const onValueChange = vi.fn(); + const slider = createSlider(createOptions({ getPercent: () => 50, onValueChange, changeThrottle: 100 })); slider.thumbProps.onKeyDown(keyboardEvent('ArrowRight')); - // Keyboard commits fire immediately, not throttled - expect(onValueCommit).toHaveBeenCalledOnce(); - expect(onValueCommit).toHaveBeenCalledWith(51); + expect(onValueChange).toHaveBeenCalledOnce(); + expect(onValueChange).toHaveBeenCalledWith(51); slider.destroy(); }); - it('defaults commitThrottle to 0 when not provided', () => { - vi.useFakeTimers(); - - const onValueCommit = vi.fn(); + it('defaults changeThrottle to 0 when not provided', () => { + const onValueChange = vi.fn(); const el = createMockElement({ left: 0, width: 200 }); - // No commitThrottle option — should default to 0 (disabled) - const slider = createSlider(createOptions({ getElement: () => el, onValueCommit })); + const slider = createSlider(createOptions({ getElement: () => el, onValueChange })); slider.rootProps.onPointerDown(pointerEvent({ clientX: 50 })); - onValueCommit.mockClear(); + onValueChange.mockClear(); + // Every drag move fires immediately (no throttle). firePointerMove(slider, { clientX: 60 }); firePointerMove(slider, { clientX: 80 }); firePointerMove(slider, { clientX: 100 }); - vi.advanceTimersByTime(200); - - // No throttled commits — default is disabled - expect(onValueCommit).not.toHaveBeenCalled(); + expect(onValueChange).toHaveBeenCalledTimes(2); slider.destroy(); - vi.useRealTimers(); }); }); }); diff --git a/packages/html/src/ui/time-slider/tests/time-slider-element.test.ts b/packages/html/src/ui/time-slider/tests/time-slider-element.test.ts index 85ee1a9d..64aa8977 100644 --- a/packages/html/src/ui/time-slider/tests/time-slider-element.test.ts +++ b/packages/html/src/ui/time-slider/tests/time-slider-element.test.ts @@ -28,7 +28,7 @@ describe('TimeSliderElement', () => { it('initializes with default property values', () => { const slider = createElement(TimeSliderElement); expect(slider.label).toBe('Seek'); - expect(slider.commitThrottle).toBe(100); + expect(slider.changeThrottle).toBe(100); expect(slider.step).toBe(1); expect(slider.largeStep).toBe(10); expect(slider.orientation).toBe('horizontal'); diff --git a/packages/html/src/ui/time-slider/time-slider-element.ts b/packages/html/src/ui/time-slider/time-slider-element.ts index 93122b22..1e2d684e 100644 --- a/packages/html/src/ui/time-slider/time-slider-element.ts +++ b/packages/html/src/ui/time-slider/time-slider-element.ts @@ -24,7 +24,7 @@ export class TimeSliderElement extends MediaElement { static override properties = { label: { type: String }, - commitThrottle: { type: Number, attribute: 'commit-throttle' }, + changeThrottle: { type: Number, attribute: 'change-throttle' }, step: { type: Number }, largeStep: { type: Number, attribute: 'large-step' }, orientation: { type: String }, @@ -33,7 +33,7 @@ export class TimeSliderElement extends MediaElement { } satisfies PropertyDeclarationMap>; label = TimeSliderCore.defaultProps.label; - commitThrottle = TimeSliderCore.defaultProps.commitThrottle; + changeThrottle = TimeSliderCore.defaultProps.changeThrottle; step = TimeSliderCore.defaultProps.step; largeStep = TimeSliderCore.defaultProps.largeStep; orientation = TimeSliderCore.defaultProps.orientation; @@ -68,11 +68,15 @@ export class TimeSliderElement extends MediaElement { }, getStepPercent: () => this.#core.getStepPercent(), getLargeStepPercent: () => this.#core.getLargeStepPercent(), + onValueChange: (percent) => { + const media = this.#timeState.value; + if (media) media.seek(this.#core.rawValueFromPercent(percent)); + }, onValueCommit: (percent) => { const media = this.#timeState.value; if (media) media.seek(this.#core.rawValueFromPercent(percent)); }, - commitThrottle: this.commitThrottle, + changeThrottle: this.changeThrottle, onDragStart: () => { this.dispatchEvent(new CustomEvent('drag-start', { bubbles: true })); }, diff --git a/packages/react/src/ui/hooks/use-slider.ts b/packages/react/src/ui/hooks/use-slider.ts index e5309e9d..6c30cd36 100644 --- a/packages/react/src/ui/hooks/use-slider.ts +++ b/packages/react/src/ui/hooks/use-slider.ts @@ -22,7 +22,7 @@ export interface UseSliderOptions | 'getPercent' | 'getStepPercent' | 'getLargeStepPercent' - | 'commitThrottle' + | 'changeThrottle' | 'onValueChange' | 'onValueCommit' | 'onDragStart' @@ -74,7 +74,7 @@ export function useSlider( getPercent: () => optionsRef.current.getPercent(), getStepPercent: () => optionsRef.current.getStepPercent(), getLargeStepPercent: () => optionsRef.current.getLargeStepPercent(), - commitThrottle: optionsRef.current.commitThrottle, + changeThrottle: optionsRef.current.changeThrottle, adjustPercent: optionsRef.current.adjustPercent, onValueChange: (percent) => optionsRef.current.onValueChange?.(percent), onValueCommit: (percent) => optionsRef.current.onValueCommit?.(percent), diff --git a/packages/react/src/ui/time-slider/time-slider-root.tsx b/packages/react/src/ui/time-slider/time-slider-root.tsx index 50a15f04..62628769 100644 --- a/packages/react/src/ui/time-slider/time-slider-root.tsx +++ b/packages/react/src/ui/time-slider/time-slider-root.tsx @@ -26,7 +26,7 @@ export const TimeSliderRoot = forwardRef( className, style, label, - commitThrottle = TimeSliderCore.defaultProps.commitThrottle, + changeThrottle = TimeSliderCore.defaultProps.changeThrottle, step = TimeSliderCore.defaultProps.step, largeStep = TimeSliderCore.defaultProps.largeStep, orientation, @@ -71,10 +71,14 @@ export const TimeSliderRoot = forwardRef( getLargeStepPercent: () => core.getLargeStepPercent(), orientation, disabled, - commitThrottle, + changeThrottle, adjustPercent: (rawPercent, thumbSize, trackSize) => core.adjustPercentForAlignment(rawPercent, thumbSize, trackSize), getCSSVars: getTimeSliderCSSVars, + onValueChange: (percent) => { + const media = mediaRef.current; + if (media) media.seek(core.rawValueFromPercent(percent)); + }, onValueCommit: (percent) => { const media = mediaRef.current; if (media) media.seek(core.rawValueFromPercent(percent)); diff --git a/packages/utils/src/function/tests/throttle.test.ts b/packages/utils/src/function/tests/throttle.test.ts index eec5b649..27a3890f 100644 --- a/packages/utils/src/function/tests/throttle.test.ts +++ b/packages/utils/src/function/tests/throttle.test.ts @@ -99,3 +99,135 @@ describe('throttle', () => { expect(() => throttled.cancel()).not.toThrow(); }); }); + +describe('throttle with leading: true', () => { + it('invokes immediately on the first call', () => { + vi.useFakeTimers(); + + const callback = vi.fn(); + const throttled = throttle(callback, 100, { leading: true }); + + throttled('first'); + + expect(callback).toHaveBeenCalledOnce(); + expect(callback).toHaveBeenCalledWith('first'); + + vi.useRealTimers(); + }); + + it('coalesces calls within the cooldown to a trailing invocation', () => { + vi.useFakeTimers(); + + const callback = vi.fn(); + const throttled = throttle(callback, 100, { leading: true }); + + throttled('first'); + expect(callback).toHaveBeenCalledOnce(); + + throttled('second'); + throttled('third'); + + // Still only the leading call so far. + expect(callback).toHaveBeenCalledOnce(); + + vi.advanceTimersByTime(100); + + // Trailing fires with latest args. + expect(callback).toHaveBeenCalledTimes(2); + expect(callback).toHaveBeenLastCalledWith('third'); + + vi.useRealTimers(); + }); + + it('resets to leading after cooldown expires with no pending calls', () => { + vi.useFakeTimers(); + + const callback = vi.fn(); + const throttled = throttle(callback, 100, { leading: true }); + + // First leading call. + throttled('batch-1'); + expect(callback).toHaveBeenCalledOnce(); + + // Let cooldown expire without any trailing calls. + vi.advanceTimersByTime(100); + expect(callback).toHaveBeenCalledOnce(); + + // Next call should be treated as a new leading invocation. + throttled('batch-2'); + expect(callback).toHaveBeenCalledTimes(2); + expect(callback).toHaveBeenLastCalledWith('batch-2'); + + vi.useRealTimers(); + }); + + it('chains trailing into a new cooldown window', () => { + vi.useFakeTimers(); + + const callback = vi.fn(); + const throttled = throttle(callback, 100, { leading: true }); + + // Leading call. + throttled('a'); + expect(callback).toHaveBeenCalledOnce(); + + // Pending trailing. + throttled('b'); + + // Trailing fires at 100ms, starting a new cooldown. + vi.advanceTimersByTime(100); + expect(callback).toHaveBeenCalledTimes(2); + expect(callback).toHaveBeenLastCalledWith('b'); + + // Another pending during the new cooldown. + throttled('c'); + vi.advanceTimersByTime(100); + expect(callback).toHaveBeenCalledTimes(3); + expect(callback).toHaveBeenLastCalledWith('c'); + + vi.useRealTimers(); + }); + + it('cancel prevents trailing invocation', () => { + vi.useFakeTimers(); + + const callback = vi.fn(); + const throttled = throttle(callback, 100, { leading: true }); + + throttled('leading'); + throttled('pending'); + expect(callback).toHaveBeenCalledOnce(); + + throttled.cancel(); + + vi.advanceTimersByTime(200); + + // Only the leading call, no trailing. + expect(callback).toHaveBeenCalledOnce(); + + vi.useRealTimers(); + }); + + it('accepts new leading call after cancel', () => { + vi.useFakeTimers(); + + const callback = vi.fn(); + const throttled = throttle(callback, 100, { leading: true }); + + throttled('first'); + throttled.cancel(); + + throttled('after-cancel'); + expect(callback).toHaveBeenCalledTimes(2); + expect(callback).toHaveBeenLastCalledWith('after-cancel'); + + vi.useRealTimers(); + }); + + it('cancel is a no-op when no timer is pending', () => { + const callback = vi.fn(); + const throttled = throttle(callback, 100, { leading: true }); + + expect(() => throttled.cancel()).not.toThrow(); + }); +}); diff --git a/packages/utils/src/function/throttle.ts b/packages/utils/src/function/throttle.ts index e6b26326..313f474d 100644 --- a/packages/utils/src/function/throttle.ts +++ b/packages/utils/src/function/throttle.ts @@ -5,22 +5,69 @@ export interface Throttled { cancel(): void; } +export interface ThrottleOptions { + /** + * When `true`, the first call invokes `fn` immediately (leading edge) and + * starts the cooldown window. Calls during cooldown are coalesced and fire + * on the trailing edge. If no calls arrive during the window the next call + * is treated as a fresh leading invocation. + */ + leading?: boolean; +} + /** - * Trailing-edge throttle: the first call schedules a timer; subsequent calls - * within the window update the arguments. The function fires once per `ms` - * window with the latest arguments. + * Throttle: limits `fn` to at most once per `ms` window. + * + * - Default (no options): trailing-edge only — the first call schedules a + * timer; subsequent calls within the window update the arguments. The + * function fires once per window with the latest arguments. + * - `{ leading: true }`: leading + trailing — the first call invokes + * immediately and opens a cooldown window. Subsequent calls within the + * window are coalesced to a single trailing-edge invocation. */ -export function throttle(fn: (...args: Args) => void, ms: number): Throttled { +export function throttle( + fn: (...args: Args) => void, + ms: number, + options?: ThrottleOptions +): Throttled { + const leading = options?.leading ?? false; + let timerId: ReturnType | null = null; let latestArgs: Args; + let hasPending = false; + + function startCooldown(): void { + timerId = setTimeout(() => { + timerId = null; + + if (hasPending) { + hasPending = false; + fn(...latestArgs); + startCooldown(); + } + }, ms); + } const throttled = (...args: Args): void => { latestArgs = args; - if (timerId !== null) return; - timerId = setTimeout(() => { - timerId = null; - fn(...latestArgs); - }, ms); + + if (leading) { + if (timerId === null) { + // No active window — fire immediately (leading edge). + fn(...latestArgs); + startCooldown(); + } else { + // Inside cooldown — mark pending for trailing edge. + hasPending = true; + } + } else { + // Trailing-only (original behavior). + if (timerId !== null) return; + timerId = setTimeout(() => { + timerId = null; + fn(...latestArgs); + }, ms); + } }; throttled.cancel = (): void => { @@ -28,6 +75,7 @@ export function throttle(fn: (...args: Args) => void, ms clearTimeout(timerId); timerId = null; } + hasPending = false; }; return throttled;