Add lint rule for inaccessible disabled Button (WordPress#62080)

* Add lint rule for inaccessible disabled `Button` * Exclude react native files * Include files in root `storybook` folder * Fix in Storybook editor playground (matches actual behavior) * Fix in install block button (is clearly a busy state) * Ignore in gallery image reordering buttons * Fix in LinkControl copy link button (aleviates confusion) * Ignore in edit-site pagination buttons (not confusing, and useful) * Fix in enable custom fields (is clearly a busy state) * Fix in DataViews list view Empty action menu should still be perceivable to aleviate confusion, and does not clutter tab order due to Composite use. * Fix in DataViews CompactItemActions (is dropdown trigger) * Fix in template part title modal disabled and aria-disabled are set with an identical condition, which doesn't make sense but signals the intent to keep it focusable. * Fix in PageList block (should be perceivable, esp. because the description is always there) * Fix in ConvertToLinksModal button (should be perceivable, and doesn't clutter tab order) * Fix in edit-site "Apply globally" button (should be perceivable, and doesn't clutter tab order) * Fix in edit-site nav menu rename modal (should be perceivable to signal that input is invalid) * Fix in RevisionsButtons (button is never visible when `areStylesEqual`) * Fix in RevisionsButtons (contains important info and should be perceivable) * Fix in GlobalStylesSidebar (should be perceivable) * Fix in PostPublishPanel cancel button (can cause focus loss) * Fix in PostPreviewButton (should be perceivable) * Defer decision in ButtonBlockAppender * Fix in reusable blocks import form (should be perceivable, can cause focus loss) * Adapt test for PostPreviewButton * Improve rigidity of accessible disabled detection in test * Add disable reason for ButtonBlockAppender Co-authored-by: mirka <0mirka00@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: DaniGuardiola <daniguardiola@git.wordpress.org>
patil-vipul · Jun 17, 2024 · 73545b8 · 73545b8
1 parent 62e5f67
commit 73545b8
Show file tree

Hide file tree

Showing 21 changed files with 64 additions and 4 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -253,6 +253,24 @@ module.exports = {
 				],
 			},
 		},
+		{
+			files: [
+				'packages/*/src/**/*.[tj]s?(x)',
+				'storybook/stories/**/*.[tj]s?(x)',
+			],
+			excludedFiles: [ '**/*.native.js' ],
+			rules: {
+				'no-restricted-syntax': [
+					'error',
+					{
+						selector:
+							'JSXOpeningElement[name.name="Button"]:not(:has(JSXAttribute[name.name="__experimentalIsFocusable"])) 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.)',
+					},
+				],
+			},
+		},
 		{
 			files: [
 				// Components package.

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,6 +42,7 @@ export default function InstallButton( { attributes, block, clientId } ) {
 					}
 				} )
 			}
+			__experimentalIsFocusable
 			disabled={ isInstallingBlock }
 			isBusy={ isInstallingBlock }
 			variant="primary"

diff --git a/packages/block-editor/src/components/button-block-appender/index.js b/packages/block-editor/src/components/button-block-appender/index.js
@@ -60,6 +60,8 @@ function ButtonBlockAppender(
 						onClick={ onToggle }
 						aria-haspopup={ isToggleButton ? 'true' : undefined }
 						aria-expanded={ isToggleButton ? isOpen : undefined }
+						// Disable reason: There shouldn't be a case where this button is disabled but not visually hidden.
+						// eslint-disable-next-line no-restricted-syntax
 						disabled={ disabled }
 						label={ label }
 					>

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

diff --git a/packages/block-library/src/gallery/v1/gallery-image.js b/packages/block-library/src/gallery/v1/gallery-image.js
@@ -222,13 +222,17 @@ class GalleryImage extends Component {
 						onClick={ isFirstItem ? undefined : onMoveBackward }
 						label={ __( 'Move image backward' ) }
 						aria-disabled={ isFirstItem }
+						// Disable reason: Truly disable when image is not selected.
+						// eslint-disable-next-line no-restricted-syntax
 						disabled={ ! isSelected }
 					/>
 					<Button
 						icon={ chevronRight }
 						onClick={ isLastItem ? undefined : onMoveForward }
 						label={ __( 'Move image forward' ) }
 						aria-disabled={ isLastItem }
+						// Disable reason: Truly disable when image is not selected.
+						// eslint-disable-next-line no-restricted-syntax
 						disabled={ ! isSelected }
 					/>
 				</ButtonGroup>
@@ -237,12 +241,16 @@ class GalleryImage extends Component {
 						icon={ edit }
 						onClick={ this.onEdit }
 						label={ __( 'Replace image' ) }
+						// Disable reason: Truly disable when image is not selected.
+						// eslint-disable-next-line no-restricted-syntax
 						disabled={ ! isSelected }
 					/>
 					<Button
 						icon={ closeSmall }
 						onClick={ onRemove }
 						label={ __( 'Remove image' ) }
+						// Disable reason: Truly disable when image is not selected.
+						// eslint-disable-next-line no-restricted-syntax
 						disabled={ ! isSelected }
 					/>
 				</ButtonGroup>

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,6 +36,7 @@ export function ConvertToLinksModal( { onClick, onClose, disabled } ) {
 				</Button>
 				<Button
 					variant="primary"
+					__experimentalIsFocusable
 					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,6 +338,7 @@ export default function PageListEdit( {
 						<p>{ convertDescription }</p>
 						<Button
 							variant="primary"
+							__experimentalIsFocusable
 							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,8 +43,8 @@ export default function TitleModal( { areaLabel, onClose, onSubmit } ) {
 						<Button
 							variant="primary"
 							type="submit"
+							__experimentalIsFocusable
 							disabled={ ! title.length }
-							aria-disabled={ ! title.length }
 						>
 							{ __( 'Create' ) }
 						</Button>

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

diff --git a/packages/dataviews/src/view-list.tsx b/packages/dataviews/src/view-list.tsx
@@ -255,6 +255,7 @@ function ListItem< Item extends AnyItem >( {
 												size="compact"
 												icon={ moreVertical }
 												label={ __( 'Actions' ) }
+												__experimentalIsFocusable
 												disabled={ ! actions.length }
 												onKeyDown={ ( event: {
 													key: string;

diff --git a/packages/edit-post/src/components/preferences-modal/enable-custom-fields.js b/packages/edit-post/src/components/preferences-modal/enable-custom-fields.js
@@ -42,6 +42,7 @@ export function CustomFieldsConfirmation( { willEnable } ) {
 				className="edit-post-preferences-modal__custom-fields-confirmation-button"
 				variant="secondary"
 				isBusy={ isReloading }
+				__experimentalIsFocusable
 				disabled={ isReloading }
 				onClick={ () => {
 					setIsReloading( true );

diff --git a/packages/edit-site/src/components/global-styles-sidebar/index.js b/packages/edit-site/src/components/global-styles-sidebar/index.js
@@ -152,6 +152,7 @@ export default function GlobalStylesSidebar() {
 							isPressed={
 								isStyleBookOpened || isRevisionsStyleBookOpened
 							}
+							__experimentalIsFocusable
 							disabled={ shouldClearCanvasContainerView }
 							onClick={ toggleStyleBook }
 							size="compact"
@@ -162,6 +163,7 @@ export default function GlobalStylesSidebar() {
 							label={ __( 'Revisions' ) }
 							icon={ backup }
 							onClick={ toggleRevisions }
+							__experimentalIsFocusable
 							disabled={ ! hasRevisions }
 							isPressed={
 								isRevisionsOpened || isRevisionsStyleBookOpened

diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js b/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js
@@ -163,6 +163,7 @@ function RevisionsButtons( {
 					>
 						<Button
 							className="edit-site-global-styles-screen-revisions__revision-button"
+							__experimentalIsFocusable
 							disabled={ isSelected }
 							onClick={ () => {
 								onChange( revision );
@@ -219,7 +220,6 @@ function RevisionsButtons( {
 								</p>
 							) : (
 								<Button
-									disabled={ areStylesEqual }
 									size="compact"
 									variant="primary"
 									className="edit-site-global-styles-screen-revisions__apply-button"

diff --git a/packages/edit-site/src/components/pagination/index.js b/packages/edit-site/src/components/pagination/index.js
@@ -46,6 +46,8 @@ export default function Pagination( {
 				<Button
 					variant={ buttonVariant }
 					onClick={ () => changePage( 1 ) }
+					// Disable reason: Would not cause confusion, and allows quicker access to a relevant nav button.
+					// eslint-disable-next-line no-restricted-syntax
 					disabled={ disabled || currentPage === 1 }
 					aria-label={ __( 'First page' ) }
 				>
@@ -54,6 +56,8 @@ export default function Pagination( {
 				<Button
 					variant={ buttonVariant }
 					onClick={ () => changePage( currentPage - 1 ) }
+					// Disable reason: Would not cause confusion, and allows quicker access to a relevant nav button.
+					// eslint-disable-next-line no-restricted-syntax
 					disabled={ disabled || currentPage === 1 }
 					aria-label={ __( 'Previous page' ) }
 				>
@@ -72,6 +76,8 @@ export default function Pagination( {
 				<Button
 					variant={ buttonVariant }
 					onClick={ () => changePage( currentPage + 1 ) }
+					// Disable reason: Would not cause confusion, and allows quicker access to a relevant nav button.
+					// eslint-disable-next-line no-restricted-syntax
 					disabled={ disabled || currentPage === numPages }
 					aria-label={ __( 'Next page' ) }
 				>
@@ -80,6 +86,8 @@ export default function Pagination( {
 				<Button
 					variant={ buttonVariant }
 					onClick={ () => changePage( numPages ) }
+					// Disable reason: Would not cause confusion, and allows quicker access to a relevant nav button.
+					// eslint-disable-next-line no-restricted-syntax
 					disabled={ disabled || currentPage === numPages }
 					aria-label={ __( 'Last page' ) }
 				>

diff --git a/packages/edit-site/src/components/sidebar-navigation-screen-navigation-menu/rename-modal.js b/packages/edit-site/src/components/sidebar-navigation-screen-navigation-menu/rename-modal.js
@@ -43,6 +43,7 @@ export default function RenameModal( { menuTitle, onClose, onSave } ) {
 
 						<Button
 							__next40pxDefaultSize
+							__experimentalIsFocusable
 							disabled={ ! isEditedMenuTitleValid }
 							variant="primary"
 							type="submit"

diff --git a/packages/edit-site/src/hooks/push-changes-to-global-styles/index.js b/packages/edit-site/src/hooks/push-changes-to-global-styles/index.js
@@ -339,6 +339,7 @@ function PushChangesToGlobalStylesControl( {
 			<Button
 				__next40pxDefaultSize
 				variant="secondary"
+				__experimentalIsFocusable
 				disabled={ changes.length === 0 }
 				onClick={ pushChanges }
 			>

diff --git a/packages/editor/src/components/post-preview-button/index.js b/packages/editor/src/components/post-preview-button/index.js
@@ -183,6 +183,7 @@ export default function PostPreviewButton( {
 			className={ className || 'editor-post-preview' }
 			href={ href }
 			target={ targetId }
+			__experimentalIsFocusable
 			disabled={ ! isSaveable }
 			onClick={ openPreviewWindow }
 			role={ role }

diff --git a/packages/editor/src/components/post-preview-button/test/index.js b/packages/editor/src/components/post-preview-button/test/index.js
@@ -139,12 +139,16 @@ describe( 'PostPreviewButton', () => {
 		).toBeInTheDocument();
 	} );
 
-	it( 'should be disabled if post is not saveable.', () => {
+	it( 'should be accessibly disabled if post is not saveable.', () => {
 		mockUseSelect( { isEditedPostSaveable: () => false } );
 
 		render( <PostPreviewButton /> );
 
-		expect( screen.getByRole( 'button' ) ).toBeDisabled();
+		expect( screen.getByRole( 'button' ) ).toBeEnabled();
+		expect( screen.getByRole( 'button' ) ).toHaveAttribute(
+			'aria-disabled',
+			'true'
+		);
 	} );
 
 	it( 'should not be disabled if post is saveable.', () => {
@@ -153,6 +157,10 @@ describe( 'PostPreviewButton', () => {
 		render( <PostPreviewButton /> );
 
 		expect( screen.getByRole( 'button' ) ).toBeEnabled();
+		expect( screen.getByRole( 'button' ) ).not.toHaveAttribute(
+			'aria-disabled',
+			'true'
+		);
 	} );
 
 	it( 'should set `href` to edited post preview link if specified.', () => {

diff --git a/packages/editor/src/components/post-publish-panel/index.js b/packages/editor/src/components/post-publish-panel/index.js
@@ -93,6 +93,7 @@ export class PostPublishPanel extends Component {
 							</div>
 							<div className="editor-post-publish-panel__header-cancel-button">
 								<Button
+									__experimentalIsFocusable
 									disabled={ isSavingNonPostEntityChanges }
 									onClick={ onClose }
 									variant="secondary"

diff --git a/packages/list-reusable-blocks/src/components/import-form/index.js b/packages/list-reusable-blocks/src/components/import-form/index.js
@@ -86,6 +86,7 @@ function ImportForm( { instanceId, onUpload } ) {
 			<Button
 				type="submit"
 				isBusy={ isLoading }
+				__experimentalIsFocusable
 				disabled={ ! file || isLoading }
 				variant="secondary"
 				className="list-reusable-blocks-import-form__button"

diff --git a/storybook/stories/playground/with-undo-redo/index.js b/storybook/stories/playground/with-undo-redo/index.js
@@ -49,12 +49,14 @@ export default function EditorWithUndoRedo() {
 					<Button
 						onClick={ undo }
 						disabled={ ! hasUndo }
+						__experimentalIsFocusable
 						icon={ undoIcon }
 						label="Undo"
 					/>
 					<Button
 						onClick={ redo }
 						disabled={ ! hasRedo }
+						__experimentalIsFocusable
 						icon={ redoIcon }
 						label="Redo"
 					/>