Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Slider] Fix thumb positioning #1411

Merged
merged 7 commits into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/react/src/slider/control/SliderControl.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { NOOP } from '../../utils/noop';

const testRootContext: SliderRootContext = {
active: -1,
handleInputChange: NOOP,
commitValue: NOOP,
dragging: false,
disabled: false,
getFingerState: () => ({
Expand All @@ -15,15 +15,14 @@ const testRootContext: SliderRootContext = {
percentageValues: [0],
thumbIndex: 0,
}),
setValue: NOOP,
handleInputChange: NOOP,
largeStep: 10,
lastChangedValueRef: { current: null },
thumbMap: new Map(),
max: 100,
min: 0,
minStepsBetweenValues: 0,
name: '',
onValueCommitted: NOOP,
orientation: 'horizontal',
state: {
activeThumbIndex: -1,
Expand All @@ -47,6 +46,7 @@ const testRootContext: SliderRootContext = {
setDragging: NOOP,
setPercentageValues: NOOP,
setThumbMap: NOOP,
setValue: NOOP,
step: 1,
tabIndex: null,
thumbRefs: { current: [] },
Expand Down
4 changes: 2 additions & 2 deletions packages/react/src/slider/control/SliderControl.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ const SliderControl = React.forwardRef(function SliderControl(
const { render: renderProp, className, ...otherProps } = props;

const {
commitValue,
disabled,
dragging,
getFingerState,
lastChangedValueRef,
minStepsBetweenValues,
onValueCommitted,
percentageValues,
registerSliderControl,
setActive,
Expand All @@ -38,12 +38,12 @@ const SliderControl = React.forwardRef(function SliderControl(
} = useSliderRootContext();

const { getRootProps } = useSliderControl({
commitValue,
disabled,
dragging,
getFingerState,
lastChangedValueRef,
minStepsBetweenValues,
onValueCommitted,
percentageValues,
registerSliderControl,
rootRef: forwardedRef,
Expand Down
8 changes: 3 additions & 5 deletions packages/react/src/slider/control/useSliderControl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export function useSliderControl(
getFingerState,
lastChangedValueRef,
minStepsBetweenValues,
onValueCommitted,
commitValue,
percentageValues,
registerSliderControl,
rootRef: externalRef,
Expand Down Expand Up @@ -128,11 +128,9 @@ export function useSliderControl(
setActive(-1);

commitValidation(lastChangedValueRef.current ?? finger.value);

onValueCommitted(lastChangedValueRef.current ?? finger.value, nativeEvent);
commitValue(lastChangedValueRef.current ?? finger.value, nativeEvent);

touchIdRef.current = null;

// eslint-disable-next-line @typescript-eslint/no-use-before-define
stopListening();
});
Expand Down Expand Up @@ -280,7 +278,7 @@ export namespace useSliderControl {
| 'getFingerState'
| 'lastChangedValueRef'
| 'minStepsBetweenValues'
| 'onValueCommitted'
| 'commitValue'
| 'percentageValues'
| 'registerSliderControl'
| 'setActive'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { NOOP } from '../../utils/noop';

const testRootContext: SliderRootContext = {
active: -1,
handleInputChange: NOOP,
commitValue: NOOP,
dragging: false,
disabled: false,
getFingerState: () => ({
Expand All @@ -15,15 +15,14 @@ const testRootContext: SliderRootContext = {
percentageValues: [0],
thumbIndex: 0,
}),
setValue: NOOP,
handleInputChange: NOOP,
largeStep: 10,
lastChangedValueRef: { current: null },
thumbMap: new Map(),
max: 100,
min: 0,
minStepsBetweenValues: 0,
name: '',
onValueCommitted: NOOP,
orientation: 'horizontal',
state: {
activeThumbIndex: -1,
Expand All @@ -47,6 +46,7 @@ const testRootContext: SliderRootContext = {
setDragging: NOOP,
setPercentageValues: NOOP,
setThumbMap: NOOP,
setValue: NOOP,
step: 1,
tabIndex: null,
thumbRefs: { current: [] },
Expand Down
90 changes: 65 additions & 25 deletions packages/react/src/slider/root/useSliderRoot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { areArraysEqual } from '../../utils/areArraysEqual';
import { clamp } from '../../utils/clamp';
import { mergeReactProps } from '../../utils/mergeReactProps';
import { ownerDocument } from '../../utils/owner';
import type { GenericHTMLProps } from '../../utils/types';
import { useControlled } from '../../utils/useControlled';
import { useEnhancedEffect } from '../../utils/useEnhancedEffect';
import { useEventCallback } from '../../utils/useEventCallback';
Expand Down Expand Up @@ -54,6 +55,14 @@ function findClosest(values: readonly number[], currentValue: number) {
return closestIndex;
}

function valueArrayToPercentages(values: number[], min: number, max: number) {
const output = [];
for (let i = 0; i < values.length; i += 1) {
output.push(valueToPercent(values[i], min, max));
}
return output;
}

export function focusThumb(
thumbIndex: number,
sliderRef: React.RefObject<HTMLElement | null>,
Expand Down Expand Up @@ -192,11 +201,7 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo
}, [max, min, range, valueUnwrapped]);

function initializePercentageValues() {
const vals = [];
for (let i = 0; i < values.length; i += 1) {
vals.push(valueToPercent(values[i], min, max));
}
return vals;
return valueArrayToPercentages(values, min, max);
}

const [percentageValues, setPercentageValues] = React.useState<readonly number[]>(
Expand Down Expand Up @@ -233,8 +238,22 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo
},
);

// for pointer drag only
const commitValue = useEventCallback((value: number | readonly number[], event: Event) => {
if (Array.isArray(value)) {
const newPercentageValues = valueArrayToPercentages(value, min, max);
if (!areArraysEqual(newPercentageValues, percentageValues)) {
setPercentageValues(newPercentageValues);
}
} else if (typeof value === 'number') {
setPercentageValues([valueToPercent(value, min, max)]);
}
onValueCommitted(value, event);
});

const handleRootRef = useForkRef(rootRef, sliderRef);

// for keypresses only
const handleInputChange = useEventCallback(
(valueInput: number, index: number, event: React.KeyboardEvent | React.ChangeEvent) => {
const newValue = getSliderValue(valueInput, index, min, max, range, values);
Expand Down Expand Up @@ -347,6 +366,24 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo
},
);

useEnhancedEffect(() => {
if (valueProp === undefined || dragging) {
return;
}

if (typeof valueUnwrapped === 'number') {
const newPercentageValue = valueToPercent(valueUnwrapped, min, max);
if (newPercentageValue !== percentageValues[0]) {
setPercentageValues([newPercentageValue]);
}
} else if (Array.isArray(valueUnwrapped)) {
const newPercentageValues = valueArrayToPercentages(valueUnwrapped, min, max);
if (!areArraysEqual(newPercentageValues, percentageValues)) {
setPercentageValues(newPercentageValues);
}
}
}, [dragging, min, max, percentageValues, setPercentageValues, valueProp, valueUnwrapped]);

useEnhancedEffect(() => {
const activeEl = activeElement(ownerDocument(sliderRef.current));
if (disabled && sliderRef.current?.contains(activeEl)) {
Expand Down Expand Up @@ -375,12 +412,13 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo

return React.useMemo(
() => ({
getRootProps,
'aria-labelledby': ariaLabelledby,
active,
commitValue,
disabled,
dragging,
getFingerState,
getRootProps,
handleInputChange,
largeStep,
lastChangedValueRef,
Expand All @@ -404,12 +442,13 @@ export function useSliderRoot(parameters: useSliderRoot.Parameters): useSliderRo
values,
}),
[
getRootProps,
active,
ariaLabelledby,
commitValue,
disabled,
dragging,
getFingerState,
getRootProps,
handleInputChange,
largeStep,
lastChangedValueRef,
Expand Down Expand Up @@ -535,34 +574,27 @@ export namespace useSliderRoot {
}

export interface ReturnValue {
getRootProps: (
externalProps?: React.ComponentPropsWithRef<'div'>,
) => React.ComponentPropsWithRef<'div'>;
/**
* The index of the active thumb.
*/
active: number;
'aria-labelledby'?: string;
handleInputChange: (
valueInput: number,
index: number,
event: React.KeyboardEvent | React.ChangeEvent,
) => void;
/**
* Function to be called when drag ends. Invokes onValueCommitted.
*/
commitValue: (newValue: number | readonly number[], event: Event) => void;
dragging: boolean;
disabled: boolean;
getFingerState: (
fingerPosition: FingerPosition | null,
shouldCaptureThumbIndex?: boolean,
offset?: number,
) => FingerState | null;
/**
* Callback to invoke change handlers after internal value state is updated.
*/
setValue: (
newValue: number | number[],
newPercentageValues: readonly number[],
activeThumb: number,
event: Event,
getRootProps: (externalProps?: GenericHTMLProps) => GenericHTMLProps;
handleInputChange: (
valueInput: number,
index: number,
event: React.KeyboardEvent | React.ChangeEvent,
) => void;
/**
* The large step value of the slider when incrementing or decrementing while the shift key is held,
Expand All @@ -584,23 +616,31 @@ export namespace useSliderRoot {
*/
minStepsBetweenValues: number;
name: string;
onValueCommitted: (value: number | readonly number[], event: Event) => void;
/**
* The component orientation.
* @default 'horizontal'
*/
orientation: Orientation;
registerSliderControl: (element: HTMLElement | null) => void;
/**
* The value(s) of the slider as percentages
*/
percentageValues: readonly number[];
registerSliderControl: (element: HTMLElement | null) => void;
setActive: React.Dispatch<React.SetStateAction<number>>;
setDragging: React.Dispatch<React.SetStateAction<boolean>>;
setPercentageValues: React.Dispatch<React.SetStateAction<readonly number[]>>;
setThumbMap: React.Dispatch<
React.SetStateAction<Map<Node, CompositeMetadata<ThumbMetadata> | null>>
>;
/**
* Callback fired when dragging and invokes onValueChange.
*/
setValue: (
newValue: number | number[],
newPercentageValues: readonly number[],
activeThumb: number,
event: Event,
) => void;
/**
* The step increment of the slider when incrementing or decrementing. It will snap
* to multiples of this value. Decimal values are supported.
Expand Down
Loading