seiyria / bootstrap-slider

A slider control for Bootstrap 3 & 4.
http://seiyria.github.io/bootstrap-slider/
Other
3k stars 1.14k forks source link

Fix calculation of percentage values #881

Closed jespirit closed 5 years ago

jespirit commented 5 years ago

The percentage values were being miscalculated.

Old equation was taking into account negative min values.

Wrong (val / this.options.max) * 100;

Correct ((val - this.options.min) / (this.options.max - this.options.min)) * 100;

min, max = -100, 100
val = 10
// old
percentage
= (10 / 100) * 100
= 10%

// fix
percentage
= [ (10 - (-100)) / (100 - (-100) ] * 100
= [ (10 + 100) / (100 + 100) ] * 100
= 110 / 200 * 100
= 55%

So I just make call to this._toPercentage().

jespirit commented 5 years ago

That is a difficult question. I'll see what I can do.

jespirit commented 5 years ago

The percentage is used for checking if one handle overtakes another using either the keyboard or mouse and adjusts the percentages for both handles by switching them.

The code still seems to work even though the equation for calculating the percentage is incorrect. The percentage is incorrectly calculated based on a scale from 0 to max, regardless of the value for min.

// Current implementation
min,max = -100, +100
val, percentage
10, 10%
20, 20%
...
90, 90%
(percentages can be negative, which is obviously wrong, should be from 0 to 100)
-70, -70%
-60, -60%
...

Note: The percentages can be negative when the value is negative.

However, the logic still works when checking if one handle overtakes another because the percentages are based on the scale from 0 to max, (eg. 70% < 90%) and even for negative numbers (eg. -70% < -60%).

As for the logarithmic scale, I have no idea. I can't say I know a lot about logarithmic scale.

I added two unit tests to check if the correct handles are receiving focus.

rovolution commented 5 years ago

@jespirit thank you for doing that!!