From 78d217ec20c85d4ec9485e40de1f9542a83aa29d Mon Sep 17 00:00:00 2001 From: "Hyeonjun (Jello)" Date: Mon, 26 Dec 2022 09:00:43 +0000 Subject: [PATCH] Enhance the `Slider` Component (#1070) * 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 --- .changeset/beige-rice-shout.md | 5 ++ packages/bezier-react/jest.setup.ts | 7 +++ .../Forms/Slider/Slider.stories.tsx | 7 ++- .../components/Forms/Slider/Slider.test.tsx | 50 +++++++++++++++++++ .../src/components/Forms/Slider/Slider.tsx | 41 ++++++++++----- 5 files changed, 96 insertions(+), 14 deletions(-) create mode 100644 .changeset/beige-rice-shout.md diff --git a/.changeset/beige-rice-shout.md b/.changeset/beige-rice-shout.md new file mode 100644 index 0000000000..acf430ca08 --- /dev/null +++ b/.changeset/beige-rice-shout.md @@ -0,0 +1,5 @@ +--- +"@channel.io/bezier-react": patch +--- + +Enhance the Slider component diff --git a/packages/bezier-react/jest.setup.ts b/packages/bezier-react/jest.setup.ts index bb96e68efb..906040113f 100644 --- a/packages/bezier-react/jest.setup.ts +++ b/packages/bezier-react/jest.setup.ts @@ -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()) diff --git a/packages/bezier-react/src/components/Forms/Slider/Slider.stories.tsx b/packages/bezier-react/src/components/Forms/Slider/Slider.stories.tsx index 64a2aa9a68..024244f393 100644 --- a/packages/bezier-react/src/components/Forms/Slider/Slider.stories.tsx +++ b/packages/bezier-react/src/components/Forms/Slider/Slider.stories.tsx @@ -53,8 +53,9 @@ const Template: Story = (args) => export const Primary = Template.bind({}) Primary.args = { - width: '36', + width: '285', defaultValue: [5], + value: [2], min: 0, max: 10, step: 1, @@ -64,6 +65,7 @@ export const Uncontrolled = Template.bind({}) Uncontrolled.args = { width: '285', defaultValue: [5], + value: [2], min: 0, max: 10, step: 1, @@ -73,6 +75,7 @@ export const Disabled = Template.bind({}) Disabled.args = { width: '285', defaultValue: [5], + value: [2], min: 0, max: 10, step: 1, @@ -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, @@ -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, diff --git a/packages/bezier-react/src/components/Forms/Slider/Slider.test.tsx b/packages/bezier-react/src/components/Forms/Slider/Slider.test.tsx index 8196d87d54..09ea375e84 100644 --- a/packages/bezier-react/src/components/Forms/Slider/Slider.test.tsx +++ b/packages/bezier-react/src/components/Forms/Slider/Slider.test.tsx @@ -1,5 +1,6 @@ /* External dependencies */ import React from 'react' +import userEvent from '@testing-library/user-event' /* Internal dependencies */ import { LightFoundation } from 'Foundation' @@ -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() + + 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() + + 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') + }) + }) }) diff --git a/packages/bezier-react/src/components/Forms/Slider/Slider.tsx b/packages/bezier-react/src/components/Forms/Slider/Slider.tsx index 4bb2042f60..87b563a612 100644 --- a/packages/bezier-react/src/components/Forms/Slider/Slider.tsx +++ b/packages/bezier-react/src/components/Forms/Slider/Slider.tsx @@ -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 */ @@ -14,30 +19,39 @@ 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, + forwardedRef: React.Ref, ) { const [currentValue, setCurrentValue] = useState(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 = useCallback(() => { - if (!disabled && isFunction(onThumbPointerDown)) { + if (!disabled) { onThumbPointerDown(currentValue) } }, [ @@ -45,8 +59,9 @@ export const Slider = forwardRef(function Slider( onThumbPointerDown, currentValue, ]) + const handlePointerUp: React.PointerEventHandler = useCallback(() => { - if (!disabled && isFunction(onThumbPointerUp)) { + if (!disabled) { onThumbPointerUp(currentValue) } }, [ @@ -89,10 +104,10 @@ export const Slider = forwardRef(function Slider( guideValue={guideValue} /> )) } - { defaultValue.map((v) => ( + { currentValue.map((v, i) => (