Accessibility: Fix custom color palette (#62753)

Co-authored-by: ellatrix <ellatrix@git.wordpress.org> Co-authored-by: afercia <afercia@git.wordpress.org> Co-authored-by: joedolson <joedolson@git.wordpress.org>
WordPress · Jun 25, 2024 · bc6d443 · bc6d443
1 parent 0e03332
commit bc6d443
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 52 deletions.
diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md
@@ -5,6 +5,7 @@
 ### Enhancements
 
 - Add `text-wrap: balance` fallback to all instances of `text-wrap: pretty` for greater cross browser compatibility. ([#62233](https://github.com/WordPress/gutenberg/pull/62233))
+- `__experimentalPaletteEdit`: improve the accessibility. ([#62753](https://github.com/WordPress/gutenberg/pull/62753))
 
 ### Bug Fixes
 

diff --git a/packages/components/src/palette-edit/index.tsx b/packages/components/src/palette-edit/index.tsx
@@ -15,10 +15,7 @@ import {
 } from '@wordpress/element';
 import { __, sprintf } from '@wordpress/i18n';
 import { lineSolid, moreVertical, plus } from '@wordpress/icons';
-import {
-	__experimentalUseFocusOutside as useFocusOutside,
-	useDebounce,
-} from '@wordpress/compose';
+import { useDebounce } from '@wordpress/compose';
 
 /**
  * Internal dependencies
@@ -174,16 +171,13 @@ function Option< T extends Color | Gradient >( {
 	canOnlyChangeValues,
 	element,
 	onChange,
-	isEditing,
-	onStartEditing,
 	onRemove,
-	onStopEditing,
 	popoverProps: receivedPopoverProps,
 	slugPrefix,
 	isGradient,
 }: OptionProps< T > ) {
-	const focusOutsideProps = useFocusOutside( onStopEditing );
 	const value = isGradient ? element.gradient : element.color;
+	const [ isEditingColor, setIsEditingColor ] = useState( false );
 
 	// Use internal state instead of a ref to make sure that the component
 	// re-renders when the popover's anchor updates.
@@ -198,26 +192,23 @@ function Option< T extends Color | Gradient >( {
 	);
 
 	return (
-		<PaletteItem
-			className={ isEditing ? 'is-selected' : undefined }
-			as={ isEditing ? 'div' : 'button' }
-			onClick={ onStartEditing }
-			aria-label={
-				isEditing
-					? undefined
-					: sprintf(
-							// translators: %s is a color or gradient name, e.g. "Red".
-							__( 'Edit: %s' ),
-							element.name.trim().length ? element.name : value
-					  )
-			}
-			ref={ setPopoverAnchor }
-			{ ...( isEditing ? { ...focusOutsideProps } : {} ) }
-		>
+		<PaletteItem ref={ setPopoverAnchor } as="div">
 			<HStack justify="flex-start">
-				<IndicatorStyled colorValue={ value } />
+				<Button
+					onClick={ () => {
+						setIsEditingColor( true );
+					} }
+					aria-label={ sprintf(
+						// translators: %s is a color or gradient name, e.g. "Red".
+						__( 'Edit: %s' ),
+						element.name.trim().length ? element.name : value
+					) }
+					style={ { padding: 0 } }
+				>
+					<IndicatorStyled colorValue={ value } />
+				</Button>
 				<FlexItem>
-					{ isEditing && ! canOnlyChangeValues ? (
+					{ ! canOnlyChangeValues ? (
 						<NameInput
 							label={
 								isGradient
@@ -244,23 +235,30 @@ function Option< T extends Color | Gradient >( {
 						</NameContainer>
 					) }
 				</FlexItem>
-				{ isEditing && ! canOnlyChangeValues && (
+				{ ! canOnlyChangeValues && (
 					<FlexItem>
 						<RemoveButton
 							size="small"
 							icon={ lineSolid }
-							label={ __( 'Remove color' ) }
+							label={ sprintf(
+								// translators: %s is a color or gradient name, e.g. "Red".
+								__( 'Remove color: %s' ),
+								element.name.trim().length
+									? element.name
+									: value
+							) }
 							onClick={ onRemove }
 						/>
 					</FlexItem>
 				) }
 			</HStack>
-			{ isEditing && (
+			{ isEditingColor && (
 				<ColorPickerPopover
 					isGradient={ isGradient }
 					onChange={ onChange }
 					element={ element }
 					popoverProps={ popoverProps }
+					onClose={ () => setIsEditingColor( false ) }
 				/>
 			) }
 		</PaletteItem>
@@ -270,12 +268,11 @@ function Option< T extends Color | Gradient >( {
 function PaletteEditListView< T extends Color | Gradient >( {
 	elements,
 	onChange,
-	editingElement,
-	setEditingElement,
 	canOnlyChangeValues,
 	slugPrefix,
 	isGradient,
 	popoverProps,
+	addColorRef,
 }: PaletteEditListViewProps< T > ) {
 	// When unmounting the component if there are empty elements (the user did not complete the insertion) clean them.
 	const elementsReference = useRef< typeof elements >();
@@ -294,11 +291,6 @@ function PaletteEditListView< T extends Color | Gradient >( {
 						canOnlyChangeValues={ canOnlyChangeValues }
 						key={ index }
 						element={ element }
-						onStartEditing={ () => {
-							if ( editingElement !== index ) {
-								setEditingElement( index );
-							}
-						} }
 						onChange={ ( newElement ) => {
 							debounceOnChange(
 								elements.map(
@@ -312,7 +304,6 @@ function PaletteEditListView< T extends Color | Gradient >( {
 							);
 						} }
 						onRemove={ () => {
-							setEditingElement( null );
 							const newElements = elements.filter(
 								( _currentElement, currentIndex ) => {
 									if ( currentIndex === index ) {
@@ -324,12 +315,7 @@ function PaletteEditListView< T extends Color | Gradient >( {
 							onChange(
 								newElements.length ? newElements : undefined
 							);
-						} }
-						isEditing={ index === editingElement }
-						onStopEditing={ () => {
-							if ( index === editingElement ) {
-								setEditingElement( null );
-							}
+							addColorRef.current?.focus();
 						} }
 						slugPrefix={ slugPrefix }
 						popoverProps={ popoverProps }
@@ -408,6 +394,8 @@ export function PaletteEdit( {
 		[ isGradient, elements ]
 	);
 
+	const addColorRef = useRef< HTMLButtonElement | null >( null );
+
 	return (
 		<PaletteEditStyles>
 			<HStack>
@@ -428,6 +416,7 @@ export function PaletteEdit( {
 					) }
 					{ ! canOnlyChangeValues && (
 						<Button
+							ref={ addColorRef }
 							size="small"
 							isPressed={ isAdding }
 							icon={ plus }
@@ -551,11 +540,10 @@ export function PaletteEdit( {
 							elements={ elements }
 							// @ts-expect-error TODO: Don't know how to resolve
 							onChange={ onChange }
-							editingElement={ editingElement }
-							setEditingElement={ setEditingElement }
 							slugPrefix={ slugPrefix }
 							isGradient={ isGradient }
 							popoverProps={ popoverProps }
+							addColorRef={ addColorRef }
 						/>
 					) }
 					{ ! isEditing && editingElement !== null && (

diff --git a/packages/components/src/palette-edit/test/index.tsx b/packages/components/src/palette-edit/test/index.tsx
@@ -306,7 +306,7 @@ describe( 'PaletteEdit', () => {
 		await click( screen.getByRole( 'button', { name: 'Edit: Primary' } ) );
 		await click(
 			screen.getByRole( 'button', {
-				name: 'Remove color',
+				name: 'Remove color: Primary',
 			} )
 		);
 
@@ -337,9 +337,7 @@ describe( 'PaletteEdit', () => {
 			} )
 		);
 		await click( screen.getByRole( 'button', { name: 'Edit: Primary' } ) );
-		const nameInput = screen.getByRole( 'textbox', {
-			name: 'Color name',
-		} );
+		const nameInput = screen.getByDisplayValue( 'Primary' );
 
 		await clearInput( nameInput as HTMLInputElement );
 

diff --git a/packages/components/src/palette-edit/types.ts b/packages/components/src/palette-edit/types.ts
@@ -116,11 +116,8 @@ export type OptionProps< T extends Color | Gradient > = {
 	onChange: ( newElement: T ) => void;
 	isGradient: T extends Gradient ? true : false;
 	canOnlyChangeValues: PaletteEditProps[ 'canOnlyChangeValues' ];
-	isEditing: boolean;
 	key: Key;
 	onRemove: MouseEventHandler< HTMLButtonElement >;
-	onStartEditing: () => void;
-	onStopEditing: () => void;
 	popoverProps?: PaletteEditProps[ 'popoverProps' ];
 	slugPrefix: string;
 };
@@ -130,6 +127,7 @@ export type PaletteEditListViewProps< T extends Color | Gradient > = {
 	onChange: ( newElements?: T[] ) => void;
 	isGradient: T extends Gradient ? true : false;
 	canOnlyChangeValues: PaletteEditProps[ 'canOnlyChangeValues' ];
+	addColorRef: React.RefObject< HTMLButtonElement >;
 	editingElement?: EditingElement;
 	popoverProps?: PaletteEditProps[ 'popoverProps' ];
 	setEditingElement: ( newEditingElement?: EditingElement ) => void;