diff --git a/packages/@react-aria/slider/src/useSlider.ts b/packages/@react-aria/slider/src/useSlider.ts index 668d40822d2..4556a5af3d7 100644 --- a/packages/@react-aria/slider/src/useSlider.ts +++ b/packages/@react-aria/slider/src/useSlider.ts @@ -11,7 +11,7 @@ */ import {AriaSliderProps} from '@react-types/slider'; -import {clamp, mergeProps, useGlobalListeners} from '@react-aria/utils'; +import {clamp, mergeProps, useEffectEvent, useGlobalListeners} from '@react-aria/utils'; import {DOMAttributes} from '@react-types/shared'; import {getSliderThumbId, sliderIds} from './utils'; import React, {LabelHTMLAttributes, OutputHTMLAttributes, RefObject, useRef} from 'react'; @@ -61,46 +61,48 @@ export function useSlider( let {addGlobalListener, removeGlobalListener} = useGlobalListeners(); // When the user clicks or drags the track, we want the motion to set and drag the - // closest thumb. Hence we also need to install useMove() on the track element. + // closest thumb. Hence, we also need to install useMove() on the track element. // Here, we keep track of which index is the "closest" to the drag start point. // It is set onMouseDown/onTouchDown; see trackProps below. const realTimeTrackDraggingIndex = useRef(null); - const stateRef = useRef(null); - stateRef.current = state; + const reverseX = direction === 'rtl'; const currentPosition = useRef(null); - const {moveProps} = useMove({ - onMoveStart() { - currentPosition.current = null; - }, - onMove({deltaX, deltaY}) { - let {height, width} = trackRef.current.getBoundingClientRect(); - let size = isVertical ? height : width; + let onMoveStart = useEffectEvent(() => { + currentPosition.current = null; + }); + let onMove = useEffectEvent(({deltaX, deltaY}) => { + let {height, width} = trackRef.current.getBoundingClientRect(); + let size = isVertical ? height : width; - if (currentPosition.current == null) { - currentPosition.current = stateRef.current.getThumbPercent(realTimeTrackDraggingIndex.current) * size; - } + if (currentPosition.current == null) { + currentPosition.current = state.getThumbPercent(realTimeTrackDraggingIndex.current) * size; + } - let delta = isVertical ? deltaY : deltaX; - if (isVertical || reverseX) { - delta = -delta; - } + let delta = isVertical ? deltaY : deltaX; + if (isVertical || reverseX) { + delta = -delta; + } - currentPosition.current += delta; + currentPosition.current += delta; - if (realTimeTrackDraggingIndex.current != null && trackRef.current) { - const percent = clamp(currentPosition.current / size, 0, 1); - stateRef.current.setThumbPercent(realTimeTrackDraggingIndex.current, percent); - } - }, - onMoveEnd() { - if (realTimeTrackDraggingIndex.current != null) { - stateRef.current.setThumbDragging(realTimeTrackDraggingIndex.current, false); - realTimeTrackDraggingIndex.current = null; - } + if (realTimeTrackDraggingIndex.current != null && trackRef.current) { + const percent = clamp(currentPosition.current / size, 0, 1); + state.setThumbPercent(realTimeTrackDraggingIndex.current, percent); } }); + let onMoveEnd = useEffectEvent(() => { + if (realTimeTrackDraggingIndex.current != null) { + state.setThumbDragging(realTimeTrackDraggingIndex.current, false); + realTimeTrackDraggingIndex.current = null; + } + }); + const {moveProps} = useMove({ + onMoveStart, + onMove, + onMoveEnd + }); let currentPointer = useRef(undefined); let onDownTrack = (e: React.UIEvent, id: number, clientX: number, clientY: number) => { diff --git a/packages/@react-aria/slider/src/useSliderThumb.ts b/packages/@react-aria/slider/src/useSliderThumb.ts index 249097a1947..b9762027cbe 100644 --- a/packages/@react-aria/slider/src/useSliderThumb.ts +++ b/packages/@react-aria/slider/src/useSliderThumb.ts @@ -1,8 +1,16 @@ import {AriaSliderThumbProps} from '@react-types/slider'; -import {clamp, focusWithoutScrolling, mergeProps, useGlobalListeners} from '@react-aria/utils'; +import {clamp, focusWithoutScrolling, mergeProps, useGlobalListeners, useLayoutEffect} from '@react-aria/utils'; import {DOMAttributes} from '@react-types/shared'; import {getSliderThumbId, sliderIds} from './utils'; -import React, {ChangeEvent, InputHTMLAttributes, LabelHTMLAttributes, RefObject, useCallback, useEffect, useRef} from 'react'; +import React, { + ChangeEvent, + InputHTMLAttributes, + LabelHTMLAttributes, + RefObject, + useCallback, + useEffect, + useRef +} from 'react'; import {SliderState} from '@react-stately/slider'; import {useFocusable} from '@react-aria/focus'; import {useKeyboard, useMove} from '@react-aria/interactions'; @@ -83,7 +91,9 @@ export function useSliderThumb( }, [isFocused, focusInput]); const stateRef = useRef(null); - stateRef.current = state; + useLayoutEffect(() => { + stateRef.current = state; + }); let reverseX = direction === 'rtl'; let currentPosition = useRef(null); diff --git a/packages/@react-spectrum/slider/test/RangeSlider.test.tsx b/packages/@react-spectrum/slider/test/RangeSlider.test.tsx index c2952bb2363..7ff0f2bb68c 100644 --- a/packages/@react-spectrum/slider/test/RangeSlider.test.tsx +++ b/packages/@react-spectrum/slider/test/RangeSlider.test.tsx @@ -14,7 +14,7 @@ import {fireEvent, render} from '@react-spectrum/test-utils'; import {press, testKeypresses} from './utils'; import {Provider} from '@adobe/react-spectrum'; import {RangeSlider} from '../'; -import React, {useState} from 'react'; +import React, {useCallback, useState} from 'react'; import {theme} from '@react-spectrum/theme-default'; import userEvent from '@testing-library/user-event'; @@ -126,11 +126,14 @@ describe('RangeSlider', function () { }); it('can be controlled', function () { - let renders = []; + let setValues = []; function Test() { - let [value, setValue] = useState({start: 20, end: 40}); - renders.push(value); + let [value, _setValue] = useState({start: 20, end: 40}); + let setValue = useCallback((val) => { + setValues.push(val); + _setValue(val); + }, [_setValue]); return (); } @@ -154,7 +157,7 @@ describe('RangeSlider', function () { expect(sliderRight).toHaveAttribute('aria-valuetext', '50'); expect(output).toHaveTextContent('30 – 50'); - expect(renders).toStrictEqual([{start: 20, end: 40}, {start: 30, end: 40}, {start: 30, end: 50}]); + expect(setValues).toStrictEqual([{start: 30, end: 40}, {start: 30, end: 50}]); }); it('supports a custom valueLabel', function () { diff --git a/packages/@react-spectrum/slider/test/Slider.test.tsx b/packages/@react-spectrum/slider/test/Slider.test.tsx index c28974c8920..4acd318497e 100644 --- a/packages/@react-spectrum/slider/test/Slider.test.tsx +++ b/packages/@react-spectrum/slider/test/Slider.test.tsx @@ -13,7 +13,7 @@ import {act, fireEvent, installMouseEvent, render} from '@react-spectrum/test-utils'; import {press, testKeypresses} from './utils'; import {Provider} from '@adobe/react-spectrum'; -import React, {useState} from 'react'; +import React, {useCallback, useState} from 'react'; import {Slider} from '../'; import {theme} from '@react-spectrum/theme-default'; import userEvent from '@testing-library/user-event'; @@ -119,11 +119,14 @@ describe('Slider', function () { }); it('can be controlled', function () { - let renders = []; + let setValues = []; function Test() { - let [value, setValue] = useState(50); - renders.push(value); + let [value, _setValue] = useState(50); + let setValue = useCallback((val) => { + setValues.push(val); + _setValue(val); + }, [_setValue]); return (); } @@ -141,7 +144,7 @@ describe('Slider', function () { expect(slider).toHaveAttribute('aria-valuetext', '55'); expect(output).toHaveTextContent('55'); - expect(renders).toStrictEqual([50, 55]); + expect(setValues).toStrictEqual([55]); }); it('supports a custom getValueLabel', function () { diff --git a/packages/@react-stately/slider/src/useSliderState.ts b/packages/@react-stately/slider/src/useSliderState.ts index 2132000bd45..f4533439d60 100644 --- a/packages/@react-stately/slider/src/useSliderState.ts +++ b/packages/@react-stately/slider/src/useSliderState.ts @@ -14,7 +14,7 @@ import {clamp, snapValueToStep} from '@react-aria/utils'; import {Orientation} from '@react-types/shared'; import {SliderProps} from '@react-types/slider'; import {useControlledState} from '@react-stately/utils'; -import {useMemo, useRef, useState} from 'react'; +import {useEffect, useMemo, useRef, useState} from 'react'; export interface SliderState { /** @@ -192,9 +192,11 @@ export function useSliderState(props: SliderStateOp const [focusedIndex, setFocusedIndex] = useState(undefined); const valuesRef = useRef(null); - valuesRef.current = values; const isDraggingsRef = useRef(null); - isDraggingsRef.current = isDraggings; + useEffect(() => { + valuesRef.current = values; + isDraggingsRef.current = isDraggings; + }); function getValuePercent(value: number) { return (value - minValue) / (maxValue - minValue); diff --git a/packages/@react-stately/utils/test/useControlledState.test.js b/packages/@react-stately/utils/test/useControlledState.test.js index 1a6875c0528..e904a397c9b 100644 --- a/packages/@react-stately/utils/test/useControlledState.test.js +++ b/packages/@react-stately/utils/test/useControlledState.test.js @@ -99,12 +99,16 @@ describe('useControlledState tests', function () { let {getByRole, getByTestId} = render(); let button = getByRole('button'); getByTestId('5'); - expect(renderSpy).toBeCalledTimes(1); + if (!process.env.STRICT_MODE) { + expect(renderSpy).toBeCalledTimes(1); + } actDOM(() => userEvent.click(button) ); getByTestId('6'); - expect(renderSpy).toBeCalledTimes(2); + if (!process.env.STRICT_MODE) { + expect(renderSpy).toBeCalledTimes(2); + } }); it('can handle controlled setValue behavior', () => {