-
Notifications
You must be signed in to change notification settings - Fork 1.2k
ColorArea: add HSB and HSL support #2570
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.
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
Changes from all commits
b63490a
ba5698d
3ecc458
a5466b9
76922ca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,8 @@ | |
import {AriaColorSliderProps} from '@react-types/color'; | ||
import {ColorSliderState} from '@react-stately/color'; | ||
import {HTMLAttributes, RefObject} from 'react'; | ||
import {mergeProps} from '@react-aria/utils'; | ||
import {mergeProps, snapValueToStep} from '@react-aria/utils'; | ||
import {useKeyboard} from '@react-aria/interactions'; | ||
import {useLocale} from '@react-aria/i18n'; | ||
import {useSlider, useSliderThumb} from '@react-aria/slider'; | ||
|
||
|
@@ -61,6 +62,43 @@ export function useColorSlider(props: ColorSliderAriaOptions, state: ColorSlider | |
inputRef | ||
}, state); | ||
|
||
let {minValue, maxValue, step, pageSize} = state.value.getChannelRange(channel); | ||
let {keyboardProps} = useKeyboard({ | ||
onKeyDown(e) { | ||
// these are the cases that useMove or useSlider don't handle | ||
if (!/^(PageUp|PageDown|Home|End)$/.test(e.key)) { | ||
e.continuePropagation(); | ||
return; | ||
} | ||
// same handling as useMove, stopPropagation to prevent useSlider from handling the event as well. | ||
e.preventDefault(); | ||
let channelValue = state.value.getChannelValue(channel); | ||
let pageStep = Math.max(pageSize, step); | ||
let newValue = channelValue; | ||
switch (e.key) { | ||
case 'PageUp': | ||
newValue = channelValue + pageStep > maxValue ? maxValue : snapValueToStep(channelValue + pageStep, minValue, maxValue, pageStep); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why would pressing PageUp ever cause the user to see the maxValue? Isn't PageUp moving upward, i.e. closer to the minValue? When I tested this pageup/pagedown seemed backwards. Also, why does snapValueToStep have this broder issue when moving up, but not with PageDown? This feels like a bug with snapValueToStep. I noticed in |
||
break; | ||
case 'PageDown': | ||
newValue = snapValueToStep(channelValue - pageStep, minValue, maxValue, pageStep); | ||
break; | ||
case 'Home': | ||
newValue = minValue; | ||
break; | ||
case 'End': | ||
newValue = maxValue; | ||
break; | ||
} | ||
// remember to set this so that onChangeEnd is fired | ||
state.setThumbDragging(0, true); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't the setThumDragging only change if the value changes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to set it and then immediately unset it just so that onChangeEnd is fired |
||
if (newValue !== channelValue) { | ||
state.setValue(state.value.withChannelValue(channel, newValue)); | ||
} | ||
// wait a frame to ensure value has changed then unset this so that onChangeEnd is fired | ||
requestAnimationFrame(() => state.setThumbDragging(0, false)); | ||
} | ||
}); | ||
|
||
let generateBackground = () => { | ||
let value = state.getDisplayColor(); | ||
let to: string; | ||
|
@@ -113,7 +151,7 @@ export function useColorSlider(props: ColorSliderAriaOptions, state: ColorSlider | |
background: generateBackground() | ||
} | ||
}, | ||
inputProps, | ||
inputProps: mergeProps(inputProps, keyboardProps), | ||
thumbProps: { | ||
...thumbProps, | ||
style: { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,14 +11,12 @@ | |
*/ | ||
|
||
import {action} from '@storybook/addon-actions'; | ||
import {ColorArea, ColorSlider} from '../'; | ||
import {ColorArea, ColorField, ColorSlider, ColorWheel} from '../'; | ||
import {ColorChannel, SpectrumColorAreaProps} from '@react-types/color'; | ||
import {Flex} from '@adobe/react-spectrum'; | ||
import {Meta, Story} from '@storybook/react'; | ||
import {parseColor} from '@react-stately/color'; | ||
import {normalizeColor, parseColor} from '@react-stately/color'; | ||
import React, {useState} from 'react'; | ||
import {Text} from '@react-spectrum/text'; | ||
|
||
|
||
const meta: Meta<SpectrumColorAreaProps> = { | ||
title: 'ColorArea', | ||
|
@@ -31,41 +29,70 @@ const Template: Story<SpectrumColorAreaProps> = (args) => ( | |
<ColorAreaExample {...args} /> | ||
); | ||
|
||
let RGB: Set<ColorChannel> = new Set(['red', 'green', 'blue']); | ||
let difference = (a, b): Set<ColorChannel> => new Set([...a].filter(x => !b.has(x))); | ||
|
||
function ColorAreaExample(props: SpectrumColorAreaProps) { | ||
let {xChannel, yChannel, isDisabled} = props; | ||
let channels = new Set([xChannel, yChannel]); | ||
let zChannel: ColorChannel = difference(RGB, channels).keys().next().value; | ||
let [color, setColor] = useState(props.defaultValue || parseColor('#ff00ff')); | ||
return (<div role="group" aria-label="RGB Color Picker"> | ||
<Flex gap="size-500" alignItems="center"> | ||
<Flex direction="column" gap="size-50" alignItems="center"> | ||
let defaultValue = typeof props.defaultValue === 'string' ? parseColor(props.defaultValue) : props.defaultValue; | ||
let [color, setColor] = useState(defaultValue || parseColor('#ff00ff')); | ||
let xyChannels = new Set([xChannel, yChannel]); | ||
let colorSpace = color.getColorSpace(); | ||
let zChannel: ColorChannel = difference(color.getColorChannels(), xyChannels).keys().next().value; | ||
let isHue = zChannel === 'hue'; | ||
function onChange(e) { | ||
try { | ||
e = normalizeColor(e); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding
|
||
// eslint-disable-next-line no-empty | ||
} catch (error) { | ||
e = undefined; | ||
return; | ||
} | ||
const newColor = (e || color).toFormat(colorSpace); | ||
if (props.onChange) { | ||
props.onChange(newColor); | ||
} | ||
setColor(newColor); | ||
} | ||
return (<div role="group" aria-label={`${colorSpace.toUpperCase()} Color Picker`}> | ||
<Flex gap="size-500" alignItems="start"> | ||
<Flex direction="column" gap={isHue ? 0 : 'size-50'} alignItems="center"> | ||
<ColorArea | ||
size={isHue ? 'size-1200' : null} | ||
{...props} | ||
value={color} | ||
onChange={(e) => { | ||
if (props.onChange) { | ||
props.onChange(e); | ||
} | ||
setColor(e); | ||
}} /> | ||
<ColorSlider | ||
value={color} | ||
onChange={(e) => { | ||
if (props.onChange) { | ||
props.onChange(e); | ||
} | ||
setColor(e); | ||
}} | ||
onChangeEnd={props.onChangeEnd} | ||
channel={zChannel} | ||
isDisabled={isDisabled} /> | ||
onChange={onChange} | ||
onChangeEnd={props.onChangeEnd} /> | ||
{isHue ? ( | ||
<ColorWheel | ||
value={color} | ||
onChange={onChange} | ||
onChangeEnd={props.onChangeEnd} | ||
isDisabled={isDisabled} | ||
size={'size-2400'} | ||
UNSAFE_style={{ | ||
marginTop: 'calc( -.75 * var(--spectrum-global-dimension-size-2400))' | ||
}} /> | ||
) : ( | ||
<ColorSlider | ||
value={color} | ||
onChange={onChange} | ||
onChangeEnd={props.onChangeEnd} | ||
channel={zChannel} | ||
isDisabled={isDisabled} /> | ||
)} | ||
</Flex> | ||
<Flex direction="column" alignItems="center" gap="size-100" minWidth={'size-2000'}> | ||
<div role="img" aria-label={`color swatch: ${color.toString('rgb')}`} title={`${color.toString('hex')}`} style={{width: '100px', height: '100px', background: color.toString('css')}} /> | ||
<Text>{color.toString('hex')}</Text> | ||
<Flex direction="column" alignItems="center" gap="size-100" minWidth="size-1200"> | ||
<div role="img" aria-label={`color swatch: ${color.toString('rgb')}`} title={`${color.toString('hex')}`} style={{width: '96px', height: '96px', background: color.toString('css')}} /> | ||
<ColorField | ||
label="HEX Color" | ||
value={color} | ||
onChange={onChange} | ||
onKeyDown={event => | ||
event.key === 'Enter' && | ||
onChange((event.target as HTMLInputElement).value) | ||
} | ||
isDisabled={isDisabled} | ||
width="size-1200" /> | ||
</Flex> | ||
</Flex> | ||
</div>); | ||
|
@@ -84,7 +111,7 @@ XBlueYRed.storyName = 'RGB xChannel="blue", yChannel="red"'; | |
XBlueYRed.args = {...XBlueYGreen.args, xChannel: 'blue', yChannel: 'red'}; | ||
|
||
export let XRedYBlue = Template.bind({}); | ||
XRedYBlue.storyName = 'GB xChannel="red", yChannel="blue"'; | ||
XRedYBlue.storyName = 'RGB xChannel="red", yChannel="blue"'; | ||
XRedYBlue.args = {...XBlueYGreen.args, xChannel: 'red', yChannel: 'blue'}; | ||
|
||
export let XRedYGreen = Template.bind({}); | ||
|
@@ -120,3 +147,81 @@ XBlueYGreenSize3000.args = {...XBlueYGreen.args, size: 'size-3000'}; | |
export let XBlueYGreenSize600 = Template.bind({}); | ||
XBlueYGreenSize600.storyName = 'RGB xChannel="blue", yChannel="green", size="size-600"'; | ||
XBlueYGreenSize600.args = {...XBlueYGreen.args, size: 'size-600'}; | ||
|
||
export let XSaturationYLightness = Template.bind({}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This a lot of new stories to test. I know we are just following the pattern of the RGB stories above. Wondering if it would make more sense to add these to chromatic storybook and only having one or two examples of this or a dropdown to change a single story between all of these, or checkboxes to change toggle the examples between all these combinations? |
||
XSaturationYLightness.storyName = 'HSL xChannel="saturation", yChannel="lightness"'; | ||
XSaturationYLightness.args = {xChannel: 'saturation', yChannel: 'lightness', defaultValue: 'hsl(0, 100%, 50%)', onChange: action('onChange'), onChangeEnd: action('onChangeEnd')}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we care about the onChange and onChangeEnd events for these stories? I'm speaking to these not being on every RGB story. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. they are on every rgb story in the latest on the color-area branch, they're inherited from the base one |
||
|
||
export let XLightnessYSaturation = Template.bind({}); | ||
XLightnessYSaturation.storyName = 'HSL xChannel="lightness", yChannel="saturation"'; | ||
XLightnessYSaturation.args = {xChannel: 'lightness', yChannel: 'saturation', defaultValue: 'hsl(0, 100%, 50%)', onChange: action('onChange'), onChangeEnd: action('onChangeEnd')}; | ||
|
||
/* TODO: what does a disabled color area look like? */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this todo resolved or need to another PR or an issue? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we've got this pretty much resolved, I'll remove the comment, thanks |
||
export let XSaturationYLightnessisDisabled = Template.bind({}); | ||
XSaturationYLightnessisDisabled.storyName = 'HSL xChannel="saturation", yChannel="lightness", isDisabled'; | ||
XSaturationYLightnessisDisabled.args = {...XSaturationYLightness.args, isDisabled: true}; | ||
|
||
export let XHueYSaturationHSL = Template.bind({}); | ||
XHueYSaturationHSL.storyName = 'HSL xChannel="hue", yChannel="saturation"'; | ||
XHueYSaturationHSL.args = {xChannel: 'hue', yChannel: 'saturation', defaultValue: 'hsl(0, 100%, 50%)', onChange: action('onChange'), onChangeEnd: action('onChangeEnd')}; | ||
|
||
export let XSaturationYHueHSL = Template.bind({}); | ||
XSaturationYHueHSL.storyName = 'HSL xChannel="saturation", yChannel="hue"'; | ||
XSaturationYHueHSL.args = {xChannel: 'saturation', yChannel: 'hue', defaultValue: 'hsl(0, 100%, 50%)', onChange: action('onChange'), onChangeEnd: action('onChangeEnd')}; | ||
|
||
/* TODO: what does a disabled color area look like? */ | ||
export let XHueYSaturationHSLisDisabled = Template.bind({}); | ||
XHueYSaturationHSLisDisabled.storyName = 'HSL xChannel="hue", yChannel="saturation", isDisabled'; | ||
XHueYSaturationHSLisDisabled.args = {...XHueYSaturationHSL.args, isDisabled: true}; | ||
Comment on lines
+172
to
+175
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good question, maybe something to bring up to spectrum? I think making everything gray (like what you have) is probably easiest since we won't have to worry about if the disabled handle is too hard too see and it maybe confusing if we only change the handle style but leave the color area filled in |
||
|
||
export let XHueYLightnessHSL = Template.bind({}); | ||
XHueYLightnessHSL.storyName = 'HSL xChannel="hue", yChannel="lightness"'; | ||
XHueYLightnessHSL.args = {xChannel: 'hue', yChannel: 'lightness', defaultValue: 'hsl(0, 100%, 50%)', onChange: action('onChange'), onChangeEnd: action('onChangeEnd')}; | ||
|
||
export let XLightnessYHueHSL = Template.bind({}); | ||
XLightnessYHueHSL.storyName = 'HSL xChannel="lightness", yChannel="hue"'; | ||
XLightnessYHueHSL.args = {xChannel: 'lightness', yChannel: 'hue', defaultValue: 'hsl(0, 100%, 50%)', onChange: action('onChange'), onChangeEnd: action('onChangeEnd')}; | ||
|
||
/* TODO: what does a disabled color area look like? */ | ||
export let XHueYLightnessHSLisDisabled = Template.bind({}); | ||
XHueYLightnessHSLisDisabled.storyName = 'HSL xChannel="hue", yChannel="lightness", isDisabled'; | ||
XHueYLightnessHSLisDisabled.args = {...XHueYLightnessHSL.args, isDisabled: true}; | ||
|
||
export let XSaturationYBrightness = Template.bind({}); | ||
XSaturationYBrightness.storyName = 'HSB xChannel="saturation", yChannel="brightness"'; | ||
XSaturationYBrightness.args = {xChannel: 'saturation', yChannel: 'brightness', defaultValue: 'hsb(0, 100%, 100%)', onChange: action('onChange'), onChangeEnd: action('onChangeEnd')}; | ||
|
||
export let XBrightnessYSaturation = Template.bind({}); | ||
XBrightnessYSaturation.storyName = 'HSB xChannel="brightness", yChannel="saturation"'; | ||
XBrightnessYSaturation.args = {xChannel: 'brightness', yChannel: 'saturation', defaultValue: 'hsb(0, 100%, 100%)', onChange: action('onChange'), onChangeEnd: action('onChangeEnd')}; | ||
|
||
/* TODO: what does a disabled color area look like? */ | ||
export let XSaturationYBrightnessisDisabled = Template.bind({}); | ||
XSaturationYBrightnessisDisabled.storyName = 'HSB xChannel="saturation", yChannel="brightness", isDisabled'; | ||
XSaturationYBrightnessisDisabled.args = {...XSaturationYBrightness.args, isDisabled: true}; | ||
|
||
export let XHueYSaturationHSB = Template.bind({}); | ||
XHueYSaturationHSB.storyName = 'HSB xChannel="hue", yChannel="saturation"'; | ||
XHueYSaturationHSB.args = {xChannel: 'hue', yChannel: 'saturation', defaultValue: 'hsb(0, 100%, 100%)', onChange: action('onChange'), onChangeEnd: action('onChangeEnd')}; | ||
|
||
export let XSaturationYHueHSB = Template.bind({}); | ||
XSaturationYHueHSB.storyName = 'HSB xChannel="saturation", yChannel="hue"'; | ||
XSaturationYHueHSB.args = {xChannel: 'saturation', yChannel: 'hue', defaultValue: 'hsb(0, 100%, 100%)', onChange: action('onChange'), onChangeEnd: action('onChangeEnd')}; | ||
|
||
/* TODO: what does a disabled color area look like? */ | ||
export let XHueYSaturationHSBisDisabled = Template.bind({}); | ||
XHueYSaturationHSBisDisabled.storyName = 'HSB xChannel="hue", yChannel="saturation", isDisabled'; | ||
XHueYSaturationHSBisDisabled.args = {...XHueYSaturationHSB.args, isDisabled: true}; | ||
|
||
export let XHueYBrightnessHSB = Template.bind({}); | ||
XHueYBrightnessHSB.storyName = 'HSB xChannel="hue", yChannel="brightness"'; | ||
XHueYBrightnessHSB.args = {xChannel: 'hue', yChannel: 'brightness', defaultValue: 'hsb(0, 100%, 100%)', onChange: action('onChange'), onChangeEnd: action('onChangeEnd')}; | ||
|
||
export let XBrightnessYHueHSB = Template.bind({}); | ||
XBrightnessYHueHSB.storyName = 'HSB xChannel="brightness", yChannel="hue"'; | ||
XBrightnessYHueHSB.args = {xChannel: 'brightness', yChannel: 'hue', defaultValue: 'hsb(0, 100%, 100%)', onChange: action('onChange'), onChangeEnd: action('onChangeEnd')}; | ||
|
||
/* TODO: what does a disabled color area look like? */ | ||
export let XBrightnessYHueHSBisDisabled = Template.bind({}); | ||
XBrightnessYHueHSBisDisabled.storyName = 'HSB xChannel="brightness", yChannel="hue", isDisabled'; | ||
XBrightnessYHueHSBisDisabled.args = {...XBrightnessYHueHSB.args, isDisabled: true}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change going to cause issues if I'm using an app and I change my locale? I'm also curious if there could be issues with this component being used in a collaborative app where one person has their locale in one language/region and the other has it set to a different one. This code should be transforming the data for display to the user, but double checking.