Skip to content

Commit 463837d

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 cc238b6 commit 463837d

File tree

3 files changed

+94
-5
lines changed

3 files changed

+94
-5
lines changed

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

Lines changed: 40 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,43 @@ export function useColorSlider(props: ColorSliderAriaOptions, state: ColorSlider
6162
inputRef
6263
}, state);
6364

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

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ XBlueYRed.storyName = 'RGB xChannel="blue", yChannel="red"';
8484
XBlueYRed.args = {...XBlueYGreen.args, xChannel: 'blue', yChannel: 'red'};
8585

8686
export let XRedYBlue = Template.bind({});
87-
XRedYBlue.storyName = 'GB xChannel="red", yChannel="blue"';
87+
XRedYBlue.storyName = 'RGB xChannel="red", yChannel="blue"';
8888
XRedYBlue.args = {...XBlueYGreen.args, xChannel: 'red', yChannel: 'blue'};
8989

9090
export let XRedYGreen = Template.bind({});

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

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,20 @@ import userEvent from '@testing-library/user-event';
1919

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

2324
beforeAll(() => {
2425
jest.spyOn(window.HTMLElement.prototype, 'offsetWidth', 'get').mockImplementation(() => 100);
2526
jest.spyOn(window.HTMLElement.prototype, 'offsetHeight', 'get').mockImplementation(() => 100);
2627
// @ts-ignore
27-
jest.spyOn(window, 'requestAnimationFrame').mockImplementation((cb) => cb());
28+
jest.spyOn(window, 'requestAnimationFrame').mockImplementation((cb) => setTimeout(cb, 0));
2829
// @ts-ignore
2930
jest.useFakeTimers('legacy');
3031
});
3132

3233
afterEach(() => {
3334
onChangeSpy.mockClear();
35+
onChangeEndSpy.mockClear();
3436
// for restoreTextSelection
3537
act(() => {jest.runAllTimers();});
3638
});
@@ -264,16 +266,65 @@ describe('ColorSlider', () => {
264266
describe('keyboard events', () => {
265267
it('works', () => {
266268
let defaultColor = parseColor('#000000');
267-
let {getByRole} = render(<ColorSlider defaultValue={defaultColor} onChange={onChangeSpy} channel="red" />);
269+
let {getByRole} = render(<ColorSlider defaultValue={defaultColor} onChange={onChangeSpy} onChangeEnd={onChangeEndSpy} channel="red" />);
268270
let slider = getByRole('slider');
269271
act(() => {slider.focus();});
270272

271273
fireEvent.keyDown(slider, {key: 'Right'});
274+
act(() => {jest.runAllTimers();});
272275
expect(onChangeSpy).toHaveBeenCalledTimes(1);
273276
expect(onChangeSpy.mock.calls[0][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 1).toString('hexa'));
277+
expect(onChangeEndSpy).toHaveBeenCalledTimes(1);
278+
expect(onChangeEndSpy.mock.calls[0][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 1).toString('hexa'));
279+
274280
fireEvent.keyDown(slider, {key: 'Left'});
281+
act(() => {jest.runAllTimers();});
275282
expect(onChangeSpy).toHaveBeenCalledTimes(2);
276283
expect(onChangeSpy.mock.calls[1][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 0).toString('hexa'));
284+
expect(onChangeEndSpy).toHaveBeenCalledTimes(2);
285+
expect(onChangeEndSpy.mock.calls[1][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 0).toString('hexa'));
286+
287+
fireEvent.keyDown(slider, {key: 'PageUp'});
288+
act(() => {jest.runAllTimers();});
289+
expect(onChangeSpy).toHaveBeenCalledTimes(3);
290+
expect(onChangeSpy.mock.calls[2][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 16).toString('hexa'));
291+
expect(onChangeEndSpy).toHaveBeenCalledTimes(3);
292+
expect(onChangeEndSpy.mock.calls[2][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 16).toString('hexa'));
293+
294+
fireEvent.keyDown(slider, {key: 'Right'});
295+
act(() => {jest.runAllTimers();});
296+
expect(onChangeSpy).toHaveBeenCalledTimes(4);
297+
expect(onChangeSpy.mock.calls[3][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 17).toString('hexa'));
298+
expect(onChangeEndSpy).toHaveBeenCalledTimes(4);
299+
expect(onChangeEndSpy.mock.calls[3][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 17).toString('hexa'));
300+
301+
fireEvent.keyDown(slider, {key: 'PageDown'});
302+
act(() => {jest.runAllTimers();});
303+
expect(onChangeSpy).toHaveBeenCalledTimes(5);
304+
expect(onChangeSpy.mock.calls[4][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 0).toString('hexa'));
305+
expect(onChangeEndSpy).toHaveBeenCalledTimes(5);
306+
expect(onChangeEndSpy.mock.calls[4][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 0).toString('hexa'));
307+
308+
fireEvent.keyDown(slider, {key: 'End'});
309+
act(() => {jest.runAllTimers();});
310+
expect(onChangeSpy).toHaveBeenCalledTimes(6);
311+
expect(onChangeSpy.mock.calls[5][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 255).toString('hexa'));
312+
expect(onChangeEndSpy).toHaveBeenCalledTimes(6);
313+
expect(onChangeEndSpy.mock.calls[5][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 255).toString('hexa'));
314+
315+
fireEvent.keyDown(slider, {key: 'PageDown'});
316+
act(() => {jest.runAllTimers();});
317+
expect(onChangeSpy).toHaveBeenCalledTimes(7);
318+
expect(onChangeSpy.mock.calls[6][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 240).toString('hexa'));
319+
expect(onChangeEndSpy).toHaveBeenCalledTimes(7);
320+
expect(onChangeEndSpy.mock.calls[6][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 240).toString('hexa'));
321+
322+
fireEvent.keyDown(slider, {key: 'Home'});
323+
act(() => {jest.runAllTimers();});
324+
expect(onChangeSpy).toHaveBeenCalledTimes(8);
325+
expect(onChangeSpy.mock.calls[7][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 0).toString('hexa'));
326+
expect(onChangeEndSpy).toHaveBeenCalledTimes(8);
327+
expect(onChangeEndSpy.mock.calls[7][0].toString('hexa')).toBe(defaultColor.withChannelValue('red', 0).toString('hexa'));
277328
});
278329

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

0 commit comments

Comments
 (0)