fix: check for invalid configuration in limit settings (#638)

draggie · BartoszKlonowski · web-flow · commit f1c48ad7e2db · 2024-09-24T12:34:07.000+02:00
* fix: check for invalid configuration in limit settings

* fix: add limit check with warning

* chore: pr improvements

* fix: pr improvements

* Remove outdated comment

* Correct the minor typo

---------

Co-authored-by: Bartosz Klonowski &lt;70535775+BartoszKlonowski@users.noreply.github.com&gt;
diff --git a/package/android/src/main/java/com/reactnativecommunity/slider/ReactSlider.java b/package/android/src/main/java/com/reactnativecommunity/slider/ReactSlider.java
@@ -11,6 +11,7 @@
 import android.graphics.BitmapFactory;
 import android.graphics.drawable.BitmapDrawable;
 import android.os.Build;
+import android.util.Log;
 import android.util.AttributeSet;
 import android.view.accessibility.AccessibilityEvent;
 import android.view.accessibility.AccessibilityManager;
@@ -104,6 +105,15 @@ private void disableStateListAnimatorIfNeeded() {
     updateAll();
   }
 
+  /*package*/ int getValidProgressValue(int progress) {
+      if (progress < getLowerLimit()) {
+        progress = getLowerLimit();
+      } else if (progress > getUpperLimit()) {
+        progress = getUpperLimit();
+      }
+      return progress;
+  }
+
   /* package */ void setValue(double value) {
     mValue = value;
     updateValue();
@@ -222,16 +232,29 @@ private void updateAll() {
     updateValue();
   }
 
-  /** Update limit based on props limit, max and min */
+  /** Update limit based on props limit, max and min
+   * Fallback to upper limit if invalid configuration provided
+  */
   private void updateLowerLimit() {
     double limit = Math.max(mRealLowerLimit, mMinValue);
-    mLowerLimit = (int) Math.round((limit - mMinValue) / (mMaxValue - mMinValue) * getTotalSteps());
+    int lowerLimit = (int) Math.round((limit - mMinValue) / (mMaxValue - mMinValue) * getTotalSteps());
+    if(lowerLimit > mUpperLimit) {
+      Log.d("Invalid configuration", "upperLimit < lowerLimit; lowerLimit not set");
+    }else {
+      mLowerLimit = Math.min(lowerLimit, mUpperLimit);
+    }
   }
 
-  /** Update limit based on props limit, max and min */
+  /** Update limit based on props limit, max and min
+  */
   private void updateUpperLimit() {
     double limit = Math.min(mRealUpperLimit, mMaxValue);
-    mUpperLimit = (int) Math.round((limit - mMinValue) / (mMaxValue - mMinValue) * getTotalSteps());
+    int upperLimit = (int) Math.round((limit - mMinValue) / (mMaxValue - mMinValue) * getTotalSteps());
+    if (mLowerLimit > upperLimit) {
+          Log.d("Invalid configuration", "upperLimit < lowerLimit; upperLimit not set");
+    } else {
+          mUpperLimit = upperLimit;
+    }
   }
 
   /** Update value only (optimization in case only value is set). */
diff --git a/package/android/src/newarch/java/com/reactnativecommunity/slider/ReactSliderManager.java b/package/android/src/newarch/java/com/reactnativecommunity/slider/ReactSliderManager.java
@@ -48,13 +48,8 @@ protected ViewManagerDelegate<ReactSlider> getDelegate() {
             public void onProgressChanged(SeekBar seekbar, int progress, boolean fromUser) {
               ReactSlider slider = (ReactSlider)seekbar;
 
-              if(progress < slider.getLowerLimit()) {
-                progress = slider.getLowerLimit();
-                seekbar.setProgress(progress);
-              } else if (progress > slider.getUpperLimit()) {
-                progress = slider.getUpperLimit();
-                seekbar.setProgress(progress);
-              }
+              progress = slider.getValidProgressValue(progress);
+              seekbar.setProgress(progress);
 
               ReactContext reactContext = (ReactContext) seekbar.getContext();
               int reactTag = seekbar.getId();
diff --git a/package/android/src/oldarch/java/com/reactnativecommunity/slider/ReactSliderManager.java b/package/android/src/oldarch/java/com/reactnativecommunity/slider/ReactSliderManager.java
@@ -30,13 +30,8 @@ public class ReactSliderManager extends SimpleViewManager<ReactSlider> {
             public void onProgressChanged(SeekBar seekbar, int progress, boolean fromUser) {
               ReactSlider slider = (ReactSlider)seekbar;
 
-              if(progress < slider.getLowerLimit()) {
-                progress = slider.getLowerLimit();
-                seekbar.setProgress(progress);
-              } else if(progress > slider.getUpperLimit()) {
-                progress = slider.getUpperLimit();
-                seekbar.setProgress(progress);
-              }
+              progress = slider.getValidProgressValue(progress);
+              seekbar.setProgress(progress);
 
               ReactContext reactContext = (ReactContext) seekbar.getContext();
               if(fromUser) {
diff --git a/package/ios/RNCSlider.m b/package/ios/RNCSlider.m
@@ -14,6 +14,14 @@ @implementation RNCSlider
   bool _maximumTrackImageSet;
 }
 
+- (instancetype)init {
+  if (self = [super init]) {
+      _upperLimit = FLT_MAX;
+      _lowerLimit = FLT_MIN;
+  }
+  return self;
+}
+
 - (instancetype)initWithFrame:(CGRect)frame
 {
     return [super initWithFrame:frame];
@@ -46,7 +54,7 @@ - (void)setupAccessibility:(float)value
     if (sliderValue && [sliderValue intValue] == 1) {
       spokenUnits = [spokenUnits substringToIndex:stringLength-1];
     }
-    
+
     self.accessibilityValue = [NSString stringWithFormat:@"%@ %@", sliderValue, spokenUnits];
   }
 }
diff --git a/package/ios/RNCSliderComponentView.mm b/package/ios/RNCSliderComponentView.mm
@@ -47,12 +47,12 @@ - (instancetype)initWithFrame:(CGRect)frame
          forControlEvents:(UIControlEventTouchUpInside |
                            UIControlEventTouchUpOutside |
                            UIControlEventTouchCancel)];
-        
+
         UITapGestureRecognizer *tapGesturer;
         tapGesturer = [[UITapGestureRecognizer alloc] initWithTarget: self action:@selector(tapHandler:)];
         [tapGesturer setNumberOfTapsRequired: 1];
         [slider addGestureRecognizer:tapGesturer];
-        
+
         slider.value = (float)defaultProps->value;
         self.contentView = slider;
     }
@@ -65,12 +65,12 @@ - (void)tapHandler:(UITapGestureRecognizer *)gesture {
     }
     RNCSlider *slider = (RNCSlider *)gesture.view;
     slider.isSliding = _isSliding;
-    
+
     // Ignore this tap if in the middle of a slide.
     if (_isSliding) {
         return;
     }
-    
+
     if (!slider.tapToSeek) {
         return;
     }
@@ -88,14 +88,14 @@ - (void)tapHandler:(UITapGestureRecognizer *)gesture {
     }
 
     [slider setValue:[slider discreteValue:value] animated: YES];
-    
+
     std::dynamic_pointer_cast<const RNCSliderEventEmitter>(_eventEmitter)
     ->onRNCSliderSlidingStart(RNCSliderEventEmitter::OnRNCSliderSlidingStart{.value = static_cast<Float>(slider.lastValue)});
-    
+
     // Trigger onValueChange to address https://github.com/react-native-community/react-native-slider/issues/212
     std::dynamic_pointer_cast<const RNCSliderEventEmitter>(_eventEmitter)
     ->onRNCSliderValueChange(RNCSliderEventEmitter::OnRNCSliderValueChange{.value = static_cast<Float>(slider.value)});
-    
+
     std::dynamic_pointer_cast<const RNCSliderEventEmitter>(_eventEmitter)
     ->onRNCSliderSlidingComplete(RNCSliderEventEmitter::OnRNCSliderSlidingComplete{.value = static_cast<Float>(slider.value)});
 }
@@ -122,7 +122,7 @@ - (void)sliderTouchEnd:(RNCSlider *)sender
 - (void)RNCSendSliderEvent:(RNCSlider *)sender withContinuous:(BOOL)continuous isSlidingStart:(BOOL)isSlidingStart
 {
     float value = [sender discreteValue:sender.value];
-    
+
     if (value < sender.lowerLimit) {
         value = sender.lowerLimit;
         [sender setValue:value animated:NO];
@@ -134,7 +134,7 @@ - (void)RNCSendSliderEvent:(RNCSlider *)sender withContinuous:(BOOL)continuous i
     if(!sender.isSliding) {
         [sender setValue:value animated:NO];
     }
-    
+
     if (continuous) {
         if (sender.lastValue != value)  {
             std::dynamic_pointer_cast<const RNCSliderEventEmitter>(_eventEmitter)
@@ -150,15 +150,15 @@ - (void)RNCSendSliderEvent:(RNCSlider *)sender withContinuous:(BOOL)continuous i
             ->onRNCSliderSlidingStart(RNCSliderEventEmitter::OnRNCSliderSlidingStart{.value = static_cast<Float>(value)});
         }
     }
-    
+
     sender.lastValue = value;
 }
 
 - (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared &)oldProps
 {
     const auto &oldScreenProps = *std::static_pointer_cast<const RNCSliderProps>(_props);
     const auto &newScreenProps = *std::static_pointer_cast<const RNCSliderProps>(props);
-    
+
     if (oldScreenProps.value != newScreenProps.value) {
         if (!slider.isSliding) {
             slider.value = newScreenProps.value;
@@ -176,11 +176,19 @@ - (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared &
     if (oldScreenProps.maximumValue != newScreenProps.maximumValue) {
         [slider setMaximumValue:newScreenProps.maximumValue];
     }
-    if (oldScreenProps.lowerLimit != newScreenProps.lowerLimit) {
-        slider.lowerLimit = newScreenProps.lowerLimit;
+    if (slider.lowerLimit != newScreenProps.lowerLimit) {
+        if(newScreenProps.lowerLimit > slider.upperLimit){
+            NSLog(@"Invalid configuration: upperLimit < lowerLimit; lowerLimit not set");
+        } else {
+            slider.lowerLimit = newScreenProps.lowerLimit;
+        }
     }
-    if (oldScreenProps.upperLimit != newScreenProps.upperLimit) {
-        slider.upperLimit = newScreenProps.upperLimit;
+    if (slider.upperLimit != newScreenProps.upperLimit) {
+        if(newScreenProps.upperLimit < slider.lowerLimit){
+            NSLog(@"Invalid configuration: upperLimit < lowerLimit; upperLimit not set");
+        } else {
+            slider.upperLimit = newScreenProps.upperLimit;
+        }
     }
     if (oldScreenProps.tapToSeek != newScreenProps.tapToSeek) {
         slider.tapToSeek = newScreenProps.tapToSeek;
diff --git a/package/ios/RNCSliderManager.m b/package/ios/RNCSliderManager.m
@@ -158,8 +158,24 @@ - (void)sliderTouchEnd:(RNCSlider *)sender
 RCT_EXPORT_VIEW_PROPERTY(maximumTrackImage, UIImage);
 RCT_EXPORT_VIEW_PROPERTY(minimumValue, float);
 RCT_EXPORT_VIEW_PROPERTY(maximumValue, float);
-RCT_EXPORT_VIEW_PROPERTY(lowerLimit, float);
-RCT_EXPORT_VIEW_PROPERTY(upperLimit, float);
+RCT_CUSTOM_VIEW_PROPERTY(lowerLimit, float, RNCSlider) {
+  float lowerLimit = [RCTConvert float:json];
+
+  if (lowerLimit > view.upperLimit) {
+      NSLog(@"Invalid configuration: upperLimit < lowerLimit; lowerLimit not set");
+    } else {
+      view.lowerLimit = lowerLimit;
+    }
+}
+RCT_CUSTOM_VIEW_PROPERTY(upperLimit, float, RNCSlider) {
+  float upperLimit = [RCTConvert float:json];
+
+  if (upperLimit < view.lowerLimit) {
+      NSLog(@"Invalid configuration: upperLimit < lowerLimit; upperLimit not set");
+    } else {
+      view.upperLimit = upperLimit;
+    }
+}
 RCT_EXPORT_VIEW_PROPERTY(minimumTrackTintColor, UIColor);
 RCT_EXPORT_VIEW_PROPERTY(maximumTrackTintColor, UIColor);
 RCT_EXPORT_VIEW_PROPERTY(onRNCSliderValueChange, RCTBubblingEventBlock);
diff --git a/package/src/Slider.tsx b/package/src/Slider.tsx
@@ -1,4 +1,4 @@
-import React, {useState} from 'react';
+import React, {useEffect, useState} from 'react';
 import {
   Image,
   Platform,
@@ -281,6 +281,14 @@ const SliderComponent = (
           default: constants.LIMIT_MAX_VALUE,
         });
 
+  useEffect(() => {
+    if (lowerLimit >= upperLimit) {
+      console.warn(
+        'Invalid configuration: lower limit is supposed to be smaller than upper limit',
+      );
+    }
+  }, [lowerLimit, upperLimit]);
+
   return (
     <View
       onLayout={(event) => {

Original file line number	Diff line number	Diff line change
`@@ -14,6 +14,14 @@ @implementation RNCSlider`
`14`	`14`	`bool _maximumTrackImageSet;`
`15`	`15`	`}`
`16`	`16`
	`17`	`+- (instancetype)init {`
	`18`	`+ if (self = [super init]) {`
	`19`	`+ _upperLimit = FLT_MAX;`
	`20`	`+ _lowerLimit = FLT_MIN;`
	`21`	`+ }`
	`22`	`+ return self;`
	`23`	`+}`
	`24`	`+`
`17`	`25`	`- (instancetype)initWithFrame:(CGRect)frame`
`18`	`26`	`{`
`19`	`27`	`return [super initWithFrame:frame];`
`@@ -46,7 +54,7 @@ - (void)setupAccessibility:(float)value`
`46`	`54`	`if (sliderValue && [sliderValue intValue] == 1) {`
`47`	`55`	`spokenUnits = [spokenUnits substringToIndex:stringLength-1];`
`48`	`56`	`}`
`49`		`-`
	`57`	`+`
`50`	`58`	`self.accessibilityValue = [NSString stringWithFormat:@"%@ %@", sliderValue, spokenUnits];`
`51`	`59`	`}`
`52`	`60`	`}`