From 40192eac76656dcd91d2e405021dad3314205891 Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Mon, 23 Dec 2024 18:30:54 +0800 Subject: [PATCH 01/13] useEventCallback and misc cleanup --- .../src/slider/control/SliderControl.test.tsx | 1 - .../src/slider/control/SliderControl.tsx | 2 - .../src/slider/control/useSliderControl.ts | 19 +--- .../slider/indicator/SliderIndicator.test.tsx | 1 - packages/react/src/slider/root/SliderRoot.tsx | 36 +++---- .../react/src/slider/root/useSliderRoot.ts | 98 ++++++++----------- .../src/slider/thumb/SliderThumb.test.tsx | 1 - .../src/slider/track/SliderTrack.test.tsx | 1 - .../src/slider/value/SliderValue.test.tsx | 1 - 9 files changed, 62 insertions(+), 98 deletions(-) diff --git a/packages/react/src/slider/control/SliderControl.test.tsx b/packages/react/src/slider/control/SliderControl.test.tsx index da57da3d3a..279d7d7192 100644 --- a/packages/react/src/slider/control/SliderControl.test.tsx +++ b/packages/react/src/slider/control/SliderControl.test.tsx @@ -6,7 +6,6 @@ import { NOOP } from '../../utils/noop'; const testRootContext: SliderRootContext = { active: -1, - areValuesEqual: () => true, changeValue: NOOP, direction: 'ltr', dragging: false, diff --git a/packages/react/src/slider/control/SliderControl.tsx b/packages/react/src/slider/control/SliderControl.tsx index 21e5529fa0..370c001a6e 100644 --- a/packages/react/src/slider/control/SliderControl.tsx +++ b/packages/react/src/slider/control/SliderControl.tsx @@ -21,7 +21,6 @@ const SliderControl = React.forwardRef(function SliderControl( const { render: renderProp, className, ...otherProps } = props; const { - areValuesEqual, disabled, dragging, getFingerNewValue, @@ -39,7 +38,6 @@ const SliderControl = React.forwardRef(function SliderControl( } = useSliderRootContext(); const { getRootProps } = useSliderControl({ - areValuesEqual, disabled, dragging, getFingerNewValue, diff --git a/packages/react/src/slider/control/useSliderControl.ts b/packages/react/src/slider/control/useSliderControl.ts index 28470dbae2..5beca5ada0 100644 --- a/packages/react/src/slider/control/useSliderControl.ts +++ b/packages/react/src/slider/control/useSliderControl.ts @@ -19,7 +19,6 @@ export function useSliderControl( parameters: useSliderControl.Parameters, ): useSliderControl.ReturnValue { const { - areValuesEqual, disabled, dragging, getFingerNewValue, @@ -90,9 +89,7 @@ export function useSliderControl( setDragging(true); } - if (handleValueChange && !areValuesEqual(newValue)) { - handleValueChange(newValue, activeIndex, nativeEvent); - } + handleValueChange(newValue, activeIndex, nativeEvent); } }); @@ -117,9 +114,7 @@ export function useSliderControl( commitValidation(newFingerValue.newValue); - if (onValueCommitted) { - onValueCommitted(newFingerValue.newValue, nativeEvent); - } + onValueCommitted(newFingerValue.newValue, nativeEvent); touchIdRef.current = null; @@ -154,9 +149,7 @@ export function useSliderControl( setValueState(newValue); - if (handleValueChange && !areValuesEqual(newValue)) { - handleValueChange(newValue, activeIndex, nativeEvent); - } + handleValueChange(newValue, activeIndex, nativeEvent); } moveCountRef.current = 0; @@ -245,9 +238,7 @@ export function useSliderControl( } else { setValueState(newValue); - if (handleValueChange && !areValuesEqual(newValue)) { - handleValueChange(newValue, activeIndex, event); - } + handleValueChange(newValue, activeIndex, event.nativeEvent); } } @@ -260,7 +251,6 @@ export function useSliderControl( }); }, [ - areValuesEqual, disabled, getFingerNewValue, handleRootRef, @@ -286,7 +276,6 @@ export namespace useSliderControl { export interface Parameters extends Pick< useSliderRoot.ReturnValue, - | 'areValuesEqual' | 'disabled' | 'dragging' | 'getFingerNewValue' diff --git a/packages/react/src/slider/indicator/SliderIndicator.test.tsx b/packages/react/src/slider/indicator/SliderIndicator.test.tsx index fa84166aba..381f0f381f 100644 --- a/packages/react/src/slider/indicator/SliderIndicator.test.tsx +++ b/packages/react/src/slider/indicator/SliderIndicator.test.tsx @@ -6,7 +6,6 @@ import { NOOP } from '../../utils/noop'; const testRootContext: SliderRootContext = { active: -1, - areValuesEqual: () => true, changeValue: NOOP, direction: 'ltr', dragging: false, diff --git a/packages/react/src/slider/root/SliderRoot.tsx b/packages/react/src/slider/root/SliderRoot.tsx index 22e18422f0..bb5d25c840 100644 --- a/packages/react/src/slider/root/SliderRoot.tsx +++ b/packages/react/src/slider/root/SliderRoot.tsx @@ -1,6 +1,7 @@ 'use client'; import * as React from 'react'; import PropTypes from 'prop-types'; +import { NOOP } from '../../utils/noop'; import type { BaseUIComponentProps } from '../../utils/types'; import { useComponentRenderer } from '../../utils/useComponentRenderer'; import type { FieldRoot } from '../../field/root/FieldRoot'; @@ -34,8 +35,8 @@ const SliderRoot = React.forwardRef(function SliderRoot( min, minStepsBetweenValues, name, - onValueChange, - onValueCommitted, + onValueChange: onValueChangeProp, + onValueCommitted: onValueCommittedProp, orientation = 'horizontal', step, tabIndex, @@ -59,8 +60,8 @@ const SliderRoot = React.forwardRef(function SliderRoot( min, minStepsBetweenValues, name, - onValueChange, - onValueCommitted, + onValueChange: onValueChangeProp ?? NOOP, + onValueCommitted: onValueCommittedProp ?? NOOP, orientation, rootRef: forwardedRef, step, @@ -158,19 +159,20 @@ export namespace SliderRoot { } export interface Props - extends Pick< - useSliderRoot.Parameters, - | 'disabled' - | 'max' - | 'min' - | 'minStepsBetweenValues' - | 'name' - | 'onValueChange' - | 'onValueCommitted' - | 'orientation' - | 'largeStep' - | 'step' - | 'value' + extends Partial< + Pick< + useSliderRoot.Parameters, + | 'disabled' + | 'max' + | 'min' + | 'minStepsBetweenValues' + | 'name' + | 'onValueChange' + | 'onValueCommitted' + | 'orientation' + | 'largeStep' + | 'step' + > >, Omit, 'defaultValue' | 'onChange' | 'values'> { /** diff --git a/packages/react/src/slider/root/useSliderRoot.ts b/packages/react/src/slider/root/useSliderRoot.ts index 6860b7a608..2fb8e163ac 100644 --- a/packages/react/src/slider/root/useSliderRoot.ts +++ b/packages/react/src/slider/root/useSliderRoot.ts @@ -5,10 +5,11 @@ import { areArraysEqual } from '../../utils/areArraysEqual'; import { clamp } from '../../utils/clamp'; import { mergeReactProps } from '../../utils/mergeReactProps'; import { ownerDocument } from '../../utils/owner'; +import { useBaseUiId } from '../../utils/useBaseUiId'; import { useControlled } from '../../utils/useControlled'; import { useEnhancedEffect } from '../../utils/useEnhancedEffect'; +import { useEventCallback } from '../../utils/useEventCallback'; import { useForkRef } from '../../utils/useForkRef'; -import { useBaseUiId } from '../../utils/useBaseUiId'; import { valueToPercent } from '../../utils/valueToPercent'; import type { CompositeMetadata } from '../../composite/list/CompositeList'; import type { TextDirection } from '../../direction-provider/DirectionContext'; @@ -21,6 +22,19 @@ import { setValueIndex } from '../utils/setValueIndex'; import { getSliderValue } from '../utils/getSliderValue'; import { ThumbMetadata } from '../thumb/useSliderThumb'; +function areValuesEqual( + newValue: number | readonly number[], + oldValue: number | readonly number[], +) { + if (typeof newValue === 'number' && typeof oldValue === 'number') { + return newValue === oldValue; + } + if (Array.isArray(newValue) && Array.isArray(oldValue)) { + return areArraysEqual(newValue, oldValue); + } + return false; +} + function findClosest(values: number[], currentValue: number) { const { index: closestIndex } = values.reduce<{ distance: number; index: number } | null>( @@ -143,6 +157,8 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo commitValidation, } = useFieldControlValidation(); + // The internal valueState is potentially unsorted, e.g. to support frozen arrays + // https://github.com/mui/material-ui/pull/28472 const [valueState, setValueState] = useControlled({ controlled: valueProp, default: defaultValue ?? min, @@ -190,9 +206,9 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo [inputValidationRef], ); - const handleValueChange = React.useCallback( - (value: number | number[], thumbIndex: number, event: Event | React.SyntheticEvent) => { - if (!onValueChange) { + const handleValueChange = useEventCallback( + (newValue: number | readonly number[], thumbIndex: number, event: Event) => { + if (areValuesEqual(newValue, valueState)) { return; } @@ -200,18 +216,16 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo // This allows seamless integration with the most popular form libraries. // https://github.com/mui/material-ui/issues/13485#issuecomment-676048492 // Clone the event to not override `target` of the original event. - const nativeEvent = (event as React.SyntheticEvent).nativeEvent || event; // @ts-ignore The nativeEvent is function, not object - const clonedEvent = new nativeEvent.constructor(nativeEvent.type, nativeEvent); + const clonedEvent = new event.constructor(event.type, event); Object.defineProperty(clonedEvent, 'target', { writable: true, - value: { value, name }, + value: { value: newValue, name }, }); - onValueChange(value, clonedEvent, thumbIndex); + onValueChange(newValue, clonedEvent, thumbIndex); }, - [name, onValueChange], ); const range = Array.isArray(valueState); @@ -224,20 +238,7 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo const handleRootRef = useForkRef(rootRef, sliderRef); - const areValuesEqual = React.useCallback( - (newValue: number | ReadonlyArray): boolean => { - if (typeof newValue === 'number' && typeof valueState === 'number') { - return newValue === valueState; - } - if (typeof newValue === 'object' && typeof valueState === 'object') { - return areArraysEqual(newValue, valueState); - } - return false; - }, - [valueState], - ); - - const changeValue = React.useCallback( + const changeValue = useEventCallback( (valueInput: number, index: number, event: React.KeyboardEvent | React.ChangeEvent) => { const newValue = getSliderValue({ valueInput, @@ -256,39 +257,19 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo setValueState(newValue); setDirty(newValue !== validityData.initialValue); - if (handleValueChange && !areValuesEqual(newValue) && event) { - handleValueChange(newValue, index, event); - } + handleValueChange(newValue, index, event.nativeEvent); setTouched(true); commitValidation(newValue); - if (onValueCommitted && event) { - onValueCommitted(newValue, event.nativeEvent); - } + onValueCommitted(newValue, event.nativeEvent); } }, - [ - min, - max, - range, - step, - minStepsBetweenValues, - values, - setValueState, - setDirty, - validityData.initialValue, - handleValueChange, - areValuesEqual, - onValueCommitted, - setTouched, - commitValidation, - ], ); const previousIndexRef = React.useRef(null); - const getFingerNewValue = React.useCallback( + const getFingerNewValue = useEventCallback( ({ finger, move = false, @@ -364,7 +345,6 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo return { newValue, activeIndex, newPercentageValue: percent }; }, - [direction, max, min, minStepsBetweenValues, orientation, range, step, values], ); useEnhancedEffect(() => { @@ -397,7 +377,6 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo () => ({ getRootProps, active, - areValuesEqual, 'aria-labelledby': ariaLabelledby, changeValue, direction, @@ -428,7 +407,6 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo [ getRootProps, active, - areValuesEqual, ariaLabelledby, changeValue, direction, @@ -513,7 +491,11 @@ export namespace useSliderRoot { * You can pull out the new value by accessing `event.target.value` (any). * @param {number} activeThumbIndex Index of the currently moved thumb. */ - onValueChange?: (value: number | number[], event: Event, activeThumbIndex: number) => void; + onValueChange: ( + value: number | readonly number[], + event: Event, + activeThumbIndex: number, + ) => void; /** * Callback function that is fired when the `pointerup` is triggered. * @@ -521,7 +503,7 @@ export namespace useSliderRoot { * @param {Event} event The corresponding event that initiated the change. * **Warning**: This is a generic event not a change event. */ - onValueCommitted?: (value: number | number[], event: Event) => void; + onValueCommitted: (value: number | number[], event: Event) => void; /** * The component orientation. * @default 'horizontal' @@ -562,11 +544,6 @@ export namespace useSliderRoot { * The index of the active thumb. */ active: number; - /** - * A function that compares a new value with the internal value of the slider. - * The internal value is potentially unsorted, e.g. to support frozen arrays: https://github.com/mui/material-ui/pull/28472 - */ - areValuesEqual: (newValue: number | ReadonlyArray) => boolean; 'aria-labelledby'?: string; changeValue: ( valueInput: number, @@ -582,10 +559,13 @@ export namespace useSliderRoot { offset?: number; activeIndex?: number; }) => { newValue: number | number[]; activeIndex: number; newPercentageValue: number } | null; + /** + * Callback to invoke change handlers after internal value state is updated. + */ handleValueChange: ( - value: number | number[], + newValue: number | readonly number[], activeThumb: number, - event: React.SyntheticEvent | Event, + event: Event, ) => void; /** * The large step value of the slider when incrementing or decrementing while the shift key is held, @@ -606,7 +586,7 @@ export namespace useSliderRoot { */ minStepsBetweenValues: number; name?: string; - onValueCommitted?: (value: number | number[], event: Event) => void; + onValueCommitted: (value: number | number[], event: Event) => void; /** * The component orientation. * @default 'horizontal' diff --git a/packages/react/src/slider/thumb/SliderThumb.test.tsx b/packages/react/src/slider/thumb/SliderThumb.test.tsx index b7549e245a..abfc3ae07f 100644 --- a/packages/react/src/slider/thumb/SliderThumb.test.tsx +++ b/packages/react/src/slider/thumb/SliderThumb.test.tsx @@ -6,7 +6,6 @@ import { NOOP } from '../../utils/noop'; const testRootContext: SliderRootContext = { active: -1, - areValuesEqual: () => true, changeValue: NOOP, direction: 'ltr', dragging: false, diff --git a/packages/react/src/slider/track/SliderTrack.test.tsx b/packages/react/src/slider/track/SliderTrack.test.tsx index e53b90b198..bf468ef4ab 100644 --- a/packages/react/src/slider/track/SliderTrack.test.tsx +++ b/packages/react/src/slider/track/SliderTrack.test.tsx @@ -6,7 +6,6 @@ import { NOOP } from '../../utils/noop'; const testRootContext: SliderRootContext = { active: -1, - areValuesEqual: () => true, changeValue: NOOP, direction: 'ltr', dragging: false, diff --git a/packages/react/src/slider/value/SliderValue.test.tsx b/packages/react/src/slider/value/SliderValue.test.tsx index cb340a21ea..6f244d87fd 100644 --- a/packages/react/src/slider/value/SliderValue.test.tsx +++ b/packages/react/src/slider/value/SliderValue.test.tsx @@ -8,7 +8,6 @@ import { NOOP } from '../../utils/noop'; const testRootContext: SliderRootContext = { active: -1, - areValuesEqual: () => true, changeValue: NOOP, direction: 'ltr', dragging: false, From 8d6b4981b282136153d1cbbbb32006b123b1b5c9 Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Mon, 23 Dec 2024 19:35:17 +0800 Subject: [PATCH 02/13] setValueState no longer needed in context --- packages/react/src/slider/control/SliderControl.test.tsx | 1 - packages/react/src/slider/control/SliderControl.tsx | 2 -- packages/react/src/slider/control/useSliderControl.ts | 9 --------- .../react/src/slider/indicator/SliderIndicator.test.tsx | 1 - packages/react/src/slider/root/useSliderRoot.ts | 9 +++------ packages/react/src/slider/thumb/SliderThumb.test.tsx | 1 - packages/react/src/slider/track/SliderTrack.test.tsx | 1 - packages/react/src/slider/value/SliderValue.test.tsx | 1 - 8 files changed, 3 insertions(+), 22 deletions(-) diff --git a/packages/react/src/slider/control/SliderControl.test.tsx b/packages/react/src/slider/control/SliderControl.test.tsx index 279d7d7192..30a4886a8d 100644 --- a/packages/react/src/slider/control/SliderControl.test.tsx +++ b/packages/react/src/slider/control/SliderControl.test.tsx @@ -41,7 +41,6 @@ const testRootContext: SliderRootContext = { setActive: NOOP, setDragging: NOOP, setThumbMap: NOOP, - setValueState: NOOP, step: 1, thumbRefs: { current: [] }, values: [0], diff --git a/packages/react/src/slider/control/SliderControl.tsx b/packages/react/src/slider/control/SliderControl.tsx index 370c001a6e..a372081f2d 100644 --- a/packages/react/src/slider/control/SliderControl.tsx +++ b/packages/react/src/slider/control/SliderControl.tsx @@ -32,7 +32,6 @@ const SliderControl = React.forwardRef(function SliderControl( registerSliderControl, setActive, setDragging, - setValueState, step, thumbRefs, } = useSliderRootContext(); @@ -49,7 +48,6 @@ const SliderControl = React.forwardRef(function SliderControl( rootRef: forwardedRef, setActive, setDragging, - setValueState, step, thumbRefs, }); diff --git a/packages/react/src/slider/control/useSliderControl.ts b/packages/react/src/slider/control/useSliderControl.ts index 5beca5ada0..d63b917464 100644 --- a/packages/react/src/slider/control/useSliderControl.ts +++ b/packages/react/src/slider/control/useSliderControl.ts @@ -30,7 +30,6 @@ export function useSliderControl( rootRef: externalRef, setActive, setDragging, - setValueState, step, thumbRefs, } = parameters; @@ -83,8 +82,6 @@ export function useSliderControl( focusThumb({ sliderRef: controlRef, activeIndex, setActive }); if (validateMinimumDistance(newValue, step, minStepsBetweenValues)) { - setValueState(newValue); - if (!dragging && moveCountRef.current > INTENTIONAL_DRAG_COUNT_THRESHOLD) { setDragging(true); } @@ -147,8 +144,6 @@ export function useSliderControl( focusThumb({ sliderRef: controlRef, activeIndex, setActive }); - setValueState(newValue); - handleValueChange(newValue, activeIndex, nativeEvent); } @@ -236,8 +231,6 @@ export function useSliderControl( offsetRef.current = offset; } else { - setValueState(newValue); - handleValueChange(newValue, activeIndex, event.nativeEvent); } } @@ -259,7 +252,6 @@ export function useSliderControl( handleValueChange, percentageValues, setActive, - setValueState, thumbRefs, ], ); @@ -286,7 +278,6 @@ export namespace useSliderControl { | 'registerSliderControl' | 'setActive' | 'setDragging' - | 'setValueState' | 'step' | 'thumbRefs' > { diff --git a/packages/react/src/slider/indicator/SliderIndicator.test.tsx b/packages/react/src/slider/indicator/SliderIndicator.test.tsx index 381f0f381f..9e000505c1 100644 --- a/packages/react/src/slider/indicator/SliderIndicator.test.tsx +++ b/packages/react/src/slider/indicator/SliderIndicator.test.tsx @@ -41,7 +41,6 @@ const testRootContext: SliderRootContext = { setActive: NOOP, setDragging: NOOP, setThumbMap: NOOP, - setValueState: NOOP, step: 1, thumbRefs: { current: [] }, values: [0], diff --git a/packages/react/src/slider/root/useSliderRoot.ts b/packages/react/src/slider/root/useSliderRoot.ts index 2fb8e163ac..7cd372d0ad 100644 --- a/packages/react/src/slider/root/useSliderRoot.ts +++ b/packages/react/src/slider/root/useSliderRoot.ts @@ -212,6 +212,8 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo return; } + setValueState(newValue); + // Redefine target to allow name and value to be read. // This allows seamless integration with the most popular form libraries. // https://github.com/mui/material-ui/issues/13485#issuecomment-676048492 @@ -254,10 +256,8 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo } if (validateMinimumDistance(newValue, step, minStepsBetweenValues)) { - setValueState(newValue); - setDirty(newValue !== validityData.initialValue); - handleValueChange(newValue, index, event.nativeEvent); + setDirty(newValue !== validityData.initialValue); setTouched(true); commitValidation(newValue); @@ -397,7 +397,6 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo setActive, setDragging, setThumbMap, - setValueState, step, tabIndex, thumbMap, @@ -426,7 +425,6 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo setActive, setDragging, setThumbMap, - setValueState, step, tabIndex, thumbMap, @@ -600,7 +598,6 @@ export namespace useSliderRoot { setActive: (activeIndex: number) => void; setDragging: (isDragging: boolean) => void; setThumbMap: (map: Map | null>) => void; - setValueState: (newValue: number | number[]) => 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 abfc3ae07f..ba11c426e6 100644 --- a/packages/react/src/slider/thumb/SliderThumb.test.tsx +++ b/packages/react/src/slider/thumb/SliderThumb.test.tsx @@ -41,7 +41,6 @@ const testRootContext: SliderRootContext = { setActive: NOOP, setDragging: NOOP, setThumbMap: NOOP, - setValueState: NOOP, step: 1, thumbRefs: { current: [] }, values: [0], diff --git a/packages/react/src/slider/track/SliderTrack.test.tsx b/packages/react/src/slider/track/SliderTrack.test.tsx index bf468ef4ab..3a1ee48ff5 100644 --- a/packages/react/src/slider/track/SliderTrack.test.tsx +++ b/packages/react/src/slider/track/SliderTrack.test.tsx @@ -41,7 +41,6 @@ const testRootContext: SliderRootContext = { setActive: NOOP, setDragging: NOOP, setThumbMap: NOOP, - setValueState: NOOP, step: 1, thumbRefs: { current: [] }, values: [0], diff --git a/packages/react/src/slider/value/SliderValue.test.tsx b/packages/react/src/slider/value/SliderValue.test.tsx index 6f244d87fd..85aca41db5 100644 --- a/packages/react/src/slider/value/SliderValue.test.tsx +++ b/packages/react/src/slider/value/SliderValue.test.tsx @@ -43,7 +43,6 @@ const testRootContext: SliderRootContext = { setActive: NOOP, setDragging: NOOP, setThumbMap: NOOP, - setValueState: NOOP, step: 1, thumbRefs: { current: [] }, values: [0], From 5ef82447621c7aef1c153e06fe402b2c9ef369fe Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Mon, 23 Dec 2024 22:49:58 +0800 Subject: [PATCH 03/13] Better internal naming --- .../src/slider/control/SliderControl.test.tsx | 5 +- .../src/slider/control/SliderControl.tsx | 4 +- .../src/slider/control/useSliderControl.ts | 63 +++++++--------- .../slider/indicator/SliderIndicator.test.tsx | 5 +- .../react/src/slider/root/useSliderRoot.ts | 74 ++++++++++--------- .../src/slider/thumb/SliderThumb.test.tsx | 5 +- .../src/slider/track/SliderTrack.test.tsx | 5 +- .../src/slider/value/SliderValue.test.tsx | 5 +- 8 files changed, 83 insertions(+), 83 deletions(-) diff --git a/packages/react/src/slider/control/SliderControl.test.tsx b/packages/react/src/slider/control/SliderControl.test.tsx index 30a4886a8d..9ff9a342d5 100644 --- a/packages/react/src/slider/control/SliderControl.test.tsx +++ b/packages/react/src/slider/control/SliderControl.test.tsx @@ -10,9 +10,9 @@ const testRootContext: SliderRootContext = { direction: 'ltr', dragging: false, disabled: false, - getFingerNewValue: () => ({ + getValueAtFinger: () => ({ newValue: 0, - activeIndex: 0, + closestThumbIndex: 0, newPercentageValue: 0, }), handleValueChange: NOOP, @@ -21,6 +21,7 @@ const testRootContext: SliderRootContext = { max: 100, min: 0, minStepsBetweenValues: 0, + onValueCommitted: NOOP, orientation: 'horizontal', state: { activeThumbIndex: -1, diff --git a/packages/react/src/slider/control/SliderControl.tsx b/packages/react/src/slider/control/SliderControl.tsx index a372081f2d..28e8cb3bd5 100644 --- a/packages/react/src/slider/control/SliderControl.tsx +++ b/packages/react/src/slider/control/SliderControl.tsx @@ -23,7 +23,7 @@ const SliderControl = React.forwardRef(function SliderControl( const { disabled, dragging, - getFingerNewValue, + getValueAtFinger, handleValueChange, minStepsBetweenValues, onValueCommitted, @@ -39,7 +39,7 @@ const SliderControl = React.forwardRef(function SliderControl( const { getRootProps } = useSliderControl({ disabled, dragging, - getFingerNewValue, + getValueAtFinger, handleValueChange, minStepsBetweenValues, onValueCommitted, diff --git a/packages/react/src/slider/control/useSliderControl.ts b/packages/react/src/slider/control/useSliderControl.ts index d63b917464..bc1d8a8ccf 100644 --- a/packages/react/src/slider/control/useSliderControl.ts +++ b/packages/react/src/slider/control/useSliderControl.ts @@ -8,8 +8,8 @@ import { useEventCallback } from '../../utils/useEventCallback'; import { focusThumb, trackFinger, - type useSliderRoot, validateMinimumDistance, + type useSliderRoot, } from '../root/useSliderRoot'; import { useFieldControlValidation } from '../../field/control/useFieldControlValidation'; @@ -21,7 +21,7 @@ export function useSliderControl( const { disabled, dragging, - getFingerNewValue, + getValueAtFinger, handleValueChange, onValueCommitted, minStepsBetweenValues, @@ -51,9 +51,9 @@ export function useSliderControl( const offsetRef = React.useRef(0); const handleTouchMove = useEventCallback((nativeEvent: TouchEvent | PointerEvent) => { - const finger = trackFinger(nativeEvent, touchIdRef); + const fingerPosition = trackFinger(nativeEvent, touchIdRef); - if (!finger) { + if (fingerPosition == null) { return; } @@ -67,41 +67,34 @@ export function useSliderControl( return; } - const newFingerValue = getFingerNewValue({ - finger, - move: true, - offset: offsetRef.current, - }); + const newFingerValue = getValueAtFinger(fingerPosition, false, offsetRef.current); if (!newFingerValue) { return; } - const { newValue, activeIndex } = newFingerValue; + const { newValue, closestThumbIndex } = newFingerValue; - focusThumb({ sliderRef: controlRef, activeIndex, setActive }); + focusThumb({ sliderRef: controlRef, activeIndex: closestThumbIndex, setActive }); if (validateMinimumDistance(newValue, step, minStepsBetweenValues)) { if (!dragging && moveCountRef.current > INTENTIONAL_DRAG_COUNT_THRESHOLD) { setDragging(true); } - handleValueChange(newValue, activeIndex, nativeEvent); + handleValueChange(newValue, closestThumbIndex, nativeEvent); } }); const handleTouchEnd = useEventCallback((nativeEvent: TouchEvent | PointerEvent) => { - const finger = trackFinger(nativeEvent, touchIdRef); + const fingerPosition = trackFinger(nativeEvent, touchIdRef); setDragging(false); - if (!finger) { + if (fingerPosition == null) { return; } - const newFingerValue = getFingerNewValue({ - finger, - move: true, - }); + const newFingerValue = getValueAtFinger(fingerPosition, true); if (!newFingerValue) { return; @@ -130,21 +123,19 @@ export function useSliderControl( touchIdRef.current = touch.identifier; } - const finger = trackFinger(nativeEvent, touchIdRef); + const fingerPosition = trackFinger(nativeEvent, touchIdRef); - if (finger !== false) { - const newFingerValue = getFingerNewValue({ - finger, - }); + if (fingerPosition != null) { + const newFingerValue = getValueAtFinger(fingerPosition); if (!newFingerValue) { return; } - const { newValue, activeIndex } = newFingerValue; + const { newValue, closestThumbIndex } = newFingerValue; - focusThumb({ sliderRef: controlRef, activeIndex, setActive }); + focusThumb({ sliderRef: controlRef, activeIndex: closestThumbIndex, setActive }); - handleValueChange(newValue, activeIndex, nativeEvent); + handleValueChange(newValue, closestThumbIndex, nativeEvent); } moveCountRef.current = 0; @@ -206,20 +197,18 @@ export function useSliderControl( // Avoid text selection event.preventDefault(); - const finger = trackFinger(event, touchIdRef); + const fingerPosition = trackFinger(event, touchIdRef); - if (finger !== false) { - const newFingerValue = getFingerNewValue({ - finger, - }); + if (fingerPosition != null) { + const newFingerValue = getValueAtFinger(fingerPosition); - if (!newFingerValue) { + if (newFingerValue == null) { return; } - const { newValue, activeIndex, newPercentageValue } = newFingerValue; + const { newValue, closestThumbIndex, newPercentageValue } = newFingerValue; - focusThumb({ sliderRef: controlRef, activeIndex, setActive }); + focusThumb({ sliderRef: controlRef, activeIndex: closestThumbIndex, setActive }); // if the event lands on a thumb, don't change the value, just get the // percentageValue difference represented by the distance between the click origin @@ -231,7 +220,7 @@ export function useSliderControl( offsetRef.current = offset; } else { - handleValueChange(newValue, activeIndex, event.nativeEvent); + handleValueChange(newValue, closestThumbIndex, event.nativeEvent); } } @@ -245,7 +234,7 @@ export function useSliderControl( }, [ disabled, - getFingerNewValue, + getValueAtFinger, handleRootRef, handleTouchMove, handleTouchEnd, @@ -270,7 +259,7 @@ export namespace useSliderControl { useSliderRoot.ReturnValue, | 'disabled' | 'dragging' - | 'getFingerNewValue' + | 'getValueAtFinger' | 'handleValueChange' | 'minStepsBetweenValues' | 'onValueCommitted' diff --git a/packages/react/src/slider/indicator/SliderIndicator.test.tsx b/packages/react/src/slider/indicator/SliderIndicator.test.tsx index 9e000505c1..b8aee8db08 100644 --- a/packages/react/src/slider/indicator/SliderIndicator.test.tsx +++ b/packages/react/src/slider/indicator/SliderIndicator.test.tsx @@ -10,9 +10,9 @@ const testRootContext: SliderRootContext = { direction: 'ltr', dragging: false, disabled: false, - getFingerNewValue: () => ({ + getValueAtFinger: () => ({ newValue: 0, - activeIndex: 0, + closestThumbIndex: 0, newPercentageValue: 0, }), handleValueChange: NOOP, @@ -21,6 +21,7 @@ const testRootContext: SliderRootContext = { max: 100, min: 0, minStepsBetweenValues: 0, + onValueCommitted: NOOP, orientation: 'horizontal', state: { activeThumbIndex: -1, diff --git a/packages/react/src/slider/root/useSliderRoot.ts b/packages/react/src/slider/root/useSliderRoot.ts index 7cd372d0ad..1358974387 100644 --- a/packages/react/src/slider/root/useSliderRoot.ts +++ b/packages/react/src/slider/root/useSliderRoot.ts @@ -102,7 +102,7 @@ export function validateMinimumDistance( export function trackFinger( event: TouchEvent | PointerEvent | React.PointerEvent, touchIdRef: React.RefObject, -) { +): FingerPosition | null { // The event is TouchEvent if (touchIdRef.current !== undefined && (event as TouchEvent).changedTouches) { const touchEvent = event as TouchEvent; @@ -116,7 +116,7 @@ export function trackFinger( } } - return false; + return null; } // The event is PointerEvent @@ -258,10 +258,8 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo if (validateMinimumDistance(newValue, step, minStepsBetweenValues)) { handleValueChange(newValue, index, event.nativeEvent); setDirty(newValue !== validityData.initialValue); - setTouched(true); commitValidation(newValue); - onValueCommitted(newValue, event.nativeEvent); } }, @@ -269,17 +267,17 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo const previousIndexRef = React.useRef(null); - const getFingerNewValue = useEventCallback( - ({ - finger, - move = false, - offset = 0, - }: { - finger: { x: number; y: number }; + const getValueAtFinger = useEventCallback( + ( + fingerPosition: FingerPosition | null, // `move` is used to distinguish between when this is called by touchstart vs touchmove/end - move?: boolean; - offset?: number; - }) => { + move: boolean = false, + offset: number = 0, + ) => { + if (fingerPosition == null) { + return null; + } + const { current: sliderControl } = controlRef; if (!sliderControl) { @@ -293,9 +291,9 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo let percent; if (isVertical) { - percent = (bottom - finger.y) / height + offset; + percent = (bottom - fingerPosition.y) / height + offset; } else { - percent = (finger.x - left) / width + offset * (isRtl ? -1 : 1); + percent = (fingerPosition.x - left) / width + offset * (isRtl ? -1 : 1); } percent = Math.min(percent, 1); @@ -311,39 +309,39 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo } newValue = clamp(newValue, min, max); - let activeIndex = 0; + let closestThumbIndex = 0; if (!range) { - return { newValue, activeIndex, newPercentageValue: percent }; + return { newValue, closestThumbIndex, newPercentageValue: percent }; } if (!move) { - activeIndex = findClosest(values, newValue)!; + closestThumbIndex = findClosest(values, newValue)!; } else { - activeIndex = previousIndexRef.current!; + closestThumbIndex = previousIndexRef.current!; } // Bound the new value to the thumb's neighbours. newValue = clamp( newValue, - values[activeIndex - 1] + minStepsBetweenValues || -Infinity, - values[activeIndex + 1] - minStepsBetweenValues || Infinity, + values[closestThumbIndex - 1] + minStepsBetweenValues || -Infinity, + values[closestThumbIndex + 1] - minStepsBetweenValues || Infinity, ); const previousValue = newValue; newValue = setValueIndex({ values, newValue, - index: activeIndex, + index: closestThumbIndex, }); // Potentially swap the index if needed. if (!move) { - activeIndex = newValue.indexOf(previousValue); - previousIndexRef.current = activeIndex; + closestThumbIndex = newValue.indexOf(previousValue); + previousIndexRef.current = closestThumbIndex; } - return { newValue, activeIndex, newPercentageValue: percent }; + return { newValue, closestThumbIndex, newPercentageValue: percent }; }, ); @@ -382,7 +380,7 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo direction, disabled, dragging, - getFingerNewValue, + getValueAtFinger, handleValueChange, largeStep, max, @@ -411,7 +409,7 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo direction, disabled, dragging, - getFingerNewValue, + getValueAtFinger, handleValueChange, largeStep, max, @@ -434,6 +432,11 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo ); } +export type FingerPosition = { + x: number; + y: number; +}; + export namespace useSliderRoot { export type Orientation = 'horizontal' | 'vertical'; @@ -551,12 +554,15 @@ export namespace useSliderRoot { dragging: boolean; direction: TextDirection; disabled: boolean; - getFingerNewValue: (args: { - finger: { x: number; y: number }; - move?: boolean; - offset?: number; - activeIndex?: number; - }) => { newValue: number | number[]; activeIndex: number; newPercentageValue: number } | null; + getValueAtFinger: ( + fingerPosition: FingerPosition | null, + move?: boolean, + offset?: number, + ) => { + newValue: number | number[]; + closestThumbIndex: number; + newPercentageValue: number; + } | null; /** * Callback to invoke change handlers after internal value state is updated. */ diff --git a/packages/react/src/slider/thumb/SliderThumb.test.tsx b/packages/react/src/slider/thumb/SliderThumb.test.tsx index ba11c426e6..a948273c0b 100644 --- a/packages/react/src/slider/thumb/SliderThumb.test.tsx +++ b/packages/react/src/slider/thumb/SliderThumb.test.tsx @@ -10,9 +10,9 @@ const testRootContext: SliderRootContext = { direction: 'ltr', dragging: false, disabled: false, - getFingerNewValue: () => ({ + getValueAtFinger: () => ({ newValue: 0, - activeIndex: 0, + closestThumbIndex: 0, newPercentageValue: 0, }), handleValueChange: NOOP, @@ -21,6 +21,7 @@ const testRootContext: SliderRootContext = { max: 100, min: 0, minStepsBetweenValues: 0, + onValueCommitted: NOOP, orientation: 'horizontal', state: { activeThumbIndex: -1, diff --git a/packages/react/src/slider/track/SliderTrack.test.tsx b/packages/react/src/slider/track/SliderTrack.test.tsx index 3a1ee48ff5..f4982d1055 100644 --- a/packages/react/src/slider/track/SliderTrack.test.tsx +++ b/packages/react/src/slider/track/SliderTrack.test.tsx @@ -10,9 +10,9 @@ const testRootContext: SliderRootContext = { direction: 'ltr', dragging: false, disabled: false, - getFingerNewValue: () => ({ + getValueAtFinger: () => ({ newValue: 0, - activeIndex: 0, + closestThumbIndex: 0, newPercentageValue: 0, }), handleValueChange: NOOP, @@ -21,6 +21,7 @@ const testRootContext: SliderRootContext = { max: 100, min: 0, minStepsBetweenValues: 0, + onValueCommitted: NOOP, orientation: 'horizontal', state: { activeThumbIndex: -1, diff --git a/packages/react/src/slider/value/SliderValue.test.tsx b/packages/react/src/slider/value/SliderValue.test.tsx index 85aca41db5..eee12bda60 100644 --- a/packages/react/src/slider/value/SliderValue.test.tsx +++ b/packages/react/src/slider/value/SliderValue.test.tsx @@ -12,9 +12,9 @@ const testRootContext: SliderRootContext = { direction: 'ltr', dragging: false, disabled: false, - getFingerNewValue: () => ({ + getValueAtFinger: () => ({ newValue: 0, - activeIndex: 0, + closestThumbIndex: 0, newPercentageValue: 0, }), handleValueChange: NOOP, @@ -23,6 +23,7 @@ const testRootContext: SliderRootContext = { max: 100, min: 0, minStepsBetweenValues: 0, + onValueCommitted: NOOP, orientation: 'horizontal', state: { activeThumbIndex: -1, From 287319e3fc28e42d24f02bcbbb18489d0f19472a Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Mon, 23 Dec 2024 23:01:32 +0800 Subject: [PATCH 04/13] Avoid objects and misc improvements --- .../src/slider/control/SliderControl.test.tsx | 6 +-- .../src/slider/control/SliderControl.tsx | 4 +- .../src/slider/control/useSliderControl.ts | 48 +++++++++--------- .../slider/indicator/SliderIndicator.test.tsx | 6 +-- .../react/src/slider/root/useSliderRoot.ts | 49 ++++++++++--------- .../src/slider/thumb/SliderThumb.test.tsx | 6 +-- .../src/slider/track/SliderTrack.test.tsx | 6 +-- .../src/slider/value/SliderValue.test.tsx | 6 +-- 8 files changed, 66 insertions(+), 65 deletions(-) diff --git a/packages/react/src/slider/control/SliderControl.test.tsx b/packages/react/src/slider/control/SliderControl.test.tsx index 9ff9a342d5..3186c66785 100644 --- a/packages/react/src/slider/control/SliderControl.test.tsx +++ b/packages/react/src/slider/control/SliderControl.test.tsx @@ -10,10 +10,10 @@ const testRootContext: SliderRootContext = { direction: 'ltr', dragging: false, disabled: false, - getValueAtFinger: () => ({ - newValue: 0, + getFingerState: () => ({ + value: 0, closestThumbIndex: 0, - newPercentageValue: 0, + percentageValue: 0, }), handleValueChange: NOOP, largeStep: 10, diff --git a/packages/react/src/slider/control/SliderControl.tsx b/packages/react/src/slider/control/SliderControl.tsx index 28e8cb3bd5..1f9c527978 100644 --- a/packages/react/src/slider/control/SliderControl.tsx +++ b/packages/react/src/slider/control/SliderControl.tsx @@ -23,7 +23,7 @@ const SliderControl = React.forwardRef(function SliderControl( const { disabled, dragging, - getValueAtFinger, + getFingerState, handleValueChange, minStepsBetweenValues, onValueCommitted, @@ -39,7 +39,7 @@ const SliderControl = React.forwardRef(function SliderControl( const { getRootProps } = useSliderControl({ disabled, dragging, - getValueAtFinger, + getFingerState, handleValueChange, minStepsBetweenValues, onValueCommitted, diff --git a/packages/react/src/slider/control/useSliderControl.ts b/packages/react/src/slider/control/useSliderControl.ts index bc1d8a8ccf..c3615e5619 100644 --- a/packages/react/src/slider/control/useSliderControl.ts +++ b/packages/react/src/slider/control/useSliderControl.ts @@ -21,7 +21,7 @@ export function useSliderControl( const { disabled, dragging, - getValueAtFinger, + getFingerState, handleValueChange, onValueCommitted, minStepsBetweenValues, @@ -67,22 +67,20 @@ export function useSliderControl( return; } - const newFingerValue = getValueAtFinger(fingerPosition, false, offsetRef.current); + const finger = getFingerState(fingerPosition, false, offsetRef.current); - if (!newFingerValue) { + if (finger == null) { return; } - const { newValue, closestThumbIndex } = newFingerValue; + focusThumb(finger.closestThumbIndex, controlRef, setActive); - focusThumb({ sliderRef: controlRef, activeIndex: closestThumbIndex, setActive }); - - if (validateMinimumDistance(newValue, step, minStepsBetweenValues)) { + if (validateMinimumDistance(finger.value, step, minStepsBetweenValues)) { if (!dragging && moveCountRef.current > INTENTIONAL_DRAG_COUNT_THRESHOLD) { setDragging(true); } - handleValueChange(newValue, closestThumbIndex, nativeEvent); + handleValueChange(finger.value, finger.closestThumbIndex, nativeEvent); } }); @@ -94,17 +92,17 @@ export function useSliderControl( return; } - const newFingerValue = getValueAtFinger(fingerPosition, true); + const finger = getFingerState(fingerPosition, true); - if (!newFingerValue) { + if (finger == null) { return; } setActive(-1); - commitValidation(newFingerValue.newValue); + commitValidation(finger.value); - onValueCommitted(newFingerValue.newValue, nativeEvent); + onValueCommitted(finger.value, nativeEvent); touchIdRef.current = null; @@ -126,16 +124,15 @@ export function useSliderControl( const fingerPosition = trackFinger(nativeEvent, touchIdRef); if (fingerPosition != null) { - const newFingerValue = getValueAtFinger(fingerPosition); + const finger = getFingerState(fingerPosition); - if (!newFingerValue) { + if (finger == null) { return; } - const { newValue, closestThumbIndex } = newFingerValue; - focusThumb({ sliderRef: controlRef, activeIndex: closestThumbIndex, setActive }); + focusThumb(finger.closestThumbIndex, controlRef, setActive); - handleValueChange(newValue, closestThumbIndex, nativeEvent); + handleValueChange(finger.value, finger.closestThumbIndex, nativeEvent); } moveCountRef.current = 0; @@ -200,15 +197,13 @@ export function useSliderControl( const fingerPosition = trackFinger(event, touchIdRef); if (fingerPosition != null) { - const newFingerValue = getValueAtFinger(fingerPosition); + const finger = getFingerState(fingerPosition); - if (newFingerValue == null) { + if (finger == null) { return; } - const { newValue, closestThumbIndex, newPercentageValue } = newFingerValue; - - focusThumb({ sliderRef: controlRef, activeIndex: closestThumbIndex, setActive }); + focusThumb(finger.closestThumbIndex, controlRef, setActive); // if the event lands on a thumb, don't change the value, just get the // percentageValue difference represented by the distance between the click origin @@ -216,11 +211,12 @@ export function useSliderControl( if (thumbRefs.current.includes(event.target as HTMLElement)) { const targetThumbIndex = (event.target as HTMLElement).getAttribute('data-index'); - const offset = percentageValues[Number(targetThumbIndex)] / 100 - newPercentageValue; + const offset = + percentageValues[Number(targetThumbIndex)] / 100 - finger.percentageValue; offsetRef.current = offset; } else { - handleValueChange(newValue, closestThumbIndex, event.nativeEvent); + handleValueChange(finger.value, finger.closestThumbIndex, event.nativeEvent); } } @@ -234,7 +230,7 @@ export function useSliderControl( }, [ disabled, - getValueAtFinger, + getFingerState, handleRootRef, handleTouchMove, handleTouchEnd, @@ -259,7 +255,7 @@ export namespace useSliderControl { useSliderRoot.ReturnValue, | 'disabled' | 'dragging' - | 'getValueAtFinger' + | 'getFingerState' | 'handleValueChange' | 'minStepsBetweenValues' | 'onValueCommitted' diff --git a/packages/react/src/slider/indicator/SliderIndicator.test.tsx b/packages/react/src/slider/indicator/SliderIndicator.test.tsx index b8aee8db08..e204da50e3 100644 --- a/packages/react/src/slider/indicator/SliderIndicator.test.tsx +++ b/packages/react/src/slider/indicator/SliderIndicator.test.tsx @@ -10,10 +10,10 @@ const testRootContext: SliderRootContext = { direction: 'ltr', dragging: false, disabled: false, - getValueAtFinger: () => ({ - newValue: 0, + getFingerState: () => ({ + value: 0, closestThumbIndex: 0, - newPercentageValue: 0, + percentageValue: 0, }), handleValueChange: NOOP, largeStep: 10, diff --git a/packages/react/src/slider/root/useSliderRoot.ts b/packages/react/src/slider/root/useSliderRoot.ts index 1358974387..c6b7ce51f6 100644 --- a/packages/react/src/slider/root/useSliderRoot.ts +++ b/packages/react/src/slider/root/useSliderRoot.ts @@ -55,25 +55,30 @@ function findClosest(values: number[], currentValue: number) { return closestIndex; } -export function focusThumb({ - sliderRef, - activeIndex, - setActive, -}: { - sliderRef: React.RefObject; - activeIndex: number; - setActive?: (num: number) => void; -}) { +export function focusThumb( + thumbIndex: number, + sliderRef: React.RefObject, + setActive?: useSliderRoot.ReturnValue['setActive'], +) { + if (!sliderRef.current) { + return; + } + const doc = ownerDocument(sliderRef.current); + if ( - !sliderRef.current?.contains(doc.activeElement) || - Number(doc?.activeElement?.getAttribute('data-index')) !== activeIndex + !sliderRef.current.contains(doc.activeElement) || + Number(doc?.activeElement?.getAttribute('data-index')) !== thumbIndex ) { - sliderRef.current?.querySelector(`[type="range"][data-index="${activeIndex}"]`).focus(); + ( + sliderRef.current.querySelector( + `[type="range"][data-index="${thumbIndex}"]`, + ) as HTMLInputElement + ).focus(); } if (setActive) { - setActive(activeIndex); + setActive(thumbIndex); } } @@ -252,7 +257,7 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo }); if (range) { - focusThumb({ sliderRef, activeIndex: index }); + focusThumb(index, sliderRef); } if (validateMinimumDistance(newValue, step, minStepsBetweenValues)) { @@ -267,7 +272,7 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo const previousIndexRef = React.useRef(null); - const getValueAtFinger = useEventCallback( + const getFingerState = useEventCallback( ( fingerPosition: FingerPosition | null, // `move` is used to distinguish between when this is called by touchstart vs touchmove/end @@ -312,7 +317,7 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo let closestThumbIndex = 0; if (!range) { - return { newValue, closestThumbIndex, newPercentageValue: percent }; + return { value: newValue, percentageValue: percent, closestThumbIndex }; } if (!move) { @@ -341,7 +346,7 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo previousIndexRef.current = closestThumbIndex; } - return { newValue, closestThumbIndex, newPercentageValue: percent }; + return { value: newValue, percentageValue: percent, closestThumbIndex }; }, ); @@ -380,7 +385,7 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo direction, disabled, dragging, - getValueAtFinger, + getFingerState, handleValueChange, largeStep, max, @@ -409,7 +414,7 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo direction, disabled, dragging, - getValueAtFinger, + getFingerState, handleValueChange, largeStep, max, @@ -554,14 +559,14 @@ export namespace useSliderRoot { dragging: boolean; direction: TextDirection; disabled: boolean; - getValueAtFinger: ( + getFingerState: ( fingerPosition: FingerPosition | null, move?: boolean, offset?: number, ) => { - newValue: number | number[]; + value: number | number[]; + percentageValue: number; closestThumbIndex: number; - newPercentageValue: number; } | null; /** * Callback to invoke change handlers after internal value state is updated. diff --git a/packages/react/src/slider/thumb/SliderThumb.test.tsx b/packages/react/src/slider/thumb/SliderThumb.test.tsx index a948273c0b..2330500692 100644 --- a/packages/react/src/slider/thumb/SliderThumb.test.tsx +++ b/packages/react/src/slider/thumb/SliderThumb.test.tsx @@ -10,10 +10,10 @@ const testRootContext: SliderRootContext = { direction: 'ltr', dragging: false, disabled: false, - getValueAtFinger: () => ({ - newValue: 0, + getFingerState: () => ({ + value: 0, closestThumbIndex: 0, - newPercentageValue: 0, + percentageValue: 0, }), handleValueChange: NOOP, largeStep: 10, diff --git a/packages/react/src/slider/track/SliderTrack.test.tsx b/packages/react/src/slider/track/SliderTrack.test.tsx index f4982d1055..45f9bed6a4 100644 --- a/packages/react/src/slider/track/SliderTrack.test.tsx +++ b/packages/react/src/slider/track/SliderTrack.test.tsx @@ -10,10 +10,10 @@ const testRootContext: SliderRootContext = { direction: 'ltr', dragging: false, disabled: false, - getValueAtFinger: () => ({ - newValue: 0, + getFingerState: () => ({ + value: 0, closestThumbIndex: 0, - newPercentageValue: 0, + percentageValue: 0, }), handleValueChange: NOOP, largeStep: 10, diff --git a/packages/react/src/slider/value/SliderValue.test.tsx b/packages/react/src/slider/value/SliderValue.test.tsx index eee12bda60..ab7c87117f 100644 --- a/packages/react/src/slider/value/SliderValue.test.tsx +++ b/packages/react/src/slider/value/SliderValue.test.tsx @@ -12,10 +12,10 @@ const testRootContext: SliderRootContext = { direction: 'ltr', dragging: false, disabled: false, - getValueAtFinger: () => ({ - newValue: 0, + getFingerState: () => ({ + value: 0, closestThumbIndex: 0, - newPercentageValue: 0, + percentageValue: 0, }), handleValueChange: NOOP, largeStep: 10, From 8dad49f625ee6a4c680ec95550a4fafc474bd0a1 Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Mon, 23 Dec 2024 23:43:10 +0800 Subject: [PATCH 05/13] More naming --- .../src/slider/control/SliderControl.test.tsx | 4 +- .../src/slider/control/SliderControl.tsx | 4 +- .../src/slider/control/useSliderControl.ts | 12 +++--- .../slider/indicator/SliderIndicator.test.tsx | 4 +- .../react/src/slider/root/useSliderRoot.ts | 38 +++++++++---------- .../src/slider/thumb/SliderThumb.test.tsx | 4 +- .../react/src/slider/thumb/SliderThumb.tsx | 4 +- .../react/src/slider/thumb/useSliderThumb.ts | 15 ++++---- .../src/slider/track/SliderTrack.test.tsx | 4 +- .../src/slider/value/SliderValue.test.tsx | 4 +- 10 files changed, 44 insertions(+), 49 deletions(-) diff --git a/packages/react/src/slider/control/SliderControl.test.tsx b/packages/react/src/slider/control/SliderControl.test.tsx index 3186c66785..33d1456f1f 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, - changeValue: NOOP, + handleInputChange: NOOP, direction: 'ltr', dragging: false, disabled: false, @@ -15,7 +15,7 @@ const testRootContext: SliderRootContext = { closestThumbIndex: 0, percentageValue: 0, }), - handleValueChange: NOOP, + setValue: NOOP, largeStep: 10, thumbMap: new Map(), max: 100, diff --git a/packages/react/src/slider/control/SliderControl.tsx b/packages/react/src/slider/control/SliderControl.tsx index 1f9c527978..46e449ee6f 100644 --- a/packages/react/src/slider/control/SliderControl.tsx +++ b/packages/react/src/slider/control/SliderControl.tsx @@ -24,7 +24,7 @@ const SliderControl = React.forwardRef(function SliderControl( disabled, dragging, getFingerState, - handleValueChange, + setValue, minStepsBetweenValues, onValueCommitted, state, @@ -40,7 +40,7 @@ const SliderControl = React.forwardRef(function SliderControl( disabled, dragging, getFingerState, - handleValueChange, + setValue, minStepsBetweenValues, onValueCommitted, percentageValues, diff --git a/packages/react/src/slider/control/useSliderControl.ts b/packages/react/src/slider/control/useSliderControl.ts index c3615e5619..cfa23db030 100644 --- a/packages/react/src/slider/control/useSliderControl.ts +++ b/packages/react/src/slider/control/useSliderControl.ts @@ -22,7 +22,7 @@ export function useSliderControl( disabled, dragging, getFingerState, - handleValueChange, + setValue, onValueCommitted, minStepsBetweenValues, percentageValues, @@ -80,7 +80,7 @@ export function useSliderControl( setDragging(true); } - handleValueChange(finger.value, finger.closestThumbIndex, nativeEvent); + setValue(finger.value, finger.closestThumbIndex, nativeEvent); } }); @@ -132,7 +132,7 @@ export function useSliderControl( focusThumb(finger.closestThumbIndex, controlRef, setActive); - handleValueChange(finger.value, finger.closestThumbIndex, nativeEvent); + setValue(finger.value, finger.closestThumbIndex, nativeEvent); } moveCountRef.current = 0; @@ -216,7 +216,7 @@ export function useSliderControl( offsetRef.current = offset; } else { - handleValueChange(finger.value, finger.closestThumbIndex, event.nativeEvent); + setValue(finger.value, finger.closestThumbIndex, event.nativeEvent); } } @@ -234,7 +234,7 @@ export function useSliderControl( handleRootRef, handleTouchMove, handleTouchEnd, - handleValueChange, + setValue, percentageValues, setActive, thumbRefs, @@ -256,7 +256,7 @@ export namespace useSliderControl { | 'disabled' | 'dragging' | 'getFingerState' - | 'handleValueChange' + | 'setValue' | 'minStepsBetweenValues' | 'onValueCommitted' | 'percentageValues' diff --git a/packages/react/src/slider/indicator/SliderIndicator.test.tsx b/packages/react/src/slider/indicator/SliderIndicator.test.tsx index e204da50e3..6da101fc2c 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, - changeValue: NOOP, + handleInputChange: NOOP, direction: 'ltr', dragging: false, disabled: false, @@ -15,7 +15,7 @@ const testRootContext: SliderRootContext = { closestThumbIndex: 0, percentageValue: 0, }), - handleValueChange: NOOP, + setValue: NOOP, largeStep: 10, thumbMap: new Map(), max: 100, diff --git a/packages/react/src/slider/root/useSliderRoot.ts b/packages/react/src/slider/root/useSliderRoot.ts index c6b7ce51f6..286df0e4b8 100644 --- a/packages/react/src/slider/root/useSliderRoot.ts +++ b/packages/react/src/slider/root/useSliderRoot.ts @@ -162,9 +162,9 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo commitValidation, } = useFieldControlValidation(); - // The internal valueState is potentially unsorted, e.g. to support frozen arrays + // The internal value is potentially unsorted, e.g. to support frozen arrays // https://github.com/mui/material-ui/pull/28472 - const [valueState, setValueState] = useControlled({ + const [valueUnwrapped, setValueUnwrapped] = useControlled({ controlled: valueProp, default: defaultValue ?? min, name: 'Slider', @@ -190,7 +190,7 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo useField({ id, commitValidation, - value: valueState, + value: valueUnwrapped, controlRef, }); @@ -211,13 +211,13 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo [inputValidationRef], ); - const handleValueChange = useEventCallback( + const setValue = useEventCallback( (newValue: number | readonly number[], thumbIndex: number, event: Event) => { - if (areValuesEqual(newValue, valueState)) { + if (areValuesEqual(newValue, valueUnwrapped)) { return; } - setValueState(newValue); + setValueUnwrapped(newValue); // Redefine target to allow name and value to be read. // This allows seamless integration with the most popular form libraries. @@ -235,17 +235,17 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo }, ); - const range = Array.isArray(valueState); + const range = Array.isArray(valueUnwrapped); const values = React.useMemo(() => { - return (range ? valueState.slice().sort(asc) : [valueState]).map((val) => + return (range ? valueUnwrapped.slice().sort(asc) : [valueUnwrapped]).map((val) => val == null ? min : clamp(val, min, max), ); - }, [max, min, range, valueState]); + }, [max, min, range, valueUnwrapped]); const handleRootRef = useForkRef(rootRef, sliderRef); - const changeValue = useEventCallback( + const handleInputChange = useEventCallback( (valueInput: number, index: number, event: React.KeyboardEvent | React.ChangeEvent) => { const newValue = getSliderValue({ valueInput, @@ -261,7 +261,7 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo } if (validateMinimumDistance(newValue, step, minStepsBetweenValues)) { - handleValueChange(newValue, index, event.nativeEvent); + setValue(newValue, index, event.nativeEvent); setDirty(newValue !== validityData.initialValue); setTouched(true); commitValidation(newValue); @@ -381,12 +381,12 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo getRootProps, active, 'aria-labelledby': ariaLabelledby, - changeValue, + handleInputChange, direction, disabled, dragging, getFingerState, - handleValueChange, + setValue, largeStep, max, min, @@ -410,12 +410,12 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo getRootProps, active, ariaLabelledby, - changeValue, + handleInputChange, direction, disabled, dragging, getFingerState, - handleValueChange, + setValue, largeStep, max, min, @@ -551,7 +551,7 @@ export namespace useSliderRoot { */ active: number; 'aria-labelledby'?: string; - changeValue: ( + handleInputChange: ( valueInput: number, index: number, event: React.KeyboardEvent | React.ChangeEvent, @@ -571,11 +571,7 @@ export namespace useSliderRoot { /** * Callback to invoke change handlers after internal value state is updated. */ - handleValueChange: ( - newValue: number | readonly number[], - activeThumb: number, - event: Event, - ) => void; + setValue: (newValue: number | readonly number[], activeThumb: number, event: Event) => void; /** * The large step value of the slider when incrementing or decrementing while the shift key is held, * or when using Page-Up or Page-Down keys. Snaps to multiples of this value. diff --git a/packages/react/src/slider/thumb/SliderThumb.test.tsx b/packages/react/src/slider/thumb/SliderThumb.test.tsx index 2330500692..1cd31bf0b7 100644 --- a/packages/react/src/slider/thumb/SliderThumb.test.tsx +++ b/packages/react/src/slider/thumb/SliderThumb.test.tsx @@ -6,7 +6,7 @@ import { NOOP } from '../../utils/noop'; const testRootContext: SliderRootContext = { active: -1, - changeValue: NOOP, + handleInputChange: NOOP, direction: 'ltr', dragging: false, disabled: false, @@ -15,7 +15,7 @@ const testRootContext: SliderRootContext = { closestThumbIndex: 0, percentageValue: 0, }), - handleValueChange: NOOP, + setValue: NOOP, largeStep: 10, thumbMap: new Map(), max: 100, diff --git a/packages/react/src/slider/thumb/SliderThumb.tsx b/packages/react/src/slider/thumb/SliderThumb.tsx index 7a09f0e956..404279815b 100644 --- a/packages/react/src/slider/thumb/SliderThumb.tsx +++ b/packages/react/src/slider/thumb/SliderThumb.tsx @@ -52,7 +52,7 @@ const SliderThumb = React.forwardRef(function SliderThumb( const { active: activeIndex, 'aria-labelledby': ariaLabelledby, - changeValue, + handleInputChange, direction, disabled: contextDisabled, format, @@ -81,7 +81,7 @@ const SliderThumb = React.forwardRef(function SliderThumb( 'aria-label': ariaLabel, 'aria-labelledby': ariaLabelledby, 'aria-valuetext': ariaValuetext, - changeValue, + handleInputChange, direction, disabled: disabledProp || contextDisabled, format, diff --git a/packages/react/src/slider/thumb/useSliderThumb.ts b/packages/react/src/slider/thumb/useSliderThumb.ts index 9171896545..4719d29ac6 100644 --- a/packages/react/src/slider/thumb/useSliderThumb.ts +++ b/packages/react/src/slider/thumb/useSliderThumb.ts @@ -52,7 +52,7 @@ export function useSliderThumb(parameters: useSliderThumb.Parameters): useSlider 'aria-label': ariaLabel, 'aria-labelledby': ariaLabelledby, 'aria-valuetext': ariaValuetext, - changeValue, + handleInputChange, direction, disabled, format, @@ -207,7 +207,7 @@ export function useSliderThumb(parameters: useSliderThumb.Parameters): useSlider } if (newValue !== null) { - changeValue(newValue, index, event); + handleInputChange(newValue, index, event); event.preventDefault(); } }, @@ -219,7 +219,7 @@ export function useSliderThumb(parameters: useSliderThumb.Parameters): useSlider }); }, [ - changeValue, + handleInputChange, commitValidation, disabled, getThumbStyle, @@ -262,9 +262,8 @@ export function useSliderThumb(parameters: useSliderThumb.Parameters): useSlider max, min, name, - onChange(event: React.ChangeEvent) { - // @ts-ignore - changeValue(event.target.valueAsNumber, index, event); + onChange(event: React.ChangeEvent) { + handleInputChange(event.target.valueAsNumber, index, event); }, ref: mergedInputRef, step, @@ -285,7 +284,7 @@ export function useSliderThumb(parameters: useSliderThumb.Parameters): useSlider ariaLabel, ariaLabelledby, ariaValuetext, - changeValue, + handleInputChange, disabled, format, getAriaLabel, @@ -322,7 +321,7 @@ export namespace useSliderThumb { useSliderRoot.ReturnValue, | 'active' | 'aria-labelledby' - | 'changeValue' + | 'handleInputChange' | 'direction' | 'largeStep' | 'max' diff --git a/packages/react/src/slider/track/SliderTrack.test.tsx b/packages/react/src/slider/track/SliderTrack.test.tsx index 45f9bed6a4..a05ca0f196 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, - changeValue: NOOP, + handleInputChange: NOOP, direction: 'ltr', dragging: false, disabled: false, @@ -15,7 +15,7 @@ const testRootContext: SliderRootContext = { closestThumbIndex: 0, percentageValue: 0, }), - handleValueChange: NOOP, + setValue: NOOP, largeStep: 10, thumbMap: new Map(), max: 100, diff --git a/packages/react/src/slider/value/SliderValue.test.tsx b/packages/react/src/slider/value/SliderValue.test.tsx index ab7c87117f..53a74111ce 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, - changeValue: NOOP, + handleInputChange: NOOP, direction: 'ltr', dragging: false, disabled: false, @@ -17,7 +17,7 @@ const testRootContext: SliderRootContext = { closestThumbIndex: 0, percentageValue: 0, }), - handleValueChange: NOOP, + setValue: NOOP, largeStep: 10, thumbMap: new Map(), max: 100, From 6215fd94ab2493331eb040ab544f0a7300355950 Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Tue, 24 Dec 2024 00:15:43 +0800 Subject: [PATCH 06/13] All hook params are required --- docs/reference/generated/slider-root.json | 4 + docs/reference/generated/slider-thumb.json | 5 ++ .../src/slider/control/SliderControl.test.tsx | 2 + .../slider/indicator/SliderIndicator.test.tsx | 2 + packages/react/src/slider/root/SliderRoot.tsx | 34 ++++---- .../src/slider/root/SliderRootContext.ts | 1 + .../react/src/slider/root/useSliderRoot.ts | 47 +++++------ .../src/slider/thumb/SliderThumb.test.tsx | 2 + .../react/src/slider/thumb/SliderThumb.tsx | 49 ++++++++---- .../react/src/slider/thumb/useSliderThumb.ts | 80 +++++++++++-------- .../src/slider/track/SliderTrack.test.tsx | 2 + .../src/slider/value/SliderValue.test.tsx | 2 + .../react/src/slider/value/SliderValue.tsx | 13 ++- .../react/src/slider/value/useSliderValue.ts | 11 ++- 14 files changed, 153 insertions(+), 101 deletions(-) diff --git a/docs/reference/generated/slider-root.json b/docs/reference/generated/slider-root.json index 804dc5ff26..d432e1c472 100644 --- a/docs/reference/generated/slider-root.json +++ b/docs/reference/generated/slider-root.json @@ -26,6 +26,10 @@ "type": "(value, event) => void", "description": "Callback function that is fired when the `pointerup` is triggered." }, + "tabIndex": { + "type": "number", + "description": "Optional tab index attribute for the thumb components." + }, "step": { "type": "number", "default": "1", diff --git a/docs/reference/generated/slider-thumb.json b/docs/reference/generated/slider-thumb.json index 558237081c..88568a6eda 100644 --- a/docs/reference/generated/slider-thumb.json +++ b/docs/reference/generated/slider-thumb.json @@ -18,6 +18,11 @@ "type": "function(formattedValue: string, value: number, index: number) => string", "description": "Accepts a function which returns a string value that provides a user-friendly name for the current value of the slider.\nThis is important for screen reader users." }, + "tabIndex": { + "type": "number", + "default": "null", + "description": "Optional tab index attribute for the thumb components." + }, "className": { "type": "string | (state) => string", "description": "CSS class applied to the element, or a function that\nreturns a class based on the component’s state." diff --git a/packages/react/src/slider/control/SliderControl.test.tsx b/packages/react/src/slider/control/SliderControl.test.tsx index 33d1456f1f..9af2b0b692 100644 --- a/packages/react/src/slider/control/SliderControl.test.tsx +++ b/packages/react/src/slider/control/SliderControl.test.tsx @@ -21,6 +21,7 @@ const testRootContext: SliderRootContext = { max: 100, min: 0, minStepsBetweenValues: 0, + name: '', onValueCommitted: NOOP, orientation: 'horizontal', state: { @@ -43,6 +44,7 @@ const testRootContext: SliderRootContext = { setDragging: NOOP, setThumbMap: NOOP, step: 1, + tabIndex: null, thumbRefs: { current: [] }, values: [0], }; diff --git a/packages/react/src/slider/indicator/SliderIndicator.test.tsx b/packages/react/src/slider/indicator/SliderIndicator.test.tsx index 6da101fc2c..b7faffef3d 100644 --- a/packages/react/src/slider/indicator/SliderIndicator.test.tsx +++ b/packages/react/src/slider/indicator/SliderIndicator.test.tsx @@ -21,6 +21,7 @@ const testRootContext: SliderRootContext = { max: 100, min: 0, minStepsBetweenValues: 0, + name: '', onValueCommitted: NOOP, orientation: 'horizontal', state: { @@ -43,6 +44,7 @@ const testRootContext: SliderRootContext = { setDragging: NOOP, setThumbMap: NOOP, step: 1, + tabIndex: null, thumbRefs: { current: [] }, values: [0], }; diff --git a/packages/react/src/slider/root/SliderRoot.tsx b/packages/react/src/slider/root/SliderRoot.tsx index bb5d25c840..15523aff68 100644 --- a/packages/react/src/slider/root/SliderRoot.tsx +++ b/packages/react/src/slider/root/SliderRoot.tsx @@ -3,6 +3,7 @@ import * as React from 'react'; import PropTypes from 'prop-types'; import { NOOP } from '../../utils/noop'; import type { BaseUIComponentProps } from '../../utils/types'; +import { useBaseUiId } from '../../utils/useBaseUiId'; import { useComponentRenderer } from '../../utils/useComponentRenderer'; import type { FieldRoot } from '../../field/root/FieldRoot'; import { CompositeList } from '../../composite/list/CompositeList'; @@ -27,45 +28,45 @@ const SliderRoot = React.forwardRef(function SliderRoot( className, defaultValue, disabled: disabledProp = false, - id, + id: idProp, format, - largeStep, + largeStep = 10, render, - max, - min, - minStepsBetweenValues, - name, + max = 100, + min = 0, + minStepsBetweenValues = 0, + name: nameProp, onValueChange: onValueChangeProp, onValueCommitted: onValueCommittedProp, orientation = 'horizontal', - step, - tabIndex, + step = 1, + tabIndex: externalTabIndex, value, ...otherProps } = props; + const id = useBaseUiId(idProp); const direction = useDirection(); const { labelId, state: fieldState, disabled: fieldDisabled } = useFieldRootContext(); const disabled = fieldDisabled || disabledProp; const { getRootProps, ...slider } = useSliderRoot({ - 'aria-labelledby': ariaLabelledby ?? labelId, + 'aria-labelledby': ariaLabelledby ?? labelId ?? '', defaultValue, direction, disabled, - id, + id: id ?? '', largeStep, max, min, minStepsBetweenValues, - name, + name: nameProp ?? '', onValueChange: onValueChangeProp ?? NOOP, onValueCommitted: onValueCommittedProp ?? NOOP, orientation, rootRef: forwardedRef, step, - tabIndex, value, }); @@ -101,8 +102,9 @@ const SliderRoot = React.forwardRef(function SliderRoot( ...slider, format, state, + tabIndex: externalTabIndex ?? null, }), - [slider, format, state], + [slider, format, state, externalTabIndex], ); const { renderElement } = useComponentRenderer({ @@ -190,6 +192,10 @@ export namespace SliderRoot { * Options to format the input value. */ format?: Intl.NumberFormatOptions; + /** + * Optional tab index attribute for the thumb components. + */ + tabIndex?: number; /** * The value of the slider. * For ranged sliders, provide an array with two values. @@ -319,7 +325,7 @@ SliderRoot.propTypes /* remove-proptypes */ = { */ step: PropTypes.number, /** - * @ignore + * Optional tab index attribute for the thumb components. */ tabIndex: PropTypes.number, /** diff --git a/packages/react/src/slider/root/SliderRootContext.ts b/packages/react/src/slider/root/SliderRootContext.ts index a2b3c5f3dd..521bad010a 100644 --- a/packages/react/src/slider/root/SliderRootContext.ts +++ b/packages/react/src/slider/root/SliderRootContext.ts @@ -6,6 +6,7 @@ import type { useSliderRoot } from './useSliderRoot'; export interface SliderRootContext extends Omit { format?: Intl.NumberFormatOptions; state: SliderRoot.State; + tabIndex: number | null; } export const SliderRootContext = React.createContext(undefined); diff --git a/packages/react/src/slider/root/useSliderRoot.ts b/packages/react/src/slider/root/useSliderRoot.ts index 286df0e4b8..16d07d2a53 100644 --- a/packages/react/src/slider/root/useSliderRoot.ts +++ b/packages/react/src/slider/root/useSliderRoot.ts @@ -5,7 +5,6 @@ import { areArraysEqual } from '../../utils/areArraysEqual'; import { clamp } from '../../utils/clamp'; import { mergeReactProps } from '../../utils/mergeReactProps'; import { ownerDocument } from '../../utils/owner'; -import { useBaseUiId } from '../../utils/useBaseUiId'; import { useControlled } from '../../utils/useControlled'; import { useEnhancedEffect } from '../../utils/useEnhancedEffect'; import { useEventCallback } from '../../utils/useEventCallback'; @@ -139,7 +138,7 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo defaultValue, direction = 'ltr', disabled = false, - id: idProp, + id, largeStep = 10, max = 100, min = 0, @@ -150,7 +149,6 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo orientation = 'horizontal', rootRef, step = 1, - tabIndex, value: valueProp, } = parameters; @@ -174,8 +172,6 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo const controlRef: React.RefObject = React.useRef(null); const thumbRefs = React.useRef<(HTMLElement | null)[]>([]); - const id = useBaseUiId(idProp); - const [thumbMap, setThumbMap] = React.useState( () => new Map | null>(), ); @@ -379,14 +375,13 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo return React.useMemo( () => ({ getRootProps, - active, 'aria-labelledby': ariaLabelledby, - handleInputChange, + active, direction, disabled, dragging, getFingerState, - setValue, + handleInputChange, largeStep, max, min, @@ -400,8 +395,8 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo setActive, setDragging, setThumbMap, + setValue, step, - tabIndex, thumbMap, thumbRefs, values, @@ -410,12 +405,11 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo getRootProps, active, ariaLabelledby, - handleInputChange, direction, disabled, dragging, getFingerState, - setValue, + handleInputChange, largeStep, max, min, @@ -428,8 +422,8 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo setActive, setDragging, setThumbMap, + setValue, step, - tabIndex, thumbMap, thumbRefs, values, @@ -449,11 +443,11 @@ export namespace useSliderRoot { /** * The id of the slider element. */ - id?: string; + id: string; /** * The id of the element containing a label for the slider. */ - 'aria-labelledby'?: string; + 'aria-labelledby': string; /** * The default value. Use when the component is not controlled. */ @@ -467,28 +461,28 @@ export namespace useSliderRoot { * Whether the component should ignore user interaction. * @default false */ - disabled?: boolean; + disabled: boolean; /** * The maximum allowed value of the slider. * Should not be equal to min. * @default 100 */ - max?: number; + max: number; /** * The minimum allowed value of the slider. * Should not be equal to max. * @default 0 */ - min?: number; + min: number; /** * The minimum steps between values in a range slider. * @default 0 */ - minStepsBetweenValues?: number; + minStepsBetweenValues: number; /** * Identifies the field when a form is submitted. */ - name?: string; + name: string; /** * Callback function that is fired when the slider's value changed. * @@ -514,27 +508,23 @@ export namespace useSliderRoot { * The component orientation. * @default 'horizontal' */ - orientation?: Orientation; + orientation: Orientation; /** * The ref attached to the root of the Slider. */ - rootRef?: React.Ref; + rootRef: React.Ref; /** * The granularity with which the slider can step through values when using Page Up/Page Down or Shift + Arrow Up/Arrow Down. * @default 10 */ - largeStep?: number; + largeStep: number; /** * The granularity with which the slider can step through values. (A "discrete" slider.) * The `min` prop serves as the origin for the valid values. * We recommend (max - min) to be evenly divisible by the step. * @default 1 */ - step?: number; - /** - * Tab index attribute of the Thumb component's `input` element. - */ - tabIndex?: number; + step: number; /** * The value of the slider. * For ranged sliders, provide an array with two values. @@ -590,7 +580,7 @@ export namespace useSliderRoot { * The minimum steps between values in a range slider. */ minStepsBetweenValues: number; - name?: string; + name: string; onValueCommitted: (value: number | number[], event: Event) => void; /** * The component orientation. @@ -613,7 +603,6 @@ export namespace useSliderRoot { step: number; thumbMap: Map | null>; thumbRefs: React.MutableRefObject<(HTMLElement | null)[]>; - tabIndex?: number; /** * The value(s) of the slider */ diff --git a/packages/react/src/slider/thumb/SliderThumb.test.tsx b/packages/react/src/slider/thumb/SliderThumb.test.tsx index 1cd31bf0b7..bca5ebe0b8 100644 --- a/packages/react/src/slider/thumb/SliderThumb.test.tsx +++ b/packages/react/src/slider/thumb/SliderThumb.test.tsx @@ -21,6 +21,7 @@ const testRootContext: SliderRootContext = { max: 100, min: 0, minStepsBetweenValues: 0, + name: '', onValueCommitted: NOOP, orientation: 'horizontal', state: { @@ -43,6 +44,7 @@ const testRootContext: SliderRootContext = { setDragging: NOOP, setThumbMap: NOOP, step: 1, + tabIndex: null, thumbRefs: { current: [] }, values: [0], }; diff --git a/packages/react/src/slider/thumb/SliderThumb.tsx b/packages/react/src/slider/thumb/SliderThumb.tsx index 404279815b..a7b53f4433 100644 --- a/packages/react/src/slider/thumb/SliderThumb.tsx +++ b/packages/react/src/slider/thumb/SliderThumb.tsx @@ -3,8 +3,10 @@ import * as React from 'react'; import PropTypes from 'prop-types'; import { getStyleHookProps } from '../../utils/getStyleHookProps'; import { mergeReactProps } from '../../utils/mergeReactProps'; +import { NOOP } from '../../utils/noop'; import { resolveClassName } from '../../utils/resolveClassName'; import { BaseUIComponentProps } from '../../utils/types'; +import { useBaseUiId } from '../../utils/useBaseUiId'; import { useForkRef } from '../../utils/useForkRef'; import type { SliderRoot } from '../root/SliderRoot'; import { useSliderRootContext } from '../root/SliderRootContext'; @@ -40,13 +42,20 @@ const SliderThumb = React.forwardRef(function SliderThumb( 'aria-valuetext': ariaValuetext, className, disabled: disabledProp = false, - getAriaLabel, - getAriaValueText, - id, - inputId, + getAriaLabel: getAriaLabelProp, + getAriaValueText: getAriaValueTextProp, + id: idProp, + inputId: inputIdProp, + onBlur: onBlurProp, + onFocus: onFocusProp, + onKeyDown: onKeyDownProp, + tabIndex: tabIndexProp, ...otherProps } = props; + const id = useBaseUiId(idProp); + const inputId = useBaseUiId(inputIdProp); + const render = renderProp ?? defaultRender; const { @@ -55,7 +64,7 @@ const SliderThumb = React.forwardRef(function SliderThumb( handleInputChange, direction, disabled: contextDisabled, - format, + format = null, largeStep, max, min, @@ -65,7 +74,7 @@ const SliderThumb = React.forwardRef(function SliderThumb( state, percentageValues, step, - tabIndex, + tabIndex: contextTabIndex, values, } = useSliderRootContext(); @@ -78,27 +87,30 @@ const SliderThumb = React.forwardRef(function SliderThumb( const { getRootProps, getThumbInputProps, disabled, index } = useSliderThumb({ active: activeIndex, - 'aria-label': ariaLabel, - 'aria-labelledby': ariaLabelledby, - 'aria-valuetext': ariaValuetext, + 'aria-label': ariaLabel ?? '', + 'aria-labelledby': ariaLabelledby ?? '', + 'aria-valuetext': ariaValuetext ?? '', handleInputChange, direction, disabled: disabledProp || contextDisabled, format, - getAriaLabel, - getAriaValueText, - id, - inputId, + getAriaLabel: getAriaLabelProp ?? null, + getAriaValueText: getAriaValueTextProp ?? null, + id: id ?? '', + inputId: inputId ?? '', largeStep, max, min, minStepsBetweenValues, name, + onBlur: onBlurProp ?? NOOP, + onFocus: onFocusProp ?? NOOP, + onKeyDown: onKeyDownProp ?? NOOP, orientation, percentageValues, rootRef: mergedRef, step, - tabIndex, + tabIndex: tabIndexProp ?? contextTabIndex, values, }); @@ -145,7 +157,7 @@ export namespace SliderThumb { export interface Props extends Partial>, - Omit, 'render'> { + Omit, 'render' | 'tabIndex'> { onPointerLeave?: React.PointerEventHandler; onPointerOver?: React.PointerEventHandler; onBlur?: React.FocusEventHandler; @@ -199,6 +211,7 @@ SliderThumb.propTypes /* remove-proptypes */ = { * Accepts a function which returns a string value that provides a user-friendly name for the input associated with the thumb * @param {number} index The index of the input * @returns {string} + * @type {((index: number) => string) | null} */ getAriaLabel: PropTypes.func, /** @@ -208,6 +221,7 @@ SliderThumb.propTypes /* remove-proptypes */ = { * @param {number} value The thumb's numerical value. * @param {number} index The thumb's index. * @returns {string} + * @type {((formattedValue: string, value: number, index: number) => string) | null} */ getAriaValueText: PropTypes.func, /** @@ -248,4 +262,9 @@ SliderThumb.propTypes /* remove-proptypes */ = { PropTypes.func, PropTypes.node, ]), + /** + * Optional tab index attribute for the thumb components. + * @default null + */ + tabIndex: PropTypes.number, } as any; diff --git a/packages/react/src/slider/thumb/useSliderThumb.ts b/packages/react/src/slider/thumb/useSliderThumb.ts index 4719d29ac6..c7b697ee21 100644 --- a/packages/react/src/slider/thumb/useSliderThumb.ts +++ b/packages/react/src/slider/thumb/useSliderThumb.ts @@ -4,8 +4,15 @@ import { formatNumber } from '../../utils/formatNumber'; import { mergeReactProps } from '../../utils/mergeReactProps'; import { GenericHTMLProps } from '../../utils/types'; import { useForkRef } from '../../utils/useForkRef'; -import { useBaseUiId } from '../../utils/useBaseUiId'; import { visuallyHidden } from '../../utils/visuallyHidden'; +import { + ARROW_DOWN, + ARROW_UP, + ARROW_RIGHT, + ARROW_LEFT, + HOME, + END, +} from '../../composite/composite'; import { useCompositeListItem } from '../../composite/list/useCompositeListItem'; import { useFieldControlValidation } from '../../field/control/useFieldControlValidation'; import { useFieldRootContext } from '../../field/root/FieldRootContext'; @@ -56,10 +63,10 @@ export function useSliderThumb(parameters: useSliderThumb.Parameters): useSlider direction, disabled, format, - getAriaLabel, - getAriaValueText, - id: idParam, - inputId: inputIdParam, + getAriaLabel = null, + getAriaValueText = null, + id: thumbId, + inputId, largeStep, max, min, @@ -69,7 +76,7 @@ export function useSliderThumb(parameters: useSliderThumb.Parameters): useSlider percentageValues, rootRef: externalRef, step, - tabIndex, + tabIndex: externalTabIndex, values: sliderValues, } = parameters; @@ -80,14 +87,10 @@ export function useSliderThumb(parameters: useSliderThumb.Parameters): useSlider commitValidation, } = useFieldControlValidation(); - const thumbId = useBaseUiId(idParam); - const thumbRef = React.useRef(null); const inputRef = React.useRef(null); const mergedInputRef = useForkRef(inputRef, inputValidationRef); - const inputId = useBaseUiId(inputIdParam); - const thumbMetadata = React.useMemo( () => ({ inputId, @@ -154,10 +157,10 @@ export function useSliderThumb(parameters: useSliderThumb.Parameters): useSlider let newValue = null; const isRange = sliderValues.length > 1; switch (event.key) { - case 'ArrowUp': + case ARROW_UP: newValue = getNewValue(thumbValue, event.shiftKey ? largeStep : step, 1, min, max); break; - case 'ArrowRight': + case ARROW_RIGHT: newValue = getNewValue( thumbValue, event.shiftKey ? largeStep : step, @@ -166,10 +169,10 @@ export function useSliderThumb(parameters: useSliderThumb.Parameters): useSlider max, ); break; - case 'ArrowDown': + case ARROW_DOWN: newValue = getNewValue(thumbValue, event.shiftKey ? largeStep : step, -1, min, max); break; - case 'ArrowLeft': + case ARROW_LEFT: newValue = getNewValue( thumbValue, event.shiftKey ? largeStep : step, @@ -184,7 +187,7 @@ export function useSliderThumb(parameters: useSliderThumb.Parameters): useSlider case 'PageDown': newValue = getNewValue(thumbValue, largeStep, -1, min, max); break; - case 'End': + case END: newValue = max; if (isRange) { @@ -193,7 +196,7 @@ export function useSliderThumb(parameters: useSliderThumb.Parameters): useSlider : max; } break; - case 'Home': + case HOME: newValue = min; if (isRange) { @@ -215,14 +218,15 @@ export function useSliderThumb(parameters: useSliderThumb.Parameters): useSlider style: { ...getThumbStyle(), }, - tabIndex: tabIndex ?? (disabled ? undefined : 0), + tabIndex: externalTabIndex ?? (disabled ? undefined : 0), }); }, [ - handleInputChange, commitValidation, disabled, + externalTabIndex, getThumbStyle, + handleInputChange, index, isRtl, largeStep, @@ -233,7 +237,6 @@ export function useSliderThumb(parameters: useSliderThumb.Parameters): useSlider setTouched, sliderValues, step, - tabIndex, thumbId, thumbValue, ], @@ -247,15 +250,16 @@ export function useSliderThumb(parameters: useSliderThumb.Parameters): useSlider } return mergeReactProps(getInputValidationProps(externalProps), { - 'aria-label': getAriaLabel ? getAriaLabel(index) : ariaLabel, + 'aria-label': getAriaLabel != null ? getAriaLabel(index) : ariaLabel, 'aria-labelledby': ariaLabelledby, 'aria-orientation': orientation, 'aria-valuemax': max, 'aria-valuemin': min, 'aria-valuenow': thumbValue, - 'aria-valuetext': getAriaValueText - ? getAriaValueText(formatNumber(thumbValue, [], format), thumbValue, index) - : (ariaValuetext ?? getDefaultAriaValueText(sliderValues, index, format)), + 'aria-valuetext': + getAriaValueText != null + ? getAriaValueText(formatNumber(thumbValue, [], format ?? undefined), thumbValue, index) + : ariaValuetext || getDefaultAriaValueText(sliderValues, index, format ?? undefined), 'data-index': index, disabled, id: inputId, @@ -269,7 +273,6 @@ export function useSliderThumb(parameters: useSliderThumb.Parameters): useSlider step, style: { ...visuallyHidden, - direction: isRtl ? 'rtl' : 'ltr', // So that VoiceOver's focus indicator matches the thumb's dimensions width: '100%', height: '100%', @@ -331,27 +334,28 @@ export namespace useSliderThumb { | 'orientation' | 'percentageValues' | 'step' - | 'tabIndex' | 'values' > { /** * The label for the input element. */ - 'aria-label'?: string; + 'aria-label': string; /** * A string value that provides a user-friendly name for the current value of the slider. */ - 'aria-valuetext'?: string; + 'aria-valuetext': string; /** * Options to format the input value. + * @default null */ - format?: Intl.NumberFormatOptions; + format: Intl.NumberFormatOptions | null; /** * Accepts a function which returns a string value that provides a user-friendly name for the input associated with the thumb * @param {number} index The index of the input * @returns {string} + * @type {((index: number) => string) | null} */ - getAriaLabel?: (index: number) => string; + getAriaLabel?: ((index: number) => string) | null; /** * Accepts a function which returns a string value that provides a user-friendly name for the current value of the slider. * This is important for screen reader users. @@ -359,15 +363,21 @@ export namespace useSliderThumb { * @param {number} value The thumb's numerical value. * @param {number} index The thumb's index. * @returns {string} + * @type {((formattedValue: string, value: number, index: number) => string) | null} */ - getAriaValueText?: (formattedValue: string, value: number, index: number) => string; - id?: React.HTMLAttributes['id']; - inputId?: React.HTMLAttributes['id']; + getAriaValueText: ((formattedValue: string, value: number, index: number) => string) | null; + id: string; + inputId: string; disabled: boolean; - onBlur?: React.FocusEventHandler; - onFocus?: React.FocusEventHandler; - onKeyDown?: React.KeyboardEventHandler; + onBlur: React.FocusEventHandler; + onFocus: React.FocusEventHandler; + onKeyDown: React.KeyboardEventHandler; rootRef?: React.Ref; + /** + * Optional tab index attribute for the thumb components. + * @default null + */ + tabIndex: number | null; } export interface ReturnValue { diff --git a/packages/react/src/slider/track/SliderTrack.test.tsx b/packages/react/src/slider/track/SliderTrack.test.tsx index a05ca0f196..f84ba363e9 100644 --- a/packages/react/src/slider/track/SliderTrack.test.tsx +++ b/packages/react/src/slider/track/SliderTrack.test.tsx @@ -21,6 +21,7 @@ const testRootContext: SliderRootContext = { max: 100, min: 0, minStepsBetweenValues: 0, + name: '', onValueCommitted: NOOP, orientation: 'horizontal', state: { @@ -43,6 +44,7 @@ const testRootContext: SliderRootContext = { setDragging: NOOP, setThumbMap: NOOP, step: 1, + tabIndex: null, thumbRefs: { current: [] }, values: [0], }; diff --git a/packages/react/src/slider/value/SliderValue.test.tsx b/packages/react/src/slider/value/SliderValue.test.tsx index 53a74111ce..5056c3715c 100644 --- a/packages/react/src/slider/value/SliderValue.test.tsx +++ b/packages/react/src/slider/value/SliderValue.test.tsx @@ -23,6 +23,7 @@ const testRootContext: SliderRootContext = { max: 100, min: 0, minStepsBetweenValues: 0, + name: '', onValueCommitted: NOOP, orientation: 'horizontal', state: { @@ -45,6 +46,7 @@ const testRootContext: SliderRootContext = { setDragging: NOOP, setThumbMap: NOOP, step: 1, + tabIndex: null, thumbRefs: { current: [] }, values: [0], }; diff --git a/packages/react/src/slider/value/SliderValue.tsx b/packages/react/src/slider/value/SliderValue.tsx index 7c596e455d..71514660c9 100644 --- a/packages/react/src/slider/value/SliderValue.tsx +++ b/packages/react/src/slider/value/SliderValue.tsx @@ -17,12 +17,13 @@ const SliderValue = React.forwardRef(function SliderValue( props: SliderValue.Props, forwardedRef: React.ForwardedRef, ) { - const { render, className, children, ...otherProps } = props; + const { 'aria-live': ariaLive = 'off', render, className, children, ...otherProps } = props; const { thumbMap, state, values, format } = useSliderRootContext(); const { getRootProps, formattedValues } = useSliderValue({ - format, + 'aria-live': ariaLive, + format: format ?? null, thumbMap, values, }); @@ -55,6 +56,10 @@ const SliderValue = React.forwardRef(function SliderValue( export namespace SliderValue { export interface Props extends Omit, 'children'> { + /** + * @default 'off' + */ + 'aria-live'?: React.AriaAttributes['aria-live']; children?: | null | ((formattedValues: readonly string[], values: readonly number[]) => React.ReactNode); @@ -68,6 +73,10 @@ SliderValue.propTypes /* remove-proptypes */ = { // │ These PropTypes are generated from the TypeScript type definitions. │ // │ To update them, edit the TypeScript types and run `pnpm proptypes`. │ // └─────────────────────────────────────────────────────────────────────┘ + /** + * @default 'off' + */ + 'aria-live': PropTypes.oneOf(['assertive', 'off', 'polite']), /** * @ignore */ diff --git a/packages/react/src/slider/value/useSliderValue.ts b/packages/react/src/slider/value/useSliderValue.ts index 2c1b71f051..f0385b617f 100644 --- a/packages/react/src/slider/value/useSliderValue.ts +++ b/packages/react/src/slider/value/useSliderValue.ts @@ -5,7 +5,7 @@ import { mergeReactProps } from '../../utils/mergeReactProps'; import type { useSliderRoot } from '../root/useSliderRoot'; export function useSliderValue(parameters: useSliderValue.Parameters): useSliderValue.ReturnValue { - const { 'aria-live': ariaLive = 'off', format: formatParam, thumbMap, values } = parameters; + const { 'aria-live': ariaLive, format: formatParam, thumbMap, values } = parameters; const outputFor = React.useMemo(() => { let htmlFor = ''; @@ -20,9 +20,7 @@ export function useSliderValue(parameters: useSliderValue.Parameters): useSlider const formattedValues = React.useMemo(() => { const arr = []; for (let i = 0; i < values.length; i += 1) { - arr.push( - formatNumber(values[i], [], Array.isArray(formatParam) ? formatParam[i] : formatParam), - ); + arr.push(formatNumber(values[i], [], formatParam ?? undefined)); } return arr; }, [formatParam, values]); @@ -50,11 +48,12 @@ export function useSliderValue(parameters: useSliderValue.Parameters): useSlider export namespace useSliderValue { export interface Parameters extends Pick { - 'aria-live'?: React.AriaAttributes['aria-live']; + 'aria-live': React.AriaAttributes['aria-live']; /** * Options to format the input value. + * @default null */ - format?: Intl.NumberFormatOptions | Intl.NumberFormatOptions[]; + format: Intl.NumberFormatOptions | null; } export interface ReturnValue { From 5145bb78b6cd4bc2d2d120c51b40307da2dac653 Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Wed, 1 Jan 2025 17:25:57 +0800 Subject: [PATCH 07/13] Replace previousThumbIndex with closestThumbIndex --- .../src/slider/control/useSliderControl.ts | 6 ++-- .../react/src/slider/root/useSliderRoot.ts | 32 ++++++++----------- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/packages/react/src/slider/control/useSliderControl.ts b/packages/react/src/slider/control/useSliderControl.ts index cfa23db030..8cf1b0fa34 100644 --- a/packages/react/src/slider/control/useSliderControl.ts +++ b/packages/react/src/slider/control/useSliderControl.ts @@ -92,7 +92,7 @@ export function useSliderControl( return; } - const finger = getFingerState(fingerPosition, true); + const finger = getFingerState(fingerPosition, false); if (finger == null) { return; @@ -124,7 +124,7 @@ export function useSliderControl( const fingerPosition = trackFinger(nativeEvent, touchIdRef); if (fingerPosition != null) { - const finger = getFingerState(fingerPosition); + const finger = getFingerState(fingerPosition, true); if (finger == null) { return; @@ -197,7 +197,7 @@ export function useSliderControl( const fingerPosition = trackFinger(event, touchIdRef); if (fingerPosition != null) { - const finger = getFingerState(fingerPosition); + const finger = getFingerState(fingerPosition, true); if (finger == null) { return; diff --git a/packages/react/src/slider/root/useSliderRoot.ts b/packages/react/src/slider/root/useSliderRoot.ts index 16d07d2a53..2005b1e91d 100644 --- a/packages/react/src/slider/root/useSliderRoot.ts +++ b/packages/react/src/slider/root/useSliderRoot.ts @@ -266,13 +266,17 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo }, ); - const previousIndexRef = React.useRef(null); + const closestThumbIndexRef = React.useRef(null); const getFingerState = useEventCallback( ( fingerPosition: FingerPosition | null, - // `move` is used to distinguish between when this is called by touchstart vs touchmove/end - move: boolean = false, + + /** + * When `true`, closestThumbIndexRef is updated. + * It's `true` when called by touchstart or pointerdown. + */ + shouldCaptureThumbIndex: boolean = false, offset: number = 0, ) => { if (fingerPosition == null) { @@ -288,7 +292,7 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo const isRtl = direction === 'rtl'; const isVertical = orientation === 'vertical'; - const { width, height, bottom, left } = sliderControl!.getBoundingClientRect(); + const { width, height, bottom, left } = sliderControl.getBoundingClientRect(); let percent; if (isVertical) { @@ -310,18 +314,17 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo } newValue = clamp(newValue, min, max); - let closestThumbIndex = 0; if (!range) { - return { value: newValue, percentageValue: percent, closestThumbIndex }; + return { value: newValue, percentageValue: percent, closestThumbIndex: 0 }; } - if (!move) { - closestThumbIndex = findClosest(values, newValue)!; - } else { - closestThumbIndex = previousIndexRef.current!; + if (shouldCaptureThumbIndex) { + closestThumbIndexRef.current = findClosest(values, newValue) ?? 0; } + const closestThumbIndex = closestThumbIndexRef.current ?? 0; + // Bound the new value to the thumb's neighbours. newValue = clamp( newValue, @@ -329,26 +332,19 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo values[closestThumbIndex + 1] - minStepsBetweenValues || Infinity, ); - const previousValue = newValue; newValue = setValueIndex({ values, newValue, index: closestThumbIndex, }); - // Potentially swap the index if needed. - if (!move) { - closestThumbIndex = newValue.indexOf(previousValue); - previousIndexRef.current = closestThumbIndex; - } - return { value: newValue, percentageValue: percent, closestThumbIndex }; }, ); useEnhancedEffect(() => { const activeEl = activeElement(ownerDocument(sliderRef.current)); - if (disabled && sliderRef.current!.contains(activeEl)) { + if (disabled && sliderRef.current?.contains(activeEl)) { // This is necessary because Firefox and Safari will keep focus // on a disabled element: // https://codesandbox.io/p/sandbox/mui-pr-22247-forked-h151h?file=/src/App.js From e1deb669cba9e7f3a18d4e0f32096ec13ca0b29d Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Wed, 1 Jan 2025 18:00:59 +0800 Subject: [PATCH 08/13] Split out utils --- packages/react/src/slider/root/useSliderRoot.ts | 5 +++-- packages/react/src/slider/utils/percentToValue.ts | 3 +++ .../react/src/slider/{utils.ts => utils/roundValueToStep.ts} | 4 ---- 3 files changed, 6 insertions(+), 6 deletions(-) create mode 100644 packages/react/src/slider/utils/percentToValue.ts rename packages/react/src/slider/{utils.ts => utils/roundValueToStep.ts} (86%) diff --git a/packages/react/src/slider/root/useSliderRoot.ts b/packages/react/src/slider/root/useSliderRoot.ts index 2005b1e91d..7c5b0d9646 100644 --- a/packages/react/src/slider/root/useSliderRoot.ts +++ b/packages/react/src/slider/root/useSliderRoot.ts @@ -15,10 +15,11 @@ import type { TextDirection } from '../../direction-provider/DirectionContext'; import { useField } from '../../field/useField'; import { useFieldRootContext } from '../../field/root/FieldRootContext'; import { useFieldControlValidation } from '../../field/control/useFieldControlValidation'; -import { percentToValue, roundValueToStep } from '../utils'; import { asc } from '../utils/asc'; -import { setValueIndex } from '../utils/setValueIndex'; import { getSliderValue } from '../utils/getSliderValue'; +import { percentToValue } from '../utils/percentToValue'; +import { roundValueToStep } from '../utils/roundValueToStep'; +import { setValueIndex } from '../utils/setValueIndex'; import { ThumbMetadata } from '../thumb/useSliderThumb'; function areValuesEqual( diff --git a/packages/react/src/slider/utils/percentToValue.ts b/packages/react/src/slider/utils/percentToValue.ts new file mode 100644 index 0000000000..cc12d737b7 --- /dev/null +++ b/packages/react/src/slider/utils/percentToValue.ts @@ -0,0 +1,3 @@ +export function percentToValue(percent: number, min: number, max: number) { + return (max - min) * percent + min; +} diff --git a/packages/react/src/slider/utils.ts b/packages/react/src/slider/utils/roundValueToStep.ts similarity index 86% rename from packages/react/src/slider/utils.ts rename to packages/react/src/slider/utils/roundValueToStep.ts index ed473cffd4..741f662f75 100644 --- a/packages/react/src/slider/utils.ts +++ b/packages/react/src/slider/utils/roundValueToStep.ts @@ -11,10 +11,6 @@ function getDecimalPrecision(num: number) { return decimalPart ? decimalPart.length : 0; } -export function percentToValue(percent: number, min: number, max: number) { - return (max - min) * percent + min; -} - export function roundValueToStep(value: number, step: number, min: number) { const nearest = Math.round((value - min) / step) * step + min; return Number(nearest.toFixed(getDecimalPrecision(step))); From afe70571efe1f5bf074e540314c5a90bed31d1b3 Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Thu, 2 Jan 2025 15:30:30 +0800 Subject: [PATCH 09/13] Add new state for unrounded percentageValues --- .../src/slider/control/SliderControl.test.tsx | 4 +- .../src/slider/control/useSliderControl.ts | 20 +-- .../slider/indicator/SliderIndicator.test.tsx | 4 +- packages/react/src/slider/root/SliderRoot.tsx | 6 +- .../react/src/slider/root/useSliderRoot.ts | 138 ++++++++++++------ .../src/slider/thumb/SliderThumb.test.tsx | 4 +- .../src/slider/track/SliderTrack.test.tsx | 4 +- .../react/src/slider/utils/getSliderValue.ts | 12 +- .../slider/utils/replaceArrayItemAtIndex.ts | 7 + .../react/src/slider/utils/setValueIndex.ts | 15 -- .../src/slider/value/SliderValue.test.tsx | 4 +- 11 files changed, 131 insertions(+), 87 deletions(-) create mode 100644 packages/react/src/slider/utils/replaceArrayItemAtIndex.ts delete mode 100644 packages/react/src/slider/utils/setValueIndex.ts diff --git a/packages/react/src/slider/control/SliderControl.test.tsx b/packages/react/src/slider/control/SliderControl.test.tsx index 9af2b0b692..07d4982407 100644 --- a/packages/react/src/slider/control/SliderControl.test.tsx +++ b/packages/react/src/slider/control/SliderControl.test.tsx @@ -12,8 +12,9 @@ const testRootContext: SliderRootContext = { disabled: false, getFingerState: () => ({ value: 0, - closestThumbIndex: 0, percentageValue: 0, + percentageValues: [0], + thumbIndex: 0, }), setValue: NOOP, largeStep: 10, @@ -42,6 +43,7 @@ const testRootContext: SliderRootContext = { registerSliderControl: NOOP, setActive: NOOP, setDragging: NOOP, + setPercentageValues: NOOP, setThumbMap: NOOP, step: 1, tabIndex: null, diff --git a/packages/react/src/slider/control/useSliderControl.ts b/packages/react/src/slider/control/useSliderControl.ts index 8cf1b0fa34..fc865654c5 100644 --- a/packages/react/src/slider/control/useSliderControl.ts +++ b/packages/react/src/slider/control/useSliderControl.ts @@ -73,14 +73,14 @@ export function useSliderControl( return; } - focusThumb(finger.closestThumbIndex, controlRef, setActive); + focusThumb(finger.thumbIndex, controlRef, setActive); if (validateMinimumDistance(finger.value, step, minStepsBetweenValues)) { if (!dragging && moveCountRef.current > INTENTIONAL_DRAG_COUNT_THRESHOLD) { setDragging(true); } - setValue(finger.value, finger.closestThumbIndex, nativeEvent); + setValue(finger.value, finger.percentageValues, finger.thumbIndex, nativeEvent); } }); @@ -130,9 +130,9 @@ export function useSliderControl( return; } - focusThumb(finger.closestThumbIndex, controlRef, setActive); + focusThumb(finger.thumbIndex, controlRef, setActive); - setValue(finger.value, finger.closestThumbIndex, nativeEvent); + setValue(finger.value, finger.percentageValues, finger.thumbIndex, nativeEvent); } moveCountRef.current = 0; @@ -203,20 +203,16 @@ export function useSliderControl( return; } - focusThumb(finger.closestThumbIndex, controlRef, setActive); + focusThumb(finger.thumbIndex, controlRef, setActive); // if the event lands on a thumb, don't change the value, just get the // percentageValue difference represented by the distance between the click origin // and the coordinates of the value on the track area if (thumbRefs.current.includes(event.target as HTMLElement)) { - const targetThumbIndex = (event.target as HTMLElement).getAttribute('data-index'); - - const offset = - percentageValues[Number(targetThumbIndex)] / 100 - finger.percentageValue; - - offsetRef.current = offset; + offsetRef.current = + percentageValues[finger.thumbIndex] / 100 - finger.percentageValue; } else { - setValue(finger.value, finger.closestThumbIndex, event.nativeEvent); + setValue(finger.value, finger.percentageValues, finger.thumbIndex, event.nativeEvent); } } diff --git a/packages/react/src/slider/indicator/SliderIndicator.test.tsx b/packages/react/src/slider/indicator/SliderIndicator.test.tsx index b7faffef3d..49ac011d9c 100644 --- a/packages/react/src/slider/indicator/SliderIndicator.test.tsx +++ b/packages/react/src/slider/indicator/SliderIndicator.test.tsx @@ -12,8 +12,9 @@ const testRootContext: SliderRootContext = { disabled: false, getFingerState: () => ({ value: 0, - closestThumbIndex: 0, percentageValue: 0, + percentageValues: [0], + thumbIndex: 0, }), setValue: NOOP, largeStep: 10, @@ -42,6 +43,7 @@ const testRootContext: SliderRootContext = { registerSliderControl: NOOP, setActive: NOOP, setDragging: NOOP, + setPercentageValues: NOOP, setThumbMap: NOOP, step: 1, tabIndex: null, diff --git a/packages/react/src/slider/root/SliderRoot.tsx b/packages/react/src/slider/root/SliderRoot.tsx index 15523aff68..ad3e1cad8e 100644 --- a/packages/react/src/slider/root/SliderRoot.tsx +++ b/packages/react/src/slider/root/SliderRoot.tsx @@ -157,7 +157,7 @@ export namespace SliderRoot { /** * The raw number value of the slider. */ - values: ReadonlyArray; + values: readonly number[]; } export interface Props @@ -182,7 +182,7 @@ export namespace SliderRoot { * * To render a controlled slider, use the `value` prop instead. */ - defaultValue?: number | ReadonlyArray; + defaultValue?: number | readonly number[]; /** * Whether the component should ignore user interaction. * @default false @@ -200,7 +200,7 @@ export namespace SliderRoot { * The value of the slider. * For ranged sliders, provide an array with two values. */ - value?: number | ReadonlyArray; + value?: number | readonly number[]; } } diff --git a/packages/react/src/slider/root/useSliderRoot.ts b/packages/react/src/slider/root/useSliderRoot.ts index 7c5b0d9646..cfe3f6960d 100644 --- a/packages/react/src/slider/root/useSliderRoot.ts +++ b/packages/react/src/slider/root/useSliderRoot.ts @@ -18,8 +18,8 @@ import { useFieldControlValidation } from '../../field/control/useFieldControlVa import { asc } from '../utils/asc'; import { getSliderValue } from '../utils/getSliderValue'; import { percentToValue } from '../utils/percentToValue'; +import { replaceArrayItemAtIndex } from '../utils/replaceArrayItemAtIndex'; import { roundValueToStep } from '../utils/roundValueToStep'; -import { setValueIndex } from '../utils/setValueIndex'; import { ThumbMetadata } from '../thumb/useSliderThumb'; function areValuesEqual( @@ -35,7 +35,7 @@ function areValuesEqual( return false; } -function findClosest(values: number[], currentValue: number) { +function findClosest(values: readonly number[], currentValue: number) { const { index: closestIndex } = values.reduce<{ distance: number; index: number } | null>( (acc, value: number, index: number) => { @@ -208,14 +208,41 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo [inputValidationRef], ); + const range = Array.isArray(valueUnwrapped); + + const values = React.useMemo(() => { + return (range ? valueUnwrapped.slice().sort(asc) : [valueUnwrapped]).map((val) => + val == null ? min : clamp(val, min, max), + ); + }, [max, min, range, valueUnwrapped]); + + function initializePercentageValues() { + // console.log('initializePercentageValues'); + const vals = []; + for (let i = 0; i < values.length; i += 1) { + vals.push(valueToPercent(values[i], min, max)); + } + return vals; + } + + const [percentageValues, setPercentageValues] = React.useState( + initializePercentageValues, + ); + // console.log('percentageValues', percentageValues); + const setValue = useEventCallback( - (newValue: number | readonly number[], thumbIndex: number, event: Event) => { + ( + newValue: number | readonly number[], + newPercentageValues: readonly number[], + thumbIndex: number, + event: Event, + ) => { if (areValuesEqual(newValue, valueUnwrapped)) { return; } setValueUnwrapped(newValue); - + setPercentageValues(newPercentageValues); // Redefine target to allow name and value to be read. // This allows seamless integration with the most popular form libraries. // https://github.com/mui/material-ui/issues/13485#issuecomment-676048492 @@ -232,14 +259,6 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo }, ); - const range = Array.isArray(valueUnwrapped); - - const values = React.useMemo(() => { - return (range ? valueUnwrapped.slice().sort(asc) : [valueUnwrapped]).map((val) => - val == null ? min : clamp(val, min, max), - ); - }, [max, min, range, valueUnwrapped]); - const handleRootRef = useForkRef(rootRef, sliderRef); const handleInputChange = useEventCallback( @@ -258,7 +277,20 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo } if (validateMinimumDistance(newValue, step, minStepsBetweenValues)) { - setValue(newValue, index, event.nativeEvent); + if (Array.isArray(newValue)) { + setValue( + newValue, + replaceArrayItemAtIndex( + percentageValues, + index, + valueToPercent(newValue[index], min, max), + ), + index, + event.nativeEvent, + ); + } else { + setValue(newValue, [valueToPercent(newValue, min, max)], index, event.nativeEvent); + } setDirty(newValue !== validityData.initialValue); setTouched(true); commitValidation(newValue); @@ -272,12 +304,14 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo const getFingerState = useEventCallback( ( fingerPosition: FingerPosition | null, - /** * When `true`, closestThumbIndexRef is updated. * It's `true` when called by touchstart or pointerdown. */ shouldCaptureThumbIndex: boolean = false, + /** + * The pixel distance between the finger origin and the center of the thumb. + */ offset: number = 0, ) => { if (fingerPosition == null) { @@ -294,30 +328,29 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo const isVertical = orientation === 'vertical'; const { width, height, bottom, left } = sliderControl.getBoundingClientRect(); - let percent; - if (isVertical) { - percent = (bottom - fingerPosition.y) / height + offset; - } else { - percent = (fingerPosition.x - left) / width + offset * (isRtl ? -1 : 1); - } + // percent is a value between 0 and 1, e.g. "41%" is `0.41` + const valueRescaled = isVertical + ? (bottom - fingerPosition.y) / height + offset + : (fingerPosition.x - left) / width + offset * (isRtl ? -1 : 1); - percent = Math.min(percent, 1); + let percentageValue = Math.min(valueRescaled, 1); if (isRtl && !isVertical) { - percent = 1 - percent; - } - - let newValue; - newValue = percentToValue(percent, min, max); - if (step) { - newValue = roundValueToStep(newValue, step, min); + percentageValue = 1 - percentageValue; } + let newValue = percentToValue(percentageValue, min, max); + newValue = roundValueToStep(newValue, step, min); newValue = clamp(newValue, min, max); if (!range) { - return { value: newValue, percentageValue: percent, closestThumbIndex: 0 }; + return { + value: newValue, + percentageValue, + percentageValues: [percentageValue * 100], + thumbIndex: 0, + }; } if (shouldCaptureThumbIndex) { @@ -333,13 +366,16 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo values[closestThumbIndex + 1] - minStepsBetweenValues || Infinity, ); - newValue = setValueIndex({ - values, - newValue, - index: closestThumbIndex, - }); - - return { value: newValue, percentageValue: percent, closestThumbIndex }; + return { + value: replaceArrayItemAtIndex(values, closestThumbIndex, newValue), + percentageValue, + percentageValues: replaceArrayItemAtIndex( + percentageValues, + closestThumbIndex, + percentageValue * 100, + ), + thumbIndex: closestThumbIndex, + }; }, ); @@ -386,11 +422,12 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo name, onValueCommitted, orientation, - percentageValues: values.map((v) => valueToPercent(v, min, max)), + percentageValues, range, registerSliderControl, setActive, setDragging, + setPercentageValues, setThumbMap, setValue, step, @@ -414,10 +451,12 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo name, onValueCommitted, orientation, + percentageValues, range, registerSliderControl, setActive, setDragging, + setPercentageValues, setThumbMap, setValue, step, @@ -448,7 +487,7 @@ export namespace useSliderRoot { /** * The default value. Use when the component is not controlled. */ - defaultValue?: number | ReadonlyArray; + defaultValue?: number | readonly number[]; /** * Sets the direction. For right-to-left languages, the lowest value is on the right-hand side. * @default 'ltr' @@ -526,7 +565,7 @@ export namespace useSliderRoot { * The value of the slider. * For ranged sliders, provide an array with two values. */ - value?: number | ReadonlyArray; + value?: number | readonly number[]; } export interface ReturnValue { @@ -548,17 +587,23 @@ export namespace useSliderRoot { disabled: boolean; getFingerState: ( fingerPosition: FingerPosition | null, - move?: boolean, + shouldCaptureThumbIndex?: boolean, offset?: number, ) => { value: number | number[]; percentageValue: number; - closestThumbIndex: number; + percentageValues: number[]; + thumbIndex: number; } | null; /** * Callback to invoke change handlers after internal value state is updated. */ - setValue: (newValue: number | readonly number[], activeThumb: number, event: Event) => void; + setValue: ( + newValue: number | readonly number[], + newPercentageValue: readonly number[], + activeThumb: number, + event: Event, + ) => void; /** * The large step value of the slider when incrementing or decrementing while the shift key is held, * or when using Page-Up or Page-Down keys. Snaps to multiples of this value. @@ -589,9 +634,12 @@ export namespace useSliderRoot { * The value(s) of the slider as percentages */ percentageValues: readonly number[]; - setActive: (activeIndex: number) => void; - setDragging: (isDragging: boolean) => void; - setThumbMap: (map: Map | null>) => void; + setActive: React.Dispatch>; + setDragging: React.Dispatch>; + setPercentageValues: React.Dispatch>; + setThumbMap: React.Dispatch< + React.SetStateAction | null>> + >; /** * 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 bca5ebe0b8..86e58c3368 100644 --- a/packages/react/src/slider/thumb/SliderThumb.test.tsx +++ b/packages/react/src/slider/thumb/SliderThumb.test.tsx @@ -12,8 +12,9 @@ const testRootContext: SliderRootContext = { disabled: false, getFingerState: () => ({ value: 0, - closestThumbIndex: 0, percentageValue: 0, + percentageValues: [0], + thumbIndex: 0, }), setValue: NOOP, largeStep: 10, @@ -42,6 +43,7 @@ const testRootContext: SliderRootContext = { registerSliderControl: NOOP, setActive: NOOP, setDragging: NOOP, + setPercentageValues: NOOP, setThumbMap: NOOP, step: 1, tabIndex: null, diff --git a/packages/react/src/slider/track/SliderTrack.test.tsx b/packages/react/src/slider/track/SliderTrack.test.tsx index f84ba363e9..f8e75b411e 100644 --- a/packages/react/src/slider/track/SliderTrack.test.tsx +++ b/packages/react/src/slider/track/SliderTrack.test.tsx @@ -12,8 +12,9 @@ const testRootContext: SliderRootContext = { disabled: false, getFingerState: () => ({ value: 0, - closestThumbIndex: 0, percentageValue: 0, + percentageValues: [0], + thumbIndex: 0, }), setValue: NOOP, largeStep: 10, @@ -42,6 +43,7 @@ const testRootContext: SliderRootContext = { registerSliderControl: NOOP, setActive: NOOP, setDragging: NOOP, + setPercentageValues: NOOP, setThumbMap: NOOP, step: 1, tabIndex: null, diff --git a/packages/react/src/slider/utils/getSliderValue.ts b/packages/react/src/slider/utils/getSliderValue.ts index efe7d2019f..b031f84d92 100644 --- a/packages/react/src/slider/utils/getSliderValue.ts +++ b/packages/react/src/slider/utils/getSliderValue.ts @@ -1,5 +1,5 @@ import { clamp } from '../../utils/clamp'; -import { setValueIndex } from './setValueIndex'; +import { replaceArrayItemAtIndex } from './replaceArrayItemAtIndex'; interface GetSliderValueParameters { valueInput: number; @@ -18,14 +18,12 @@ export function getSliderValue(params: GetSliderValueParameters) { newValue = clamp(newValue, min, max); if (range) { - // Bound the new value to the thumb's neighbours. - newValue = clamp(newValue, values[index - 1] || -Infinity, values[index + 1] || Infinity); - - newValue = setValueIndex({ + newValue = replaceArrayItemAtIndex( values, - newValue, index, - }); + // Bound the new value to the thumb's neighbours. + clamp(newValue, values[index - 1] || -Infinity, values[index + 1] || Infinity), + ); } return newValue; diff --git a/packages/react/src/slider/utils/replaceArrayItemAtIndex.ts b/packages/react/src/slider/utils/replaceArrayItemAtIndex.ts new file mode 100644 index 0000000000..4864f9591e --- /dev/null +++ b/packages/react/src/slider/utils/replaceArrayItemAtIndex.ts @@ -0,0 +1,7 @@ +import { asc } from './asc'; + +export function replaceArrayItemAtIndex(array: readonly number[], index: number, newValue: number) { + const output = array.slice(); + output[index] = newValue; + return output.sort(asc); +} diff --git a/packages/react/src/slider/utils/setValueIndex.ts b/packages/react/src/slider/utils/setValueIndex.ts deleted file mode 100644 index 291e096495..0000000000 --- a/packages/react/src/slider/utils/setValueIndex.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { asc } from './asc'; - -export function setValueIndex({ - values, - newValue, - index, -}: { - values: readonly number[]; - newValue: number; - index: number; -}) { - const output = values.slice(); - output[index] = newValue; - return output.sort(asc); -} diff --git a/packages/react/src/slider/value/SliderValue.test.tsx b/packages/react/src/slider/value/SliderValue.test.tsx index 5056c3715c..c060d813d8 100644 --- a/packages/react/src/slider/value/SliderValue.test.tsx +++ b/packages/react/src/slider/value/SliderValue.test.tsx @@ -14,8 +14,9 @@ const testRootContext: SliderRootContext = { disabled: false, getFingerState: () => ({ value: 0, - closestThumbIndex: 0, percentageValue: 0, + percentageValues: [0], + thumbIndex: 0, }), setValue: NOOP, largeStep: 10, @@ -44,6 +45,7 @@ const testRootContext: SliderRootContext = { registerSliderControl: NOOP, setActive: NOOP, setDragging: NOOP, + setPercentageValues: NOOP, setThumbMap: NOOP, step: 1, tabIndex: null, From f6cbce4260bef677a5799902c1bdbc10b4aa0e9f Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Thu, 2 Jan 2025 16:55:42 +0800 Subject: [PATCH 10/13] Some polish --- .../src/slider/control/SliderControl.test.tsx | 2 +- .../src/slider/control/useSliderControl.ts | 41 +++++++-- .../slider/indicator/SliderIndicator.test.tsx | 2 +- .../react/src/slider/root/useSliderRoot.ts | 87 ++++++------------- .../src/slider/thumb/SliderThumb.test.tsx | 2 +- .../react/src/slider/thumb/useSliderThumb.ts | 13 +-- .../src/slider/track/SliderTrack.test.tsx | 2 +- .../react/src/slider/utils/getSliderValue.ts | 20 ++--- .../react/src/slider/utils/percentToValue.ts | 3 - .../src/slider/value/SliderValue.test.tsx | 2 +- 10 files changed, 75 insertions(+), 99 deletions(-) delete mode 100644 packages/react/src/slider/utils/percentToValue.ts diff --git a/packages/react/src/slider/control/SliderControl.test.tsx b/packages/react/src/slider/control/SliderControl.test.tsx index 07d4982407..9eef837772 100644 --- a/packages/react/src/slider/control/SliderControl.test.tsx +++ b/packages/react/src/slider/control/SliderControl.test.tsx @@ -12,7 +12,7 @@ const testRootContext: SliderRootContext = { disabled: false, getFingerState: () => ({ value: 0, - percentageValue: 0, + valueRescaled: 0, percentageValues: [0], thumbIndex: 0, }), diff --git a/packages/react/src/slider/control/useSliderControl.ts b/packages/react/src/slider/control/useSliderControl.ts index fc865654c5..ef6fd13269 100644 --- a/packages/react/src/slider/control/useSliderControl.ts +++ b/packages/react/src/slider/control/useSliderControl.ts @@ -7,14 +7,41 @@ import { useForkRef } from '../../utils/useForkRef'; import { useEventCallback } from '../../utils/useEventCallback'; import { focusThumb, - trackFinger, validateMinimumDistance, + type FingerPosition, type useSliderRoot, } from '../root/useSliderRoot'; import { useFieldControlValidation } from '../../field/control/useFieldControlValidation'; const INTENTIONAL_DRAG_COUNT_THRESHOLD = 2; +function trackFinger( + event: TouchEvent | PointerEvent | React.PointerEvent, + touchIdRef: React.RefObject, +): FingerPosition | null { + // The event is TouchEvent + if (touchIdRef.current !== undefined && (event as TouchEvent).changedTouches) { + const touchEvent = event as TouchEvent; + for (let i = 0; i < touchEvent.changedTouches.length; i += 1) { + const touch = touchEvent.changedTouches[i]; + if (touch.identifier === touchIdRef.current) { + return { + x: touch.clientX, + y: touch.clientY, + }; + } + } + + return null; + } + + // The event is PointerEvent + return { + x: (event as PointerEvent).clientX, + y: (event as PointerEvent).clientY, + }; +} + export function useSliderControl( parameters: useSliderControl.Parameters, ): useSliderControl.ReturnValue { @@ -42,12 +69,11 @@ export function useSliderControl( // A number that uniquely identifies the current finger in the touch session. const touchIdRef = React.useRef(null); - const moveCountRef = React.useRef(0); - - // offset distance between: - // 1. pointerDown coordinates and - // 2. the exact intersection of the center of the thumb and the track + /** + * The difference between the value at the finger origin and the value at + * the center of the thumb scaled down to fit the range [0, 1] + */ const offsetRef = React.useRef(0); const handleTouchMove = useEventCallback((nativeEvent: TouchEvent | PointerEvent) => { @@ -209,8 +235,7 @@ export function useSliderControl( // percentageValue difference represented by the distance between the click origin // and the coordinates of the value on the track area if (thumbRefs.current.includes(event.target as HTMLElement)) { - offsetRef.current = - percentageValues[finger.thumbIndex] / 100 - finger.percentageValue; + offsetRef.current = percentageValues[finger.thumbIndex] / 100 - finger.valueRescaled; } else { setValue(finger.value, finger.percentageValues, finger.thumbIndex, event.nativeEvent); } diff --git a/packages/react/src/slider/indicator/SliderIndicator.test.tsx b/packages/react/src/slider/indicator/SliderIndicator.test.tsx index 49ac011d9c..63114597bb 100644 --- a/packages/react/src/slider/indicator/SliderIndicator.test.tsx +++ b/packages/react/src/slider/indicator/SliderIndicator.test.tsx @@ -12,7 +12,7 @@ const testRootContext: SliderRootContext = { disabled: false, getFingerState: () => ({ value: 0, - percentageValue: 0, + valueRescaled: 0, percentageValues: [0], thumbIndex: 0, }), diff --git a/packages/react/src/slider/root/useSliderRoot.ts b/packages/react/src/slider/root/useSliderRoot.ts index cfe3f6960d..7da886c635 100644 --- a/packages/react/src/slider/root/useSliderRoot.ts +++ b/packages/react/src/slider/root/useSliderRoot.ts @@ -17,7 +17,6 @@ import { useFieldRootContext } from '../../field/root/FieldRootContext'; import { useFieldControlValidation } from '../../field/control/useFieldControlValidation'; import { asc } from '../utils/asc'; import { getSliderValue } from '../utils/getSliderValue'; -import { percentToValue } from '../utils/percentToValue'; import { replaceArrayItemAtIndex } from '../utils/replaceArrayItemAtIndex'; import { roundValueToStep } from '../utils/roundValueToStep'; import { ThumbMetadata } from '../thumb/useSliderThumb'; @@ -104,33 +103,6 @@ export function validateMinimumDistance( return Math.min(...distances) >= step * minStepsBetweenValues; } -export function trackFinger( - event: TouchEvent | PointerEvent | React.PointerEvent, - touchIdRef: React.RefObject, -): FingerPosition | null { - // The event is TouchEvent - if (touchIdRef.current !== undefined && (event as TouchEvent).changedTouches) { - const touchEvent = event as TouchEvent; - for (let i = 0; i < touchEvent.changedTouches.length; i += 1) { - const touch = touchEvent.changedTouches[i]; - if (touch.identifier === touchIdRef.current) { - return { - x: touch.clientX, - y: touch.clientY, - }; - } - } - - return null; - } - - // The event is PointerEvent - return { - x: (event as PointerEvent).clientX, - y: (event as PointerEvent).clientY, - }; -} - /** */ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRoot.ReturnValue { @@ -211,13 +183,13 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo const range = Array.isArray(valueUnwrapped); const values = React.useMemo(() => { - return (range ? valueUnwrapped.slice().sort(asc) : [valueUnwrapped]).map((val) => - val == null ? min : clamp(val, min, max), - ); + if (!range) { + return [clamp(valueUnwrapped as number, min, max)]; + } + return valueUnwrapped.slice().sort(asc); }, [max, min, range, valueUnwrapped]); function initializePercentageValues() { - // console.log('initializePercentageValues'); const vals = []; for (let i = 0; i < values.length; i += 1) { vals.push(valueToPercent(values[i], min, max)); @@ -228,7 +200,6 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo const [percentageValues, setPercentageValues] = React.useState( initializePercentageValues, ); - // console.log('percentageValues', percentageValues); const setValue = useEventCallback( ( @@ -263,14 +234,7 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo const handleInputChange = useEventCallback( (valueInput: number, index: number, event: React.KeyboardEvent | React.ChangeEvent) => { - const newValue = getSliderValue({ - valueInput, - min, - max, - index, - range, - values, - }); + const newValue = getSliderValue(valueInput, index, min, max, range, values); if (range) { focusThumb(index, sliderRef); @@ -310,10 +274,11 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo */ shouldCaptureThumbIndex: boolean = false, /** - * The pixel distance between the finger origin and the center of the thumb. + * The difference between the value at the finger origin and the value at + * the center of the thumb scaled down to fit the range [0, 1] */ offset: number = 0, - ) => { + ): FingerState | null => { if (fingerPosition == null) { return null; } @@ -329,26 +294,26 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo const { width, height, bottom, left } = sliderControl.getBoundingClientRect(); - // percent is a value between 0 and 1, e.g. "41%" is `0.41` - const valueRescaled = isVertical + // the value at the finger origin scaled down to fit the range [0, 1] + let valueRescaled = isVertical ? (bottom - fingerPosition.y) / height + offset : (fingerPosition.x - left) / width + offset * (isRtl ? -1 : 1); - let percentageValue = Math.min(valueRescaled, 1); + valueRescaled = Math.min(valueRescaled, 1); if (isRtl && !isVertical) { - percentageValue = 1 - percentageValue; + valueRescaled = 1 - valueRescaled; } - let newValue = percentToValue(percentageValue, min, max); + let newValue = (max - min) * valueRescaled + min; newValue = roundValueToStep(newValue, step, min); newValue = clamp(newValue, min, max); if (!range) { return { value: newValue, - percentageValue, - percentageValues: [percentageValue * 100], + valueRescaled, + percentageValues: [valueRescaled * 100], thumbIndex: 0, }; } @@ -368,11 +333,11 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo return { value: replaceArrayItemAtIndex(values, closestThumbIndex, newValue), - percentageValue, + valueRescaled, percentageValues: replaceArrayItemAtIndex( percentageValues, closestThumbIndex, - percentageValue * 100, + valueRescaled * 100, ), thumbIndex: closestThumbIndex, }; @@ -467,10 +432,17 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo ); } -export type FingerPosition = { +export interface FingerPosition { x: number; y: number; -}; +} + +interface FingerState { + value: number | number[]; + valueRescaled: number; + percentageValues: number[]; + thumbIndex: number; +} export namespace useSliderRoot { export type Orientation = 'horizontal' | 'vertical'; @@ -589,12 +561,7 @@ export namespace useSliderRoot { fingerPosition: FingerPosition | null, shouldCaptureThumbIndex?: boolean, offset?: number, - ) => { - value: number | number[]; - percentageValue: number; - percentageValues: number[]; - thumbIndex: number; - } | null; + ) => FingerState | null; /** * Callback to invoke change handlers after internal value state is updated. */ diff --git a/packages/react/src/slider/thumb/SliderThumb.test.tsx b/packages/react/src/slider/thumb/SliderThumb.test.tsx index 86e58c3368..fa7b44a92f 100644 --- a/packages/react/src/slider/thumb/SliderThumb.test.tsx +++ b/packages/react/src/slider/thumb/SliderThumb.test.tsx @@ -12,7 +12,7 @@ const testRootContext: SliderRootContext = { disabled: false, getFingerState: () => ({ value: 0, - percentageValue: 0, + valueRescaled: 0, percentageValues: [0], thumbIndex: 0, }), diff --git a/packages/react/src/slider/thumb/useSliderThumb.ts b/packages/react/src/slider/thumb/useSliderThumb.ts index c7b697ee21..86db210a24 100644 --- a/packages/react/src/slider/thumb/useSliderThumb.ts +++ b/packages/react/src/slider/thumb/useSliderThumb.ts @@ -143,14 +143,7 @@ export function useSliderThumb(parameters: useSliderThumb.Parameters): useSlider } setTouched(true); commitValidation( - getSliderValue({ - valueInput: thumbValue, - min, - max, - index, - range: sliderValues.length > 1, - values: sliderValues, - }), + getSliderValue(thumbValue, index, min, max, sliderValues.length > 1, sliderValues), ); }, onKeyDown(event: React.KeyboardEvent) { @@ -215,9 +208,7 @@ export function useSliderThumb(parameters: useSliderThumb.Parameters): useSlider } }, ref: mergedThumbRef, - style: { - ...getThumbStyle(), - }, + style: getThumbStyle(), tabIndex: externalTabIndex ?? (disabled ? undefined : 0), }); }, diff --git a/packages/react/src/slider/track/SliderTrack.test.tsx b/packages/react/src/slider/track/SliderTrack.test.tsx index f8e75b411e..aca1fb5d5b 100644 --- a/packages/react/src/slider/track/SliderTrack.test.tsx +++ b/packages/react/src/slider/track/SliderTrack.test.tsx @@ -12,7 +12,7 @@ const testRootContext: SliderRootContext = { disabled: false, getFingerState: () => ({ value: 0, - percentageValue: 0, + valueRescaled: 0, percentageValues: [0], thumbIndex: 0, }), diff --git a/packages/react/src/slider/utils/getSliderValue.ts b/packages/react/src/slider/utils/getSliderValue.ts index b031f84d92..f82ef09aa5 100644 --- a/packages/react/src/slider/utils/getSliderValue.ts +++ b/packages/react/src/slider/utils/getSliderValue.ts @@ -1,18 +1,14 @@ import { clamp } from '../../utils/clamp'; import { replaceArrayItemAtIndex } from './replaceArrayItemAtIndex'; -interface GetSliderValueParameters { - valueInput: number; - index: number; - min: number; - max: number; - range: boolean; - values: readonly number[]; -} - -export function getSliderValue(params: GetSliderValueParameters) { - const { valueInput, index, min, max, range, values } = params; - +export function getSliderValue( + valueInput: number, + index: number, + min: number, + max: number, + range: boolean, + values: readonly number[], +) { let newValue: number | number[] = valueInput; newValue = clamp(newValue, min, max); diff --git a/packages/react/src/slider/utils/percentToValue.ts b/packages/react/src/slider/utils/percentToValue.ts deleted file mode 100644 index cc12d737b7..0000000000 --- a/packages/react/src/slider/utils/percentToValue.ts +++ /dev/null @@ -1,3 +0,0 @@ -export function percentToValue(percent: number, min: number, max: number) { - return (max - min) * percent + min; -} diff --git a/packages/react/src/slider/value/SliderValue.test.tsx b/packages/react/src/slider/value/SliderValue.test.tsx index c060d813d8..ed960c2601 100644 --- a/packages/react/src/slider/value/SliderValue.test.tsx +++ b/packages/react/src/slider/value/SliderValue.test.tsx @@ -14,7 +14,7 @@ const testRootContext: SliderRootContext = { disabled: false, getFingerState: () => ({ value: 0, - percentageValue: 0, + valueRescaled: 0, percentageValues: [0], thumbIndex: 0, }), From aa160c4c7ae979d50ffd28252e0a1282dc1d5917 Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Fri, 3 Jan 2025 15:14:11 +0800 Subject: [PATCH 11/13] Remove direction from slider context --- .../react/src/slider/control/SliderControl.test.tsx | 1 - .../src/slider/indicator/SliderIndicator.test.tsx | 1 - .../react/src/slider/indicator/SliderIndicator.tsx | 3 +-- .../react/src/slider/indicator/useSliderIndicator.ts | 5 +---- packages/react/src/slider/root/SliderRoot.tsx | 3 --- packages/react/src/slider/root/useSliderRoot.ts | 12 ++---------- packages/react/src/slider/thumb/SliderThumb.test.tsx | 1 - packages/react/src/slider/thumb/SliderThumb.tsx | 2 -- packages/react/src/slider/thumb/useSliderThumb.ts | 4 ++-- packages/react/src/slider/track/SliderTrack.test.tsx | 1 - packages/react/src/slider/value/SliderValue.test.tsx | 1 - 11 files changed, 6 insertions(+), 28 deletions(-) diff --git a/packages/react/src/slider/control/SliderControl.test.tsx b/packages/react/src/slider/control/SliderControl.test.tsx index 9eef837772..4e55a7a590 100644 --- a/packages/react/src/slider/control/SliderControl.test.tsx +++ b/packages/react/src/slider/control/SliderControl.test.tsx @@ -7,7 +7,6 @@ import { NOOP } from '../../utils/noop'; const testRootContext: SliderRootContext = { active: -1, handleInputChange: NOOP, - direction: 'ltr', dragging: false, disabled: false, getFingerState: () => ({ diff --git a/packages/react/src/slider/indicator/SliderIndicator.test.tsx b/packages/react/src/slider/indicator/SliderIndicator.test.tsx index 63114597bb..04eb7806f9 100644 --- a/packages/react/src/slider/indicator/SliderIndicator.test.tsx +++ b/packages/react/src/slider/indicator/SliderIndicator.test.tsx @@ -7,7 +7,6 @@ import { NOOP } from '../../utils/noop'; const testRootContext: SliderRootContext = { active: -1, handleInputChange: NOOP, - direction: 'ltr', dragging: false, disabled: false, getFingerState: () => ({ diff --git a/packages/react/src/slider/indicator/SliderIndicator.tsx b/packages/react/src/slider/indicator/SliderIndicator.tsx index 7c09a0ae60..653d6d67b8 100644 --- a/packages/react/src/slider/indicator/SliderIndicator.tsx +++ b/packages/react/src/slider/indicator/SliderIndicator.tsx @@ -20,10 +20,9 @@ const SliderIndicator = React.forwardRef(function SliderIndicator( ) { const { render, className, ...otherProps } = props; - const { direction, disabled, orientation, state, percentageValues } = useSliderRootContext(); + const { disabled, orientation, state, percentageValues } = useSliderRootContext(); const { getRootProps } = useSliderIndicator({ - direction, disabled, orientation, percentageValues, diff --git a/packages/react/src/slider/indicator/useSliderIndicator.ts b/packages/react/src/slider/indicator/useSliderIndicator.ts index e8271ccb91..b8cef2d682 100644 --- a/packages/react/src/slider/indicator/useSliderIndicator.ts +++ b/packages/react/src/slider/indicator/useSliderIndicator.ts @@ -71,10 +71,7 @@ export function useSliderIndicator( export namespace useSliderIndicator { export interface Parameters - extends Pick< - useSliderRoot.ReturnValue, - 'direction' | 'disabled' | 'orientation' | 'percentageValues' - > {} + extends Pick {} export interface ReturnValue { getRootProps: (externalProps?: GenericHTMLProps) => GenericHTMLProps; diff --git a/packages/react/src/slider/root/SliderRoot.tsx b/packages/react/src/slider/root/SliderRoot.tsx index ad3e1cad8e..b0d56ac5b0 100644 --- a/packages/react/src/slider/root/SliderRoot.tsx +++ b/packages/react/src/slider/root/SliderRoot.tsx @@ -7,7 +7,6 @@ import { useBaseUiId } from '../../utils/useBaseUiId'; import { useComponentRenderer } from '../../utils/useComponentRenderer'; import type { FieldRoot } from '../../field/root/FieldRoot'; import { CompositeList } from '../../composite/list/CompositeList'; -import { useDirection } from '../../direction-provider/DirectionContext'; import { sliderStyleHookMapping } from './styleHooks'; import { useSliderRoot } from './useSliderRoot'; import { SliderRootContext } from './SliderRootContext'; @@ -46,7 +45,6 @@ const SliderRoot = React.forwardRef(function SliderRoot( } = props; const id = useBaseUiId(idProp); - const direction = useDirection(); const { labelId, state: fieldState, disabled: fieldDisabled } = useFieldRootContext(); const disabled = fieldDisabled || disabledProp; @@ -54,7 +52,6 @@ const SliderRoot = React.forwardRef(function SliderRoot( const { getRootProps, ...slider } = useSliderRoot({ 'aria-labelledby': ariaLabelledby ?? labelId ?? '', defaultValue, - direction, disabled, id: id ?? '', largeStep, diff --git a/packages/react/src/slider/root/useSliderRoot.ts b/packages/react/src/slider/root/useSliderRoot.ts index 7da886c635..1c3d0e6ae7 100644 --- a/packages/react/src/slider/root/useSliderRoot.ts +++ b/packages/react/src/slider/root/useSliderRoot.ts @@ -11,7 +11,7 @@ import { useEventCallback } from '../../utils/useEventCallback'; import { useForkRef } from '../../utils/useForkRef'; import { valueToPercent } from '../../utils/valueToPercent'; import type { CompositeMetadata } from '../../composite/list/CompositeList'; -import type { TextDirection } from '../../direction-provider/DirectionContext'; +import { useDirection } from '../../direction-provider/DirectionContext'; import { useField } from '../../field/useField'; import { useFieldRootContext } from '../../field/root/FieldRootContext'; import { useFieldControlValidation } from '../../field/control/useFieldControlValidation'; @@ -109,7 +109,6 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo const { 'aria-labelledby': ariaLabelledby, defaultValue, - direction = 'ltr', disabled = false, id, largeStep = 10, @@ -125,6 +124,7 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo value: valueProp, } = parameters; + const direction = useDirection(); const { setControlId, setTouched, setDirty, validityData } = useFieldRootContext(); const { @@ -375,7 +375,6 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo getRootProps, 'aria-labelledby': ariaLabelledby, active, - direction, disabled, dragging, getFingerState, @@ -404,7 +403,6 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo getRootProps, active, ariaLabelledby, - direction, disabled, dragging, getFingerState, @@ -460,11 +458,6 @@ export namespace useSliderRoot { * The default value. Use when the component is not controlled. */ defaultValue?: number | readonly number[]; - /** - * Sets the direction. For right-to-left languages, the lowest value is on the right-hand side. - * @default 'ltr' - */ - direction: TextDirection; /** * Whether the component should ignore user interaction. * @default false @@ -555,7 +548,6 @@ export namespace useSliderRoot { event: React.KeyboardEvent | React.ChangeEvent, ) => void; dragging: boolean; - direction: TextDirection; disabled: boolean; getFingerState: ( fingerPosition: FingerPosition | null, diff --git a/packages/react/src/slider/thumb/SliderThumb.test.tsx b/packages/react/src/slider/thumb/SliderThumb.test.tsx index fa7b44a92f..55b4d7c727 100644 --- a/packages/react/src/slider/thumb/SliderThumb.test.tsx +++ b/packages/react/src/slider/thumb/SliderThumb.test.tsx @@ -7,7 +7,6 @@ import { NOOP } from '../../utils/noop'; const testRootContext: SliderRootContext = { active: -1, handleInputChange: NOOP, - direction: 'ltr', dragging: false, disabled: false, getFingerState: () => ({ diff --git a/packages/react/src/slider/thumb/SliderThumb.tsx b/packages/react/src/slider/thumb/SliderThumb.tsx index a7b53f4433..0f3b166457 100644 --- a/packages/react/src/slider/thumb/SliderThumb.tsx +++ b/packages/react/src/slider/thumb/SliderThumb.tsx @@ -62,7 +62,6 @@ const SliderThumb = React.forwardRef(function SliderThumb( active: activeIndex, 'aria-labelledby': ariaLabelledby, handleInputChange, - direction, disabled: contextDisabled, format = null, largeStep, @@ -91,7 +90,6 @@ const SliderThumb = React.forwardRef(function SliderThumb( 'aria-labelledby': ariaLabelledby ?? '', 'aria-valuetext': ariaValuetext ?? '', handleInputChange, - direction, disabled: disabledProp || contextDisabled, format, getAriaLabel: getAriaLabelProp ?? null, diff --git a/packages/react/src/slider/thumb/useSliderThumb.ts b/packages/react/src/slider/thumb/useSliderThumb.ts index 86db210a24..b0e8c1042a 100644 --- a/packages/react/src/slider/thumb/useSliderThumb.ts +++ b/packages/react/src/slider/thumb/useSliderThumb.ts @@ -14,6 +14,7 @@ import { END, } from '../../composite/composite'; import { useCompositeListItem } from '../../composite/list/useCompositeListItem'; +import { useDirection } from '../../direction-provider/DirectionContext'; import { useFieldControlValidation } from '../../field/control/useFieldControlValidation'; import { useFieldRootContext } from '../../field/root/FieldRootContext'; import { getSliderValue } from '../utils/getSliderValue'; @@ -60,7 +61,6 @@ export function useSliderThumb(parameters: useSliderThumb.Parameters): useSlider 'aria-labelledby': ariaLabelledby, 'aria-valuetext': ariaValuetext, handleInputChange, - direction, disabled, format, getAriaLabel = null, @@ -80,6 +80,7 @@ export function useSliderThumb(parameters: useSliderThumb.Parameters): useSlider values: sliderValues, } = parameters; + const direction = useDirection(); const { setTouched } = useFieldRootContext(); const { getInputValidationProps, @@ -316,7 +317,6 @@ export namespace useSliderThumb { | 'active' | 'aria-labelledby' | 'handleInputChange' - | 'direction' | 'largeStep' | 'max' | 'min' diff --git a/packages/react/src/slider/track/SliderTrack.test.tsx b/packages/react/src/slider/track/SliderTrack.test.tsx index aca1fb5d5b..4219709da6 100644 --- a/packages/react/src/slider/track/SliderTrack.test.tsx +++ b/packages/react/src/slider/track/SliderTrack.test.tsx @@ -7,7 +7,6 @@ import { NOOP } from '../../utils/noop'; const testRootContext: SliderRootContext = { active: -1, handleInputChange: NOOP, - direction: 'ltr', dragging: false, disabled: false, getFingerState: () => ({ diff --git a/packages/react/src/slider/value/SliderValue.test.tsx b/packages/react/src/slider/value/SliderValue.test.tsx index ed960c2601..3d6e62f2e0 100644 --- a/packages/react/src/slider/value/SliderValue.test.tsx +++ b/packages/react/src/slider/value/SliderValue.test.tsx @@ -9,7 +9,6 @@ import { NOOP } from '../../utils/noop'; const testRootContext: SliderRootContext = { active: -1, handleInputChange: NOOP, - direction: 'ltr', dragging: false, disabled: false, getFingerState: () => ({ From 69165b823173c868702b774ef6d4dfd5500044b9 Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Fri, 3 Jan 2025 15:41:58 +0800 Subject: [PATCH 12/13] Clamp scaled down value --- packages/react/src/slider/root/useSliderRoot.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/slider/root/useSliderRoot.ts b/packages/react/src/slider/root/useSliderRoot.ts index 1c3d0e6ae7..2f816e0f21 100644 --- a/packages/react/src/slider/root/useSliderRoot.ts +++ b/packages/react/src/slider/root/useSliderRoot.ts @@ -299,7 +299,7 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo ? (bottom - fingerPosition.y) / height + offset : (fingerPosition.x - left) / width + offset * (isRtl ? -1 : 1); - valueRescaled = Math.min(valueRescaled, 1); + valueRescaled = clamp(valueRescaled, 0, 1); if (isRtl && !isVertical) { valueRescaled = 1 - valueRescaled; From 57d18c50fccc37bbbc4deb084991b892a97856c1 Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Tue, 7 Jan 2025 15:11:17 +0800 Subject: [PATCH 13/13] Callback array values should not be readonly --- packages/react/src/slider/root/useSliderRoot.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/react/src/slider/root/useSliderRoot.ts b/packages/react/src/slider/root/useSliderRoot.ts index 2f816e0f21..5dcdc0687b 100644 --- a/packages/react/src/slider/root/useSliderRoot.ts +++ b/packages/react/src/slider/root/useSliderRoot.ts @@ -203,7 +203,7 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo const setValue = useEventCallback( ( - newValue: number | readonly number[], + newValue: number | number[], newPercentageValues: readonly number[], thumbIndex: number, event: Event, @@ -492,11 +492,7 @@ export namespace useSliderRoot { * You can pull out the new value by accessing `event.target.value` (any). * @param {number} activeThumbIndex Index of the currently moved thumb. */ - onValueChange: ( - value: number | readonly number[], - event: Event, - activeThumbIndex: number, - ) => void; + onValueChange: (value: number | number[], event: Event, activeThumbIndex: number) => void; /** * Callback function that is fired when the `pointerup` is triggered. * @@ -558,8 +554,8 @@ export namespace useSliderRoot { * Callback to invoke change handlers after internal value state is updated. */ setValue: ( - newValue: number | readonly number[], - newPercentageValue: readonly number[], + newValue: number | number[], + newPercentageValues: readonly number[], activeThumb: number, event: Event, ) => void;