mantinedev / mantine

A fully featured React components library
https://mantine.dev
MIT License
26.87k stars 1.9k forks source link

Slider not showing correct values #2444

Closed yaldram closed 2 years ago

yaldram commented 2 years ago

What package has an issue

@mantine/core

Describe the bug

For the Slider component if I have the following code -

 <Slider
    labelAlwaysOn
    min={1}
     max={100}
     step={10} 
  />

My min = 1 & max = 100. I am getting 101 as my max value -

image

What version of @mantine/hooks page do you have in package.json?

5.3.1

If possible, please include a link to a codesandbox with the reproduced problem

No response

Do you know how to fix the issue

No

Are you willing to participate in fixing this issue and create a pull request with the fix

Yes

Possible fix

I think under - src/mantine-core/src/Slider/utils/get-change-value/get-change-value.ts on line number 22 the Math.round() is causing this issue.

yaldram commented 2 years ago

Technically this is not a Bug, this is how the math works, I have seen the same behaviour in other libraries

rtivital commented 2 years ago

Thanks for reporting

yaldram commented 2 years ago

We can do the following under src/mantine-core/src/Slider/utils/get-change-value/get-change-value.ts -

return Math.max(Math.min(nextValue, max), min);

instead of just

return nextValue;
yaldram commented 2 years ago

Also check the following scenario -

 <Slider
    labelAlwaysOn
    min={-100}
     max={1}
     step={10} 
  />

It should go back to 1, but unfortunately sticks at 0. I checked the same behaviour for Material UI Slider it goes to 1.

yaldram commented 2 years ago

Also Please check the following scenario for the RangeSlider with minRange = stepSize it does not work -

<RangeSlider
          labelAlwaysOn
          min={-100}
          max={10}
          step={10}
          minRange={10}
          defaultValue={[1, 10]}
/>

Check this example from React Material UI - https://codesandbox.io/s/ph7nci?file=/demo.tsx

yaldram commented 2 years ago

Also Please check the following scenario the RangeSlider is stuck at 81 - 100 I am unable to choose a value 91 -100 -

     <RangeSlider labelAlwaysOn min={1} max={100} step={10} minRange={10} />
yaldram commented 2 years ago

I would say if we have a non-uniform scale like min - 1 to max - 100 as opposed to min - 0 to max - 100 we are gonna face such issues. The Material UI Slider it does not have min range at all.

yaldram commented 2 years ago

Material UI has a demo with minRange & stepSize here - https://codesandbox.io/s/tezue3?file=/demo.tsx, but it does not work well for negative numbers.