Skip to content

Commit 61a13d3

Browse files
author
Michael Jordan
committed
fix(#2664): useColorSlider: refine PageUp/PageDown behaviors
- Omit change events when value doesn't change. - Refine snapValueToStep for incrementing or decrementing from max.
1 parent 04e93b8 commit 61a13d3

File tree

3 files changed

+128
-19
lines changed

3 files changed

+128
-19
lines changed

‎packages/@react-aria/color/src/useColorSlider.ts‎

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@
1313
import {AriaColorSliderProps} from '@react-types/color';
1414
import {ColorSliderState} from '@react-stately/color';
1515
import {HTMLAttributes, RefObject} from 'react';
16-
import {mergeProps} from '@react-aria/utils';
16+
import {mergeProps, snapValueToStep} from '@react-aria/utils';
17+
import {useKeyboard} from '@react-aria/interactions';
1718
import {useLocale} from '@react-aria/i18n';
1819
import {useSlider, useSliderThumb} from '@react-aria/slider';
1920

@@ -61,6 +62,44 @@ export function useColorSlider(props: ColorSliderAriaOptions, state: ColorSlider
6162
inputRef
6263
}, state);
6364

65+
let {minValue, maxValue, step, pageSize} = state.value.getChannelRange(channel);
66+
67+
let {keyboardProps} = useKeyboard({
68+
onKeyDown(e) {
69+
// these are the cases that useMove or useSlider don't handle
70+
if (!/^(PageUp|PageDown|Home|End)$/.test(e.key)) {
71+
e.continuePropagation();
72+
return;
73+
}
74+
// same handling as useMove, stopPropagation to prevent useSlider from handling the event as well.
75+
e.preventDefault();
76+
let channelValue = state.value.getChannelValue(channel);
77+
let pageStep = Math.max(pageSize, step);
78+
let newValue = channelValue;
79+
switch (e.key) {
80+
case 'PageUp':
81+
newValue = channelValue + pageStep > maxValue ? maxValue : snapValueToStep(channelValue + pageStep, minValue, maxValue, pageStep);
82+
break;
83+
case 'PageDown':
84+
newValue = snapValueToStep(channelValue - pageStep, minValue, maxValue, pageStep);
85+
break;
86+
case 'Home':
87+
newValue = minValue;
88+
break;
89+
case 'End':
90+
newValue = maxValue;
91+
break;
92+
}
93+
if (newValue !== channelValue) {
94+
// remember to set this so that onChangeEnd is fired
95+
state.setThumbDragging(0, true);
96+
state.setValue(state.value.withChannelValue(channel, newValue));
97+
// wait a frame to ensure value has changed then unset this so that onChangeEnd is fired
98+
setTimeout(() => state.setThumbDragging(0, false));
99+
}
100+
}
101+
});
102+
64103
let generateBackground = () => {
65104
let value = state.getDisplayColor();
66105
let to: string;
@@ -113,7 +152,7 @@ export function useColorSlider(props: ColorSliderAriaOptions, state: ColorSlider
113152
background: generateBackground()
114153
}
115154
},
116-
inputProps,
155+
inputProps: mergeProps(inputProps, keyboardProps),
117156
thumbProps: {
118157
...thumbProps,
119158
style: {

‎packages/@react-spectrum/color/stories/ColorArea.stories.tsx‎

Lines changed: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,12 @@
1111
*/
1212

1313
import {action} from '@storybook/addon-actions';
14-
import {ColorArea, ColorSlider} from '../';
14+
import {ColorArea, ColorField, ColorSlider} from '../';
1515
import {ColorChannel, SpectrumColorAreaProps} from '@react-types/color';
1616
import {Flex} from '@adobe/react-spectrum';
1717
import {Meta, Story} from '@storybook/react';
1818
import {parseColor} from '@react-stately/color';
19-
import React, {useState} from 'react';
19+
import React, {useRef, useState} from 'react';
2020
import {Text} from '@react-spectrum/text';
2121

2222

@@ -39,8 +39,9 @@ function ColorAreaExample(props: SpectrumColorAreaProps) {
3939
let channels = new Set([xChannel, yChannel]);
4040
let zChannel: ColorChannel = difference(RGB, channels).keys().next().value;
4141
let [color, setColor] = useState(props.defaultValue || parseColor('#ff00ff'));
42+
let colorFieldRef = useRef(null);
4243
return (<div role="group" aria-label="RGB Color Picker">
43-
<Flex gap="size-500" alignItems="center">
44+
<Flex gap="size-200" alignItems="start">
4445
<Flex direction="column" gap="size-50" alignItems="center">
4546
<ColorArea
4647
{...props}
@@ -50,7 +51,8 @@ function ColorAreaExample(props: SpectrumColorAreaProps) {
5051
props.onChange(e);
5152
}
5253
setColor(e);
53-
}} />
54+
}}
55+
onChangeEnd={props.onChangeEnd} />
5456
<ColorSlider
5557
value={color}
5658
onChange={(e) => {
@@ -63,9 +65,26 @@ function ColorAreaExample(props: SpectrumColorAreaProps) {
6365
channel={zChannel}
6466
isDisabled={isDisabled} />
6567
</Flex>
66-
<Flex direction="column" alignItems="center" gap="size-100" minWidth={'size-2000'}>
68+
<Flex direction="column" alignItems="center" gap="size-200" minWidth={'size-2000'}>
6769
<div role="img" aria-label={`color swatch: ${color.toString('rgb')}`} title={`${color.toString('hex')}`} style={{width: '100px', height: '100px', background: color.toString('css')}} />
68-
<Text>{color.toString('hex')}</Text>
70+
<ColorField
71+
ref={colorFieldRef}
72+
aria-label="RGB Color"
73+
value={color}
74+
onChange={(e) => {
75+
if (e && props.onChange) {
76+
props.onChange(e);
77+
}
78+
setColor(e || color);
79+
}}
80+
onKeyDown={e => {
81+
let newColor = parseColor((e.target as HTMLInputElement).value);
82+
if (e.key === 'Enter' && newColor) {
83+
setColor(newColor);
84+
}
85+
}}
86+
isDisabled={isDisabled}
87+
width="100px" />
6988
</Flex>
7089
</Flex>
7190
</div>);
@@ -77,42 +96,42 @@ XBlueYGreen.args = {xChannel: 'blue', yChannel: 'green', onChange: action('onCha
7796

7897
export let XGreenYBlue = Template.bind({});
7998
XGreenYBlue.storyName = 'RGB xChannel="green", yChannel="blue"';
80-
XGreenYBlue.args = {xChannel: 'green', yChannel: 'blue', onChange: action('onChange')};
99+
XGreenYBlue.args = {xChannel: 'green', yChannel: 'blue', onChange: action('onChange'), onChangeEnd: action('onChangeEnd')};
81100

82101
export let XBlueYRed = Template.bind({});
83102
XBlueYRed.storyName = 'RGB xChannel="blue", yChannel="red"';
84-
XBlueYRed.args = {xChannel: 'blue', yChannel: 'red', onChange: action('onChange')};
103+
XBlueYRed.args = {xChannel: 'blue', yChannel: 'red', onChange: action('onChange'), onChangeEnd: action('onChangeEnd')};
85104

86105
export let XRedYBlue = Template.bind({});
87106
XRedYBlue.storyName = 'GB xChannel="red", yChannel="blue"';
88-
XRedYBlue.args = {xChannel: 'red', yChannel: 'blue', onChange: action('onChange')};
107+
XRedYBlue.args = {xChannel: 'red', yChannel: 'blue', onChange: action('onChange'), onChangeEnd: action('onChangeEnd')};
89108

90109
export let XRedYGreen = Template.bind({});
91110
XRedYGreen.storyName = 'RGB xChannel="red", yChannel="green"';
92-
XRedYGreen.args = {xChannel: 'red', yChannel: 'green', onChange: action('onChange')};
111+
XRedYGreen.args = {xChannel: 'red', yChannel: 'green', onChange: action('onChange'), onChangeEnd: action('onChangeEnd')};
93112

94113
export let XGreenYRed = Template.bind({});
95114
XGreenYRed.storyName = 'RGB xChannel="green", yChannel="red"';
96-
XGreenYRed.args = {xChannel: 'green', yChannel: 'red', onChange: action('onChange')};
115+
XGreenYRed.args = {xChannel: 'green', yChannel: 'red', onChange: action('onChange'), onChangeEnd: action('onChangeEnd')};
97116

98117
export let XBlueYGreenStep16 = Template.bind({});
99118
XBlueYGreenStep16.storyName = 'RGB xChannel="blue", yChannel="green", step="16"';
100-
XBlueYGreenStep16.args = {...XBlueYGreen.args, xChannelStep: 16, yChannelStep: 16};
119+
XBlueYGreenStep16.args = {...XBlueYGreen.args, xChannelStep: 16, yChannelStep: 16, onChange: action('onChange'), onChangeEnd: action('onChangeEnd')};
101120

102121
/* TODO: what does a disabled color area look like? */
103122
export let XBlueYGreenisDisabled = Template.bind({});
104123
XBlueYGreenisDisabled.storyName = 'RGB xChannel="blue", yChannel="green", isDisabled';
105-
XBlueYGreenisDisabled.args = {...XBlueYGreen.args, isDisabled: true};
124+
XBlueYGreenisDisabled.args = {...XBlueYGreen.args, isDisabled: true, onChange: action('onChange'), onChangeEnd: action('onChangeEnd')};
106125

107126
/* TODO: how do we visually label and how to do we aria-label */
108127
export let XBlueYGreenAriaLabelled = Template.bind({});
109128
XBlueYGreenAriaLabelled.storyName = 'RGB xChannel="blue", yChannel="green", aria-label="foo"';
110-
XBlueYGreenAriaLabelled.args = {...XBlueYGreen.args, label: undefined, ariaLabel: 'foo'};
129+
XBlueYGreenAriaLabelled.args = {...XBlueYGreen.args, label: undefined, ariaLabel: 'foo', onChange: action('onChange'), onChangeEnd: action('onChangeEnd')};
111130

112131
export let XBlueYGreenSize3000 = Template.bind({});
113132
XBlueYGreenSize3000.storyName = 'RGB xChannel="blue", yChannel="green", size="size-3000"';
114-
XBlueYGreenSize3000.args = {...XBlueYGreen.args, size: 'size-3000'};
133+
XBlueYGreenSize3000.args = {...XBlueYGreen.args, size: 'size-3000', onChange: action('onChange'), onChangeEnd: action('onChangeEnd')};
115134

116135
export let XBlueYGreenSize600 = Template.bind({});
117136
XBlueYGreenSize600.storyName = 'RGB xChannel="blue", yChannel="green", size="size-600"';
118-
XBlueYGreenSize600.args = {...XBlueYGreen.args, size: 'size-600'};
137+
XBlueYGreenSize600.args = {...XBlueYGreen.args, size: 'size-600', onChange: action('onChange'), onChangeEnd: action('onChangeEnd')};

‎packages/@react-spectrum/color/test/ColorSlider.test.tsx‎

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,11 @@ import userEvent from '@testing-library/user-event';
1919

2020
describe('ColorSlider', () => {
2121
let onChangeSpy = jest.fn();
22+
let onChangeEndSpy = jest.fn();
2223

2324
afterEach(() => {
2425
onChangeSpy.mockClear();
26+
onChangeEndSpy.mockClear();
2527
});
2628

2729
beforeAll(() => {
@@ -275,16 +277,65 @@ describe('ColorSlider', () => {
275277
describe('keyboard events', () => {
276278
it('works', () => {
277279
let defaultColor = parseColor('#000000');
278-
let {getByRole} = render(<ColorSlider defaultValue={defaultColor} onChange={onChangeSpy} channel="red" />);
280+
let {getByRole} = render(<ColorSlider defaultValue={defaultColor} onChange={onChangeSpy} onChangeEnd={onChangeEndSpy} channel="red" />);
279281
let slider = getByRole('slider');
280282
act(() => {slider.focus();});
281283

282284
fireEvent.keyDown(slider, {key: 'Right'});
285+
act(() => jest.runAllTimers());
283286
expect(onChangeSpy).toHaveBeenCalledTimes(1);
284287
expect(onChangeSpy.mock.calls[0][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 1).toString('hexa'));
288+
expect(onChangeEndSpy).toHaveBeenCalledTimes(1);
289+
expect(onChangeEndSpy.mock.calls[0][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 1).toString('hexa'));
290+
285291
fireEvent.keyDown(slider, {key: 'Left'});
292+
act(() => jest.runAllTimers());
286293
expect(onChangeSpy).toHaveBeenCalledTimes(2);
287294
expect(onChangeSpy.mock.calls[1][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 0).toString('hexa'));
295+
expect(onChangeEndSpy).toHaveBeenCalledTimes(2);
296+
expect(onChangeEndSpy.mock.calls[1][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 0).toString('hexa'));
297+
298+
fireEvent.keyDown(slider, {key: 'PageUp'});
299+
act(() => jest.runAllTimers());
300+
expect(onChangeSpy).toHaveBeenCalledTimes(3);
301+
expect(onChangeSpy.mock.calls[2][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 16).toString('hexa'));
302+
expect(onChangeEndSpy).toHaveBeenCalledTimes(3);
303+
expect(onChangeEndSpy.mock.calls[2][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 16).toString('hexa'));
304+
305+
fireEvent.keyDown(slider, {key: 'Right'});
306+
act(() => jest.runAllTimers());
307+
expect(onChangeSpy).toHaveBeenCalledTimes(4);
308+
expect(onChangeSpy.mock.calls[3][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 17).toString('hexa'));
309+
expect(onChangeEndSpy).toHaveBeenCalledTimes(4);
310+
expect(onChangeEndSpy.mock.calls[3][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 17).toString('hexa'));
311+
312+
fireEvent.keyDown(slider, {key: 'PageDown'});
313+
act(() => jest.runAllTimers());
314+
expect(onChangeSpy).toHaveBeenCalledTimes(5);
315+
expect(onChangeSpy.mock.calls[4][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 0).toString('hexa'));
316+
expect(onChangeEndSpy).toHaveBeenCalledTimes(5);
317+
expect(onChangeEndSpy.mock.calls[4][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 0).toString('hexa'));
318+
319+
fireEvent.keyDown(slider, {key: 'End'});
320+
act(() => jest.runAllTimers());
321+
expect(onChangeSpy).toHaveBeenCalledTimes(6);
322+
expect(onChangeSpy.mock.calls[5][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 255).toString('hexa'));
323+
expect(onChangeEndSpy).toHaveBeenCalledTimes(6);
324+
expect(onChangeEndSpy.mock.calls[5][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 255).toString('hexa'));
325+
326+
fireEvent.keyDown(slider, {key: 'PageDown'});
327+
act(() => jest.runAllTimers());
328+
expect(onChangeSpy).toHaveBeenCalledTimes(7);
329+
expect(onChangeSpy.mock.calls[6][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 240).toString('hexa'));
330+
expect(onChangeEndSpy).toHaveBeenCalledTimes(7);
331+
expect(onChangeEndSpy.mock.calls[6][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 240).toString('hexa'));
332+
333+
fireEvent.keyDown(slider, {key: 'Home'});
334+
act(() => jest.runAllTimers());
335+
expect(onChangeSpy).toHaveBeenCalledTimes(8);
336+
expect(onChangeSpy.mock.calls[7][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 0).toString('hexa'));
337+
expect(onChangeEndSpy).toHaveBeenCalledTimes(8);
338+
expect(onChangeEndSpy.mock.calls[7][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 0).toString('hexa'));
288339
});
289340

290341
it('doesn\'t work when disabled', () => {

0 commit comments

Comments
 (0)