diff --git a/packages/@react-aria/color/src/useColorWheel.ts b/packages/@react-aria/color/src/useColorWheel.ts index 7d9389ba8c4..9d171d98103 100644 --- a/packages/@react-aria/color/src/useColorWheel.ts +++ b/packages/@react-aria/color/src/useColorWheel.ts @@ -40,7 +40,6 @@ interface ColorWheelAria { export function useColorWheel(props: ColorWheelAriaProps, state: ColorWheelState, inputRef: RefObject): ColorWheelAria { let { isDisabled, - step = 1, innerRadius, outerRadius, 'aria-label': ariaLabel @@ -256,6 +255,7 @@ export function useColorWheel(props: ColorWheelAriaProps, state: ColorWheelState ...props, 'aria-label': ariaLabel }); + let step = stateRef.current.step; return { trackProps: { diff --git a/packages/@react-aria/color/test/useColorWheel.test.tsx b/packages/@react-aria/color/test/useColorWheel.test.tsx index 86f4897cfdc..2026c88fd35 100644 --- a/packages/@react-aria/color/test/useColorWheel.test.tsx +++ b/packages/@react-aria/color/test/useColorWheel.test.tsx @@ -51,7 +51,7 @@ function ColorWheel(props: ColorWheelProps) { describe('useColorWheel', () => { let onChangeSpy = jest.fn(); - + beforeAll(() => { // @ts-ignore jest.spyOn(window, 'requestAnimationFrame').mockImplementation((cb) => cb()); @@ -170,38 +170,6 @@ describe('useColorWheel', () => { expect(slider).toHaveAttribute('value', '359'); }); - it('respects step', () => { - let defaultColor = parseColor('hsl(0, 100%, 50%)'); - let {getByRole} = render(); - let slider = getByRole('slider'); - act(() => {slider.focus();}); - - fireEvent.keyDown(slider, {key: 'Right'}); - expect(onChangeSpy).toHaveBeenCalledTimes(1); - expect(onChangeSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 45).toString('hsla')); - expect(slider).toHaveAttribute('value', '45'); - fireEvent.keyDown(slider, {key: 'Left'}); - expect(onChangeSpy).toHaveBeenCalledTimes(2); - expect(onChangeSpy.mock.calls[1][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 0).toString('hsla')); - expect(slider).toHaveAttribute('value', '0'); - }); - - it('can always get back to 0 even with step', () => { - let defaultColor = parseColor('hsl(330, 100%, 50%)'); - let {getByRole} = render(); - let slider = getByRole('slider'); - act(() => {slider.focus();}); - - fireEvent.keyDown(slider, {key: 'Right'}); - expect(onChangeSpy).toHaveBeenCalledTimes(1); - expect(onChangeSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 0).toString('hsla')); - expect(slider).toHaveAttribute('value', '0'); - fireEvent.keyDown(slider, {key: 'Left'}); - expect(onChangeSpy).toHaveBeenCalledTimes(2); - expect(onChangeSpy.mock.calls[1][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 330).toString('hsla')); - expect(slider).toHaveAttribute('value', '330'); - }); - it('steps with page up/down', () => { let defaultColor = parseColor('hsl(0, 100%, 50%)'); let {getByRole} = render(); @@ -210,8 +178,8 @@ describe('useColorWheel', () => { fireEvent.keyDown(slider, {key: 'PageUp'}); expect(onChangeSpy).toHaveBeenCalledTimes(1); - expect(onChangeSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 6).toString('hsla')); - expect(slider).toHaveAttribute('value', '6'); + expect(onChangeSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 15).toString('hsla')); + expect(slider).toHaveAttribute('value', '15'); fireEvent.keyDown(slider, {key: 'PageDown'}); expect(onChangeSpy).toHaveBeenCalledTimes(2); expect(onChangeSpy.mock.calls[1][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 0).toString('hsla')); @@ -285,24 +253,6 @@ describe('useColorWheel', () => { expect(document.activeElement).not.toBe(slider); }); - it('dragging the thumb respects the step', () => { - let defaultColor = parseColor('hsl(0, 100%, 50%)'); - let {getByRole, getByTestId} = render(); - let thumb = getByTestId('thumb'); - let slider = getByRole('slider'); - let container = getByTestId('container'); - container.getBoundingClientRect = getBoundingClientRect; - - start(thumb, {pageX: CENTER + THUMB_RADIUS, pageY: CENTER}); - expect(onChangeSpy).toHaveBeenCalledTimes(0); - move(thumb, {pageX: CENTER, pageY: CENTER + THUMB_RADIUS}); - expect(onChangeSpy).toHaveBeenCalledTimes(1); - expect(onChangeSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 120).toString('hsla')); - expect(slider).toHaveAttribute('value', '120'); - end(thumb, {pageX: CENTER, pageY: CENTER + THUMB_RADIUS}); - expect(onChangeSpy).toHaveBeenCalledTimes(1); - }); - it('clicking and dragging on the track works', () => { let defaultColor = parseColor('hsl(0, 100%, 50%)'); let {getByRole, getByTestId} = render(); @@ -349,25 +299,5 @@ describe('useColorWheel', () => { expect(onChangeSpy).toHaveBeenCalledTimes(0); expect(document.activeElement).not.toBe(slider); }); - - it('clicking and dragging on the track respects the step', () => { - let defaultColor = parseColor('hsl(0, 100%, 50%)'); - let {getByRole, getByTestId} = render(); - let thumb = getByTestId('thumb'); - let slider = getByRole('slider'); - let container = getByTestId('container'); - container.getBoundingClientRect = getBoundingClientRect; - - start(container, {pageX: CENTER, pageY: CENTER + THUMB_RADIUS}); - expect(onChangeSpy).toHaveBeenCalledTimes(1); - expect(onChangeSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 120).toString('hsla')); - expect(slider).toHaveAttribute('value', '120'); - move(thumb, {pageX: CENTER, pageY: CENTER - THUMB_RADIUS}); - expect(onChangeSpy).toHaveBeenCalledTimes(2); - expect(onChangeSpy.mock.calls[1][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 240).toString('hsla')); - expect(slider).toHaveAttribute('value', '240'); - end(thumb, {pageX: CENTER, pageY: CENTER - THUMB_RADIUS}); - expect(onChangeSpy).toHaveBeenCalledTimes(2); - }); }); }); diff --git a/packages/@react-spectrum/color/stories/ColorArea.stories.tsx b/packages/@react-spectrum/color/stories/ColorArea.stories.tsx index a1e70542bff..84095c4d688 100644 --- a/packages/@react-spectrum/color/stories/ColorArea.stories.tsx +++ b/packages/@react-spectrum/color/stories/ColorArea.stories.tsx @@ -95,15 +95,6 @@ export let XGreenYRed = Template.bind({}); XGreenYRed.storyName = 'RGB xChannel="green", yChannel="red"'; XGreenYRed.args = {...XBlueYGreen.args, xChannel: 'green', yChannel: 'red'}; -export let XBlueYGreenStep16 = Template.bind({}); -XBlueYGreenStep16.storyName = 'RGB xChannel="blue", yChannel="green", step="16"'; -XBlueYGreenStep16.args = {...XBlueYGreen.args, xChannelStep: 16, yChannelStep: 16}; - -export let XBlueYGreenPageStep32 = Template.bind({}); -XBlueYGreenPageStep32.storyName = 'RGB xChannel="blue", yChannel="green", pageStep="32"'; -XBlueYGreenPageStep32.args = {...XBlueYGreen.args, xChannelPageStep: 32, yChannelPageStep: 32}; - -/* TODO: what does a disabled color area look like? */ export let XBlueYGreenisDisabled = Template.bind({}); XBlueYGreenisDisabled.storyName = 'RGB xChannel="blue", yChannel="green", isDisabled'; XBlueYGreenisDisabled.args = {...XBlueYGreen.args, isDisabled: true}; diff --git a/packages/@react-spectrum/color/stories/ColorWheel.stories.tsx b/packages/@react-spectrum/color/stories/ColorWheel.stories.tsx index b808bec399d..b84614473f2 100644 --- a/packages/@react-spectrum/color/stories/ColorWheel.stories.tsx +++ b/packages/@react-spectrum/color/stories/ColorWheel.stories.tsx @@ -26,10 +26,6 @@ storiesOf('ColorWheel', module) 'disabled', () => ) - .add( - 'step', - () => - ) .add( 'custom size', () => { diff --git a/packages/@react-spectrum/color/test/ColorArea.test.tsx b/packages/@react-spectrum/color/test/ColorArea.test.tsx index a9ef7a834b9..8115d0dc0ed 100644 --- a/packages/@react-spectrum/color/test/ColorArea.test.tsx +++ b/packages/@react-spectrum/color/test/ColorArea.test.tsx @@ -131,10 +131,10 @@ describe('ColorArea', () => { Name | props | actions | result ${'left/right'} | ${{defaultValue: parseColor('#ff00ff')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowLeft'}), backward: (elem) => pressKey(elem, {key: 'ArrowRight'})}} | ${parseColor('#ff00fe')} ${'up/down'} | ${{defaultValue: parseColor('#ff00ff')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowUp'}), backward: (elem) => pressKey(elem, {key: 'ArrowDown'})}} | ${parseColor('#ff01ff')} - ${'shiftleft/shiftright'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowLeft', shiftKey: true}), backward: (elem) => pressKey(elem, {key: 'ArrowRight', shiftKey: true})}} | ${parseColor('#f000e0')} - ${'shiftup/shiftdown'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowUp', shiftKey: true}), backward: (elem) => pressKey(elem, {key: 'ArrowDown', shiftKey: true})}} | ${parseColor('#f010f0')} - ${'pageup/pagedown'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'PageUp'}), backward: (elem) => pressKey(elem, {key: 'PageDown'})}} | ${parseColor('#f010f0')} - ${'home/end'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'Home'}), backward: (elem) => pressKey(elem, {key: 'End'})}} | ${parseColor('#f000e0')} + ${'shiftleft/shiftright'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowLeft', shiftKey: true}), backward: (elem) => pressKey(elem, {key: 'ArrowRight', shiftKey: true})}} | ${parseColor('#f000df')} + ${'shiftup/shiftdown'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowUp', shiftKey: true}), backward: (elem) => pressKey(elem, {key: 'ArrowDown', shiftKey: true})}} | ${parseColor('#f011f0')} + ${'pageup/pagedown'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'PageUp'}), backward: (elem) => pressKey(elem, {key: 'PageDown'})}} | ${parseColor('#f011f0')} + ${'home/end'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'Home'}), backward: (elem) => pressKey(elem, {key: 'End'})}} | ${parseColor('#f000df')} `('$Name', ({props, actions: {forward, backward}, result}) => { let {getAllByRole} = render( { Name | props | actions | result ${'left/right'} | ${{defaultValue: parseColor('#ff00ff')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowRight'}), backward: (elem) => pressKey(elem, {key: 'ArrowLeft'})}} | ${parseColor('#ff00fe')} ${'up/down'} | ${{defaultValue: parseColor('#ff00ff')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowUp'}), backward: (elem) => pressKey(elem, {key: 'ArrowDown'})}} | ${parseColor('#ff01ff')} - ${'shiftleft/shiftright'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowRight', shiftKey: true}), backward: (elem) => pressKey(elem, {key: 'ArrowLeft', shiftKey: true})}} | ${parseColor('#f000e0')} - ${'shiftup/shiftdown'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowUp', shiftKey: true}), backward: (elem) => pressKey(elem, {key: 'ArrowDown', shiftKey: true})}} | ${parseColor('#f010f0')} - ${'pageup/pagedown'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'PageUp'}), backward: (elem) => pressKey(elem, {key: 'PageDown'})}} | ${parseColor('#f010f0')} - ${'home/end'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'End'}), backward: (elem) => pressKey(elem, {key: 'Home'})}} | ${parseColor('#f000e0')} + ${'shiftleft/shiftright'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowRight', shiftKey: true}), backward: (elem) => pressKey(elem, {key: 'ArrowLeft', shiftKey: true})}} | ${parseColor('#f000df')} + ${'shiftup/shiftdown'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowUp', shiftKey: true}), backward: (elem) => pressKey(elem, {key: 'ArrowDown', shiftKey: true})}} | ${parseColor('#f011f0')} + ${'pageup/pagedown'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'PageUp'}), backward: (elem) => pressKey(elem, {key: 'PageDown'})}} | ${parseColor('#f011f0')} + ${'home/end'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'End'}), backward: (elem) => pressKey(elem, {key: 'Home'})}} | ${parseColor('#f000df')} `('$Name RTL', ({props, actions: {forward, backward}, result}) => { let {getAllByRole} = render( @@ -213,35 +213,6 @@ describe('ColorArea', () => { expect(onChangeSpy).not.toHaveBeenCalled(); expect(onChangeEndSpy).not.toHaveBeenCalled(); }); - - it.each` - Name | props | actions | result - ${'left/right'} | ${{defaultValue: parseColor('#ff00ff')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowLeft'}), backward: (elem) => pressKey(elem, {key: 'ArrowRight'})}} | ${parseColor('#ff00f0')} - ${'up/down'} | ${{defaultValue: parseColor('#ff00ff')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowUp'}), backward: (elem) => pressKey(elem, {key: 'ArrowDown'})}} | ${parseColor('#ff0fff')} - `('$Name with step', ({props, actions: {forward, backward}, result}) => { - let {getAllByRole} = render( - - ); - let sliders = getAllByRole('slider'); - userEvent.tab(); - - forward(sliders[0]); - expect(onChangeSpy).toHaveBeenCalledTimes(1); - expect(onChangeSpy.mock.calls[0][0].toString('rgba')).toBe(result.toString('rgba')); - expect(onChangeEndSpy).toHaveBeenCalledTimes(1); - expect(onChangeEndSpy.mock.calls[0][0].toString('rgba')).toBe(result.toString('rgba')); - - backward(sliders[0]); - expect(onChangeSpy).toHaveBeenCalledTimes(2); - expect(onChangeSpy.mock.calls[1][0].toString('rgba')).toBe(props.defaultValue.toString('rgba')); - expect(onChangeEndSpy).toHaveBeenCalledTimes(2); - expect(onChangeEndSpy.mock.calls[1][0].toString('rgba')).toBe(props.defaultValue.toString('rgba')); - }); }); describe.each` @@ -351,34 +322,6 @@ describe('ColorArea', () => { expect(onChangeEndSpy).toHaveBeenCalledTimes(0); }); - // TODO: Should it? - it('dragging the thumb respects the step', () => { - let defaultColor = parseColor('#ff00ff'); - let {getAllByRole} = render( - - ); - let sliders = getAllByRole('slider'); - let groups = getAllByRole('group'); - let thumb = sliders[0].parentElement; - let container = groups[groupIndex]; - container.getBoundingClientRect = getBoundingClientRect; - - start(thumb, {pageX: CENTER + THUMB_RADIUS, pageY: CENTER}); - expect(onChangeSpy).toHaveBeenCalledTimes(0); - - move(thumb, {pageX: CENTER, pageY: CENTER + THUMB_RADIUS}); - expect(onChangeSpy).toHaveBeenCalledTimes(1); - expect(onChangeSpy.mock.calls[0][0].toString('rgba')).toBe(parseColor('#ff0090').toString('rgba')); - - end(thumb, {pageX: CENTER, pageY: CENTER + THUMB_RADIUS}); - expect(onChangeSpy).toHaveBeenCalledTimes(1); - }); - it('clicking and dragging on the track works', () => { let defaultColor = parseColor('#ff00ff'); let {getAllByRole} = render( @@ -438,31 +381,6 @@ describe('ColorArea', () => { expect(onChangeSpy).toHaveBeenCalledTimes(0); expect(document.activeElement).not.toBe(sliders[0]); }); - - it('clicking and dragging on the track respects the step', () => { - let defaultColor = parseColor('#ff00ff'); - let {getAllByRole} = render( - - ); - let groups = getAllByRole('group'); - let container = groups[groupIndex]; - container.getBoundingClientRect = getBoundingClientRect; - - start(container, {pageX: CENTER + THUMB_RADIUS, pageY: CENTER}); - expect(onChangeSpy).toHaveBeenCalledTimes(1); - - move(container, {pageX: CENTER, pageY: CENTER + THUMB_RADIUS}); - expect(onChangeSpy).toHaveBeenCalledTimes(2); - expect(onChangeSpy.mock.calls[0][0].toString('rgba')).toBe(parseColor('#ff80f0').toString('rgba')); - - end(container, {pageX: CENTER, pageY: CENTER + THUMB_RADIUS}); - expect(onChangeSpy).toHaveBeenCalledTimes(2); - }); }); }); }); diff --git a/packages/@react-spectrum/color/test/ColorWheel.test.tsx b/packages/@react-spectrum/color/test/ColorWheel.test.tsx index abf317f6766..6bda989d83c 100644 --- a/packages/@react-spectrum/color/test/ColorWheel.test.tsx +++ b/packages/@react-spectrum/color/test/ColorWheel.test.tsx @@ -158,24 +158,6 @@ describe('ColorWheel', () => { expect(onChangeSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 359).toString('hsla')); }); - it('respects step', () => { - let defaultColor = parseColor('hsl(0, 100%, 50%)'); - let {getByRole} = render(); - let slider = getByRole('slider'); - act(() => {slider.focus();}); - - fireEvent.keyDown(slider, {key: 'Right'}); - expect(onChangeSpy).toHaveBeenCalledTimes(1); - expect(onChangeSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 45).toString('hsla')); - expect(onChangeEndSpy).toHaveBeenCalledTimes(1); - expect(onChangeEndSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 45).toString('hsla')); - fireEvent.keyDown(slider, {key: 'Left'}); - expect(onChangeSpy).toHaveBeenCalledTimes(2); - expect(onChangeSpy.mock.calls[1][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 0).toString('hsla')); - expect(onChangeEndSpy).toHaveBeenCalledTimes(2); - expect(onChangeEndSpy.mock.calls[1][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 0).toString('hsla')); - }); - it('respects page steps', () => { let defaultColor = parseColor('hsl(0, 100%, 50%)'); let {getByRole} = render(); @@ -185,9 +167,9 @@ describe('ColorWheel', () => { fireEvent.keyDown(slider, {key: 'PageUp'}); fireEvent.keyUp(slider, {key: 'PageUp'}); expect(onChangeSpy).toHaveBeenCalledTimes(1); - expect(onChangeSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 6).toString('hsla')); + expect(onChangeSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 15).toString('hsla')); expect(onChangeEndSpy).toHaveBeenCalledTimes(1); - expect(onChangeEndSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 6).toString('hsla')); + expect(onChangeEndSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 15).toString('hsla')); fireEvent.keyDown(slider, {key: 'PageDown'}); fireEvent.keyUp(slider, {key: 'PageDown'}); expect(onChangeSpy).toHaveBeenCalledTimes(2); @@ -205,9 +187,9 @@ describe('ColorWheel', () => { fireEvent.keyDown(slider, {key: 'Right', shiftKey: true}); fireEvent.keyUp(slider, {key: 'Right', shiftKey: true}); expect(onChangeSpy).toHaveBeenCalledTimes(1); - expect(onChangeSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 6).toString('hsla')); + expect(onChangeSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 15).toString('hsla')); expect(onChangeEndSpy).toHaveBeenCalledTimes(1); - expect(onChangeEndSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 6).toString('hsla')); + expect(onChangeEndSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 15).toString('hsla')); fireEvent.keyDown(slider, {key: 'Left', shiftKey: true}); fireEvent.keyUp(slider, {key: 'Left', shiftKey: true}); expect(onChangeSpy).toHaveBeenCalledTimes(2); @@ -286,23 +268,6 @@ describe('ColorWheel', () => { expect(document.activeElement).not.toBe(slider); }); - it('dragging the thumb respects the step', () => { - let defaultColor = parseColor('hsl(0, 100%, 50%)'); - let {container: _container, getByRole} = render(); - let slider = getByRole('slider'); - let container = _container.firstChild.firstChild as HTMLElement; - let thumb = slider.parentElement; - container.getBoundingClientRect = getBoundingClientRect; - - start(thumb, {pageX: CENTER + THUMB_RADIUS, pageY: CENTER}); - expect(onChangeSpy).toHaveBeenCalledTimes(0); - move(thumb, {pageX: CENTER, pageY: CENTER + THUMB_RADIUS}); - expect(onChangeSpy).toHaveBeenCalledTimes(1); - expect(onChangeSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 120).toString('hsla')); - end(thumb, {pageX: CENTER, pageY: CENTER + THUMB_RADIUS}); - expect(onChangeSpy).toHaveBeenCalledTimes(1); - }); - it('clicking and dragging on the track works', () => { let defaultColor = parseColor('hsl(0, 100%, 50%)'); let {container: _container, getByRole} = render(); @@ -348,24 +313,6 @@ describe('ColorWheel', () => { expect(document.activeElement).not.toBe(slider); }); - it('clicking and dragging on the track respects the step', () => { - let defaultColor = parseColor('hsl(0, 100%, 50%)'); - let {container: _container, getByRole} = render(); - let slider = getByRole('slider'); - let thumb = slider.parentElement; - let container = _container.firstChild.firstChild as HTMLElement; - container.getBoundingClientRect = getBoundingClientRect; - - start(container, {pageX: CENTER, pageY: CENTER + THUMB_RADIUS}); - expect(onChangeSpy).toHaveBeenCalledTimes(1); - expect(onChangeSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 120).toString('hsla')); - move(thumb, {pageX: CENTER, pageY: CENTER - THUMB_RADIUS}); - expect(onChangeSpy).toHaveBeenCalledTimes(2); - expect(onChangeSpy.mock.calls[1][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 240).toString('hsla')); - end(thumb, {pageX: CENTER, pageY: CENTER - THUMB_RADIUS}); - expect(onChangeSpy).toHaveBeenCalledTimes(2); - }); - it('clicking on the track works', () => { let defaultColor = parseColor('hsl(0, 100%, 50%)'); let {container: _container, getByRole} = render(); diff --git a/packages/@react-stately/color/src/Color.ts b/packages/@react-stately/color/src/Color.ts index cf01ce86fff..eca9ac896ff 100644 --- a/packages/@react-stately/color/src/Color.ts +++ b/packages/@react-stately/color/src/Color.ts @@ -237,7 +237,7 @@ class RGBColor extends Color { case 'red': case 'green': case 'blue': - return {minValue: 0, maxValue: 255, step: 1, pageSize: 0x10}; + return {minValue: 0x0, maxValue: 0xFF, step: 0x1, pageSize: 0x11}; case 'alpha': return {minValue: 0, maxValue: 1, step: 0.01, pageSize: 0.1}; default: diff --git a/packages/@react-stately/color/src/useColorAreaState.ts b/packages/@react-stately/color/src/useColorAreaState.ts index 1a82f98ea73..e5484e438f4 100644 --- a/packages/@react-stately/color/src/useColorAreaState.ts +++ b/packages/@react-stately/color/src/useColorAreaState.ts @@ -72,8 +72,14 @@ let difference = (a: Set, b: Set): Set => new Set([...a].filter(x => * Color area allows users to adjust two channels of an HSL, HSB or RGB color value against a two-dimensional gradient background. */ export function useColorAreaState(props: ColorAreaProps): ColorAreaState { - // TODO: docs say the step props should be one, but should it be two different values? - let {value, defaultValue, xChannel, yChannel, onChange, onChangeEnd, xChannelStep, yChannelStep} = props; + let { + value, + defaultValue, + xChannel, + yChannel, + onChange, + onChangeEnd + } = props; if (!value && !defaultValue) { defaultValue = DEFAULT_COLOR; @@ -121,17 +127,6 @@ export function useColorAreaState(props: ColorAreaProps): ColorAreaState { let {minValue: minValueX, maxValue: maxValueX, step: stepX, pageSize: pageSizeX} = xChannelRange; let {minValue: minValueY, maxValue: maxValueY, step: stepY, pageSize: pageSizeY} = yChannelRange; - if (isNaN(xChannelStep)) { - xChannelStep = stepX; - } - - if (isNaN(yChannelStep)) { - yChannelStep = stepY; - } - - let xChannelPageStep = Math.max(pageSizeX, xChannelStep); - let yChannelPageStep = Math.max(pageSizeY, yChannelStep); - let [isDragging, setDragging] = useState(false); let isDraggingRef = useRef(false).current; @@ -154,10 +149,10 @@ export function useColorAreaState(props: ColorAreaProps): ColorAreaState { return { channels, - xChannelStep, - yChannelStep, - xChannelPageStep, - yChannelPageStep, + xChannelStep: stepX, + yChannelStep: stepY, + xChannelPageStep: pageSizeX, + yChannelPageStep: pageSizeY, value: color, setValue(value) { let c = normalizeColor(value); @@ -176,12 +171,12 @@ export function useColorAreaState(props: ColorAreaProps): ColorAreaState { let newColor:Color; if (newXValue !== xValue) { // Round new value to multiple of step, clamp value between min and max - newXValue = snapValueToStep(newXValue, minValueX, maxValueX, xChannelStep); + newXValue = snapValueToStep(newXValue, minValueX, maxValueX, stepX); newColor = color.withChannelValue(channels.xChannel, newXValue); } if (newYValue !== yValue) { // Round new value to multiple of step, clamp value between min and max - newYValue = snapValueToStep(newYValue, minValueY, maxValueY, yChannelStep); + newYValue = snapValueToStep(newYValue, minValueY, maxValueY, stepY); newColor = (newColor || color).withChannelValue(channels.yChannel, newYValue); } if (newColor) { @@ -194,16 +189,16 @@ export function useColorAreaState(props: ColorAreaProps): ColorAreaState { return {x, y}; }, incrementX(stepSize) { - setXValue(snapValueToStep(xValue + stepSize, minValueX, maxValueX, stepSize)); + setXValue(xValue + stepSize > maxValueX ? maxValueX : snapValueToStep(xValue + stepSize, minValueX, maxValueX, stepX)); }, incrementY(stepSize) { - setYValue(snapValueToStep(yValue + stepSize, minValueY, maxValueY, stepSize)); + setYValue(yValue + stepSize > maxValueY ? maxValueY : snapValueToStep(yValue + stepSize, minValueY, maxValueY, stepY)); }, decrementX(stepSize) { - setXValue(snapValueToStep(xValue - stepSize, minValueX, maxValueX, stepSize)); + setXValue(snapValueToStep(xValue - stepSize, minValueX, maxValueX, stepX)); }, decrementY(stepSize) { - setYValue(snapValueToStep(yValue - stepSize, minValueY, maxValueY, stepSize)); + setYValue(snapValueToStep(yValue - stepSize, minValueY, maxValueY, stepY)); }, setDragging(isDragging) { let wasDragging = isDraggingRef; diff --git a/packages/@react-stately/color/src/useColorWheelState.ts b/packages/@react-stately/color/src/useColorWheelState.ts index a43167f58c8..a096f352117 100644 --- a/packages/@react-stately/color/src/useColorWheelState.ts +++ b/packages/@react-stately/color/src/useColorWheelState.ts @@ -85,14 +85,13 @@ function cartesianToAngle(x: number, y: number, radius: number): number { let deg = radToDeg(Math.atan2(y / radius, x / radius)); return (deg + 360) % 360; } -const PAGE_MIN_STEP_SIZE = 6; /** * Provides state management for a color wheel component. * Color wheels allow users to adjust the hue of an HSL or HSB color value on a circular track. */ export function useColorWheelState(props: ColorWheelProps): ColorWheelState { - let {defaultValue, onChange, onChangeEnd, step = 1} = props; + let {defaultValue, onChange, onChangeEnd} = props; if (!props.value && !defaultValue) { defaultValue = DEFAULT_COLOR; @@ -119,7 +118,9 @@ export function useColorWheelState(props: ColorWheelProps): ColorWheelState { } } - let pageStep = PAGE_MIN_STEP_SIZE; + let channelRange = value.getChannelRange('hue'); + let {minValue: minValueX, maxValue: maxValueX, step: step, pageSize: pageStep} = channelRange; + return { value, step, @@ -139,9 +140,9 @@ export function useColorWheelState(props: ColorWheelProps): ColorWheelState { }, increment(stepSize) { let newValue = hue + Math.max(stepSize, step); - if (newValue > 360) { + if (newValue > maxValueX) { // Make sure you can always get back to 0. - newValue = 0; + newValue = minValueX; } setHue(newValue); }, diff --git a/packages/@react-types/color/src/index.d.ts b/packages/@react-types/color/src/index.d.ts index 9ee60dec806..77ff433c084 100644 --- a/packages/@react-types/color/src/index.d.ts +++ b/packages/@react-types/color/src/index.d.ts @@ -102,11 +102,6 @@ export interface ColorWheelProps extends ValueBase { onChange?: (value: Color) => void, /** Handler that is called when the user stops dragging. */ onChangeEnd?: (value: Color) => void, - /** - * The ColorWheel's step value. - * @default 1 - */ - step?: number, /** * The default value (uncontrolled). * @default 'hsl(0, 100%, 50%)' @@ -147,11 +142,7 @@ export interface ColorAreaProps extends ValueBase { /** Handler that is called when the value changes, as the user drags. */ onChange?: (value: Color) => void, /** Handler that is called when the user stops dragging. */ - onChangeEnd?: (value: Color) => void, - /** The step value for the xChannel. */ - xChannelStep?: number, - /** The step value for the yChannel. */ - yChannelStep?: number + onChangeEnd?: (value: Color) => void } export interface AriaColorAreaProps extends ColorAreaProps, DOMProps, AriaLabelingProps {}