Button: Stabilize __experimentalIsFocusable prop (WordPress#62282)

* Button: Stabilize `__experimentalIsFocusable` prop * Add changelog * Tweak changelog Co-authored-by: Marin Atanasov <[email protected]> * Replace `__experimentalIsFocusable` with `accessibleWhenDisabled` (WordPress#63107) * Replace `__experimentalIsFocusable` with `accessibleWhenDisabled` * Replace a few more remaining instances * Revert "Replace a few more remaining instances" This reverts commit 406ef77. --------- Co-authored-by: Marin Atanasov <[email protected]> * Fix new violation --------- Co-authored-by: mirka <[email protected]> Co-authored-by: tyxla <[email protected]> Co-authored-by: stokesman <[email protected]>
carstingaxion · Jul 18, 2024 · ed021f7 · ed021f7
1 parent dbaf598
commit ed021f7
Show file tree

Hide file tree

Showing 48 changed files with 119 additions and 77 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -143,9 +143,9 @@ const restrictedSyntax = [
 const restrictedSyntaxComponents = [
 	{
 		selector:
-			'JSXOpeningElement[name.name="Button"]:not(:has(JSXAttribute[name.name="__experimentalIsFocusable"])) JSXAttribute[name.name="disabled"]',
+			'JSXOpeningElement[name.name="Button"]:not(:has(JSXAttribute[name.name="accessibleWhenDisabled"])) JSXAttribute[name.name="disabled"]',
 		message:
-			'`disabled` used without the `__experimentalIsFocusable` prop. Disabling a control without maintaining focusability can cause accessibility issues, by hiding their presence from screen reader users, or preventing focus from returning to a trigger element. (Ignore this error if you truly mean to disable.)',
+			'`disabled` used without the `accessibleWhenDisabled` prop. Disabling a control without maintaining focusability can cause accessibility issues, by hiding their presence from screen reader users, or preventing focus from returning to a trigger element. (Ignore this error if you truly mean to disable.)',
 	},
 ];
 

diff --git a/packages/block-directory/src/components/downloadable-block-list-item/index.js b/packages/block-directory/src/components/downloadable-block-list-item/index.js
@@ -96,7 +96,7 @@ function DownloadableBlockListItem( { composite, item, onClick } ) {
 		<CompositeItem
 			render={
 				<Button
-					__experimentalIsFocusable
+					accessibleWhenDisabled
 					type="button"
 					role="option"
 					className="block-directory-downloadable-block-list-item"

diff --git a/packages/block-directory/src/plugins/get-install-missing/install-button.js b/packages/block-directory/src/plugins/get-install-missing/install-button.js
@@ -42,7 +42,7 @@ export default function InstallButton( { attributes, block, clientId } ) {
 					}
 				} )
 			}
-			__experimentalIsFocusable
+			accessibleWhenDisabled
 			disabled={ isInstallingBlock }
 			isBusy={ isInstallingBlock }
 			variant="primary"

diff --git a/packages/block-editor/src/components/block-mover/button.js b/packages/block-editor/src/components/block-mover/button.js
@@ -143,7 +143,7 @@ const BlockMoverButton = forwardRef(
 					{ ...props }
 					onClick={ isDisabled ? null : onClick }
 					disabled={ isDisabled }
-					__experimentalIsFocusable
+					accessibleWhenDisabled
 				/>
 				<VisuallyHidden id={ descriptionId }>
 					{ getBlockMoverDescription(

diff --git a/packages/block-editor/src/components/block-pattern-setup/setup-toolbar.js b/packages/block-editor/src/components/block-pattern-setup/setup-toolbar.js
@@ -35,14 +35,14 @@ const CarouselNavigation = ( {
 			label={ __( 'Previous pattern' ) }
 			onClick={ handlePrevious }
 			disabled={ activeSlide === 0 }
-			__experimentalIsFocusable
+			accessibleWhenDisabled
 		/>
 		<Button
 			icon={ chevronRight }
 			label={ __( 'Next pattern' ) }
 			onClick={ handleNext }
 			disabled={ activeSlide === totalSlides - 1 }
-			__experimentalIsFocusable
+			accessibleWhenDisabled
 		/>
 	</div>
 );

diff --git a/packages/block-editor/src/components/block-patterns-paging/index.js b/packages/block-editor/src/components/block-patterns-paging/index.js
@@ -42,7 +42,7 @@ export default function Pagination( {
 							onClick={ () => changePage( 1 ) }
 							disabled={ currentPage === 1 }
 							aria-label={ __( 'First page' ) }
-							__experimentalIsFocusable
+							accessibleWhenDisabled
 						>
 							<span>«</span>
 						</Button>
@@ -51,7 +51,7 @@ export default function Pagination( {
 							onClick={ () => changePage( currentPage - 1 ) }
 							disabled={ currentPage === 1 }
 							aria-label={ __( 'Previous page' ) }
-							__experimentalIsFocusable
+							accessibleWhenDisabled
 						>
 							<span>‹</span>
 						</Button>
@@ -74,7 +74,7 @@ export default function Pagination( {
 							onClick={ () => changePage( currentPage + 1 ) }
 							disabled={ currentPage === numPages }
 							aria-label={ __( 'Next page' ) }
-							__experimentalIsFocusable
+							accessibleWhenDisabled
 						>
 							<span>›</span>
 						</Button>
@@ -84,7 +84,7 @@ export default function Pagination( {
 							disabled={ currentPage === numPages }
 							aria-label={ __( 'Last page' ) }
 							size="default"
-							__experimentalIsFocusable
+							accessibleWhenDisabled
 						>
 							<span>»</span>
 						</Button>

diff --git a/packages/block-editor/src/components/link-control/link-preview.js b/packages/block-editor/src/components/link-control/link-preview.js
@@ -166,7 +166,7 @@ export default function LinkPreview( {
 						isEmptyURL || showIconLabels ? '' : ': ' + value.url
 					) }
 					ref={ ref }
-					__experimentalIsFocusable
+					accessibleWhenDisabled
 					disabled={ isEmptyURL }
 					size="compact"
 				/>

diff --git a/packages/block-library/src/page-list/convert-to-links-modal.js b/packages/block-library/src/page-list/convert-to-links-modal.js
@@ -36,7 +36,7 @@ export function ConvertToLinksModal( { onClick, onClose, disabled } ) {
 				</Button>
 				<Button
 					variant="primary"
-					__experimentalIsFocusable
+					accessibleWhenDisabled
 					disabled={ disabled }
 					onClick={ onClick }
 				>

diff --git a/packages/block-library/src/page-list/edit.js b/packages/block-library/src/page-list/edit.js
@@ -338,7 +338,7 @@ export default function PageListEdit( {
 						<p>{ convertDescription }</p>
 						<Button
 							variant="primary"
-							__experimentalIsFocusable
+							accessibleWhenDisabled
 							disabled={ ! hasResolvedPages }
 							onClick={ convertToNavigationLinks }
 						>

diff --git a/packages/block-library/src/template-part/edit/title-modal.js b/packages/block-library/src/template-part/edit/title-modal.js
@@ -43,7 +43,7 @@ export default function TitleModal( { areaLabel, onClose, onSubmit } ) {
 						<Button
 							variant="primary"
 							type="submit"
-							__experimentalIsFocusable
+							accessibleWhenDisabled
 							disabled={ ! title.length }
 						>
 							{ __( 'Create' ) }

diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md
@@ -12,6 +12,7 @@
 ### Enhancements
 
 -   `ToolbarButton`: Deprecate `isDisabled` prop and merge with `disabled` ([#63101](https://github.com/WordPress/gutenberg/pull/63101)).
+-   `Button`: Stabilize `__experimentalIsFocusable` prop as `accessibleWhenDisabled` ([#62282](https://github.com/WordPress/gutenberg/pull/62282)).
 -   `ToolbarButton`: Always keep focusable when disabled ([#63102](https://github.com/WordPress/gutenberg/pull/63102)).
 
 ### Internal

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

diff --git a/packages/components/src/button/README.md b/packages/components/src/button/README.md
@@ -115,6 +115,17 @@ The presence of a `href` prop determines whether an `anchor` element is rendered
 
 Props not included in this set will be applied to the `a` or `button` element.
 
+#### `accessibleWhenDisabled`: `boolean`
+
+Whether to keep the button focusable when disabled.
+
+In most cases, it is recommended to set this to `true`. Disabling a control without maintaining focusability can cause accessibility issues, by hiding their presence from screen reader users, or by preventing focus from returning to a trigger element.
+
+Learn more about the [focusability of disabled controls](https://www.w3.org/WAI/ARIA/apg/practices/keyboard-interface/#focusabilityofdisabledcontrols) in the WAI-ARIA Authoring Practices Guide.
+
+-   Required: No
+-   Default: `false`
+
 #### `children`: `ReactNode`
 
 The button's children.
@@ -137,6 +148,8 @@ An accessible description for the button.
 
 Whether the button is disabled. If `true`, this will force a `button` element to be rendered, even when an `href` is given.
 
+In most cases, it is recommended to also set the `accessibleWhenDisabled` prop to `true`.
+
 -   Required: No
 
 #### `href`: `string`

diff --git a/packages/components/src/button/index.tsx b/packages/components/src/button/index.tsx
@@ -29,6 +29,7 @@ import { positionToPlacement } from '../popover/utils';
 const disabledEventsOnDisabledButton = [ 'onMouseDown', 'onClick' ] as const;
 
 function useDeprecatedProps( {
+	__experimentalIsFocusable,
 	isDefault,
 	isPrimary,
 	isSecondary,
@@ -43,7 +44,8 @@ function useDeprecatedProps( {
 	let computedSize = size;
 	let computedVariant = variant;
 
-	const newProps: { 'aria-pressed'?: boolean } = {
+	const newProps = {
+		accessibleWhenDisabled: __experimentalIsFocusable,
 		// @todo Mark `isPressed` as deprecated
 		'aria-pressed': isPressed,
 	};
@@ -91,6 +93,7 @@ export function UnforwardedButton(
 ) {
 	const {
 		__next40pxDefaultSize,
+		accessibleWhenDisabled,
 		isBusy,
 		isDestructive,
 		className,
@@ -106,7 +109,6 @@ export function UnforwardedButton(
 		size = 'default',
 		text,
 		variant,
-		__experimentalIsFocusable: isFocusable,
 		describedBy,
 		...buttonOrAnchorProps
 	} = useDeprecatedProps( props );
@@ -159,7 +161,7 @@ export function UnforwardedButton(
 		'has-icon': !! icon,
 	} );
 
-	const trulyDisabled = disabled && ! isFocusable;
+	const trulyDisabled = disabled && ! accessibleWhenDisabled;
 	const Tag = href !== undefined && ! trulyDisabled ? 'a' : 'button';
 	const buttonProps: ComponentPropsWithoutRef< 'button' > =
 		Tag === 'button'
@@ -177,7 +179,7 @@ export function UnforwardedButton(
 	const disableEventProps: {
 		[ key: string ]: ( event: MouseEvent ) => void;
 	} = {};
-	if ( disabled && isFocusable ) {
+	if ( disabled && accessibleWhenDisabled ) {
 		// In this case, the button will be disabled, but still focusable and
 		// perceivable by screen reader users.
 		buttonProps[ 'aria-disabled' ] = true;

diff --git a/packages/components/src/button/stories/e2e/index.story.tsx b/packages/components/src/button/stories/e2e/index.story.tsx
@@ -45,7 +45,7 @@ export const VariantStates: StoryFn< typeof Button > = (
 						{ ...props }
 						variant={ variant }
 						disabled
-						__experimentalIsFocusable
+						accessibleWhenDisabled
 					/>
 					<Button { ...props } variant={ variant } isBusy />
 					<Button { ...props } variant={ variant } isDestructive />

diff --git a/packages/components/src/button/test/index.tsx b/packages/components/src/button/test/index.tsx
@@ -242,7 +242,7 @@ describe( 'Button', () => {
 		} );
 
 		it( 'should add only aria-disabled attribute when disabled and isFocusable are true', () => {
-			render( <Button disabled __experimentalIsFocusable /> );
+			render( <Button disabled accessibleWhenDisabled /> );
 			const button = screen.getByRole( 'button' );
 
 			expect( button ).toBeEnabled();
@@ -619,6 +619,15 @@ describe( 'Button', () => {
 				'mixed'
 			);
 		} );
+
+		it( 'should not break when the legacy __experimentalIsFocusable prop is passed', () => {
+			// eslint-disable-next-line no-restricted-syntax
+			render( <Button disabled __experimentalIsFocusable /> );
+			const button = screen.getByRole( 'button' );
+
+			expect( button ).toBeEnabled();
+			expect( button ).toHaveAttribute( 'aria-disabled' );
+		} );
 	} );
 
 	describe( 'static typing', () => {
@@ -638,7 +647,7 @@ describe( 'Button', () => {
 			{ /* @ts-expect-error */ }
 			<Button type="invalidtype" />
 			{ /* @ts-expect-error - although the runtime behavior will allow this to be an anchor, this is probably a mistake. */ }
-			<Button disabled __experimentalIsFocusable href="foo" />
+			<Button disabled accessibleWhenDisabled href="foo" />
 		</>;
 	} );
 } );
diff --git a/packages/components/src/button/types.ts b/packages/components/src/button/types.ts
@@ -25,6 +25,19 @@ type BaseButtonProps = {
 	 * @default false
 	 */
 	__next40pxDefaultSize?: boolean;
+	/**
+	 * Whether to keep the button focusable when disabled.
+	 *
+	 * In most cases, it is recommended to set this to `true`. Disabling a control without maintaining focusability
+	 * can cause accessibility issues, by hiding their presence from screen reader users,
+	 * or by preventing focus from returning to a trigger element.
+	 *
+	 * Learn more about the [focusability of disabled controls](https://www.w3.org/WAI/ARIA/apg/practices/keyboard-interface/#focusabilityofdisabledcontrols)
+	 * in the WAI-ARIA Authoring Practices Guide.
+	 *
+	 * @default false
+	 */
+	accessibleWhenDisabled?: boolean;
 	/**
 	 * The button's children.
 	 */
@@ -105,28 +118,24 @@ type BaseButtonProps = {
 	 * 'link' (the link button styles)
 	 */
 	variant?: 'primary' | 'secondary' | 'tertiary' | 'link';
-	/**
-	 * Whether to keep the button focusable when disabled.
-	 *
-	 * @default false
-	 */
-	__experimentalIsFocusable?: boolean;
 };
 
 type _ButtonProps = {
 	/**
-	 * Whether the button is disabled.
+	 * Whether the button is disabled. If `true`, this will force a `button` element
+	 * to be rendered, even when an `href` is given.
 	 *
-	 * If `true`, this will force a `button` element to be rendered, even when an `href` is given.
+	 * In most cases, it is recommended to also set the `accessibleWhenDisabled` prop to `true`.
 	 */
 	disabled?: boolean;
 };
 
 type AnchorProps = {
 	/**
-	 * Whether the button is disabled.
+	 * Whether the button is disabled. If `true`, this will force a `button` element
+	 * to be rendered, even when an `href` is given.
 	 *
-	 * If `true`, this will force a `button` element to be rendered, even when an `href` is given.
+	 * In most cases, it is recommended to also set the `accessibleWhenDisabled` prop to `true`.
 	 */
 	disabled?: false;
 	/**
@@ -140,6 +149,14 @@ type AnchorProps = {
 };
 
 export type DeprecatedButtonProps = {
+	/**
+	 * Whether to keep the button focusable when disabled.
+	 *
+	 * @default false
+	 * @deprecated Use the `accessibleWhenDisabled` prop instead.
+	 * @ignore
+	 */
+	__experimentalIsFocusable?: boolean;
 	/**
 	 * Gives the button a default style.
 	 *

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

diff --git a/packages/components/src/font-size-picker/index.tsx b/packages/components/src/font-size-picker/index.tsx
@@ -262,7 +262,7 @@ const UnforwardedFontSizePicker = (
 							<FlexItem>
 								<Button
 									disabled={ isDisabled }
-									__experimentalIsFocusable
+									accessibleWhenDisabled
 									onClick={ () => {
 										onChange?.( undefined );
 									} }

diff --git a/packages/components/src/range-control/index.tsx b/packages/components/src/range-control/index.tsx
@@ -327,7 +327,7 @@ function UnforwardedRangeControl(
 						<Button
 							className="components-range-control__reset"
 							// If the RangeControl itself is disabled, the reset button shouldn't be in the tab sequence.
-							__experimentalIsFocusable={ ! disabled }
+							accessibleWhenDisabled={ ! 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
@@ -74,7 +74,7 @@ function UnforwardedToolbarButton(
 						className
 					) }
 					isPressed={ isActive }
-					__experimentalIsFocusable
+					accessibleWhenDisabled
 					data-toolbar-item
 					{ ...extraProps }
 					{ ...restProps }

diff --git a/packages/dataviews/src/add-filter.tsx b/packages/dataviews/src/add-filter.tsx
@@ -44,7 +44,7 @@ function AddFilter(
 		<DropdownMenu
 			trigger={
 				<Button
-					__experimentalIsFocusable
+					accessibleWhenDisabled
 					size="compact"
 					className="dataviews-filters-button"
 					variant="tertiary"

diff --git a/packages/dataviews/src/item-actions.tsx b/packages/dataviews/src/item-actions.tsx
@@ -257,7 +257,7 @@ function CompactItemActions< Item >( {
 					size="compact"
 					icon={ moreVertical }
 					label={ __( 'Actions' ) }
-					__experimentalIsFocusable
+					accessibleWhenDisabled
 					disabled={ ! actions.length }
 					className="dataviews-all-actions-button"
 				/>