Components: Fix inaccessible disabled Buttons (#62306)

* Components: Fix inaccessible disabled `Button`s * Fix in RangeControl * Fix in ToolbarButton * Fix unit test * List block: Try normalizing `disabled` * Revert "Fix in ToolbarButton" This reverts commit 2cea1eb. * Revert "List block: Try normalizing `disabled`" This reverts commit f95b0b2. * ToolbarButton: Defer fix * Revert "Fix unit test" This reverts commit ed03d97. * Add changelog * ToolbarButton: Add back focusable when not within ToolbarItem * Revert "Revert "Fix unit test"" This reverts commit 5f92fdc. Co-authored-by: mirka <[email protected]> Co-authored-by: ciampo <[email protected]>
WordPress · Jul 3, 2024 · 89bfae4 · 89bfae4
1 parent 14ee526
commit 89bfae4
Show file tree

Hide file tree

Showing 11 changed files with 28 additions and 6 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -397,6 +397,7 @@ module.exports = {
 				'no-restricted-syntax': [
 					'error',
 					...restrictedSyntax,
+					...restrictedSyntaxComponents,
 					{
 						selector:
 							':matches(Literal[value=/--wp-admin-theme-/],TemplateElement[value.cooked=/--wp-admin-theme-/])',

diff --git a/packages/block-editor/src/components/block-switcher/test/index.js b/packages/block-editor/src/components/block-switcher/test/index.js
@@ -134,19 +134,19 @@ describe( 'BlockSwitcher', () => {
 		expect( items[ 1 ] ).toHaveTextContent( headingBlockType.title );
 	} );
 
-	test( 'should render disabled block switcher when we have a single selected block without styles and we cannot remove it', () => {
+	test( 'should render accessibly disabled block switcher when we have a single selected block without styles and we cannot remove it', () => {
 		useSelect.mockImplementation( () => ( {
 			blocks: [ headingBlock1 ],
 			icon: copy,
 			hasBlockStyles: false,
 			canRemove: false,
 		} ) );
 		render( <BlockSwitcher clientIds={ [ headingBlock1.clientId ] } /> );
-		expect(
-			screen.getByRole( 'button', {
-				name: 'Block Name',
-			} )
-		).toBeDisabled();
+		const blockSwitcher = screen.getByRole( 'button', {
+			name: 'Block Name',
+		} );
+		expect( blockSwitcher ).toBeEnabled();
+		expect( blockSwitcher ).toHaveAttribute( 'aria-disabled', 'true' );
 	} );
 
 	test( 'should render message for no available transforms', async () => {

diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md
@@ -7,6 +7,7 @@
 -   `UnitControl`: Fix colors when disabled. ([#62970](https://github.com/WordPress/gutenberg/pull/62970))
 -   `useUpdateEffect`: Correctly track mounted state in strict mode. ([#62974](https://github.com/WordPress/gutenberg/pull/62974))
 -   `UnitControl`: Fix an issue where keyboard shortcuts unintentionally shift focus on Windows OS. ([#62988](https://github.com/WordPress/gutenberg/pull/62988))
+-   Fix inaccessibly disabled `Button`s ([#62306](https://github.com/WordPress/gutenberg/pull/62306)).
 
 ### Internal
 

diff --git a/packages/components/src/autocomplete/autocompleter-ui.tsx b/packages/components/src/autocomplete/autocompleter-ui.tsx
@@ -58,6 +58,7 @@ function ListBox( {
 					id={ `components-autocomplete-item-${ instanceId }-${ option.key }` }
 					role="option"
 					aria-selected={ index === selectedIndex }
+					__experimentalIsFocusable
 					disabled={ option.isDisabled }
 					className={ clsx(
 						'components-autocomplete__result',

diff --git a/packages/components/src/button/stories/e2e/index.story.tsx b/packages/components/src/button/stories/e2e/index.story.tsx
@@ -39,6 +39,7 @@ export const VariantStates: StoryFn< typeof Button > = (
 					key={ variant ?? 'undefined' }
 				>
 					<Button { ...props } variant={ variant } />
+					{ /* eslint-disable-next-line no-restricted-syntax */ }
 					<Button { ...props } variant={ variant } disabled />
 					<Button
 						{ ...props }

diff --git a/packages/components/src/button/test/index.tsx b/packages/components/src/button/test/index.tsx
@@ -235,6 +235,7 @@ describe( 'Button', () => {
 		} );
 
 		it( 'should add a disabled prop to the button', () => {
+			// eslint-disable-next-line no-restricted-syntax
 			render( <Button disabled /> );
 
 			expect( screen.getByRole( 'button' ) ).toBeDisabled();
@@ -536,6 +537,7 @@ describe( 'Button', () => {
 
 		it( 'should become a button again when disabled is supplied', () => {
 			// @ts-expect-error - a button should not have `href`
+			// eslint-disable-next-line no-restricted-syntax
 			render( <Button href="https://wordpress.org/" disabled /> );
 
 			expect( screen.getByRole( 'button' ) ).toBeVisible();
@@ -624,8 +626,12 @@ describe( 'Button', () => {
 			<Button href="foo" />
 			{ /* @ts-expect-error - `target` requires `href` */ }
 			<Button target="foo" />
+
+			{ /* eslint-disable no-restricted-syntax */ }
 			{ /* @ts-expect-error - `disabled` is only for buttons */ }
 			<Button href="foo" disabled />
+			{ /* eslint-enable no-restricted-syntax */ }
+
 			<Button href="foo" type="image/png" />
 			{ /* @ts-expect-error - if button, type must be submit/reset/button */ }
 			<Button type="image/png" />

diff --git a/packages/components/src/combobox-control/index.tsx b/packages/components/src/combobox-control/index.tsx
@@ -346,6 +346,8 @@ function ComboboxControl( props: ComboboxControlProps ) {
 								<Button
 									className="components-combobox-control__reset"
 									icon={ closeSmall }
+									// Disable reason: Focus returns to input field when reset is clicked.
+									// eslint-disable-next-line no-restricted-syntax
 									disabled={ ! value }
 									onClick={ handleOnReset }
 									label={ __( 'Reset' ) }

diff --git a/packages/components/src/dropdown-menu/index.tsx b/packages/components/src/dropdown-menu/index.tsx
@@ -199,6 +199,7 @@ function UnconnectedDropdownMenu( dropdownMenuProps: DropdownMenuProps ) {
 											? control.role
 											: 'menuitem'
 									}
+									__experimentalIsFocusable
 									disabled={ control.isDisabled }
 								>
 									{ control.title }

diff --git a/packages/components/src/form-token-field/token.tsx b/packages/components/src/form-token-field/token.tsx
@@ -74,6 +74,8 @@ export default function Token( {
 				className="components-form-token-field__remove-token"
 				icon={ closeSmall }
 				onClick={ ! disabled ? onClick : undefined }
+				// Disable reason: Even when FormTokenField itself is accessibly disabled, token reset buttons shouldn't be in the tab sequence.
+				// eslint-disable-next-line no-restricted-syntax
 				disabled={ disabled }
 				label={ messages.remove }
 				aria-describedby={ `components-form-token-field__token-text-${ instanceId }` }

diff --git a/packages/components/src/range-control/index.tsx b/packages/components/src/range-control/index.tsx
@@ -326,6 +326,8 @@ function UnforwardedRangeControl(
 					<ActionRightWrapper>
 						<Button
 							className="components-range-control__reset"
+							// If the RangeControl itself is disabled, the reset button shouldn't be in the tab sequence.
+							__experimentalIsFocusable={ ! disabled }
 							disabled={ disabled || value === undefined }
 							variant="secondary"
 							size="small"

diff --git a/packages/components/src/toolbar/toolbar-button/index.tsx b/packages/components/src/toolbar/toolbar-button/index.tsx
@@ -60,6 +60,7 @@ function UnforwardedToolbarButton(
 						className
 					) }
 					isPressed={ isActive }
+					__experimentalIsFocusable
 					disabled={ isDisabled }
 					data-toolbar-item
 					{ ...extraProps }
@@ -85,6 +86,10 @@ function UnforwardedToolbarButton(
 				<Button
 					label={ title }
 					isPressed={ isActive }
+					// TODO: Should be focusable disabled, but adding `__experimentalIsFocusable` will trigger a
+					// focus bug when ToolbarButton is disabled via the `disabled` prop.
+					// Must address first: https://github.com/WordPress/gutenberg/issues/63070
+					// eslint-disable-next-line no-restricted-syntax
 					disabled={ isDisabled }
 					{ ...toolbarItemProps }
 				>