Skip to content

Commit 1c7aa38

Browse files
snowystingersnowystinger
authored
ColorArea and ColorWheel step and pagestep size fit evenly in range (#2888)
Remove step and page step from ColorArea and ColorWheel API Use step and page step defined in color space, make sure the evenly fit into the color space
1 parent dbb1ad2 commit 1c7aa38

File tree

10 files changed

+42
-273
lines changed

10 files changed

+42
-273
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ interface ColorWheelAria {
4040
export function useColorWheel(props: ColorWheelAriaProps, state: ColorWheelState, inputRef: RefObject<HTMLElement>): ColorWheelAria {
4141
let {
4242
isDisabled,
43-
step = 1,
4443
innerRadius,
4544
outerRadius,
4645
'aria-label': ariaLabel
@@ -256,6 +255,7 @@ export function useColorWheel(props: ColorWheelAriaProps, state: ColorWheelState
256255
...props,
257256
'aria-label': ariaLabel
258257
});
258+
let step = stateRef.current.step;
259259

260260
return {
261261
trackProps: {

‎packages/@react-aria/color/test/useColorWheel.test.tsx

Lines changed: 3 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ function ColorWheel(props: ColorWheelProps) {
5151

5252
describe('useColorWheel', () => {
5353
let onChangeSpy = jest.fn();
54-
54+
5555
beforeAll(() => {
5656
// @ts-ignore
5757
jest.spyOn(window, 'requestAnimationFrame').mockImplementation((cb) => cb());
@@ -170,38 +170,6 @@ describe('useColorWheel', () => {
170170
expect(slider).toHaveAttribute('value', '359');
171171
});
172172

173-
it('respects step', () => {
174-
let defaultColor = parseColor('hsl(0, 100%, 50%)');
175-
let {getByRole} = render(<ColorWheel defaultValue={defaultColor} onChange={onChangeSpy} step={45} />);
176-
let slider = getByRole('slider');
177-
act(() => {slider.focus();});
178-
179-
fireEvent.keyDown(slider, {key: 'Right'});
180-
expect(onChangeSpy).toHaveBeenCalledTimes(1);
181-
expect(onChangeSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 45).toString('hsla'));
182-
expect(slider).toHaveAttribute('value', '45');
183-
fireEvent.keyDown(slider, {key: 'Left'});
184-
expect(onChangeSpy).toHaveBeenCalledTimes(2);
185-
expect(onChangeSpy.mock.calls[1][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 0).toString('hsla'));
186-
expect(slider).toHaveAttribute('value', '0');
187-
});
188-
189-
it('can always get back to 0 even with step', () => {
190-
let defaultColor = parseColor('hsl(330, 100%, 50%)');
191-
let {getByRole} = render(<ColorWheel defaultValue={defaultColor} onChange={onChangeSpy} step={110} />);
192-
let slider = getByRole('slider');
193-
act(() => {slider.focus();});
194-
195-
fireEvent.keyDown(slider, {key: 'Right'});
196-
expect(onChangeSpy).toHaveBeenCalledTimes(1);
197-
expect(onChangeSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 0).toString('hsla'));
198-
expect(slider).toHaveAttribute('value', '0');
199-
fireEvent.keyDown(slider, {key: 'Left'});
200-
expect(onChangeSpy).toHaveBeenCalledTimes(2);
201-
expect(onChangeSpy.mock.calls[1][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 330).toString('hsla'));
202-
expect(slider).toHaveAttribute('value', '330');
203-
});
204-
205173
it('steps with page up/down', () => {
206174
let defaultColor = parseColor('hsl(0, 100%, 50%)');
207175
let {getByRole} = render(<ColorWheel defaultValue={defaultColor} onChange={onChangeSpy} />);
@@ -210,8 +178,8 @@ describe('useColorWheel', () => {
210178

211179
fireEvent.keyDown(slider, {key: 'PageUp'});
212180
expect(onChangeSpy).toHaveBeenCalledTimes(1);
213-
expect(onChangeSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 6).toString('hsla'));
214-
expect(slider).toHaveAttribute('value', '6');
181+
expect(onChangeSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 15).toString('hsla'));
182+
expect(slider).toHaveAttribute('value', '15');
215183
fireEvent.keyDown(slider, {key: 'PageDown'});
216184
expect(onChangeSpy).toHaveBeenCalledTimes(2);
217185
expect(onChangeSpy.mock.calls[1][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 0).toString('hsla'));
@@ -285,24 +253,6 @@ describe('useColorWheel', () => {
285253
expect(document.activeElement).not.toBe(slider);
286254
});
287255

288-
it('dragging the thumb respects the step', () => {
289-
let defaultColor = parseColor('hsl(0, 100%, 50%)');
290-
let {getByRole, getByTestId} = render(<ColorWheel defaultValue={defaultColor} onChange={onChangeSpy} step={120} />);
291-
let thumb = getByTestId('thumb');
292-
let slider = getByRole('slider');
293-
let container = getByTestId('container');
294-
container.getBoundingClientRect = getBoundingClientRect;
295-
296-
start(thumb, {pageX: CENTER + THUMB_RADIUS, pageY: CENTER});
297-
expect(onChangeSpy).toHaveBeenCalledTimes(0);
298-
move(thumb, {pageX: CENTER, pageY: CENTER + THUMB_RADIUS});
299-
expect(onChangeSpy).toHaveBeenCalledTimes(1);
300-
expect(onChangeSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 120).toString('hsla'));
301-
expect(slider).toHaveAttribute('value', '120');
302-
end(thumb, {pageX: CENTER, pageY: CENTER + THUMB_RADIUS});
303-
expect(onChangeSpy).toHaveBeenCalledTimes(1);
304-
});
305-
306256
it('clicking and dragging on the track works', () => {
307257
let defaultColor = parseColor('hsl(0, 100%, 50%)');
308258
let {getByRole, getByTestId} = render(<ColorWheel defaultValue={defaultColor} onChange={onChangeSpy} />);
@@ -349,25 +299,5 @@ describe('useColorWheel', () => {
349299
expect(onChangeSpy).toHaveBeenCalledTimes(0);
350300
expect(document.activeElement).not.toBe(slider);
351301
});
352-
353-
it('clicking and dragging on the track respects the step', () => {
354-
let defaultColor = parseColor('hsl(0, 100%, 50%)');
355-
let {getByRole, getByTestId} = render(<ColorWheel defaultValue={defaultColor} onChange={onChangeSpy} step={120} />);
356-
let thumb = getByTestId('thumb');
357-
let slider = getByRole('slider');
358-
let container = getByTestId('container');
359-
container.getBoundingClientRect = getBoundingClientRect;
360-
361-
start(container, {pageX: CENTER, pageY: CENTER + THUMB_RADIUS});
362-
expect(onChangeSpy).toHaveBeenCalledTimes(1);
363-
expect(onChangeSpy.mock.calls[0][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 120).toString('hsla'));
364-
expect(slider).toHaveAttribute('value', '120');
365-
move(thumb, {pageX: CENTER, pageY: CENTER - THUMB_RADIUS});
366-
expect(onChangeSpy).toHaveBeenCalledTimes(2);
367-
expect(onChangeSpy.mock.calls[1][0].toString('hsla')).toBe(defaultColor.withChannelValue('hue', 240).toString('hsla'));
368-
expect(slider).toHaveAttribute('value', '240');
369-
end(thumb, {pageX: CENTER, pageY: CENTER - THUMB_RADIUS});
370-
expect(onChangeSpy).toHaveBeenCalledTimes(2);
371-
});
372302
});
373303
});

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

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -95,15 +95,6 @@ export let XGreenYRed = Template.bind({});
9595
XGreenYRed.storyName = 'RGB xChannel="green", yChannel="red"';
9696
XGreenYRed.args = {...XBlueYGreen.args, xChannel: 'green', yChannel: 'red'};
9797

98-
export let XBlueYGreenStep16 = Template.bind({});
99-
XBlueYGreenStep16.storyName = 'RGB xChannel="blue", yChannel="green", step="16"';
100-
XBlueYGreenStep16.args = {...XBlueYGreen.args, xChannelStep: 16, yChannelStep: 16};
101-
102-
export let XBlueYGreenPageStep32 = Template.bind({});
103-
XBlueYGreenPageStep32.storyName = 'RGB xChannel="blue", yChannel="green", pageStep="32"';
104-
XBlueYGreenPageStep32.args = {...XBlueYGreen.args, xChannelPageStep: 32, yChannelPageStep: 32};
105-
106-
/* TODO: what does a disabled color area look like? */
10798
export let XBlueYGreenisDisabled = Template.bind({});
10899
XBlueYGreenisDisabled.storyName = 'RGB xChannel="blue", yChannel="green", isDisabled';
109100
XBlueYGreenisDisabled.args = {...XBlueYGreen.args, isDisabled: true};

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,6 @@ storiesOf('ColorWheel', module)
2626
'disabled',
2727
() => <ColorWheel isDisabled defaultValue="hsl(0, 100%, 50%)" />
2828
)
29-
.add(
30-
'step',
31-
() => <ColorWheel step={6} defaultValue="hsl(0, 100%, 50%)" />
32-
)
3329
.add(
3430
'custom size',
3531
() => {

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

Lines changed: 8 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,10 @@ describe('ColorArea', () => {
131131
Name | props | actions | result
132132
${'left/right'} | ${{defaultValue: parseColor('#ff00ff')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowLeft'}), backward: (elem) => pressKey(elem, {key: 'ArrowRight'})}} | ${parseColor('#ff00fe')}
133133
${'up/down'} | ${{defaultValue: parseColor('#ff00ff')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowUp'}), backward: (elem) => pressKey(elem, {key: 'ArrowDown'})}} | ${parseColor('#ff01ff')}
134-
${'shiftleft/shiftright'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowLeft', shiftKey: true}), backward: (elem) => pressKey(elem, {key: 'ArrowRight', shiftKey: true})}} | ${parseColor('#f000e0')}
135-
${'shiftup/shiftdown'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowUp', shiftKey: true}), backward: (elem) => pressKey(elem, {key: 'ArrowDown', shiftKey: true})}} | ${parseColor('#f010f0')}
136-
${'pageup/pagedown'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'PageUp'}), backward: (elem) => pressKey(elem, {key: 'PageDown'})}} | ${parseColor('#f010f0')}
137-
${'home/end'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'Home'}), backward: (elem) => pressKey(elem, {key: 'End'})}} | ${parseColor('#f000e0')}
134+
${'shiftleft/shiftright'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowLeft', shiftKey: true}), backward: (elem) => pressKey(elem, {key: 'ArrowRight', shiftKey: true})}} | ${parseColor('#f000df')}
135+
${'shiftup/shiftdown'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowUp', shiftKey: true}), backward: (elem) => pressKey(elem, {key: 'ArrowDown', shiftKey: true})}} | ${parseColor('#f011f0')}
136+
${'pageup/pagedown'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'PageUp'}), backward: (elem) => pressKey(elem, {key: 'PageDown'})}} | ${parseColor('#f011f0')}
137+
${'home/end'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'Home'}), backward: (elem) => pressKey(elem, {key: 'End'})}} | ${parseColor('#f000df')}
138138
`('$Name', ({props, actions: {forward, backward}, result}) => {
139139
let {getAllByRole} = render(
140140
<Component
@@ -162,10 +162,10 @@ describe('ColorArea', () => {
162162
Name | props | actions | result
163163
${'left/right'} | ${{defaultValue: parseColor('#ff00ff')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowRight'}), backward: (elem) => pressKey(elem, {key: 'ArrowLeft'})}} | ${parseColor('#ff00fe')}
164164
${'up/down'} | ${{defaultValue: parseColor('#ff00ff')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowUp'}), backward: (elem) => pressKey(elem, {key: 'ArrowDown'})}} | ${parseColor('#ff01ff')}
165-
${'shiftleft/shiftright'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowRight', shiftKey: true}), backward: (elem) => pressKey(elem, {key: 'ArrowLeft', shiftKey: true})}} | ${parseColor('#f000e0')}
166-
${'shiftup/shiftdown'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowUp', shiftKey: true}), backward: (elem) => pressKey(elem, {key: 'ArrowDown', shiftKey: true})}} | ${parseColor('#f010f0')}
167-
${'pageup/pagedown'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'PageUp'}), backward: (elem) => pressKey(elem, {key: 'PageDown'})}} | ${parseColor('#f010f0')}
168-
${'home/end'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'End'}), backward: (elem) => pressKey(elem, {key: 'Home'})}} | ${parseColor('#f000e0')}
165+
${'shiftleft/shiftright'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowRight', shiftKey: true}), backward: (elem) => pressKey(elem, {key: 'ArrowLeft', shiftKey: true})}} | ${parseColor('#f000df')}
166+
${'shiftup/shiftdown'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowUp', shiftKey: true}), backward: (elem) => pressKey(elem, {key: 'ArrowDown', shiftKey: true})}} | ${parseColor('#f011f0')}
167+
${'pageup/pagedown'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'PageUp'}), backward: (elem) => pressKey(elem, {key: 'PageDown'})}} | ${parseColor('#f011f0')}
168+
${'home/end'} | ${{defaultValue: parseColor('#f000f0')}} | ${{forward: (elem) => pressKey(elem, {key: 'End'}), backward: (elem) => pressKey(elem, {key: 'Home'})}} | ${parseColor('#f000df')}
169169
`('$Name RTL', ({props, actions: {forward, backward}, result}) => {
170170
let {getAllByRole} = render(
171171
<Provider locale="ar-AE" theme={defaultTheme}>
@@ -213,35 +213,6 @@ describe('ColorArea', () => {
213213
expect(onChangeSpy).not.toHaveBeenCalled();
214214
expect(onChangeEndSpy).not.toHaveBeenCalled();
215215
});
216-
217-
it.each`
218-
Name | props | actions | result
219-
${'left/right'} | ${{defaultValue: parseColor('#ff00ff')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowLeft'}), backward: (elem) => pressKey(elem, {key: 'ArrowRight'})}} | ${parseColor('#ff00f0')}
220-
${'up/down'} | ${{defaultValue: parseColor('#ff00ff')}} | ${{forward: (elem) => pressKey(elem, {key: 'ArrowUp'}), backward: (elem) => pressKey(elem, {key: 'ArrowDown'})}} | ${parseColor('#ff0fff')}
221-
`('$Name with step', ({props, actions: {forward, backward}, result}) => {
222-
let {getAllByRole} = render(
223-
<Component
224-
{...props}
225-
xChannelStep={0xf}
226-
yChannelStep={0xf}
227-
onChange={onChangeSpy}
228-
onChangeEnd={onChangeEndSpy} />
229-
);
230-
let sliders = getAllByRole('slider');
231-
userEvent.tab();
232-
233-
forward(sliders[0]);
234-
expect(onChangeSpy).toHaveBeenCalledTimes(1);
235-
expect(onChangeSpy.mock.calls[0][0].toString('rgba')).toBe(result.toString('rgba'));
236-
expect(onChangeEndSpy).toHaveBeenCalledTimes(1);
237-
expect(onChangeEndSpy.mock.calls[0][0].toString('rgba')).toBe(result.toString('rgba'));
238-
239-
backward(sliders[0]);
240-
expect(onChangeSpy).toHaveBeenCalledTimes(2);
241-
expect(onChangeSpy.mock.calls[1][0].toString('rgba')).toBe(props.defaultValue.toString('rgba'));
242-
expect(onChangeEndSpy).toHaveBeenCalledTimes(2);
243-
expect(onChangeEndSpy.mock.calls[1][0].toString('rgba')).toBe(props.defaultValue.toString('rgba'));
244-
});
245216
});
246217

247218
describe.each`
@@ -351,34 +322,6 @@ describe('ColorArea', () => {
351322
expect(onChangeEndSpy).toHaveBeenCalledTimes(0);
352323
});
353324

354-
// TODO: Should it?
355-
it('dragging the thumb respects the step', () => {
356-
let defaultColor = parseColor('#ff00ff');
357-
let {getAllByRole} = render(
358-
<Component
359-
xChannelStep={16}
360-
yChannelStep={16}
361-
defaultValue={defaultColor}
362-
onChange={onChangeSpy}
363-
onChangeEnd={onChangeEndSpy} />
364-
);
365-
let sliders = getAllByRole('slider');
366-
let groups = getAllByRole('group');
367-
let thumb = sliders[0].parentElement;
368-
let container = groups[groupIndex];
369-
container.getBoundingClientRect = getBoundingClientRect;
370-
371-
start(thumb, {pageX: CENTER + THUMB_RADIUS, pageY: CENTER});
372-
expect(onChangeSpy).toHaveBeenCalledTimes(0);
373-
374-
move(thumb, {pageX: CENTER, pageY: CENTER + THUMB_RADIUS});
375-
expect(onChangeSpy).toHaveBeenCalledTimes(1);
376-
expect(onChangeSpy.mock.calls[0][0].toString('rgba')).toBe(parseColor('#ff0090').toString('rgba'));
377-
378-
end(thumb, {pageX: CENTER, pageY: CENTER + THUMB_RADIUS});
379-
expect(onChangeSpy).toHaveBeenCalledTimes(1);
380-
});
381-
382325
it('clicking and dragging on the track works', () => {
383326
let defaultColor = parseColor('#ff00ff');
384327
let {getAllByRole} = render(
@@ -438,31 +381,6 @@ describe('ColorArea', () => {
438381
expect(onChangeSpy).toHaveBeenCalledTimes(0);
439382
expect(document.activeElement).not.toBe(sliders[0]);
440383
});
441-
442-
it('clicking and dragging on the track respects the step', () => {
443-
let defaultColor = parseColor('#ff00ff');
444-
let {getAllByRole} = render(
445-
<Component
446-
xChannelStep={16}
447-
yChannelStep={16}
448-
defaultValue={defaultColor}
449-
onChange={onChangeSpy}
450-
onChangeEnd={onChangeEndSpy} />
451-
);
452-
let groups = getAllByRole('group');
453-
let container = groups[groupIndex];
454-
container.getBoundingClientRect = getBoundingClientRect;
455-
456-
start(container, {pageX: CENTER + THUMB_RADIUS, pageY: CENTER});
457-
expect(onChangeSpy).toHaveBeenCalledTimes(1);
458-
459-
move(container, {pageX: CENTER, pageY: CENTER + THUMB_RADIUS});
460-
expect(onChangeSpy).toHaveBeenCalledTimes(2);
461-
expect(onChangeSpy.mock.calls[0][0].toString('rgba')).toBe(parseColor('#ff80f0').toString('rgba'));
462-
463-
end(container, {pageX: CENTER, pageY: CENTER + THUMB_RADIUS});
464-
expect(onChangeSpy).toHaveBeenCalledTimes(2);
465-
});
466384
});
467385
});
468386
});

0 commit comments

Comments
 (0)