JackDanielsAndCode / react-native-multi-slider

Pure JS react native slider component with one or two markers.
MIT License
151 stars 96 forks source link

Bug - Incorrect calculation of steps/step length #52

Open MachoPerrin opened 6 years ago

MachoPerrin commented 6 years ago

During the calculation of the width of each step on the screen, there is an issue that results in one more step than there are actual values in the slider:

this.stepLength = this.props.sliderLength/this.optionsArray.length;

This divides the width into steps based on the number of options in the array, rather than the number of gaps between each item. For example, in a slider with four options (let's say 1, 2, 3 and 4) there should be three steps (from 1 to 2, 2 to 3, and 3 to 4). However, because of this bug, the code treats it as if there are four steps. There is no change to the min/max values, but the points between the two ends are inexact and result in confusing behaviour - sometimes the slider will move without the value changing, and sometimes the value will change without the slider moving.

Having a look at the code, the solution is easy to implement:

this.stepLength = this.props.sliderLength/(this.optionsArray.length - 1);

Decreasing the options count by one will give the proper number of steps. This has to be done in two places: the getInitialState() and set(config) functions, both in Slider.js

Hopefully this fix can be implemented soon 🙂

MachoPerrin commented 6 years ago

After some more research and testing, it looks like the fix I mentioned above introduces a different bug that lets the minimum marker go past the maximum marker:

var top        = (this.state.positionTwo - this.stepLength) || this.props.sliderLength;

When calculating the highest value that the minimum slider can be, if the maximum slider is on the first step, then this.state.positionTwo - this.stepLength evaluates as 0 and this.props.sliderLength is used instead - as the default for if there is only one marker. The lower marker can then be moved past the upper marker so long as the upper marker stays at the first step.

In order to resolve this issue, a better solution to calculate the maximum depending on whether there is one or two markers would have to be determined.