callstack / react-native-slider

React Native component exposing Slider from iOS and SeekBar from Android
MIT License
1.19k stars 267 forks source link

Slider jumping back and forth when sliding. #496

Closed shermanlyh closed 1 year ago

shermanlyh commented 1 year ago

Dear Developer, I have experienced Slider jumping issue while passing a useState variable to the onValueChange props. When the sliding complete, the slider will first jump back to the previous value and then jump again to the correct destination. Not sure if my way of passing data has some problem or this is a bug of the library. Any help would be appreciated.

"react-native": "^0.71.2",
"@react-native-community/slider": "^4.4.2"

https://user-images.githubusercontent.com/81810291/219538682-cfa04fad-f355-4eae-8288-393d27e8a199.mov

Slider code

<View style={styles.textCon}>
      <TouchableOpacity
        style={styles.minMaxLabel}
        disabled={isOn ? false : true}
        onPress={() => {
          onValueChange("-");
        }}
      >
        <Decrement_icon />
      </TouchableOpacity>
      <View style={styles.sliderLayout}>
        <View style={styles.minLabel}>
          <Text style={{ color: textColor }}>{minValue}</Text>
        </View>
        <View style={styles.maxLabel}>
          <Text style={{ color: textColor }}>{maxValue}</Text>
        </View>
        <Slider
          style={styles.alarmSlider}
          minimumValue={minValue}
          maximumValue={maxValue}
          value = { lowAlarm}
          step={step}
          disabled={!isOn}
          onValueChange={onValueChange}
          minimumTrackTintColor={
            isHigh ? Constants.color.Red : Constants.color.Blue
          }
          maximumTrackTintColor={
            isHigh ? Constants.color.Red : Constants.color.Blue
          }
          thumbTintColor={isHigh ? Constants.color.Red : Constants.color.Blue}
        />
      </View>
      <TouchableOpacity
        style={styles.minMaxLabel}
        disabled={!isOn}
        onPress={() => {
          onValueChange("+");
        }}
      >
        <Increment_icon />
      </TouchableOpacity>
    </View>

onValueChange Method

const onSlidingComplete = (value) => {
    setLowAlarm(value, 2);
  };
  const onValueChange = (value) => {
    if (value == "+") {
      if (lowAlarm < maxValue) {
        setLowAlarm(lowAlarm + step);
      }
    } else if (value == "-") {
      if (lowAlarm > minValue) {
        setLowAlarm(lowAlarm - step);
      }
    } else {
      setLowAlarm(value, 2);
    }
  };

Low Alarm useState variable const [lowAlarm, setLowAlarm] = useState(10);

BartoszKlonowski commented 1 year ago

Hello @shermanlyh! Thank you for posting this with the full repro snippet 👍 Let me take a look into that as soon as possible and come back to you.

durupanya commented 1 year ago

same problem 👍

costabia commented 1 year ago

same problem

shermanlyh commented 1 year ago

For those in a hurry, please try with "react-native-slider" library, this works for me with only download and replace the import, no extra code amendment needed.

https://github.com/jeanregisser/react-native-slider

import Slider from "react-native-slider";

BartoszKlonowski commented 1 year ago

@shermanlyh Regarding the example you posted:

  1. Why is the setLowAlarm taking two arguments in onValueChange function, when it's just a state of number?
  2. You posted the onSlidingComplete function implementation, but you never actually use it anywhere?
  3. The example have many styles that are not included - for the next time please reduce the example to be as much minimal as it can be
  4. The video you linked to the report is somehow broken - I can't see anything than a white screen there, can you repost it?

What platform do you see this issue on?

Also, if you managed to find another Slider library, if this issue still valid? @durupanya @costabia If you also are blocked by the issue please post your examples and more info so that I have something to work on.

shermanlyh commented 1 year ago

Dear developer,

  1. My slider allow user to increase or decrease fixed step value by clicking the + and - icon behind the slider. Maybe It is not a good approach.

  2. As my onValueChange is not working as expected with jumping back and forth so I tried with onSlidingComplete. But I hope user can see the value change when moving the slider so I decided not to use onSlidingComplete.

  3. Sorry for that.

  4. Please see if you can view this video, it is a slider demonstration for my issue. https://drive.google.com/file/d/1WXREwCl6RfuhOnvGYwT88hM2ZsyrYYFV/view?usp=sharing

This issue happens on Android and iOS.

Android version: 12 iOS version 16.3.1

This issue still valid. Thank you for your help.

BartoszKlonowski commented 1 year ago

@shermanlyh Alright, I get it now, thanks! So, in general controlling Slider through some other components shouldn't be a "bad approach", and (if correctly implemented) should work just fine. When it comes to correctly implemented part: Controlling Slider through other components should be done by passing values to value property. Note however, that changing the position of the thumb through this prop will not send any onSlidingComplete, onSlidingStarted nor onValueChange events. So to monitor the changes in the value is up to your implementation of tracking what is passed to value prop. So if you still depends on them, maybe this is where the issue really is? Especially considering the fact that you observe this on both platforms, and Android is much different than iOS when it comes to controlled-value feature implementation under the hood.

If you could please repost the full example of your Slider scenario, including the + and - icons it would be very helpful for me to investigate it.

shermanlyh commented 1 year ago

@BartoszKlonowski Hope this helps. Thanks! Full Slider code

import React, { useRef, useEffect } from "react";
import {
  StyleSheet,
  View,
  TouchableOpacity,
  Text,
  Platform,
} from "react-native";
import Slider from "@react-native-community/slider";
import _ from "lodash";

import Constants from "../constants";
import Email_plus_icon from "../Email_plus_icon";
import Email_minus_icon from "../Email_minus_icon";
import Signal_wifi_1_bar from "../Signal_wifi_1_bar";

const AlarmRangeSelector = (props) => {
  let sliderRef = useRef();

  let {
    isOn,
    textColor,
    minValue,
    maxValue,
    step,
    initialValue,
    onValueChange,

    value,
    isHigh,
  } = props;

  return (
    <View style={styles.textCon}>
      <TouchableOpacity
        style={styles.minMaxLabel}
        disabled={isOn ? false : true}
        onPress={() => {
          onValueChange("-");
        }}
      >
        <Email_minus_icon />
      </TouchableOpacity>
      <View style={styles.sliderLayout}>
        <View style={styles.minLabel}>
          <Text style={{ color: textColor }}>{minValue}</Text>
        </View>
        <View style={styles.maxLabel}>
          <Text style={{ color: textColor }}>{maxValue}</Text>
        </View>
        <Slider
          ref={sliderRef}
          style={styles.alarmSlider}
          minimumValue={minValue}
          maximumValue={maxValue}
          step={step}
          disabled={!isOn}
          onValueChange={onValueChange}
          value={initialValue}
          minimumTrackTintColor={
            isHigh ? Constants.color.Red : Constants.color.Blue
          }
          maximumTrackTintColor={
            isHigh ? Constants.color.Red : Constants.color.Blue
          }
          thumbTintColor={isHigh ? Constants.color.Red : Constants.color.Blue}
        />
      </View>
      <TouchableOpacity
        style={styles.minMaxLabel}
        disabled={!isOn}
        onPress={() => {
          onValueChange("+");
        }}
      >
        <Email_plus_icon />
      </TouchableOpacity>
    </View>
  );
};

export default AlarmRangeSelector;

const styles = StyleSheet.create({
  alarmSlider: {
    width: 250,
    height: 50,
  },
  sliderLayout: {
    height: Platform.OS == "ios" ? 70 : 50,
    alignItems: "center",
    justifyContent: "center",
  },
  minLabel: {
    position: "absolute",
    top: 0,
    left: 10,
  },
  maxLabel: {
    position: "absolute",
    top: 0,
    right: 10,
  },

  minMaxLabel: {
    borderRadius: 25,
    height: 50,
    width: 50,
    alignItems: "center",
    justifyContent: "center",
    alignSelf: "center",
    backgroundColor: Constants.color.Blue,
  },

  textCon: {
    flexDirection: "row",
    justifyContent: "space-between",
  },
});

Code for implementing the slider

<View>
<AlarmRangeSelector
                  isHigh={false}
                  isOn={isOn}
                  minValue={minValue}
                  maxValue={maxValue}
                  step={step}
                  textColor={textColor}
                  value={lowAlarm}
                  onValueChange={onValueChange}
                  initialValue={lowAlarm}
                />
</View>

onValueChange method

const onValueChange = (value) => {
    if (value == "+") {
      if (lowAlarm < maxValue) {
        setLowAlarm(round(lowAlarm + step, 2));
      }
    } else if (value == "-") {
      if (lowAlarm > minValue) {
        setLowAlarm(round(lowAlarm - step, 2));
      }
    } else {
      setLowAlarm(round(value, 2));
    }
  };

useState variable

const [step, setStep] = useState(1);
  const [lowAlarm, setLowAlarm] = useState(0);
BartoszKlonowski commented 1 year ago

@shermanlyh Awesome! Let me go through the implementation of yours and locate the cause of Slider jumping. I'll get back to you later today (or in worst case tomorrow).

BartoszKlonowski commented 1 year ago

Alright! @shermanlyh, I implemented the example of your scenario and got it fully working so that:

Here is the video showing the results:

https://user-images.githubusercontent.com/70535775/223589803-47949143-ac43-4378-bfb8-2bbec1ac162a.mov

You can always check this implementation by looking at the linked PR: "Add Issue 496" in my repro app. Also you can launch this example on your own if needed.


Basically what was done here regarding your example:

Of course, I still had to simplify your implementation a bit when it comes to stylings, props, ref etc, but these changes do not affect the solution of how to programmatically control the Slider without conflicts with potential manual sliding as well.

I really hope this helps, but if you have any more questions feel free to ask here, or just find me on the Callstack's Discord!

shermanlyh commented 1 year ago

Thank you for the implementation.

So is it possible that my issue is related to the react-native library version?

I am in 0.71.2 and I saw you are in 0.70.6.

BartoszKlonowski commented 1 year ago

No, I really don't think so. Moreover, I was able to reproduce the issue you described, but then I saw that the issue real root cause is that you somehow mixed the states of the value that is used to control the Slider programmatically with how you manage the sliding events. Please take a look at the implementation I made and see if you can apply the solution to your project. Long story short - I think you only should remove the onValueChange event handler from the Slider, and use the onSlidingComplete event to get the value of thumb where user finished dragging. Use this value to set the lowAlarm, so when user presses the plus and minus icon again, you start updating the Slider's position exactly from the value where user previously finished manual drag.

shermanlyh commented 1 year ago

I have tried using onSlidingComplete before and it worked fine too. But in my application, it is required to show the value to user when user is dragging the thumb ( the middle value displayed in my video) , so I have to give up onSlidingComplete as it only updates the value when sliding complete. Or is there any method the value will still update when dragging while using onSlidingComplete?

BartoszKlonowski commented 1 year ago

@shermanlyh No problem, let me come up with showing the value, so I will update the linked PR to the repro app...

BartoszKlonowski commented 1 year ago

Alright, this commit introduces the current value displayed below the Slider, here is the video of how it works:

https://user-images.githubusercontent.com/70535775/223606340-e538b346-8de5-434d-9a8c-44f4d11a5eab.mov

As you can see I'm in fact using the onValueChange, but I don't mix it with the onValueChange used for the icon click. When separating these two, I can comfortably just set the lowAlarm value without breaking the position. Try it, and let me know what is your opinion.

shermanlyh commented 1 year ago

I have tried your method, but the jumping still occur, did I miss something?

https://user-images.githubusercontent.com/81810291/223962906-f6cd2197-512c-4b40-8dec-89b4f4c42e73.mp4

onManualValueChange and onProgrammaticChange

const onManualValueChange = (value) => {
    setLowAlarm(round(value, 2));
  };

  const onProgrammaticChange = (value) => {
    if (value == "+") {
      if (lowAlarm < maxValue) {
        setLowAlarm(round(lowAlarm + step, 2));
      }
    } else if (value == "-") {
      if (lowAlarm > minValue) {
        setLowAlarm(round(lowAlarm - step, 2));
      }
    }
  };

AlarmRangeSelector

<View style={styles.textCon}>
      <TouchableOpacity
        style={styles.minMaxLabel}
        disabled={isOn ? false : true}
        onPress={() => {
          onProgrammaticChange("-");
        }}
      >
        <Email_minus_icon />
      </TouchableOpacity>
      <View style={styles.sliderLayout}>
        <View style={styles.minLabel}>
          <Text style={{ color: textColor }}>{minValue}</Text>
        </View>
        <View style={styles.maxLabel}>
          <Text style={{ color: textColor }}>{maxValue}</Text>
        </View>
        <Slider
          style={styles.alarmSlider}
          minimumValue={minValue}
          maximumValue={maxValue}
          step={step}
          disabled={!isOn}
          onValueChange={(val) => onManualValueChange(val)}
          value={initialValue}
          minimumTrackTintColor={
            isHigh ? Constants.color.Red : Constants.color.Blue
          }
          maximumTrackTintColor={
            isHigh ? Constants.color.Red : Constants.color.Blue
          }
          thumbTintColor={isHigh ? Constants.color.Red : Constants.color.Blue}
        />
      </View>
      <TouchableOpacity
        style={styles.minMaxLabel}
        disabled={!isOn}
        onPress={() => {
          onProgrammaticChange("+");
        }}
      >
        <Email_plus_icon />
      </TouchableOpacity>
    </View>
BartoszKlonowski commented 1 year ago

@shermanlyh Are you running this on the new RN architecture?

shermanlyh commented 1 year ago

No , I am in the normal one. I did not see fabric true in my metro log.

BartoszKlonowski commented 1 year ago

Sure, ok, I'm just checking all the possibilities. Well, I'm trying to compare our solutions but I don't see anything significant. I'm still eager to find out what is going on, so right now the best thing I see to try is that if you could provide me with your repository so I can launch your project and try it. Other than that, I will try to launch a project on the latest RN version, just to be sure it's not the case. Thank you for the patience!

BartoszKlonowski commented 1 year ago

Welcome back @shermanlyh! I've tried the same with the latest versions of react native but in my case the Slider works fine. Did you have a chance to prepare a reproduction repository so I can clone it and work on this issue?

BartoszKlonowski commented 1 year ago

Hello @durupanya, @costabia, Are you still facing the issue? Would you be able to post any link to your example that I could clone and analyse?

BartoszKlonowski commented 1 year ago

Let me close this as there's no more feedback from affected users, and there's no repro possibility. Still: I'm happy to reopen if I'm provided with the repository reproducing this issue.

artsnr1 commented 1 year ago

@shermanlyh did you find the solution? I am facing the same problem

durupanya commented 1 year ago

I'm sorry for the late reply @BartoszKlonowski and other firends.

i think this problem is a timing, if you use "useState" for onSlidingComplete function actually is a timing problem for get and set data for Slider value.. You can test Slider with static data, and you see not a problem.. An other test for static data without useState or get set api post data... i'm fixed this problem for me with in onSlidingComplete.. And below my onSlidingComplete function solution:

<Slider style={[{ width: '90%', height: 30 }]} disabled={activityData ? false : true} minimumValue={0} maximumValue={100} minimumTrackTintColor="#400261" maximumTrackTintColor="#400261" value={sliderValueText} step={10} onSlidingComplete={(itemValue) => { setOnSlidingCompleteSlider(activityData.Id, itemValue); }} />

async function setOnSlidingCompleteSlider(_id, sliderValue) { const response = await connectGet(updateProgress + "?id=" + _id + "&progress=" + sliderValue); setTimeout(() => { setSliderValueText(sliderValue); }, 100); }

ZoranPerin commented 7 months ago

Any solution for this?

chur1 commented 4 months ago

I wanted to leave a quick note here, I was struggling with this for quite some time. You need to have two separate variables, one to manage the value of the slider when it changes in position, and one more to set the value of the variable to the value of the slider on release. In my particular case I used useState to continuously monitor the value of the slider at any given moment because I had a custom bubble (which in turn led me to experiencing the same issue). This is an issue as the state is changing at such a high cadence. The way I solved the issue was with: (onSlidingComplete={handleSlidingComplete}). As you'll see in the code below, i had created a new variable that only changes state on release of the slider, which will then go on in my app.tsx to return the value I was after and doing so in parallel without disturbing the bubble value/slider functionality. This seemed to fix my issue. Hope this helps. Code is below.


import 'react-native-gesture-handler';
import { GestureHandlerRootView } from 'react-native-gesture-handler';
import React, { useState } from 'react';
import { View, Text, StyleSheet } from 'react-native';
import { Slider, Bubble } from 'react-native-awesome-slider';
import { useSharedValue } from 'react-native-reanimated';

const styles = StyleSheet.create({
 bubbleContainer: {
   alignSelf: 'center',
   borderColor: '#26f',
   borderWidth: 5,
   borderRadius: 5,
   backgroundColor: '#26f',
 },
 bubbleText: {
   color: 'white',
 },
});

const RadiusSlider = ({ onRadiusChange }) => {
 const [selectedRadius, setSelectedRadius] = useState(0.5);
 const [radius, setRadius] = useState(0.5);
 const progress = useSharedValue(0.5);
 const min = useSharedValue(0);
 const max = useSharedValue(10);

 const handleValueChange = (value) => {
   const newValue = value.toFixed(1);
   setSelectedRadius(newValue);
   progress.value = newValue;
 };

 const handleSlidingComplete = () => {
   setRadius(selectedRadius);
   onRadiusChange(selectedRadius);
 };

 const renderBubble = () => {
   return (
     <View style={styles.bubbleContainer}>
       <Text style={styles.bubbleText}>{`${selectedRadius} Mi`}</Text>
     </View>
   );
 };

 return (
   <GestureHandlerRootView style={{ flex: 1 }}>
     <View>
       <Slider
         progress={progress}
         minimumValue={min}
         maximumValue={max}
         onValueChange={handleValueChange}
         renderBubble={renderBubble}
         onSlidingComplete={handleSlidingComplete}
       />
     </View>
   </GestureHandlerRootView>
 );
};

export default RadiusSlider;