Skip to content
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

fix(Slider): ensure min and max value is respected when step doesn't fit exactly #4395

Merged
merged 1 commit into from
Dec 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions packages/dnb-eufemia/src/components/slider/SliderHelpers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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 }) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
103 changes: 91 additions & 12 deletions packages/dnb-eufemia/src/components/slider/__tests__/Slider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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 } }}
/>
)
Expand All @@ -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
/>
Expand All @@ -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 } }}
/>
)
Expand All @@ -404,7 +485,6 @@ describe('Slider component', () => {

rerender(
<Slider
{...props}
extensions={{ marker: { instance: SliderMarker, value: 120 } }}
/>
)
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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 })

Expand Down Expand Up @@ -484,7 +564,6 @@ describe('Slider component', () => {

render(
<Slider
{...props}
onChange={onChange}
numberFormat={{ currency: true, decimals: 1 }}
/>
Expand Down Expand Up @@ -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 })
Expand All @@ -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')
Expand Down
Loading