Skip to content

Commit

Permalink
Enhance the Slider Component (#1070)
Browse files Browse the repository at this point in the history
* feat: change currentValue through value prop change

* fix: render thumbs through currentValue props

* docs: add value prop on storybook

* chore: add changeset

* test: add a test case

* refactor: name useEffect function

* refactor: set default value on the function props

* test(Slider): divide a test case to 2 cases

* refactor: forwardedRef type

* fix: always rerender when the current value is changed

* test: add user interaction test case
  • Loading branch information
guswnsxodlf authored Dec 26, 2022
1 parent 3aea494 commit 78d217e
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 14 deletions.
5 changes: 5 additions & 0 deletions .changeset/beige-rice-shout.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@channel.io/bezier-react": patch
---

Enhance the Slider component
7 changes: 7 additions & 0 deletions packages/bezier-react/jest.setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ global.ResizeObserver = class ResizeObserver {
disconnect() {}
}

/**
* Radix-ui uses the APIs below, but the DOM in jest (JSDOM) hasn't implemented them.
* @see https://github.com/radix-ui/primitives/issues/1822
*/
window.HTMLElement.prototype.hasPointerCapture = jest.fn()
window.HTMLElement.prototype.setPointerCapture = jest.fn()

beforeEach(() => {
// @ts-ignore
jest.spyOn(window, 'requestAnimationFrame').mockImplementation(cb => cb())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,9 @@ const Template: Story<SliderProps> = (args) => <Slider {...args} />

export const Primary = Template.bind({})
Primary.args = {
width: '36',
width: '285',
defaultValue: [5],
value: [2],
min: 0,
max: 10,
step: 1,
Expand All @@ -64,6 +65,7 @@ export const Uncontrolled = Template.bind({})
Uncontrolled.args = {
width: '285',
defaultValue: [5],
value: [2],
min: 0,
max: 10,
step: 1,
Expand All @@ -73,6 +75,7 @@ export const Disabled = Template.bind({})
Disabled.args = {
width: '285',
defaultValue: [5],
value: [2],
min: 0,
max: 10,
step: 1,
Expand All @@ -83,6 +86,7 @@ export const WithGuide = Template.bind({})
WithGuide.args = {
width: '285',
defaultValue: [5],
value: [2],
guide: [0, 1, 2, 3, 4, 5, 10],
min: 0,
max: 10,
Expand All @@ -93,6 +97,7 @@ export const MultipleThumbs = Template.bind({})
MultipleThumbs.args = {
width: '285',
defaultValue: [3, 7],
value: [2, 4],
min: 0,
max: 10,
step: 1,
Expand Down
50 changes: 50 additions & 0 deletions packages/bezier-react/src/components/Forms/Slider/Slider.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* External dependencies */
import React from 'react'
import userEvent from '@testing-library/user-event'

/* Internal dependencies */
import { LightFoundation } from 'Foundation'
Expand Down Expand Up @@ -158,4 +159,53 @@ describe('Slider', () => {
})
})
})

describe('onValueChange', () => {
it('should be executed when the `value` prop changes', () => {
const onValueChange = jest.fn()
let value = [3]
const { rerender } = renderSlider({ value, onValueChange })
expect(onValueChange).toHaveBeenCalledTimes(1)

// change value with a new array
value = [5]
rerender(<Slider {...{ value, onValueChange }} />)

expect(onValueChange).toHaveBeenCalledTimes(2)
})

it('should not be executed when the `value` prop has the same reference', () => {
const onValueChange = jest.fn()
const value = [3]
const { rerender } = renderSlider({ value, onValueChange })
expect(onValueChange).toHaveBeenCalledTimes(1)

// change value with the same reference
value.splice(0, 1, 3)
rerender(<Slider {...{ value, onValueChange }} />)

expect(onValueChange).toHaveBeenCalledTimes(1)
})
})

describe('user interactions', () => {
it('Click and ArrowLeft key', async () => {
const user = userEvent.setup()
const { getAllByTestId } = renderSlider({
value: [4, 6],
})

const sliderThumbs = getAllByTestId(SLIDER_THUMB_TEST_ID)

expect(sliderThumbs).toHaveLength(2)
expect(sliderThumbs[0]).toHaveAttribute('aria-valuenow', '4')
expect(sliderThumbs[1]).toHaveAttribute('aria-valuenow', '6')

await user.click(sliderThumbs[0])
await user.keyboard('[ArrowLeft]')

expect(sliderThumbs[0]).toHaveAttribute('aria-valuenow', '3')
expect(sliderThumbs[1]).toHaveAttribute('aria-valuenow', '6')
})
})
})
41 changes: 28 additions & 13 deletions packages/bezier-react/src/components/Forms/Slider/Slider.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
/* External dependencies */
import React, { forwardRef, useState, useCallback } from 'react'
import { isFunction } from 'lodash-es'
import React, {
forwardRef,
useState,
useCallback,
useEffect,
} from 'react'
import { noop } from 'lodash-es'
import * as SliderPrimitive from '@radix-ui/react-slider'

/* Internal dependencies */
Expand All @@ -14,39 +19,49 @@ export const Slider = forwardRef(function Slider(
{
width = 36,
guide,
onThumbPointerDown,
onThumbPointerUp,
onThumbPointerDown = noop,
onThumbPointerUp = noop,
defaultValue = [5],
value,
disabled = false,
min = 0,
max = 10,
step = 1,
onValueChange,
onValueChange = noop,
minStepsBetweenThumbs = 0,
...rest
}: SliderProps,
forwardedRef: React.Ref<HTMLElement>,
forwardedRef: React.Ref<HTMLDivElement>,
) {
const [currentValue, setCurrentValue] = useState<number[]>(value ?? defaultValue)

useEffect(function updateCurrentValue() {
if (value) {
setCurrentValue(value)
onValueChange(value)
}
}, [
value,
onValueChange,
])

const handleValueChange: (value: number[]) => void = useCallback((_value) => {
setCurrentValue(_value)
if (isFunction(onValueChange)) {
onValueChange(_value)
}
onValueChange(_value)
}, [onValueChange])

const handlePointerDown: React.PointerEventHandler<HTMLElement> = useCallback(() => {
if (!disabled && isFunction(onThumbPointerDown)) {
if (!disabled) {
onThumbPointerDown(currentValue)
}
}, [
disabled,
onThumbPointerDown,
currentValue,
])

const handlePointerUp: React.PointerEventHandler<HTMLElement> = useCallback(() => {
if (!disabled && isFunction(onThumbPointerUp)) {
if (!disabled) {
onThumbPointerUp(currentValue)
}
}, [
Expand Down Expand Up @@ -89,10 +104,10 @@ export const Slider = forwardRef(function Slider(
guideValue={guideValue}
/>
)) }
{ defaultValue.map((v) => (
{ currentValue.map((v, i) => (
<SliderPrimitive.Thumb
asChild
key={`slider-thumb-${v}`}
key={`slider-thumb-${i}`}
onPointerDown={handlePointerDown}
onPointerUp={handlePointerUp}
>
Expand Down

0 comments on commit 78d217e

Please sign in to comment.