Skip to content

Commit b3cea47

Browse files
authored
fix(ColorPicker): Fix default color value in ColorPicker (#6364)
* fix default color value to have full alpha channel * add test * make color required * add missing color prop in RSP ColorPicker * fix types * fix tests * add transparent example to colorswatch docs * use previous value if input is empty/invalid * add back null * missed a few lines in tests * fix types * fix tests * remove null from ColorPickerProps
1 parent 02515b8 commit b3cea47

File tree

8 files changed

+56
-12
lines changed

8 files changed

+56
-12
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {useLocale, useLocalizedStringFormatter} from '@react-aria/i18n';
2121

2222
export interface AriaColorSwatchProps extends AriaLabelingProps, DOMProps {
2323
/** The color value to display in the swatch. */
24-
color?: string | Color | null,
24+
color?: string | Color,
2525
/**
2626
* A localized accessible name for the color.
2727
* By default, a description is generated from the color value,

packages/@react-spectrum/color/docs/ColorSwatch.mdx

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,20 @@ function Example() {
6363
}
6464
```
6565

66-
### No value
67-
68-
When no `color` prop is provided, the value is `null`, or the color is transparent, ColorSwatch displays a red slash across the swatch.
66+
Fully transparent colors are displayed with a red slash through the swatch, while partially transparent colors are shown with a checkerboard background.
6967

7068
```tsx example
71-
<ColorSwatch color={null} />
69+
import {parseColor, ColorSlider} from '@react-spectrum/color';
70+
71+
function Example() {
72+
let [color, setColor] = React.useState(parseColor('hsla(0, 100%, 50%, 0)'));
73+
return (
74+
<Flex direction="column" gap="size-100">
75+
<ColorSlider value={color} onChange={setColor} channel="alpha" />
76+
<ColorSwatch color={color} />
77+
</Flex>
78+
);
79+
}
7280
```
7381

7482
## Labeling

packages/@react-spectrum/color/src/ColorPicker.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ function ColorPicker(props: SpectrumColorPickerProps, ref: FocusableRef<HTMLButt
8080
})({isFocusVisible})}>
8181
<ColorSwatch
8282
ref={swatchRef}
83+
color={props.value}
8384
size={props.size}
8485
rounding={props.rounding}
8586
aria-label={props['aria-label']}

packages/@react-spectrum/color/src/ColorSwatchPicker.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {SpectrumColorSwatchContext, SpectrumColorSwatchProps} from './ColorSwatc
1818
import {style} from '@react-spectrum/style-macro-s1' with {type: 'macro'};
1919
import {useDOMRef, useStyleProps} from '@react-spectrum/utils';
2020

21-
export interface SpectrumColorSwatchPickerProps extends ValueBase<string | Color | null, Color>, StyleProps {
21+
export interface SpectrumColorSwatchPickerProps extends ValueBase<string | Color, Color>, StyleProps {
2222
/** The ColorSwatches within the ColorSwatchPicker. */
2323
children: ReactNode,
2424
/**

packages/@react-spectrum/color/stories/ColorField.stories.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ export const ContextualHelpStory: ColorFieldStory = {
151151
};
152152

153153
function ControlledColorField(props: SpectrumColorFieldProps) {
154-
let [color, setColor] = useState<string | Color | null | undefined>(props.value);
154+
let [color, setColor] = useState<string | Color | null | undefined>(props.value || '#000000');
155155
let onChange = (color: Color | null) => {
156156
setColor(color);
157157
if (props.onChange) { props.onChange(color); }

packages/@react-spectrum/color/test/ColorPicker.test.js

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,39 @@ describe('ColorPicker', function () {
7070
expect(within(button).getByRole('img')).toHaveAttribute('aria-label', 'dark vibrant blue');
7171
});
7272

73+
it('should have default value of black', async function () {
74+
let {getByRole, getAllByRole} = render(
75+
<Provider theme={theme}>
76+
<ColorPicker label="Fill">
77+
<ColorEditor />
78+
</ColorPicker>
79+
</Provider>
80+
);
81+
82+
let button = getByRole('button');
83+
expect(button).toHaveTextContent('Fill');
84+
expect(within(button).getByRole('img')).toHaveAttribute('aria-label', 'black');
85+
86+
await user.click(button);
87+
88+
let dialog = getByRole('dialog');
89+
expect(dialog).toHaveAttribute('aria-labelledby');
90+
expect(document.getElementById(dialog.getAttribute('aria-labelledby'))).toHaveTextContent('Fill');
91+
92+
let sliders = getAllByRole('slider');
93+
expect(sliders).toHaveLength(3);
94+
95+
let picker = getAllByRole('button')[1];
96+
expect(picker).toHaveTextContent('Hex');
97+
98+
let colorField = getAllByRole('textbox')[0];
99+
expect(colorField).toHaveAttribute('aria-label', 'Hex');
100+
101+
let alpha = getAllByRole('textbox')[1];
102+
expect(alpha).toHaveAttribute('aria-label', 'Alpha');
103+
});
104+
105+
73106
it('allows switching color space', async function () {
74107
let {getByRole, getAllByRole} = render(
75108
<Provider theme={theme}>

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import {useColor} from './useColor';
44
import {useControlledState} from '@react-stately/utils';
55
import {ValueBase} from '@react-types/shared';
66

7-
export interface ColorPickerProps extends ValueBase<string | Color | null, Color> {}
7+
export interface ColorPickerProps extends ValueBase<string | Color, Color> {}
88

99
export interface ColorPickerState {
1010
/** The current color value of the color picker. */
@@ -15,13 +15,15 @@ export interface ColorPickerState {
1515

1616
export function useColorPickerState(props: ColorPickerProps): ColorPickerState {
1717
let value = useColor(props.value);
18-
let defaultValue = useColor(props.defaultValue || '#0000')!;
18+
let defaultValue = useColor(props.defaultValue || '#000000')!;
1919
let [color, setColor] = useControlledState(value || undefined, defaultValue, props.onChange);
2020

2121
return {
2222
color,
2323
setColor(color) {
24-
setColor(color || parseColor('#0000'));
24+
if (color != null) {
25+
setColor(color || parseColor('#000000'));
26+
}
2527
}
2628
};
2729
}

packages/react-aria-components/src/ColorSwatchPicker.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import React, {createContext, ForwardedRef, forwardRef, ReactNode, useContext, u
1010
import {useLocale, useLocalizedStringFormatter} from 'react-aria';
1111

1212
export interface ColorSwatchPickerRenderProps extends Omit<ListBoxRenderProps, 'isDropTarget'> {}
13-
export interface ColorSwatchPickerProps extends ValueBase<string | Color | null, Color>, AriaLabelingProps, StyleRenderProps<ColorSwatchPickerRenderProps> {
13+
export interface ColorSwatchPickerProps extends ValueBase<string | Color, Color>, AriaLabelingProps, StyleRenderProps<ColorSwatchPickerRenderProps> {
1414
/** The children of the ColorSwatchPicker. */
1515
children?: ReactNode,
1616
/**
@@ -66,7 +66,7 @@ export interface ColorSwatchPickerItemRenderProps extends Omit<ListBoxItemRender
6666

6767
export interface ColorSwatchPickerItemProps extends RenderProps<ColorSwatchPickerItemRenderProps>, HoverEvents {
6868
/** The color of the swatch. */
69-
color: string | Color | null,
69+
color: string | Color,
7070
/** Whether the color swatch is disabled. */
7171
isDisabled?: boolean
7272
}

0 commit comments

Comments
 (0)