Skip to content
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

Fix the issue with default min/max values with default step for custom StepMarker #606

Closed
BartoszKlonowski opened this issue Jun 11, 2024 · 0 comments · Fixed by #636
Closed
Assignees

Comments

@BartoszKlonowski
Copy link
Member

Here's the commit that fixes the issue with default vs custom values of min, max and default step:

bartoszklonowski@INV-0029 package % git show 5af5333 
commit 5af53339d4caa4e4e38204c37e543cf6686507b2 (HEAD -> fix/581/default-min-max-step-values, origin/fix/581/default-min-max-step-values)
Author: BartoszKlonowski <Bartosz.Klonowski@callstack.com>
Date:   Fri Apr 26 23:16:58 2024 +0200

    Fix the issue with default vs custom min/max/step props

diff --git a/example/src/Examples.tsx b/example/src/Examples.tsx
index 969407e..495aa81 100644
--- a/example/src/Examples.tsx
+++ b/example/src/Examples.tsx
@@ -275,7 +275,7 @@ const MyStepMarker: FC<MarkerProps> = ({stepMarked, currentValue}) => {
       <View style={styles.separator} />
       <View style={styles.label}>
         {currentValue !== undefined ? (
-          <Text>{currentValue}</Text>
+          <Text>{currentValue % 1 === 0 ? currentValue : currentValue.toFixed(2)}</Text>
         ) : (
           <Text>{'-'}</Text>
         )}
@@ -299,16 +299,36 @@ const SliderExampleWithCustomMarker = (props: SliderProps) => {
     <View>
       <Text style={styles.text}>{value && +value.toFixed(3)}</Text>
       <Slider
-        step={CONSTANTS.STEP}
+        step={1}
+        style={[styles.slider, props.style]}
+        minimumValue={0}
+        maximumValue={13}
+        thumbImage={require('./resources/empty.png')}
+        tapToSeek
+        {...props}
+        value={value}
+        onValueChange={setValue}
+        StepMarker={MyStepMarker}
+        minimumTrackTintColor={'#00629A'}
+        maximumTrackTintColor={'#979EA4'}
+      />
+    </View>
+  );
+};
+
+const SliderExampleWithCustomMarkerWithDefaultProps = (props: SliderProps) => {
+  const [value, setValue] = useState(props.value ?? CONSTANTS.MIN_VALUE);
+
+  return (
+    <View>
+      <Text style={styles.text}>{value && +value.toFixed(3)}</Text>
+      <Slider
         style={[styles.slider, props.style]}
-        minimumValue={CONSTANTS.MIN_VALUE}
-        maximumValue={CONSTANTS.MAX_VALUE}
         thumbImage={require('./resources/empty.png')}
         tapToSeek
         {...props}
         value={value}
         onValueChange={setValue}
-        lowerLimit={1}
         StepMarker={MyStepMarker}
         minimumTrackTintColor={'#00629A'}
         maximumTrackTintColor={'#979EA4'}
@@ -342,7 +362,7 @@ const styles = StyleSheet.create({
   },
   label: {
     marginTop: 10,
-    width: 45,
+    width: 55,
     paddingVertical: 5,
     paddingHorizontal: 10,
     backgroundColor: '#ffffff',
@@ -361,7 +381,7 @@ const styles = StyleSheet.create({
     alignItems: 'center',
   },
   tinyLogo: {
-    marginVertical: 5,
+    marginVertical: 2,
     aspectRatio: 1,
     flex: 1,
     height: '100%',
@@ -608,6 +628,12 @@ export const examples: Props[] = [
       return <SliderExampleWithCustomMarker />;
     },
   },
+  {
+    title: 'Custom step marker but default step and min/max values',
+    render() {
+      return <SliderExampleWithCustomMarkerWithDefaultProps />;
+    },
+  },
   {
     title: 'Inverted slider direction',
     render() {
diff --git a/package/src/Slider.tsx b/package/src/Slider.tsx
index d77f28b..80fd289 100644
--- a/package/src/Slider.tsx
+++ b/package/src/Slider.tsx
@@ -216,15 +216,17 @@ const SliderComponent = (
   );
   const [width, setWidth] = useState(0);
 
-  const step = localProps.step
+  const stepResolution = localProps.step
     ? localProps.step
     : constants.DEFAULT_STEP_RESOLUTION;
 
+  const defaultStep = (localProps.maximumValue! - localProps.minimumValue!) / stepResolution;
+  const stepLength = localProps.step || defaultStep;
   const options = Array.from(
     {
-      length: (localProps.maximumValue! - localProps.minimumValue!) / step + 1,
+      length: (localProps.step ? defaultStep : stepResolution) + 1,
     },
-    (_, index) => localProps.minimumValue! + index * step,
+    (_, index) => localProps.minimumValue! + index * stepLength
   );
 
   const defaultStyle =
diff --git a/package/src/utils/constants.ts b/package/src/utils/constants.ts
index eec4967..3cfb27d 100644
--- a/package/src/utils/constants.ts
+++ b/package/src/utils/constants.ts
@@ -1,8 +1,10 @@
+import { Platform } from "react-native";
+
 export const constants = {
   MARGIN_HORIZONTAL_PADDING: 0.05,
   STEP_NUMBER_TEXT_FONT_SMALL: 8,
   STEP_NUMBER_TEXT_FONT_BIG: 12,
   LIMIT_MIN_VALUE: Number.MIN_SAFE_INTEGER,
   LIMIT_MAX_VALUE: Number.MAX_SAFE_INTEGER,
-  DEFAULT_STEP_RESOLUTION: 100,
+  DEFAULT_STEP_RESOLUTION: Platform.OS === 'android' ? 128 : 1000,
 };

Note: iOS has some problems with the resolution, as it's 10000 steps by default with plenty of digits after the point. So let's have what we have, and can revisit the default resolution on iOS later.

Once you check and apply them to your PR I'm happy to merge it 👍

Originally posted by @BartoszKlonowski in #581 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
1 participant