Slider: Fix controlled value updates (#5655)

* fix controlled Slider value updates

* Update values ref when we start dragging

* fix lint

---------

Co-authored-by: Rob Snow <rsnow@adobe.com>
Co-authored-by: Reid Barber <reid@reidbarber.com>

Author: 3 people

packages/@react-stately/slider/src/useSliderState.ts

@@ -198,6 +198,7 @@ export function useSliderState<T extends number | number[]>(props: SliderStateOp
 
   const valuesRef = useRef<number[]>(values);
   const isDraggingsRef = useRef<boolean[]>(isDraggings);
+
   let setValues = (values: number[]) => {
     valuesRef.current = values;
     setValuesState(values);
@@ -244,6 +245,9 @@ export function useSliderState<T extends number | number[]>(props: SliderStateOp
     if (isDisabled || !isThumbEditable(index)) {
       return;
     }
+    if (dragging) {
+      valuesRef.current = values;
+    }
 
     const wasDragging = isDraggingsRef.current[index];
     isDraggingsRef.current = replaceIndex(isDraggingsRef.current, index, dragging);

packages/react-aria-components/stories/Slider.stories.tsx

@@ -18,46 +18,53 @@ export default {
   title: 'React Aria Components'
 };
 
-export const SliderExample = () => (
-  <Slider
-    data-testid="slider-example"
-    defaultValue={[30, 60]}
-    style={{
-      position: 'relative',
-      display: 'flex',
-      flexDirection: 'column',
-      alignItems: 'center',
-      width: 300
-    }}>
-    <div style={{display: 'flex', alignSelf: 'stretch'}}>
-      <Label>Test</Label>
-      <SliderOutput style={{flex: '1 0 auto', textAlign: 'end'}}>
-        {({state}) => `${state.getThumbValueLabel(0)} - ${state.getThumbValueLabel(1)}`}
-      </SliderOutput>
-    </div>
-    <SliderTrack
-      style={{
-        position: 'relative',
-        height: 30,
-        width: '100%'
-      }}>
-      <div
+export const SliderExample = () => {
+  const [value, setValue] = React.useState([30, 60]);
+  return (
+    <div>
+      <Slider<number[]>
+        data-testid="slider-example"
+        value={value}
+        onChange={setValue}
         style={{
-          position: 'absolute',
-          backgroundColor: 'gray',
-          height: 3,
-          top: 13,
-          width: '100%'
-        }} />
-      <CustomThumb index={0}>
-        <Label>A</Label>
-      </CustomThumb>
-      <CustomThumb index={1}>
-        <Label>B</Label>
-      </CustomThumb>
-    </SliderTrack>
-  </Slider>
-);
+          position: 'relative',
+          display: 'flex',
+          flexDirection: 'column',
+          alignItems: 'center',
+          width: 300
+        }}>
+        <div style={{display: 'flex', alignSelf: 'stretch'}}>
+          <Label>Test</Label>
+          <SliderOutput style={{flex: '1 0 auto', textAlign: 'end'}}>
+            {({state}) => `${state.getThumbValueLabel(0)} - ${state.getThumbValueLabel(1)}`}
+          </SliderOutput>
+        </div>
+        <SliderTrack
+          style={{
+            position: 'relative',
+            height: 30,
+            width: '100%'
+          }}>
+          <div
+            style={{
+              position: 'absolute',
+              backgroundColor: 'gray',
+              height: 3,
+              top: 13,
+              width: '100%'
+            }} />
+          <CustomThumb index={0}>
+            <Label>A</Label>
+          </CustomThumb>
+          <CustomThumb index={1}>
+            <Label>B</Label>
+          </CustomThumb>
+        </SliderTrack>
+      </Slider>
+      <button onClick={() => setValue([0, 100])}>reset</button>
+    </div>
+  );
+};
 
 export const SliderCSS = (props: SliderProps) => (
   <Slider {...props} defaultValue={30} className={styles.slider}>

packages/react-aria-components/test/Slider.test.js

@@ -12,7 +12,7 @@
 
 import {fireEvent, pointerMap, render} from '@react-spectrum/test-utils';
 import {Label, Slider, SliderContext, SliderOutput, SliderThumb, SliderTrack} from '../';
-import React from 'react';
+import React, {useState} from 'react';
 import userEvent from '@testing-library/user-event';
 
 let TestSlider = ({sliderProps, thumbProps, trackProps, outputProps}) => (
@@ -209,6 +209,62 @@ describe('Slider', () => {
     expect(output).toHaveTextContent('30 – 60');
   });
 
+  it('should support multiple thumbs (controlled)', async () => {
+    function SliderClient() {
+      const [value, setValue] = useState([30, 60]);
+      return (<div>
+        <Slider value={value} onChange={setValue}>
+          <Label>Test</Label>
+          <SliderOutput>
+            {({state}) => state.values.map((_, i) => state.getThumbValueLabel(i)).join(' – ')}
+          </SliderOutput>
+          <SliderTrack>
+            {({state}) => state.values.map((_, i) => <SliderThumb key={i} index={i} className="thumb" />)}
+          </SliderTrack>
+        </Slider>
+        <button data-testid="reset-button" onClick={() => setValue([0, 100])}>reset</button>
+      </div>);
+    }
+
+    let {getAllByRole, getByTestId} = render(<SliderClient />);
+
+    let sliders = getAllByRole('slider');
+    
+    expect(sliders).toHaveLength(2);
+    expect(sliders[0]).toHaveValue('30');
+    expect(sliders[1]).toHaveValue('60');
+
+    let resetButton = getByTestId('reset-button');
+    await user.click(resetButton);
+    expect(sliders[0]).toHaveValue('0');
+    expect(sliders[1]).toHaveValue('100');
+
+    await user.tab();  // body (because we've clicked the reset button?)
+    await user.tab();
+    expect(document.activeElement).toBe(sliders[0]);
+    
+    await user.keyboard('{ArrowRight}');
+    await user.keyboard('{ArrowRight}');
+    await user.keyboard('{ArrowRight}');
+    expect(sliders[0]).toHaveValue('3');
+    expect(sliders[1]).toHaveValue('100');
+
+    await user.click(resetButton);
+    expect(sliders[0]).toHaveValue('0');
+    expect(sliders[1]).toHaveValue('100');
+
+    await user.tab();  // body
+    await user.tab();  // sliders[0]
+    await user.tab();
+    expect(document.activeElement).toBe(sliders[1]);
+    
+    await user.keyboard('{ArrowLeft}');
+    await user.keyboard('{ArrowLeft}');
+    await user.keyboard('{ArrowLeft}');
+    expect(sliders[0]).toHaveValue('0');
+    expect(sliders[1]).toHaveValue('97');
+  });
+
   it('should support clicking on the track to move the thumb', async () => {
     let onChange = jest.fn();
     let {getByRole} = renderSlider({onChange});