From b3ff8653f369e83e23388248d60c22d55a09c060 Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Tue, 4 Feb 2025 19:52:51 +0800 Subject: [PATCH 1/6] Add a test --- .../src/slider/thumb/SliderThumb.test.tsx | 82 ++++++++++++++++++- 1 file changed, 81 insertions(+), 1 deletion(-) diff --git a/packages/react/src/slider/thumb/SliderThumb.test.tsx b/packages/react/src/slider/thumb/SliderThumb.test.tsx index 7209f48ee6..ee524d2e5f 100644 --- a/packages/react/src/slider/thumb/SliderThumb.test.tsx +++ b/packages/react/src/slider/thumb/SliderThumb.test.tsx @@ -1,9 +1,38 @@ import * as React from 'react'; +import { expect } from 'chai'; +import { stub } from 'sinon'; +import { fireEvent } from '@mui/internal-test-utils'; import { Slider } from '@base-ui-components/react/slider'; -import { createRenderer, describeConformance } from '#test-utils'; +import { createRenderer, describeConformance, isJSDOM } from '#test-utils'; import { SliderRootContext } from '../root/SliderRootContext'; import { NOOP } from '../../utils/noop'; +type Touches = Array>; + +const GETBOUNDINGCLIENTRECT_HORIZONTAL_SLIDER_RETURN_VAL = { + width: 1000, + height: 10, + bottom: 10, + left: 0, + x: 0, + y: 0, + top: 0, + right: 0, + toJSON() {}, +}; + +function createTouches(touches: Touches) { + return { + changedTouches: touches.map( + (touch) => + new Touch({ + target: document.body, + ...touch, + }), + ), + }; +} + const testRootContext: SliderRootContext = { active: -1, handleInputChange: NOOP, @@ -64,4 +93,55 @@ describe('', () => { }, refInstanceof: window.HTMLDivElement, })); + + describe.skipIf(isJSDOM)('internal styles', () => { + it('positions the thumb when the value changes', async () => { + const { getByTestId } = await render( + + + + + + + + , + ); + + const sliderControl = getByTestId('control'); + + const thumbStyles = getComputedStyle(getByTestId('thumb')); + + stub(sliderControl, 'getBoundingClientRect').callsFake( + () => GETBOUNDINGCLIENTRECT_HORIZONTAL_SLIDER_RETURN_VAL, + ); + + fireEvent.touchStart( + sliderControl, + createTouches([{ identifier: 1, clientX: 20, clientY: 0 }]), + ); + fireEvent.touchMove( + document.body, + createTouches([{ identifier: 1, clientX: 199, clientY: 0 }]), + ); + fireEvent.touchMove( + document.body, + createTouches([{ identifier: 1, clientX: 199, clientY: 0 }]), + ); + + fireEvent.touchMove( + document.body, + createTouches([{ identifier: 1, clientX: 199, clientY: 0 }]), + ); + + expect(thumbStyles.getPropertyValue('left')).to.equal('199px'); + fireEvent.touchEnd(document.body, createTouches([{ identifier: 1, clientX: 0, clientY: 0 }])); + expect(thumbStyles.getPropertyValue('left')).to.equal('200px'); + }); + }); }); From 1ac42ee8c0c65a78788f295327f808d879066572 Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Tue, 4 Feb 2025 16:42:46 +0800 Subject: [PATCH 2/6] Round percentage values when drag ends --- .../src/slider/control/SliderControl.test.tsx | 6 +- .../src/slider/control/SliderControl.tsx | 4 +- .../src/slider/control/useSliderControl.ts | 8 +-- .../slider/indicator/SliderIndicator.test.tsx | 6 +- .../react/src/slider/root/useSliderRoot.ts | 58 ++++++++++++------- .../src/slider/thumb/SliderThumb.test.tsx | 6 +- .../src/slider/track/SliderTrack.test.tsx | 6 +- .../src/slider/value/SliderValue.test.tsx | 6 +- 8 files changed, 58 insertions(+), 42 deletions(-) diff --git a/packages/react/src/slider/control/SliderControl.test.tsx b/packages/react/src/slider/control/SliderControl.test.tsx index 1541e8648b..aac7cb0ac6 100644 --- a/packages/react/src/slider/control/SliderControl.test.tsx +++ b/packages/react/src/slider/control/SliderControl.test.tsx @@ -6,7 +6,7 @@ import { NOOP } from '../../utils/noop'; const testRootContext: SliderRootContext = { active: -1, - handleInputChange: NOOP, + commitValue: NOOP, dragging: false, disabled: false, getFingerState: () => ({ @@ -15,7 +15,7 @@ const testRootContext: SliderRootContext = { percentageValues: [0], thumbIndex: 0, }), - setValue: NOOP, + handleInputChange: NOOP, largeStep: 10, lastChangedValueRef: { current: null }, thumbMap: new Map(), @@ -23,7 +23,6 @@ const testRootContext: SliderRootContext = { min: 0, minStepsBetweenValues: 0, name: '', - onValueCommitted: NOOP, orientation: 'horizontal', state: { activeThumbIndex: -1, @@ -47,6 +46,7 @@ const testRootContext: SliderRootContext = { setDragging: NOOP, setPercentageValues: NOOP, setThumbMap: NOOP, + setValue: NOOP, step: 1, tabIndex: null, thumbRefs: { current: [] }, diff --git a/packages/react/src/slider/control/SliderControl.tsx b/packages/react/src/slider/control/SliderControl.tsx index 924072a8ba..88c37f50e4 100644 --- a/packages/react/src/slider/control/SliderControl.tsx +++ b/packages/react/src/slider/control/SliderControl.tsx @@ -21,12 +21,12 @@ const SliderControl = React.forwardRef(function SliderControl( const { render: renderProp, className, ...otherProps } = props; const { + commitValue, disabled, dragging, getFingerState, lastChangedValueRef, minStepsBetweenValues, - onValueCommitted, percentageValues, registerSliderControl, setActive, @@ -38,12 +38,12 @@ const SliderControl = React.forwardRef(function SliderControl( } = useSliderRootContext(); const { getRootProps } = useSliderControl({ + commitValue, disabled, dragging, getFingerState, lastChangedValueRef, minStepsBetweenValues, - onValueCommitted, percentageValues, registerSliderControl, rootRef: forwardedRef, diff --git a/packages/react/src/slider/control/useSliderControl.ts b/packages/react/src/slider/control/useSliderControl.ts index a2b38bbe6a..fe4939adfc 100644 --- a/packages/react/src/slider/control/useSliderControl.ts +++ b/packages/react/src/slider/control/useSliderControl.ts @@ -51,7 +51,7 @@ export function useSliderControl( getFingerState, lastChangedValueRef, minStepsBetweenValues, - onValueCommitted, + commitValue, percentageValues, registerSliderControl, rootRef: externalRef, @@ -128,11 +128,9 @@ export function useSliderControl( setActive(-1); commitValidation(lastChangedValueRef.current ?? finger.value); - - onValueCommitted(lastChangedValueRef.current ?? finger.value, nativeEvent); + commitValue(lastChangedValueRef.current ?? finger.value, nativeEvent); touchIdRef.current = null; - // eslint-disable-next-line @typescript-eslint/no-use-before-define stopListening(); }); @@ -280,7 +278,7 @@ export namespace useSliderControl { | 'getFingerState' | 'lastChangedValueRef' | 'minStepsBetweenValues' - | 'onValueCommitted' + | 'commitValue' | 'percentageValues' | 'registerSliderControl' | 'setActive' diff --git a/packages/react/src/slider/indicator/SliderIndicator.test.tsx b/packages/react/src/slider/indicator/SliderIndicator.test.tsx index b526120ccf..1e6dd8781e 100644 --- a/packages/react/src/slider/indicator/SliderIndicator.test.tsx +++ b/packages/react/src/slider/indicator/SliderIndicator.test.tsx @@ -6,7 +6,7 @@ import { NOOP } from '../../utils/noop'; const testRootContext: SliderRootContext = { active: -1, - handleInputChange: NOOP, + commitValue: NOOP, dragging: false, disabled: false, getFingerState: () => ({ @@ -15,7 +15,7 @@ const testRootContext: SliderRootContext = { percentageValues: [0], thumbIndex: 0, }), - setValue: NOOP, + handleInputChange: NOOP, largeStep: 10, lastChangedValueRef: { current: null }, thumbMap: new Map(), @@ -23,7 +23,6 @@ const testRootContext: SliderRootContext = { min: 0, minStepsBetweenValues: 0, name: '', - onValueCommitted: NOOP, orientation: 'horizontal', state: { activeThumbIndex: -1, @@ -47,6 +46,7 @@ const testRootContext: SliderRootContext = { setDragging: NOOP, setPercentageValues: NOOP, setThumbMap: NOOP, + setValue: NOOP, step: 1, tabIndex: null, thumbRefs: { current: [] }, diff --git a/packages/react/src/slider/root/useSliderRoot.ts b/packages/react/src/slider/root/useSliderRoot.ts index e667f9f939..4bcdb6e744 100644 --- a/packages/react/src/slider/root/useSliderRoot.ts +++ b/packages/react/src/slider/root/useSliderRoot.ts @@ -5,6 +5,7 @@ import { areArraysEqual } from '../../utils/areArraysEqual'; import { clamp } from '../../utils/clamp'; import { mergeReactProps } from '../../utils/mergeReactProps'; import { ownerDocument } from '../../utils/owner'; +import type { GenericHTMLProps } from '../../utils/types'; import { useControlled } from '../../utils/useControlled'; import { useEnhancedEffect } from '../../utils/useEnhancedEffect'; import { useEventCallback } from '../../utils/useEventCallback'; @@ -233,8 +234,22 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo }, ); + // for pointer drag only + const commitValue = useEventCallback((value: number | readonly number[], event: Event) => { + if (Array.isArray(value)) { + const newPercentageValues = []; + for (let i = 0; i < value.length; i += 1) { + newPercentageValues.push(valueToPercent(value[i], min, max)); + } + } else if (typeof value === 'number') { + setPercentageValues([valueToPercent(value, min, max)]); + } + onValueCommitted(value, event); + }); + const handleRootRef = useForkRef(rootRef, sliderRef); + // for keypresses only const handleInputChange = useEventCallback( (valueInput: number, index: number, event: React.KeyboardEvent | React.ChangeEvent) => { const newValue = getSliderValue(valueInput, index, min, max, range, values); @@ -375,12 +390,13 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo return React.useMemo( () => ({ - getRootProps, 'aria-labelledby': ariaLabelledby, active, + commitValue, disabled, dragging, getFingerState, + getRootProps, handleInputChange, largeStep, lastChangedValueRef, @@ -404,12 +420,13 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo values, }), [ - getRootProps, active, ariaLabelledby, + commitValue, disabled, dragging, getFingerState, + getRootProps, handleInputChange, largeStep, lastChangedValueRef, @@ -535,19 +552,15 @@ export namespace useSliderRoot { } export interface ReturnValue { - getRootProps: ( - externalProps?: React.ComponentPropsWithRef<'div'>, - ) => React.ComponentPropsWithRef<'div'>; /** * The index of the active thumb. */ active: number; 'aria-labelledby'?: string; - handleInputChange: ( - valueInput: number, - index: number, - event: React.KeyboardEvent | React.ChangeEvent, - ) => void; + /** + * Callback fired when drag ends and invokes onValueCommitted. + */ + commitValue: (newValue: number | readonly number[], event: Event) => void; dragging: boolean; disabled: boolean; getFingerState: ( @@ -555,14 +568,11 @@ export namespace useSliderRoot { shouldCaptureThumbIndex?: boolean, offset?: number, ) => FingerState | null; - /** - * Callback to invoke change handlers after internal value state is updated. - */ - setValue: ( - newValue: number | number[], - newPercentageValues: readonly number[], - activeThumb: number, - event: Event, + getRootProps: (externalProps?: GenericHTMLProps) => GenericHTMLProps; + handleInputChange: ( + valueInput: number, + index: number, + event: React.KeyboardEvent | React.ChangeEvent, ) => void; /** * The large step value of the slider when incrementing or decrementing while the shift key is held, @@ -584,23 +594,31 @@ export namespace useSliderRoot { */ minStepsBetweenValues: number; name: string; - onValueCommitted: (value: number | readonly number[], event: Event) => void; /** * The component orientation. * @default 'horizontal' */ orientation: Orientation; - registerSliderControl: (element: HTMLElement | null) => void; /** * The value(s) of the slider as percentages */ percentageValues: readonly number[]; + registerSliderControl: (element: HTMLElement | null) => void; setActive: React.Dispatch>; setDragging: React.Dispatch>; setPercentageValues: React.Dispatch>; setThumbMap: React.Dispatch< React.SetStateAction | null>> >; + /** + * Callback fired when dragging and invokes onValueChange. + */ + setValue: ( + newValue: number | number[], + newPercentageValues: readonly number[], + activeThumb: number, + event: Event, + ) => void; /** * The step increment of the slider when incrementing or decrementing. It will snap * to multiples of this value. Decimal values are supported. diff --git a/packages/react/src/slider/thumb/SliderThumb.test.tsx b/packages/react/src/slider/thumb/SliderThumb.test.tsx index ee524d2e5f..ae40617d53 100644 --- a/packages/react/src/slider/thumb/SliderThumb.test.tsx +++ b/packages/react/src/slider/thumb/SliderThumb.test.tsx @@ -35,7 +35,7 @@ function createTouches(touches: Touches) { const testRootContext: SliderRootContext = { active: -1, - handleInputChange: NOOP, + commitValue: NOOP, dragging: false, disabled: false, getFingerState: () => ({ @@ -44,7 +44,7 @@ const testRootContext: SliderRootContext = { percentageValues: [0], thumbIndex: 0, }), - setValue: NOOP, + handleInputChange: NOOP, largeStep: 10, lastChangedValueRef: { current: null }, thumbMap: new Map(), @@ -52,7 +52,6 @@ const testRootContext: SliderRootContext = { min: 0, minStepsBetweenValues: 0, name: '', - onValueCommitted: NOOP, orientation: 'horizontal', state: { activeThumbIndex: -1, @@ -76,6 +75,7 @@ const testRootContext: SliderRootContext = { setDragging: NOOP, setPercentageValues: NOOP, setThumbMap: NOOP, + setValue: NOOP, step: 1, tabIndex: null, thumbRefs: { current: [] }, diff --git a/packages/react/src/slider/track/SliderTrack.test.tsx b/packages/react/src/slider/track/SliderTrack.test.tsx index 19be29b312..c7e2d5db5e 100644 --- a/packages/react/src/slider/track/SliderTrack.test.tsx +++ b/packages/react/src/slider/track/SliderTrack.test.tsx @@ -6,7 +6,7 @@ import { NOOP } from '../../utils/noop'; const testRootContext: SliderRootContext = { active: -1, - handleInputChange: NOOP, + commitValue: NOOP, dragging: false, disabled: false, getFingerState: () => ({ @@ -15,7 +15,7 @@ const testRootContext: SliderRootContext = { percentageValues: [0], thumbIndex: 0, }), - setValue: NOOP, + handleInputChange: NOOP, largeStep: 10, lastChangedValueRef: { current: null }, thumbMap: new Map(), @@ -23,7 +23,6 @@ const testRootContext: SliderRootContext = { min: 0, minStepsBetweenValues: 0, name: '', - onValueCommitted: NOOP, orientation: 'horizontal', state: { activeThumbIndex: -1, @@ -47,6 +46,7 @@ const testRootContext: SliderRootContext = { setDragging: NOOP, setPercentageValues: NOOP, setThumbMap: NOOP, + setValue: NOOP, step: 1, tabIndex: null, thumbRefs: { current: [] }, diff --git a/packages/react/src/slider/value/SliderValue.test.tsx b/packages/react/src/slider/value/SliderValue.test.tsx index dcce0fc724..30fe947bc1 100644 --- a/packages/react/src/slider/value/SliderValue.test.tsx +++ b/packages/react/src/slider/value/SliderValue.test.tsx @@ -8,7 +8,7 @@ import { NOOP } from '../../utils/noop'; const testRootContext: SliderRootContext = { active: -1, - handleInputChange: NOOP, + commitValue: NOOP, dragging: false, disabled: false, getFingerState: () => ({ @@ -17,7 +17,7 @@ const testRootContext: SliderRootContext = { percentageValues: [0], thumbIndex: 0, }), - setValue: NOOP, + handleInputChange: NOOP, largeStep: 10, lastChangedValueRef: { current: null }, thumbMap: new Map(), @@ -25,7 +25,6 @@ const testRootContext: SliderRootContext = { min: 0, minStepsBetweenValues: 0, name: '', - onValueCommitted: NOOP, orientation: 'horizontal', state: { activeThumbIndex: -1, @@ -49,6 +48,7 @@ const testRootContext: SliderRootContext = { setDragging: NOOP, setPercentageValues: NOOP, setThumbMap: NOOP, + setValue: NOOP, step: 1, tabIndex: null, thumbRefs: { current: [] }, From 0bcdae3fddbb65aefdf30d9707622dcdb2ba4033 Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Tue, 4 Feb 2025 22:30:56 +0800 Subject: [PATCH 3/6] Add a test --- .../src/slider/thumb/SliderThumb.test.tsx | 38 ++++++++++++++++++- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/packages/react/src/slider/thumb/SliderThumb.test.tsx b/packages/react/src/slider/thumb/SliderThumb.test.tsx index ae40617d53..32d397ddfc 100644 --- a/packages/react/src/slider/thumb/SliderThumb.test.tsx +++ b/packages/react/src/slider/thumb/SliderThumb.test.tsx @@ -94,8 +94,8 @@ describe('', () => { refInstanceof: window.HTMLDivElement, })); - describe.skipIf(isJSDOM)('internal styles', () => { - it('positions the thumb when the value changes', async () => { + describe.skipIf(isJSDOM)('positioning styles', () => { + it('positions the thumb when dragged', async () => { const { getByTestId } = await render( ', () => { fireEvent.touchEnd(document.body, createTouches([{ identifier: 1, clientX: 0, clientY: 0 }])); expect(thumbStyles.getPropertyValue('left')).to.equal('200px'); }); + + it('positions the thumb when the controlled value changes externally', async () => { + function App() { + const [val, setVal] = React.useState(20); + return ( + +