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: check for invalid configuration in limit settings #638

Merged
merged 6 commits into from
Sep 24, 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
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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). */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
10 changes: 9 additions & 1 deletion package/ios/RNCSlider.m
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -46,7 +54,7 @@ - (void)setupAccessibility:(float)value
if (sliderValue && [sliderValue intValue] == 1) {
spokenUnits = [spokenUnits substringToIndex:stringLength-1];
}

self.accessibilityValue = [NSString stringWithFormat:@"%@ %@", sliderValue, spokenUnits];
}
}
Expand Down
38 changes: 23 additions & 15 deletions package/ios/RNCSliderComponentView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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)});
}
Expand All @@ -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];
Expand All @@ -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)
Expand All @@ -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;
Expand All @@ -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;
Expand Down
20 changes: 18 additions & 2 deletions package/ios/RNCSliderManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
10 changes: 9 additions & 1 deletion package/src/Slider.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {useState} from 'react';
import React, {useEffect, useState} from 'react';
import {
Image,
Platform,
Expand Down Expand Up @@ -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) => {
Expand Down
Loading