From 2c00b0ca0b8411522a66c68458a2ba1b5785d022 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20H=C3=B8egh?= Date: Fri, 13 Dec 2024 16:41:20 +0100 Subject: [PATCH] fix(Slider): ensure `min` and `max` value is respected when `step` doesn't fit exactly (#4395) --- .../src/components/slider/SliderHelpers.tsx | 22 ++-- .../src/components/slider/SliderProvider.tsx | 2 +- .../slider/__tests__/Slider.test.tsx | 103 ++++++++++++++++-- 3 files changed, 107 insertions(+), 20 deletions(-) diff --git a/packages/dnb-eufemia/src/components/slider/SliderHelpers.tsx b/packages/dnb-eufemia/src/components/slider/SliderHelpers.tsx index 2ea6194f64f..054544b37fb 100644 --- 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 index b2b728feef5..3c70f21601e 100644 --- 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 index 6c2e8afefcc..4e6ce1f7c66 100644 --- 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() + + 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() + + 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() + + 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() + + 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() + + 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( + + ) + + 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( ) @@ -359,7 +442,7 @@ describe('Slider component', () => { ) expect(markerElement).toHaveAttribute('style', 'left: 30%;') - rerender() + rerender() expect(sliderElement.innerHTML).not.toContain('dnb-slider__marker') }) @@ -367,7 +450,6 @@ describe('Slider component', () => { it('should render marker in vertical direction', () => { const { rerender } = render( @@ -381,7 +463,7 @@ describe('Slider component', () => { ) expect(markerElement).toHaveAttribute('style', 'top: 70%;') - rerender() + rerender() expect(sliderElement.innerHTML).not.toContain('dnb-slider__marker') }) @@ -389,7 +471,6 @@ describe('Slider component', () => { it('should have html attributes to make it accessible', () => { const { rerender } = render( ) @@ -404,7 +485,6 @@ describe('Slider component', () => { rerender( ) @@ -415,7 +495,7 @@ describe('Slider component', () => { it('shows Tooltip with info', async () => { const marker = { instance: SliderMarker, value: 30 } - render() + render() 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() + render() 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() + render() simulateMouseMove({ pageX: 80, width: 100, height: 10 }) @@ -484,7 +564,6 @@ describe('Slider component', () => { render( @@ -520,7 +599,7 @@ describe('Slider component', () => { it('will not emit onChange with same value twice', () => { const onChange = jest.fn() - render() + render() 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() + render() expect( document.querySelector('.dnb-slider__thumb .dnb-button') ).not.toHaveAttribute('type')