Skip to content

Commit f533056

Browse files
committed
fix(#2664): useColorSlider: simplify per code review
1 parent 463837d commit f533056

File tree

3 files changed

+31
-23
lines changed

3 files changed

+31
-23
lines changed

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

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
import {AriaColorSliderProps} from '@react-types/color';
1414
import {ColorSliderState} from '@react-stately/color';
1515
import {HTMLAttributes, RefObject} from 'react';
16-
import {mergeProps, snapValueToStep} from '@react-aria/utils';
16+
import {mergeProps} from '@react-aria/utils';
1717
import {useKeyboard} from '@react-aria/interactions';
1818
import {useLocale} from '@react-aria/i18n';
1919
import {useSlider, useSliderThumb} from '@react-aria/slider';
@@ -62,40 +62,32 @@ export function useColorSlider(props: ColorSliderAriaOptions, state: ColorSlider
6262
inputRef
6363
}, state);
6464

65-
let {minValue, maxValue, step, pageSize} = state.value.getChannelRange(channel);
65+
let {step, pageSize} = state.value.getChannelRange(channel);
6666
let {keyboardProps} = useKeyboard({
6767
onKeyDown(e) {
6868
// these are the cases that useMove or useSlider don't handle
69-
if (!/^(PageUp|PageDown|Home|End)$/.test(e.key)) {
69+
if (!/^(PageUp|PageDown)$/.test(e.key)) {
7070
e.continuePropagation();
7171
return;
7272
}
7373
// same handling as useMove, stopPropagation to prevent useSlider from handling the event as well.
7474
e.preventDefault();
75-
let channelValue = state.value.getChannelValue(channel);
76-
let pageStep = Math.max(pageSize, step);
77-
let newValue = channelValue;
75+
let pageStep = Math.max(
76+
props.pageSize || pageSize,
77+
props.step || step
78+
);
79+
// remember to set this so that onChangeEnd is fired
80+
state.setThumbDragging(0, true);
7881
switch (e.key) {
7982
case 'PageUp':
80-
newValue = channelValue + pageStep > maxValue ? maxValue : snapValueToStep(channelValue + pageStep, minValue, maxValue, pageStep);
83+
state.incrementThumb(0, pageStep);
8184
break;
8285
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;
86+
state.decrementThumb(0, pageStep);
9087
break;
9188
}
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-
}
9789
// wait a frame to ensure value has changed then unset this so that onChangeEnd is fired
98-
requestAnimationFrame(() => state.setThumbDragging(0, false));
90+
state.setThumbDragging(0, false);
9991
}
10092
});
10193

packages/@react-stately/color/src/Color.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ class RGBColor extends Color {
237237
case 'red':
238238
case 'green':
239239
case 'blue':
240-
return {minValue: 0, maxValue: 255, step: 1, pageSize: 0x10};
240+
return {minValue: 0x0, maxValue: 0xFF, step: 0x1, pageSize: 0x10};
241241
case 'alpha':
242242
return {minValue: 0, maxValue: 1, step: 0.01, pageSize: 0.1};
243243
default:

packages/@react-stately/color/src/useColorSliderState.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
import {Color, ColorSliderProps} from '@react-types/color';
1414
import {normalizeColor, parseColor} from './Color';
1515
import {SliderState, useSliderState} from '@react-stately/slider';
16-
import {useControlledState} from '@react-stately/utils';
16+
import {snapValueToStep, useControlledState} from '@react-stately/utils';
1717

1818
export interface ColorSliderState extends SliderState {
1919
/** The current color value represented by the color slider. */
@@ -51,19 +51,35 @@ export function useColorSliderState(props: ColorSliderStateOptions): ColorSlider
5151
setColor(color.withChannelValue(channel, v));
5252
},
5353
onChangeEnd([v]) {
54-
// onChange will have already been called with the right value, this is just to trigger onChangEnd
54+
// onChange will have already been called with the right value, this is just to trigger onChangeEnd
5555
if (props.onChangeEnd) {
5656
props.onChangeEnd(color.withChannelValue(channel, v));
5757
}
5858
}
5959
});
6060

61+
function incrementThumb(index: number, stepSize: number = 1) {
62+
let {maxValue, minValue, step} = color.getChannelRange(channel);
63+
let v = color.getChannelValue(channel);
64+
let s = Math.max(stepSize, step);
65+
sliderState.setThumbValue(index, v + s > maxValue ? maxValue : snapValueToStep(v + s, minValue, maxValue, s));
66+
}
67+
68+
function decrementThumb(index: number, stepSize: number = 1) {
69+
let {maxValue, minValue, step} = color.getChannelRange(channel);
70+
let v = color.getChannelValue(channel);
71+
let s = Math.max(stepSize, step);
72+
sliderState.setThumbValue(index, v - s < minValue ? minValue : snapValueToStep(v - s, minValue, maxValue, s));
73+
}
74+
6175
return {
6276
...sliderState,
6377
value: color,
6478
setValue(value) {
6579
setColor(normalizeColor(value));
6680
},
81+
incrementThumb,
82+
decrementThumb,
6783
getDisplayColor() {
6884
switch (channel) {
6985
case 'hue':

0 commit comments

Comments
 (0)