Skip to content

Bug - Incorrect calculation of steps/step length #52

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

Open
MachoPerrin opened this issue Jan 10, 2018 · 1 comment
Open

Bug - Incorrect calculation of steps/step length #52

MachoPerrin opened this issue Jan 10, 2018 · 1 comment

Comments

@MachoPerrin
Copy link

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant