diff --git a/packages/react/src/components/Slider/Slider.tsx b/packages/react/src/components/Slider/Slider.tsx index cd65a52ba541..07fe9d7a17bf 100644 --- a/packages/react/src/components/Slider/Slider.tsx +++ b/packages/react/src/components/Slider/Slider.tsx @@ -96,12 +96,9 @@ const defaultTranslateWithId: TFunc = ( args ) => { const template = defaultTranslations[messageId]; + const correctedValue = args?.correctedValue ?? ''; - if (args?.correctedValue) { - return template.replace('{correctedValue}', args.correctedValue); - } - - return template; + return template.replace('{correctedValue}', correctedValue); }; const defaultFormatLabel: NonNullable = ( @@ -114,6 +111,22 @@ const defaultFormatLabel: NonNullable = ( const hasUpperValue = (valueUpper: State['valueUpper']): valueUpper is number => typeof valueUpper !== 'undefined'; +const calcRawLeftPercent = ({ + max, + min, + value, +}: { + max: number; + min: number; + value: number; +}) => { + const range = max - min; + + if (range === 0) return 0; + + return clamp((value - min) / range, 0, 1); +}; + // TODO: Assuming a 16ms throttle corresponds to 60 FPS, should it be halved, // since many systems can handle 120 FPS? If it doesn't correspond to 60 FPS, // what does it correspond to? @@ -322,7 +335,6 @@ export interface SliderProps interface CalcLeftPercentProps { clientX?: number; value?: number; - range?: number; } type State = { @@ -704,8 +716,8 @@ const Slider = (props: SliderProps) => { _onDragRef.current = (evt, activeHandle) => { activeHandle = activeHandle ?? stateRef.current.activeHandle; - // Do nothing if component is disabled, or we have no event. - if (propsRef.current.disabled || propsRef.current.readOnly || !evt) { + // Do nothing if component is disabled. + if (propsRef.current.disabled || propsRef.current.readOnly) { return; } @@ -820,11 +832,6 @@ const Slider = (props: SliderProps) => { return; } - // Do nothing if we have no valid event, target, or value - if (!evt || !('target' in evt) || typeof evt.target.value !== 'string') { - return; - } - // Avoid calling calcValue for invalid numbers, but still update the state. const activeHandle = (evt.target.dataset.handlePosition as HandlePosition | undefined) ?? @@ -872,11 +879,6 @@ const Slider = (props: SliderProps) => { * Handles state change to isValid state. */ const onBlurInput = (evt: FocusEvent) => { - // Do nothing if we have no valid event, target, or value - if (!evt || !('target' in evt) || typeof evt.target.value !== 'string') { - return; - } - const { value: targetValue } = evt.target; processNewInputValue(evt.target); @@ -899,11 +901,6 @@ const Slider = (props: SliderProps) => { return; } - // Do nothing if we have no valid event, target, or value. - if (!evt || !('target' in evt) || typeof evt.target.value !== 'string') { - return; - } - if (matches(evt, [keys.Enter])) { processNewInputValue(evt.target); } @@ -970,10 +967,11 @@ const Slider = (props: SliderProps) => { } }; - const calcLeftPercent = ({ clientX, value, range }: CalcLeftPercentProps) => { + const calcLeftPercent = ({ clientX, value }: CalcLeftPercentProps) => { // TODO: Delete the optional chaining operator after `getBoundingClientRect`. const boundingRect = elementRef.current?.getBoundingClientRect?.(); let width = boundingRect ? boundingRect.right - boundingRect.left : 0; + const nextValue = value ?? props.min; // Enforce a minimum width of at least 1 for calculations if (width <= 0) { @@ -987,13 +985,13 @@ const Slider = (props: SliderProps) => { ? (boundingRect?.right ?? 0) - clientX : clientX - (boundingRect?.left ?? 0); return leftOffset / width; - } else if (value !== null && typeof value !== 'undefined' && range) { - // Prevent NaN calculation if the range is 0. - return range === 0 ? 0 : (value - props.min) / range; } - // We should never end up in this scenario, but in case we do, and to - // re-assure Typescript, return 0. - return 0; + + return calcRawLeftPercent({ + max: props.max, + min: props.min, + value: nextValue, + }); }; /** @@ -1033,11 +1031,9 @@ const Slider = (props: SliderProps) => { /** Whether to bypass the stepping logic and use the raw value. */ useRawValue?: boolean; }) => { - const range = props.max - props.min; const leftPercentRaw = calcLeftPercent({ clientX, value, - range, }); /** `leftPercentRaw` clamped between 0 and 1. */ const leftPercent = clamp(leftPercentRaw, 0, 1); @@ -1154,11 +1150,9 @@ const Slider = (props: SliderProps) => { if (handle === HandlePosition.LOWER) { return !valueUpper || newValue <= valueUpper; - } else if (handle === HandlePosition.UPPER) { - return !value || newValue >= value; } - return false; + return !value || newValue >= value; }; const isValidValue = ({ diff --git a/packages/react/src/components/Slider/__tests__/Slider-test.js b/packages/react/src/components/Slider/__tests__/Slider-test.js index 808d09c89ee6..020b84b41fee 100644 --- a/packages/react/src/components/Slider/__tests__/Slider-test.js +++ b/packages/react/src/components/Slider/__tests__/Slider-test.js @@ -67,6 +67,25 @@ const renderTwoHandleSlider = ({ ...rest, }); +const createDOMRect = (options) => { + const left = options?.left ?? 0; + const top = options?.top ?? 0; + const width = options?.width ?? 0; + const height = options?.height ?? 0; + + return { + x: left, + y: top, + top, + left, + width, + height, + right: left + width, + bottom: top + height, + toJSON: () => ({}), + }; +}; + describe('Slider', () => { beforeEach(() => { jest.clearAllMocks(); @@ -225,6 +244,26 @@ describe('Slider', () => { expect(parseInt(inputElement.getAttribute('value'))).toEqual(100); }); + it('should auto-correct values below `min` and announce the correction', async () => { + renderSlider({ + ariaLabelInput: inputAriaValue, + value: initialValue, + min: 10, + max: 100, + }); + + const inputElement = screen.getByLabelText(inputAriaValue); + + await userEvent.clear(inputElement); + await userEvent.type(inputElement, '0'); + await userEvent.tab(); + + expect(inputElement).toHaveValue(10); + expect(screen.getByRole('alert')).toHaveTextContent( + 'The inputted value "0" was corrected to the nearest allowed digit.' + ); + }); + it('should apply the given id to the element with role of slider', () => { const testId = 'slider-test-custom-id'; renderSlider({ id: testId }); @@ -295,6 +334,23 @@ describe('Slider', () => { expect(onChange).toHaveBeenCalledTimes(0); }); + it('should ignore direct change events when `readOnly` is `true`', () => { + renderSlider({ + ariaLabelInput: inputAriaValue, + value: initialValue, + max: 100, + onChange, + readOnly: true, + }); + + const inputElement = screen.getByLabelText(inputAriaValue); + + fireEvent.change(inputElement, { target: { value: '75' } }); + + expect(onChange).not.toHaveBeenCalled(); + expect(inputElement).toHaveValue(initialValue); + }); + it('should not have warning if disabled', () => { renderSlider({ ariaLabelInput: inputAriaValue, @@ -627,6 +683,32 @@ describe('Slider', () => { }); }); + it('should set state from a touch interaction', async () => { + renderSlider({ + ariaLabelInput: inputAriaValue, + value: 0, + min: 0, + max: 100, + onChange, + }); + + const slider = screen.getByRole('slider'); + const sliderRoot = screen.getByRole('presentation'); + + jest + .spyOn(sliderRoot, 'getBoundingClientRect') + .mockImplementation(() => createDOMRect({ left: 0, width: 100 })); + + fireEvent.touchStart(slider, { touches: [{ clientX: 75 }] }); + fireEvent.touchEnd(document); + + await waitFor(() => { + expect(onChange).toHaveBeenLastCalledWith({ + value: 75, + }); + }); + }); + it('should call release', () => { const { mouseDown, mouseUp, mouseMove } = fireEvent; const { container } = renderSlider({ @@ -1208,6 +1290,49 @@ describe('Slider', () => { expect(upperInput).not.toHaveAttribute('aria-invalid', 'true'); }); + it('should clear invalid state when invalid changes to false', async () => { + const { rerender } = renderTwoHandleSlider({ + invalid: true, + invalidText: 'Error message', + value: initialValueLower, + unstable_valueUpper: initialValueUpper, + min: 0, + max: 100, + }); + const lowerInput = screen.getByLabelText(defaultAriaLabelInput, { + selector: 'input', + }); + const upperInput = screen.getByLabelText(defaultAriaLabelInputUpper, { + selector: 'input', + }); + + await waitFor(() => { + expect(lowerInput).toHaveAttribute('aria-invalid', 'true'); + expect(upperInput).toHaveAttribute('aria-invalid', 'true'); + }); + + rerender( + + ); + + await waitFor(() => { + expect(lowerInput).not.toHaveAttribute('aria-invalid', 'true'); + expect(upperInput).not.toHaveAttribute('aria-invalid', 'true'); + }); + }); + describe('Error handling, expected behavior from event handlers', () => { it('handles non-number typed into input field', async () => { const { type, tab } = userEvent; @@ -1389,6 +1514,74 @@ describe('Slider', () => { }); }); + it('should choose the lower handle when dragging from the track nearer to it', async () => { + renderTwoHandleSlider({ + value: 20, + unstable_valueUpper: 80, + min: 0, + max: 100, + onChange, + }); + + const sliderRoot = screen.getByRole('presentation'); + const [lowerThumb, upperThumb] = screen.getAllByRole('slider'); + + jest + .spyOn(sliderRoot, 'getBoundingClientRect') + .mockImplementation(() => createDOMRect({ left: 0, width: 100 })); + jest + .spyOn(lowerThumb, 'getBoundingClientRect') + .mockImplementation(() => createDOMRect({ left: 20, width: 10 })); + jest + .spyOn(upperThumb, 'getBoundingClientRect') + .mockImplementation(() => createDOMRect({ left: 80, width: 10 })); + + fireEvent.mouseDown(sliderRoot, { clientX: 26 }); + fireEvent.mouseUp(document); + + expect(lowerThumb).toHaveFocus(); + await waitFor(() => { + expect(onChange).toHaveBeenLastCalledWith({ + value: 26, + valueUpper: 80, + }); + }); + }); + + it('should choose the upper handle when dragging from the track nearer to it', async () => { + renderTwoHandleSlider({ + value: 20, + unstable_valueUpper: 80, + min: 0, + max: 100, + onChange, + }); + + const sliderRoot = screen.getByRole('presentation'); + const [lowerThumb, upperThumb] = screen.getAllByRole('slider'); + + jest + .spyOn(sliderRoot, 'getBoundingClientRect') + .mockImplementation(() => createDOMRect({ left: 0, width: 100 })); + jest + .spyOn(lowerThumb, 'getBoundingClientRect') + .mockImplementation(() => createDOMRect({ left: 20, width: 10 })); + jest + .spyOn(upperThumb, 'getBoundingClientRect') + .mockImplementation(() => createDOMRect({ left: 80, width: 10 })); + + fireEvent.mouseDown(sliderRoot, { clientX: 84 }); + fireEvent.mouseUp(document); + + expect(upperThumb).toHaveFocus(); + await waitFor(() => { + expect(onChange).toHaveBeenLastCalledWith({ + value: 20, + valueUpper: 84, + }); + }); + }); + it('should call release', () => { const { mouseDown, mouseUp, mouseMove } = fireEvent; const { container } = renderTwoHandleSlider({