fix(Slider): ensure min and max value is respected when step do…

…esn't fit exactly (#4395)
dnbexperience · Dec 13, 2024 · 2c00b0c · 2c00b0c
1 parent 70df7c8
commit 2c00b0c
Show file tree

Hide file tree

Showing 3 changed files with 107 additions and 20 deletions.
diff --git a/packages/dnb-eufemia/src/components/slider/SliderHelpers.tsx b/packages/dnb-eufemia/src/components/slider/SliderHelpers.tsx
@@ -18,9 +18,6 @@ export const percentToValue = (
   return num
 }
 
-export const roundToStep = (number: number, step: number) =>
-  Math.round(number / step) * step
-
 export const getOffset = (node: HTMLElement) => {
   const { pageYOffset, pageXOffset } =
     typeof window !== 'undefined'
@@ -66,10 +63,21 @@ export const calculatePercent = (
 export const clamp = (value: number, min = 0, max = 100) =>
   Math.min(Math.max(value, min), max)
 
-export const roundValue = (value: number, step: number) => {
-  return step > 0
-    ? roundToStep(value, step)
-    : parseFloat(parseFloat(String(value)).toFixed(3))
+export const roundValue = (
+  value: number,
+  { step, min, max }: { step: number; min: number; max: number }
+) => {
+  if (step > 0) {
+    if (value >= max) {
+      return max
+    }
+    if (value <= min) {
+      return min
+    }
+    return Math.round(value / step) * step
+  }
+
+  return parseFloat(parseFloat(String(value)).toFixed(3))
 }
 
 export const createMockDiv = ({ width, height }) => {

diff --git a/packages/dnb-eufemia/src/components/slider/SliderProvider.tsx b/packages/dnb-eufemia/src/components/slider/SliderProvider.tsx
@@ -159,7 +159,7 @@ export function SliderProvider(localProps: SliderAllProps) {
       return
     }
 
-    let numberValue = roundValue(rawValue, step)
+    let numberValue = roundValue(rawValue, { step, min, max })
     let multiValues: ValueTypes = numberValue
 
     if (numberValue >= min) {

diff --git a/packages/dnb-eufemia/src/components/slider/__tests__/Slider.test.tsx b/packages/dnb-eufemia/src/components/slider/__tests__/Slider.test.tsx
@@ -162,6 +162,90 @@ describe('Slider component', () => {
     )
   })
 
+  describe('min', () => {
+    it('should respect min value', () => {
+      const onChange = jest.fn()
+
+      render(<Slider min={50} value={60} onChange={onChange} />)
+
+      simulateMouseMove({ pageX: 60, width: 100 })
+
+      expect(onChange).toHaveBeenLastCalledWith(
+        expect.objectContaining({ value: 80 })
+      )
+
+      simulateMouseMove({ pageX: -10, width: 100 })
+
+      expect(onChange).toHaveBeenLastCalledWith(
+        expect.objectContaining({ value: 50 })
+      )
+    })
+
+    it('should respect min value with too large "step"', () => {
+      const onChange = jest.fn()
+
+      render(<Slider min={5} step={10} value={50} onChange={onChange} />)
+
+      simulateMouseMove({ pageX: 0, width: 100 })
+
+      expect(onChange).toHaveBeenLastCalledWith(
+        expect.objectContaining({ value: 5 })
+      )
+    })
+  })
+
+  describe('max', () => {
+    it('should respect max value value', () => {
+      const onChange = jest.fn()
+
+      render(<Slider max={200} onChange={onChange} />)
+
+      simulateMouseMove({ pageX: 210, width: 100 })
+
+      expect(onChange).toHaveBeenCalledWith(
+        expect.objectContaining({ value: 200 })
+      )
+    })
+
+    it('should respect "step" that do not divide with max', () => {
+      const onChange = jest.fn()
+
+      render(<Slider step={3} max={100} onChange={onChange} />)
+
+      simulateMouseMove({ pageX: 100, width: 100 })
+
+      expect(onChange).toHaveBeenLastCalledWith(
+        expect.objectContaining({ value: 100 })
+      )
+    })
+
+    it('should respect max value with too large "step"', () => {
+      const onChange = jest.fn()
+
+      render(<Slider max={105} step={10} value={50} onChange={onChange} />)
+
+      simulateMouseMove({ pageX: 100, width: 100 })
+
+      expect(onChange).toHaveBeenLastCalledWith(
+        expect.objectContaining({ value: 105 })
+      )
+    })
+
+    it('should respect max value with too large "step" and large number', () => {
+      const onChange = jest.fn()
+
+      render(
+        <Slider max={2040} step={100} value={1000} onChange={onChange} />
+      )
+
+      simulateMouseMove({ pageX: 100, width: 100 })
+
+      expect(onChange).toHaveBeenLastCalledWith(
+        expect.objectContaining({ value: 2040 })
+      )
+    })
+  })
+
   describe('Tooltip', () => {
     const IS_TEST = globalThis.IS_TEST
     beforeEach(() => {
@@ -346,7 +430,6 @@ describe('Slider component', () => {
     it('should render marker in horizontal direction', () => {
       const { rerender } = render(
         <Slider
-          {...props}
           extensions={{ marker: { instance: SliderMarker, value: 30 } }}
         />
       )
@@ -359,15 +442,14 @@ describe('Slider component', () => {
       )
       expect(markerElement).toHaveAttribute('style', 'left: 30%;')
 
-      rerender(<Slider {...props} />)
+      rerender(<Slider />)
 
       expect(sliderElement.innerHTML).not.toContain('dnb-slider__marker')
     })
 
     it('should render marker in vertical direction', () => {
       const { rerender } = render(
         <Slider
-          {...props}
           extensions={{ marker: { instance: SliderMarker, value: 30 } }}
           vertical
         />
@@ -381,15 +463,14 @@ describe('Slider component', () => {
       )
       expect(markerElement).toHaveAttribute('style', 'top: 70%;')
 
-      rerender(<Slider {...props} />)
+      rerender(<Slider />)
 
       expect(sliderElement.innerHTML).not.toContain('dnb-slider__marker')
     })
 
     it('should have html attributes to make it accessible', () => {
       const { rerender } = render(
         <Slider
-          {...props}
           extensions={{ marker: { instance: SliderMarker, value: 30 } }}
         />
       )
@@ -404,7 +485,6 @@ describe('Slider component', () => {
 
       rerender(
         <Slider
-          {...props}
           extensions={{ marker: { instance: SliderMarker, value: 120 } }}
         />
       )
@@ -415,7 +495,7 @@ describe('Slider component', () => {
 
     it('shows Tooltip with info', async () => {
       const marker = { instance: SliderMarker, value: 30 }
-      render(<Slider {...props} extensions={{ marker }} />)
+      render(<Slider extensions={{ marker }} />)
 
       const sliderElement = document.querySelector('.dnb-slider')
       const markerElement = sliderElement.querySelector(
@@ -437,7 +517,7 @@ describe('Slider component', () => {
         value: 30,
         text: 'Here is the text',
       }
-      render(<Slider {...props} extensions={{ marker }} />)
+      render(<Slider extensions={{ marker }} />)
 
       const sliderElement = document.querySelector('.dnb-slider')
       const markerElement = sliderElement.querySelector(
@@ -456,7 +536,7 @@ describe('Slider component', () => {
   it('has events that return a correct value', () => {
     const onChange = jest.fn()
 
-    render(<Slider {...props} onChange={onChange} />)
+    render(<Slider onChange={onChange} />)
 
     simulateMouseMove({ pageX: 80, width: 100, height: 10 })
 
@@ -484,7 +564,6 @@ describe('Slider component', () => {
 
     render(
       <Slider
-        {...props}
         onChange={onChange}
         numberFormat={{ currency: true, decimals: 1 }}
       />
@@ -520,7 +599,7 @@ describe('Slider component', () => {
   it('will not emit onChange with same value twice', () => {
     const onChange = jest.fn()
 
-    render(<Slider {...props} onChange={onChange} />)
+    render(<Slider onChange={onChange} />)
 
     simulateMouseMove({ pageX: 80, width: 100, height: 10 })
     simulateMouseMove({ pageX: 80, width: 100, height: 10 })
@@ -530,7 +609,7 @@ describe('Slider component', () => {
   })
 
   it('should not have type=button', () => {
-    render(<Slider {...props} />)
+    render(<Slider />)
     expect(
       document.querySelector('.dnb-slider__thumb .dnb-button')
     ).not.toHaveAttribute('type')