From ef7e387070a1c1aa4852e00311faa08c4a2f9d53 Mon Sep 17 00:00:00 2001
From: Inbal Tish <33805983+Inbal-Tish@users.noreply.github.com>
Date: Mon, 5 Feb 2024 10:51:45 +0200
Subject: [PATCH] Slider 'enableShadow' prop and ColorPicker - minor UI changes
 (#2934)

* Slider 'enableShadow' prop and ColorPicker - minor UI changes

* GradientSlider - display color only changes on hue change

* Slider - bug fix - invoke onChange only when user move slider

* Change screen name to ColorPicker

---------

Co-authored-by: Ethan Sharabi <1780255+ethanshar@users.noreply.github.com>
---
 demo/src/screens/MenuStructure.js             |  2 +-
 .../IncubatorSliderScreen.tsx                 |  7 ++--
 .../colorPicker/ColorPickerDialogHeader.tsx   |  5 +--
 .../colorPicker/ColorPickerPreview.tsx        |  6 ++-
 src/components/slider/ColorSliderGroup.tsx    |  2 +-
 src/components/slider/GradientSlider.tsx      | 40 ++++++++++---------
 src/components/slider/slider.api.json         |  8 +++-
 src/incubator/Slider/Thumb.tsx                |  6 ++-
 src/incubator/Slider/index.tsx                | 10 ++++-
 9 files changed, 54 insertions(+), 32 deletions(-)

diff --git a/demo/src/screens/MenuStructure.js b/demo/src/screens/MenuStructure.js
index ea907cbb0d..05014f74ef 100644
--- a/demo/src/screens/MenuStructure.js
+++ b/demo/src/screens/MenuStructure.js
@@ -51,7 +51,7 @@ export const navigationData = {
     title: 'Form',
     screens: [
       {title: 'Checkbox', tags: 'checkbox toggle controls', screen: 'unicorn.components.CheckboxScreen'},
-      {title: 'Color Picker', tags: 'color picker control', screen: 'unicorn.components.ColorPickerScreen'},
+      {title: 'ColorPicker', tags: 'color picker control', screen: 'unicorn.components.ColorPickerScreen'},
       {title: 'Color Swatch', tags: 'color swatch and palette', screen: 'unicorn.components.ColorSwatchScreen'},
       {title: 'TextField', tags: 'text input field form', screen: 'unicorn.components.TextFieldScreen'},
       {title: 'NumberInput', tags: 'number input', screen: 'unicorn.components.NumberInputScreen'},
diff --git a/demo/src/screens/incubatorScreens/IncubatorSliderScreen.tsx b/demo/src/screens/incubatorScreens/IncubatorSliderScreen.tsx
index b2bce44d2f..d744bd5bac 100644
--- a/demo/src/screens/incubatorScreens/IncubatorSliderScreen.tsx
+++ b/demo/src/screens/incubatorScreens/IncubatorSliderScreen.tsx
@@ -21,6 +21,7 @@ const IncubatorSliderScreen = () => {
   const [sliderMaxValue, setSliderMaxValue] = useState(INITIAL_MAX);
 
   const [color, setColor] = useState(COLOR);
+  const [groupColor, setGroupColor] = useState(Colors.yellow30);
   const [alpha, setAlpha] = useState(1);
 
   const slider = useRef<Incubator.SliderRef>(null);
@@ -58,7 +59,7 @@ const IncubatorSliderScreen = () => {
   }, []);
 
   const onGroupValueChange = (value: string) => {
-    console.log('onGroupValueChange: ', value);
+    setGroupColor(value);
   };
 
   const renderValuesBox = (min: number, max?: number) => {
@@ -235,9 +236,9 @@ const IncubatorSliderScreen = () => {
           Color Slider Group
         </Text>
         <ColorSliderGroup
-          initialColor={color}
+          initialColor={groupColor}
           sliderContainerStyle={styles.slider}
-          containerStyle={styles.group}
+          containerStyle={[styles.group, {borderWidth: 12, borderColor: groupColor}]}
           showLabels
           onValueChange={onGroupValueChange}
           migrate
diff --git a/src/components/colorPicker/ColorPickerDialogHeader.tsx b/src/components/colorPicker/ColorPickerDialogHeader.tsx
index a88d2e93ee..3b0a550e0c 100644
--- a/src/components/colorPicker/ColorPickerDialogHeader.tsx
+++ b/src/components/colorPicker/ColorPickerDialogHeader.tsx
@@ -19,7 +19,7 @@ const Header = (props: HeaderProps) => {
   const {onDismiss, accessibilityLabels, testID, doneButtonColor, valid, onDonePressed} = props;
 
   return (
-    <View row spread bg-white paddingH-20 style={styles.header}>
+    <View row spread bg-$backgroundDefault paddingH-20 style={styles.header}>
       <Button
         link
         iconSource={Assets.icons.x}
@@ -47,7 +47,6 @@ const styles = StyleSheet.create({
   header: {
     height: 56,
     borderTopLeftRadius: BORDER_RADIUS,
-    borderTopRightRadius: BORDER_RADIUS,
-    backgroundColor: Colors.$backgroundDefault
+    borderTopRightRadius: BORDER_RADIUS
   }
 });
diff --git a/src/components/colorPicker/ColorPickerPreview.tsx b/src/components/colorPicker/ColorPickerPreview.tsx
index bafa087454..e6e097b64a 100644
--- a/src/components/colorPicker/ColorPickerPreview.tsx
+++ b/src/components/colorPicker/ColorPickerPreview.tsx
@@ -89,7 +89,11 @@ const styles = StyleSheet.create({
   preview: {
     height: 200,
     alignItems: 'center',
-    justifyContent: 'center'
+    justifyContent: 'center',
+    borderBottomWidth: 0.5,
+    borderBottomColor: Colors.$outlineDisabled,
+    borderTopWidth: 0.5,
+    borderTopColor: Colors.$outlineDisabled
   },
   inputContainer: {
     alignItems: 'center',
diff --git a/src/components/slider/ColorSliderGroup.tsx b/src/components/slider/ColorSliderGroup.tsx
index 70db60738d..dc7562cbcd 100644
--- a/src/components/slider/ColorSliderGroup.tsx
+++ b/src/components/slider/ColorSliderGroup.tsx
@@ -67,8 +67,8 @@ const ColorSliderGroup = (props: ColorSliderGroupProps) => {
   return (
     <SliderGroup style={containerStyle} color={color} onValueChange={onValueChange}>
       <ColorSlider type={GradientSlider.types.HUE} initialColor={initialColor} {...others}/>
-      <ColorSlider type={GradientSlider.types.LIGHTNESS} initialColor={initialColor} {...others}/>
       <ColorSlider type={GradientSlider.types.SATURATION} initialColor={initialColor} {...others}/>
+      <ColorSlider type={GradientSlider.types.LIGHTNESS} initialColor={initialColor} {...others}/>
     </SliderGroup>
   );
 };
diff --git a/src/components/slider/GradientSlider.tsx b/src/components/slider/GradientSlider.tsx
index 72f7fc3486..7f2943f5e3 100644
--- a/src/components/slider/GradientSlider.tsx
+++ b/src/components/slider/GradientSlider.tsx
@@ -1,6 +1,6 @@
 import _ from 'lodash';
 import tinycolor from 'tinycolor2';
-import React, {useCallback, useEffect, useState} from 'react';
+import React, {useCallback, useEffect, useState, useRef} from 'react';
 import {StyleProp, ViewStyle} from 'react-native';
 import {Colors} from '../../style';
 import {asBaseComponent, forwardRef, ForwardRefInjectedProps} from '../../commons/new';
@@ -83,9 +83,9 @@ const GradientSlider = (props: Props) => {
     ...others
   } = props;
 
-  const [initialColor] = useState(Colors.getHSL(propsColors));
+  const initialColor = useRef(Colors.getHSL(propsColors));
   const [color, setColor] = useState(Colors.getHSL(propsColors));
-
+  
   useEffect(() => {
     setColor(Colors.getHSL(propsColors));
   }, [propsColors]);
@@ -94,10 +94,18 @@ const GradientSlider = (props: Props) => {
     return color || sliderContext.value;
   }, [color, sliderContext.value]);
 
-  const renderDefaultGradient = useCallback(() => {
+  const getHueColor = useCallback(() => {
     const color = getColor();
+    return {
+      h: color.h,
+      s: 1,
+      l: 0.5,
+      a: 1
+    };
+  }, [getColor]);
 
-    return <Gradient color={color} numberOfSteps={gradientSteps}/>;
+  const renderDefaultGradient = useCallback(() => {
+    return <Gradient color={getColor()} numberOfSteps={gradientSteps}/>;
   }, [getColor, gradientSteps]);
 
   const renderHueGradient = useCallback(() => {
@@ -105,14 +113,12 @@ const GradientSlider = (props: Props) => {
   }, [gradientSteps]);
 
   const renderLightnessGradient = useCallback(() => {
-    const color = getColor();
-    return <Gradient type={Gradient.types.LIGHTNESS} color={color} numberOfSteps={gradientSteps}/>;
-  }, [getColor, gradientSteps]);
+    return <Gradient type={Gradient.types.LIGHTNESS} color={getHueColor()} numberOfSteps={gradientSteps}/>;
+  }, [getHueColor, gradientSteps]);
 
   const renderSaturationGradient = useCallback(() => {
-    const color = getColor();
-    return <Gradient type={Gradient.types.SATURATION} color={color} numberOfSteps={gradientSteps}/>;
-  }, [getColor, gradientSteps]);
+    return <Gradient type={Gradient.types.SATURATION} color={getHueColor()} numberOfSteps={gradientSteps}/>;
+  }, [getHueColor, gradientSteps]);
 
   const onValueChange = useCallback((value: string, alpha: number) => {
     // alpha returns for type.DEFAULT
@@ -132,7 +138,7 @@ const GradientSlider = (props: Props) => {
   [sliderContext, onValueChange]);
 
   const reset = useCallback(() => {
-    updateColor(initialColor);
+    updateColor(initialColor.current);
   }, [initialColor, updateColor]);
 
   const updateAlpha = useCallback((a: number) => {
@@ -159,8 +165,6 @@ const GradientSlider = (props: Props) => {
   },
   [getColor, updateColor]);
 
-  const _color = getColor();
-  const thumbTintColor = Colors.getHexString(_color);
   let step = 0.01;
   let maximumValue = 1;
   let value = color.a;
@@ -171,17 +175,17 @@ const GradientSlider = (props: Props) => {
     case GradientSliderTypes.HUE:
       step = 1;
       maximumValue = 359;
-      value = initialColor.h;
+      value = initialColor.current.h;
       renderTrack = renderHueGradient;
       sliderOnValueChange = updateHue;
       break;
     case GradientSliderTypes.LIGHTNESS:
-      value = initialColor.l;
+      value = initialColor.current.l;
       renderTrack = renderLightnessGradient;
       sliderOnValueChange = updateLightness;
       break;
     case GradientSliderTypes.SATURATION:
-      value = initialColor.s;
+      value = initialColor.current.s;
       renderTrack = renderSaturationGradient;
       sliderOnValueChange = updateSaturation;
       break;
@@ -199,7 +203,7 @@ const GradientSlider = (props: Props) => {
       step={step}
       maximumValue={maximumValue}
       value={value}
-      thumbTintColor={thumbTintColor}
+      thumbTintColor={Colors.getHexString(getHueColor())}
       onValueChange={sliderOnValueChange}
       containerStyle={containerStyle}
       disabled={disabled}
diff --git a/src/components/slider/slider.api.json b/src/components/slider/slider.api.json
index 97b18367db..5c3aaff2af 100644
--- a/src/components/slider/slider.api.json
+++ b/src/components/slider/slider.api.json
@@ -84,7 +84,13 @@
       "description": "If true the component will have accessibility features enabled"
     },
     {"name": "testID", "type": "string", "description": "The component test id"},
-    {"name": "migrate", "type": "boolean", "description": "Temporary prop required for migration to the Slider's new implementation"}
+    {"name": "migrate", "type": "boolean", "description": "Temporary prop required for migration to the Slider's new implementation"},
+    {
+      "name": "enableThumbShadow",
+      "type": "boolean",
+      "description": "Whether the thumb will have a shadow (with 'migrate' true only)",
+      "default": "true"
+    }
   ],
   "snippet": [
     "<Slider",
diff --git a/src/incubator/Slider/Thumb.tsx b/src/incubator/Slider/Thumb.tsx
index 9acdd80820..7077dc2061 100644
--- a/src/incubator/Slider/Thumb.tsx
+++ b/src/incubator/Slider/Thumb.tsx
@@ -23,6 +23,7 @@ interface ThumbProps extends ViewProps {
   shouldDisableRTL?: boolean;
   onSeekStart?: () => void;
   onSeekEnd?: () => void;
+  enableShadow?: boolean;
 }
 
 const SHADOW_RADIUS = 4;
@@ -44,7 +45,8 @@ const Thumb = (props: ThumbProps) => {
     shouldBounceToStep,
     stepInterpolatedValue,
     gap = 0,
-    secondary
+    secondary,
+    enableShadow
   } = props;
 
   const rtlFix = Constants.isRTL ? -1 : 1;
@@ -109,7 +111,7 @@ const Thumb = (props: ThumbProps) => {
       <View
         reanimated
         // @ts-expect-error should be fixed in version 3.5 (https://github.com/software-mansion/react-native-reanimated/pull/4881)
-        style={[styles.thumbPosition, styles.thumbShadow, animatedStyle]}
+        style={[styles.thumbPosition, enableShadow && styles.thumbShadow, animatedStyle]}
         hitSlop={hitSlop}
         onLayout={onThumbLayout}
       />
diff --git a/src/incubator/Slider/index.tsx b/src/incubator/Slider/index.tsx
index baa38916af..bb979a9be0 100644
--- a/src/incubator/Slider/index.tsx
+++ b/src/incubator/Slider/index.tsx
@@ -93,6 +93,10 @@ export interface SliderProps extends AccessibilityProps {
    * Defines how far a touch event can start away from the thumb.
    */
   thumbHitSlop?: ViewProps['hitSlop'];
+  /**
+   * Whether the thumb will have a shadow
+   */
+  enableThumbShadow?: boolean;
   /**
    * Thumb color
    */
@@ -175,7 +179,8 @@ const Slider = React.memo((props: Props) => {
     disabled,
     useGap = true,
     accessible = true,
-    testID
+    testID,
+    enableThumbShadow = true
   } = props;
 
   const accessibilityProps = useMemo(() => {
@@ -272,7 +277,7 @@ const Slider = React.memo((props: Props) => {
           maximumValue,
           stepXValue.value);
         runOnJS(onRangeChangeThrottled)(value, maxValue);
-      } else {
+      } else if (prevOffset) { // don't invoke onChange when setting the slider
         runOnJS(onValueChangeThrottled)(value);
       }
     }
@@ -366,6 +371,7 @@ const Slider = React.memo((props: Props) => {
         hitSlop={thumbHitSlop}
         shouldBounceToStep={shouldBounceToStep}
         stepInterpolatedValue={stepInterpolatedValue}
+        enableShadow={enableThumbShadow}
       />
     );
   };