< Slider /> controlled state fix (#1692)

Co-authored-by: Omri Lavi <[email protected]>
mondaycom · Oct 22, 2023 · d3c5088 · d3c5088
1 parent 97d6fbd
commit d3c5088
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 8 deletions.
diff --git a/src/components/Slider/Slider.tsx b/src/components/Slider/Slider.tsx
@@ -61,7 +61,7 @@ export type SliderProps = {
    * Optional onChange callback (for outer trigger or Controlled mode)
    * - required in Controlled Mode
    */
-  onChange?: () => void;
+  onChange?: (value: number | number[]) => void;
   /**
    * If true switch slider to RRange mode (two Thumbs)
    */
@@ -87,11 +87,9 @@ export type SliderProps = {
    */
   value?: number | number[];
   /**
-   * Default `value` if value not specified
-   *  - should be used in NON-Controlled mode to set initial Value
-   *  - in ranged mode should be an array of two numbers
+   * Function to format the value, e.g. add %. By default, returns `${value}%`
    */
-  valueFormatter?: (value: number | number[]) => string;
+  valueFormatter?: (value: number) => string;
   /**
    * Text/presentation of current/selected value
    *  - should be used in Controlled Mode only
@@ -179,7 +177,6 @@ const Slider: React.FC<SliderProps> & {
         valueText={valueText}
       >
         <div
-          // className={bem("", { disabled, "value-shown": showValue }, className)}
           className={cx(styles.slider, { [styles.valueShown]: showValue }, className)}
           data-testid={dataTestId}
           id={id}

diff --git a/src/components/Slider/SliderHooks.ts b/src/components/Slider/SliderHooks.ts
@@ -14,7 +14,7 @@ function _useSliderValue(
   isControlled: boolean,
   value: number | number[]
 ): [number | number[], (value: number | number[]) => void] {
-  const initialValue = isControlled ? (value as number[]) : defaultValue;
+  const initialValue = isControlled ? value : defaultValue;
   const [internalStateValue, setInternalStateValue] = useState(initialValue);
   if (isControlled) {
     return [value as number, NOOP];
@@ -220,6 +220,14 @@ export function useSliderValues(
   const isControlled = _useIsStateControlledFromOutside(value);
   const [actualValue, setStateSelectedValue] = _useSliderValue(defaultValue, isControlled, value);
   const valueRef = useRef(actualValue);
+
+  // Update ref when actualValue was changed from outside in controlled mode
+  useEffect(() => {
+    if (isControlled && valueRef.current !== actualValue) {
+      valueRef.current = actualValue;
+    }
+  }, [isControlled, actualValue]);
+
   const setSelectedValue = useCallback(
     (newValue: number) => {
       setStateSelectedValue(newValue);
@@ -228,7 +236,7 @@ export function useSliderValues(
     [valueRef, setStateSelectedValue]
   );
 
-  const getSelectedValue = useCallback(() => valueRef.current, [valueRef]);
+  const getSelectedValue = () => valueRef.current;
 
   const actualValueText = ensureValueText(valueText, actualValue, valueFormatter);
   return { actualValue, actualValueText, getSelectedValue, isControlled, setSelectedValue };