Skip to content

ColorArea and ColorWheel step and pagestep size fit evenly in range #2888

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Mar 3, 2022
Merged

ColorArea and ColorWheel step and pagestep size fit evenly in range #2888

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
c37706f
change default step size to evenly fit
snowystinger Feb 18, 2022
338ca6b
Bring in max value snapping
snowystinger Feb 25, 2022
fd6a510
Merge branch 'main' into color-area-step-changes
LFDanLu Feb 28, 2022
5050feb
Allow props to set page step size
snowystinger Feb 28, 2022
fd649ab
Merge branch 'main' into color-area-step-changes
LFDanLu Feb 28, 2022
1860586
Update packages/@react-stately/color/src/Color.ts
snowystinger Mar 1, 2022
4af3db2
adjust snap to values and fix tests
snowystinger Mar 2, 2022
34bf7de
Merge branch 'main' into color-area-step-changes
snowystinger Mar 2, 2022
46df036
Remove step and page step from color component APIs
snowystinger Mar 3, 2022
7516b99
fix last test
snowystinger Mar 3, 2022
72af055
Merge branch 'main' into color-area-step-changes
LFDanLu Mar 3, 2022
3d4b69c
Merge branch 'main' into color-area-step-changes
snowystinger Mar 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/@react-aria/color/src/useColorWheel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ interface ColorWheelAria {
export function useColorWheel(props: ColorWheelAriaProps, state: ColorWheelState, inputRef: RefObject<HTMLElement>): ColorWheelAria {
let {
isDisabled,
step = 1,
innerRadius,
outerRadius,
'aria-label': ariaLabel
Expand Down Expand Up @@ -256,6 +255,7 @@ export function useColorWheel(props: ColorWheelAriaProps, state: ColorWheelState
...props,
'aria-label': ariaLabel
});
let step = stateRef.current.step;

return {
trackProps: {
Expand Down
76 changes: 3 additions & 73 deletions packages/@react-aria/color/test/useColorWheel.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ function ColorWheel(props: ColorWheelProps) {

describe('useColorWheel', () => {
let onChangeSpy = jest.fn();

beforeAll(() => {
// @ts-ignore
jest.spyOn(window, 'requestAnimationFrame').mockImplementation((cb) => cb());
Expand Down Expand Up @@ -170,38 +170,6 @@ describe('useColorWheel', () => {
expect(slider).toHaveAttribute('value', '359');
});

it('respects step', () => {
let defaultColor = parseColor('hsl(0, 100%, 50%)');
let {getByRole} = render(<ColorWheel defaultValue={defaultColor} onChange={onChangeSpy} step={45} />);
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(<ColorWheel defaultValue={defaultColor} onChange={onChangeSpy} step={110} />);
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(<ColorWheel defaultValue={defaultColor} onChange={onChangeSpy} />);
Expand All @@ -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'));
Expand Down Expand Up @@ -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(<ColorWheel defaultValue={defaultColor} onChange={onChangeSpy} step={120} />);
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(<ColorWheel defaultValue={defaultColor} onChange={onChangeSpy} />);
Expand Down Expand Up @@ -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(<ColorWheel defaultValue={defaultColor} onChange={onChangeSpy} step={120} />);
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);
});
});
});
9 changes: 0 additions & 9 deletions packages/@react-spectrum/color/stories/ColorArea.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
4 changes: 0 additions & 4 deletions packages/@react-spectrum/color/stories/ColorWheel.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ storiesOf('ColorWheel', module)
'disabled',
() => <ColorWheel isDisabled defaultValue="hsl(0, 100%, 50%)" />
)
.add(
'step',
() => <ColorWheel step={6} defaultValue="hsl(0, 100%, 50%)" />
)
.add(
'custom size',
() => {
Expand Down
98 changes: 8 additions & 90 deletions packages/@react-spectrum/color/test/ColorArea.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<Component
Expand Down Expand Up @@ -162,10 +162,10 @@ describe('ColorArea', () => {
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(
<Provider locale="ar-AE" theme={defaultTheme}>
Expand Down Expand Up @@ -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(
<Component
{...props}
xChannelStep={0xf}
yChannelStep={0xf}
onChange={onChangeSpy}
onChangeEnd={onChangeEndSpy} />
);
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`
Expand Down Expand Up @@ -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(
<Component
xChannelStep={16}
yChannelStep={16}
defaultValue={defaultColor}
onChange={onChangeSpy}
onChangeEnd={onChangeEndSpy} />
);
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(
Expand Down Expand Up @@ -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(
<Component
xChannelStep={16}
yChannelStep={16}
defaultValue={defaultColor}
onChange={onChangeSpy}
onChangeEnd={onChangeEndSpy} />
);
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);
});
});
});
});
Expand Down
Loading