Skip to content

Slider 'enableShadow' prop and ColorPicker - minor UI changes #2934

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion demo/src/screens/MenuStructure.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'},
Expand Down
7 changes: 4 additions & 3 deletions demo/src/screens/incubatorScreens/IncubatorSliderScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -58,7 +59,7 @@ const IncubatorSliderScreen = () => {
}, []);

const onGroupValueChange = (value: string) => {
console.log('onGroupValueChange: ', value);
setGroupColor(value);
};

const renderValuesBox = (min: number, max?: number) => {
Expand Down Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions src/components/colorPicker/ColorPickerDialogHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -47,7 +47,6 @@ const styles = StyleSheet.create({
header: {
height: 56,
borderTopLeftRadius: BORDER_RADIUS,
borderTopRightRadius: BORDER_RADIUS,
backgroundColor: Colors.$backgroundDefault
borderTopRightRadius: BORDER_RADIUS
}
});
6 changes: 5 additions & 1 deletion src/components/colorPicker/ColorPickerPreview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Comment on lines +93 to +95
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using StyleSheet.hairlineWidth for the thin border..

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

borderTopColor: Colors.$outlineDisabled
},
inputContainer: {
alignItems: 'center',
Expand Down
2 changes: 1 addition & 1 deletion src/components/slider/ColorSliderGroup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
);
};
Expand Down
40 changes: 22 additions & 18 deletions src/components/slider/GradientSlider.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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));

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove extra space

useEffect(() => {
setColor(Colors.getHSL(propsColors));
}, [propsColors]);
Expand All @@ -94,25 +94,31 @@ const GradientSlider = (props: Props) => {
return color || sliderContext.value;
}, [color, sliderContext.value]);

const renderDefaultGradient = useCallback(() => {
const getHueColor = useCallback(() => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels like this should be a memoize value more than a callback.. WDYT?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Good idea

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(() => {
return <Gradient type={Gradient.types.HUE} numberOfSteps={gradientSteps}/>;
}, [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
Expand All @@ -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) => {
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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}
Expand Down
8 changes: 7 additions & 1 deletion src/components/slider/slider.api.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
6 changes: 4 additions & 2 deletions src/incubator/Slider/Thumb.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ interface ThumbProps extends ViewProps {
shouldDisableRTL?: boolean;
onSeekStart?: () => void;
onSeekEnd?: () => void;
enableShadow?: boolean;
}

const SHADOW_RADIUS = 4;
Expand All @@ -44,7 +45,8 @@ const Thumb = (props: ThumbProps) => {
shouldBounceToStep,
stepInterpolatedValue,
gap = 0,
secondary
secondary,
enableShadow
} = props;

const rtlFix = Constants.isRTL ? -1 : 1;
Expand Down Expand Up @@ -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}
/>
Expand Down
10 changes: 8 additions & 2 deletions src/incubator/Slider/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -175,7 +179,8 @@ const Slider = React.memo((props: Props) => {
disabled,
useGap = true,
accessible = true,
testID
testID,
enableThumbShadow = true
} = props;

const accessibilityProps = useMemo(() => {
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -366,6 +371,7 @@ const Slider = React.memo((props: Props) => {
hitSlop={thumbHitSlop}
shouldBounceToStep={shouldBounceToStep}
stepInterpolatedValue={stepInterpolatedValue}
enableShadow={enableThumbShadow}
/>
);
};
Expand Down