From dcf5942681af25f7c98b7bf856c5358e51d20861 Mon Sep 17 00:00:00 2001 From: nabsei Date: Wed, 15 Jul 2026 18:11:32 +0200 Subject: [PATCH 1/3] fix: allow ColorSwatchPicker to have no selection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ColorSwatchPicker always defaulted to black (#000000) via useColorPickerState, and disallowEmptySelection meant a black swatch could never be deselected — if the swatch list happened to contain black, it was permanently stuck as selected. ColorSwatchPicker now manages its own controlled/uncontrolled state directly (instead of going through useColorPickerState, which is shared with the more complex ColorPicker and shouldn't be changed to support a nullable color), so value/defaultValue/onChange can be null, and selectedKeys is empty when nothing is selected. The two Spectrum wrapper components (@adobe/react-spectrum and @react-spectrum/s2) just had their prop types widened to match, no behavior change there. Closes #7918 Co-authored-by: Claude --- .../src/color/ColorSwatchPicker.tsx | 2 +- .../s2/src/ColorSwatchPicker.tsx | 2 +- .../src/ColorSwatchPicker.tsx | 23 +++++++--- .../test/ColorSwatchPicker.test.js | 43 +++++++++++++++++++ 4 files changed, 61 insertions(+), 9 deletions(-) diff --git a/packages/@adobe/react-spectrum/src/color/ColorSwatchPicker.tsx b/packages/@adobe/react-spectrum/src/color/ColorSwatchPicker.tsx index 8f1841b307e..d3165b9f338 100644 --- a/packages/@adobe/react-spectrum/src/color/ColorSwatchPicker.tsx +++ b/packages/@adobe/react-spectrum/src/color/ColorSwatchPicker.tsx @@ -24,7 +24,7 @@ import {useDOMRef} from '../utils/useDOMRef'; import {useStyleProps} from '../utils/styleProps'; export interface SpectrumColorSwatchPickerProps - extends ValueBase, StyleProps { + extends ValueBase, StyleProps { /** The ColorSwatches within the ColorSwatchPicker. */ children: ReactNode; /** diff --git a/packages/@react-spectrum/s2/src/ColorSwatchPicker.tsx b/packages/@react-spectrum/s2/src/ColorSwatchPicker.tsx index cf96d46c72d..5f4dfce532e 100644 --- a/packages/@react-spectrum/s2/src/ColorSwatchPicker.tsx +++ b/packages/@react-spectrum/s2/src/ColorSwatchPicker.tsx @@ -26,7 +26,7 @@ import {useDOMRef} from './useDOMRef'; import {useSpectrumContextProps} from './useSpectrumContextProps'; export interface ColorSwatchPickerProps - extends ValueBase, StyleProps, SlotProps { + extends ValueBase, StyleProps, SlotProps { /** The ColorSwatches within the ColorSwatchPicker. */ children: ReactNode; /** diff --git a/packages/react-aria-components/src/ColorSwatchPicker.tsx b/packages/react-aria-components/src/ColorSwatchPicker.tsx index 85efcdc9d02..9621d8fbdf6 100644 --- a/packages/react-aria-components/src/ColorSwatchPicker.tsx +++ b/packages/react-aria-components/src/ColorSwatchPicker.tsx @@ -28,14 +28,14 @@ import React, { useEffect, useMemo } from 'react'; -import {useColorPickerState} from 'react-stately/useColorPickerState'; +import {useControlledState} from 'react-stately/useControlledState'; import {useLocale} from 'react-aria/I18nProvider'; import {useLocalizedStringFormatter} from 'react-aria/useLocalizedStringFormatter'; export interface ColorSwatchPickerRenderProps extends Omit {} export interface ColorSwatchPickerProps extends - ValueBase, + ValueBase, AriaLabelingProps, StyleRenderProps, GlobalDOMAttributes { @@ -68,7 +68,16 @@ export const ColorSwatchPicker = forwardRef(function ColorSwatchPicker( ref: ForwardedRef ) { [props, ref] = useContextProps(props, ref, ColorSwatchPickerContext); - let state = useColorPickerState(props); + let {value: valueProp, defaultValue: defaultValueProp, onChange} = props; + let value = useMemo( + () => (typeof valueProp === 'string' ? parseColor(valueProp) : valueProp), + [valueProp] + ); + let defaultValue = useMemo( + () => (typeof defaultValueProp === 'string' ? parseColor(defaultValueProp) : defaultValueProp), + [defaultValueProp] + ); + let [color, setColor] = useControlledState(value, defaultValue ?? null, onChange); let colorMap = useMemo(() => new Map(), []); let formatter = useLocalizedStringFormatter(intlMessages, 'react-aria-components'); @@ -84,14 +93,14 @@ export const ColorSwatchPicker = forwardRef(function ColorSwatchPicker( } layout={props.layout || 'grid'} selectionMode="single" - selectedKeys={[state.color.toString('hexa')]} + selectedKeys={color ? [color.toString('hexa')] : []} onSelectionChange={keys => { // single select, 'all' cannot occur. appease typescript. if (keys !== 'all') { - state.setColor(colorMap.get([...keys][0])); + let key = [...keys][0]; + setColor(key != null ? colorMap.get(key) : null); } - }} - disallowEmptySelection> + }}> {props.children} ); diff --git a/packages/react-aria-components/test/ColorSwatchPicker.test.js b/packages/react-aria-components/test/ColorSwatchPicker.test.js index 0d7aab5abdb..8b6730a3d5f 100644 --- a/packages/react-aria-components/test/ColorSwatchPicker.test.js +++ b/packages/react-aria-components/test/ColorSwatchPicker.test.js @@ -143,6 +143,49 @@ describe('ColorSwatchPicker', function () { expect(options[2]).toHaveAttribute('aria-selected', 'true'); }); + it('has no selection by default', async function () { + let {getByRole} = render( + + + + + + + + + ); + + let listbox = getByRole('listbox'); + let options = within(listbox).getAllByRole('option'); + expect(options[0]).toHaveAttribute('aria-selected', 'false'); + expect(options[1]).toHaveAttribute('aria-selected', 'false'); + }); + + it('supports deselecting a swatch, including black', async function () { + let onChange = jest.fn(); + let {getByRole} = render( + + + + + + + + + ); + + let listbox = getByRole('listbox'); + let options = within(listbox).getAllByRole('option'); + + await user.click(options[0]); + expect(onChange).toHaveBeenLastCalledWith(parseColor('#000000')); + expect(options[0]).toHaveAttribute('aria-selected', 'true'); + + await user.click(options[0]); + expect(onChange).toHaveBeenLastCalledWith(null); + expect(options[0]).toHaveAttribute('aria-selected', 'false'); + }); + it('handles keyboard input', async function () { let onChange = jest.fn(); let {getByRole} = render( From 5cae4b606b4d759e4a715cfceb8324d1f2477dc5 Mon Sep 17 00:00:00 2001 From: nabsei Date: Wed, 15 Jul 2026 21:52:55 +0200 Subject: [PATCH 2/3] fix: update ColorSwatchPicker doc examples for nullable onChange The value/onChange examples used plain useState(parseColor(...)), whose setter no longer matched the widened onChange type. Type the state as Color | null and guard the display text, matching the existing ColorField doc example's pattern. --- packages/dev/s2-docs/pages/react-aria/ColorSwatchPicker.mdx | 6 +++--- packages/dev/s2-docs/pages/s2/ColorSwatchPicker.mdx | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/dev/s2-docs/pages/react-aria/ColorSwatchPicker.mdx b/packages/dev/s2-docs/pages/react-aria/ColorSwatchPicker.mdx index 7d296a4dab4..d16e878681b 100644 --- a/packages/dev/s2-docs/pages/react-aria/ColorSwatchPicker.mdx +++ b/packages/dev/s2-docs/pages/react-aria/ColorSwatchPicker.mdx @@ -51,11 +51,11 @@ Use the `value` or `defaultValue` prop to set the selected color, and `onChange` ```tsx render "use client"; import {ColorSwatchPicker, ColorSwatchPickerItem} from 'vanilla-starter/ColorSwatchPicker'; -import {parseColor} from 'react-aria-components/ColorSwatchPicker'; +import {parseColor, type Color} from 'react-aria-components/ColorSwatchPicker'; import {useState} from 'react'; function Example() { - let [value, setValue] = useState(parseColor('#A00')); + let [value, setValue] = useState(parseColor('#A00')); return ( <> @@ -69,7 +69,7 @@ function Example() { -
Selected color: {value.toString('rgb')}
+
Selected color: {value ? value.toString('rgb') : 'none'}
); } diff --git a/packages/dev/s2-docs/pages/s2/ColorSwatchPicker.mdx b/packages/dev/s2-docs/pages/s2/ColorSwatchPicker.mdx index 645e0aed011..fd3ed6252e1 100644 --- a/packages/dev/s2-docs/pages/s2/ColorSwatchPicker.mdx +++ b/packages/dev/s2-docs/pages/s2/ColorSwatchPicker.mdx @@ -31,12 +31,12 @@ Use the `value` or `defaultValue` prop to set the selected color, and `onChange` ```tsx render "use client"; -import {ColorSwatchPicker, ColorSwatch, parseColor} from '@react-spectrum/s2/ColorSwatchPicker'; +import {ColorSwatchPicker, ColorSwatch, parseColor, type Color} from '@react-spectrum/s2/ColorSwatchPicker'; import {style} from '@react-spectrum/s2/style' with {type: 'macro'}; import {useState} from 'react'; function Example() { - let [value, setValue] = useState(parseColor('#e11d48')); + let [value, setValue] = useState(parseColor('#e11d48')); return ( <> @@ -51,7 +51,7 @@ function Example() { -
Selected color: {value.toString('rgb')}
+
Selected color: {value ? value.toString('rgb') : 'none'}
); } From fef45706373fcf2fff7173edd4d8108ed2deadac Mon Sep 17 00:00:00 2001 From: nabsei Date: Thu, 16 Jul 2026 14:05:33 +0200 Subject: [PATCH 3/3] fix: address review feedback on ColorSwatchPicker null selection Per review: - Move the controlled/uncontrolled color state into a new useColorSwatchPickerState stately hook, mirroring useColorPickerState's structure (no inline memos in the component), so the two are easy to diff against each other. - Restore disallowEmptySelection: once a swatch is selected, clicking it again no longer deselects it, matching how selection works elsewhere. Clearing the selection is only done by passing a controlled null value (e.g. from a form reset). --- .../src/ColorSwatchPicker.tsx | 21 +++------- .../test/ColorSwatchPicker.test.js | 38 ++++++++++++++++++- packages/react-stately/exports/index.ts | 5 +++ .../exports/useColorSwatchPickerState.ts | 18 +++++++++ .../src/color/useColorSwatchPickerState.ts | 31 +++++++++++++++ 5 files changed, 96 insertions(+), 17 deletions(-) create mode 100644 packages/react-stately/exports/useColorSwatchPickerState.ts create mode 100644 packages/react-stately/src/color/useColorSwatchPickerState.ts diff --git a/packages/react-aria-components/src/ColorSwatchPicker.tsx b/packages/react-aria-components/src/ColorSwatchPicker.tsx index 9621d8fbdf6..e64a712db88 100644 --- a/packages/react-aria-components/src/ColorSwatchPicker.tsx +++ b/packages/react-aria-components/src/ColorSwatchPicker.tsx @@ -28,7 +28,7 @@ import React, { useEffect, useMemo } from 'react'; -import {useControlledState} from 'react-stately/useControlledState'; +import {useColorSwatchPickerState} from 'react-stately/useColorSwatchPickerState'; import {useLocale} from 'react-aria/I18nProvider'; import {useLocalizedStringFormatter} from 'react-aria/useLocalizedStringFormatter'; @@ -68,16 +68,7 @@ export const ColorSwatchPicker = forwardRef(function ColorSwatchPicker( ref: ForwardedRef ) { [props, ref] = useContextProps(props, ref, ColorSwatchPickerContext); - let {value: valueProp, defaultValue: defaultValueProp, onChange} = props; - let value = useMemo( - () => (typeof valueProp === 'string' ? parseColor(valueProp) : valueProp), - [valueProp] - ); - let defaultValue = useMemo( - () => (typeof defaultValueProp === 'string' ? parseColor(defaultValueProp) : defaultValueProp), - [defaultValueProp] - ); - let [color, setColor] = useControlledState(value, defaultValue ?? null, onChange); + let state = useColorSwatchPickerState(props); let colorMap = useMemo(() => new Map(), []); let formatter = useLocalizedStringFormatter(intlMessages, 'react-aria-components'); @@ -93,14 +84,14 @@ export const ColorSwatchPicker = forwardRef(function ColorSwatchPicker( } layout={props.layout || 'grid'} selectionMode="single" - selectedKeys={color ? [color.toString('hexa')] : []} + selectedKeys={state.color ? [state.color.toString('hexa')] : []} onSelectionChange={keys => { // single select, 'all' cannot occur. appease typescript. if (keys !== 'all') { - let key = [...keys][0]; - setColor(key != null ? colorMap.get(key) : null); + state.setColor(colorMap.get([...keys][0])); } - }}> + }} + disallowEmptySelection> {props.children} ); diff --git a/packages/react-aria-components/test/ColorSwatchPicker.test.js b/packages/react-aria-components/test/ColorSwatchPicker.test.js index 8b6730a3d5f..d111f40c803 100644 --- a/packages/react-aria-components/test/ColorSwatchPicker.test.js +++ b/packages/react-aria-components/test/ColorSwatchPicker.test.js @@ -161,7 +161,7 @@ describe('ColorSwatchPicker', function () { expect(options[1]).toHaveAttribute('aria-selected', 'false'); }); - it('supports deselecting a swatch, including black', async function () { + it('supports selecting black, and does not allow deselecting via click', async function () { let onChange = jest.fn(); let {getByRole} = render( @@ -181,9 +181,43 @@ describe('ColorSwatchPicker', function () { expect(onChange).toHaveBeenLastCalledWith(parseColor('#000000')); expect(options[0]).toHaveAttribute('aria-selected', 'true'); + // Clicking the already-selected swatch again should not deselect it, + // matching how selection normally works elsewhere. await user.click(options[0]); - expect(onChange).toHaveBeenLastCalledWith(null); + expect(onChange).toHaveBeenCalledTimes(1); + expect(options[0]).toHaveAttribute('aria-selected', 'true'); + }); + + it('supports clearing the selection via a controlled null value', async function () { + let {getByRole, rerender} = render( + + + + + + + + + ); + + let listbox = getByRole('listbox'); + let options = within(listbox).getAllByRole('option'); + expect(options[0]).toHaveAttribute('aria-selected', 'true'); + + rerender( + + + + + + + + + ); + + options = within(listbox).getAllByRole('option'); expect(options[0]).toHaveAttribute('aria-selected', 'false'); + expect(options[1]).toHaveAttribute('aria-selected', 'false'); }); it('handles keyboard input', async function () { diff --git a/packages/react-stately/exports/index.ts b/packages/react-stately/exports/index.ts index e6a7f818cd6..6b56a9e615e 100644 --- a/packages/react-stately/exports/index.ts +++ b/packages/react-stately/exports/index.ts @@ -34,6 +34,10 @@ export type { } from '../src/color/useColorChannelFieldState'; export type {ColorFieldProps, ColorFieldState} from '../src/color/useColorFieldState'; export type {ColorPickerProps, ColorPickerState} from '../src/color/useColorPickerState'; +export type { + ColorSwatchPickerStateProps, + ColorSwatchPickerState +} from '../src/color/useColorSwatchPickerState'; export type { ColorSliderState, ColorSliderStateOptions, @@ -179,6 +183,7 @@ export {useColorAreaState} from '../src/color/useColorAreaState'; export {useColorChannelFieldState} from '../src/color/useColorChannelFieldState'; export {useColorFieldState} from '../src/color/useColorFieldState'; export {useColorPickerState} from '../src/color/useColorPickerState'; +export {useColorSwatchPickerState} from '../src/color/useColorSwatchPickerState'; export {useColorSliderState} from '../src/color/useColorSliderState'; export {useColorWheelState} from '../src/color/useColorWheelState'; export {useComboBoxState} from '../src/combobox/useComboBoxState'; diff --git a/packages/react-stately/exports/useColorSwatchPickerState.ts b/packages/react-stately/exports/useColorSwatchPickerState.ts new file mode 100644 index 00000000000..1f28cd8ac88 --- /dev/null +++ b/packages/react-stately/exports/useColorSwatchPickerState.ts @@ -0,0 +1,18 @@ +/* + * Copyright 2024 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +export type { + ColorSwatchPickerStateProps, + ColorSwatchPickerState +} from '../src/color/useColorSwatchPickerState'; + +export {useColorSwatchPickerState} from '../src/color/useColorSwatchPickerState'; diff --git a/packages/react-stately/src/color/useColorSwatchPickerState.ts b/packages/react-stately/src/color/useColorSwatchPickerState.ts new file mode 100644 index 00000000000..75ce5a0afd8 --- /dev/null +++ b/packages/react-stately/src/color/useColorSwatchPickerState.ts @@ -0,0 +1,31 @@ +import {Color} from './types'; +import {useColor} from './useColor'; +import {useControlledState} from '../utils/useControlledState'; +import {ValueBase} from '@react-types/shared'; + +export interface ColorSwatchPickerStateProps extends ValueBase< + string | Color | null, + Color | null +> {} + +export interface ColorSwatchPickerState { + /** The current color value of the color swatch picker. */ + color: Color | null; + /** Sets the current color value of the color swatch picker. */ + setColor(color: Color | null): void; +} + +export function useColorSwatchPickerState( + props: ColorSwatchPickerStateProps +): ColorSwatchPickerState { + let value = useColor(props.value); + let defaultValue = useColor(props.defaultValue) ?? null; + let [color, setColor] = useControlledState(value, defaultValue, props.onChange); + + return { + color, + setColor(color) { + setColor(color); + } + }; +}