From 39777ea12223ef2ab8f15cb42990676df1c13006 Mon Sep 17 00:00:00 2001
From: Andrea Fercia <a.fercia@gmail.com>
Date: Tue, 25 Jun 2024 19:12:35 +0200
Subject: [PATCH] Fix lock modal dialog accessibility and semantics (#62795)

* Use fieldset and legend instead of labeled group.

* Make lock options a nested list.

Co-authored-by: afercia <afercia@git.wordpress.org>
Co-authored-by: alexstine <alexstine@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
---
 .../src/components/block-lock/modal.js        | 177 ++++++++++--------
 .../src/components/block-lock/style.scss      |  12 +-
 2 files changed, 106 insertions(+), 83 deletions(-)

diff --git a/packages/block-editor/src/components/block-lock/modal.js b/packages/block-editor/src/components/block-lock/modal.js
index cfafa6c031bbd1..11a03c5aff4dd8 100644
--- a/packages/block-editor/src/components/block-lock/modal.js
+++ b/packages/block-editor/src/components/block-lock/modal.js
@@ -13,7 +13,6 @@ import {
 	ToggleControl,
 } from '@wordpress/components';
 import { lock as lockIcon, unlock as unlockIcon } from '@wordpress/icons';
-import { useInstanceId } from '@wordpress/compose';
 import { useDispatch, useSelect } from '@wordpress/data';
 import { getBlockType } from '@wordpress/blocks';
 
@@ -64,10 +63,6 @@ export default function BlockLockModal( { clientId, onClose } ) {
 	);
 	const { updateBlockAttributes } = useDispatch( blockEditorStore );
 	const blockInformation = useBlockDisplayInformation( clientId );
-	const instanceId = useInstanceId(
-		BlockLockModal,
-		'block-editor-block-lock-modal__options-title'
-	);
 
 	useEffect( () => {
 		setLock( {
@@ -90,11 +85,6 @@ export default function BlockLockModal( { clientId, onClose } ) {
 			overlayClassName="block-editor-block-lock-modal"
 			onRequestClose={ onClose }
 		>
-			<p>
-				{ __(
-					'Choose specific attributes to restrict or lock all available options.'
-				) }
-			</p>
 			<form
 				onSubmit={ ( event ) => {
 					event.preventDefault();
@@ -107,84 +97,107 @@ export default function BlockLockModal( { clientId, onClose } ) {
 					onClose();
 				} }
 			>
-				<div
-					role="group"
-					aria-labelledby={ instanceId }
-					className="block-editor-block-lock-modal__options"
-				>
-					<CheckboxControl
-						__nextHasNoMarginBottom
-						className="block-editor-block-lock-modal__options-title"
-						label={
-							<span id={ instanceId }>{ __( 'Lock all' ) }</span>
-						}
-						checked={ isAllChecked }
-						indeterminate={ isMixed }
-						onChange={ ( newValue ) =>
-							setLock( {
-								move: newValue,
-								remove: newValue,
-								...( allowsEditLocking
-									? { edit: newValue }
-									: {} ),
-							} )
-						}
-					/>
-					<ul className="block-editor-block-lock-modal__checklist">
-						{ allowsEditLocking && (
-							<li className="block-editor-block-lock-modal__checklist-item">
-								<CheckboxControl
-									__nextHasNoMarginBottom
-									label={ __( 'Restrict editing' ) }
-									checked={ !! lock.edit }
-									onChange={ ( edit ) =>
-										setLock( ( prevLock ) => ( {
-											...prevLock,
-											edit,
-										} ) )
-									}
-								/>
-								<Icon
-									className="block-editor-block-lock-modal__lock-icon"
-									icon={ lock.edit ? lockIcon : unlockIcon }
-								/>
-							</li>
+				<fieldset className="block-editor-block-lock-modal__options">
+					<legend>
+						{ __(
+							'Choose specific attributes to restrict or lock all available options.'
 						) }
-						<li className="block-editor-block-lock-modal__checklist-item">
+					</legend>
+					{ /*
+					 * Disable reason: The `list` ARIA role is redundant but
+					 * Safari+VoiceOver won't announce the list otherwise.
+					 */
+					/* eslint-disable jsx-a11y/no-redundant-roles */ }
+					<ul
+						role="list"
+						className="block-editor-block-lock-modal__checklist"
+					>
+						<li>
 							<CheckboxControl
 								__nextHasNoMarginBottom
-								label={ __( 'Disable movement' ) }
-								checked={ lock.move }
-								onChange={ ( move ) =>
-									setLock( ( prevLock ) => ( {
-										...prevLock,
-										move,
-									} ) )
+								className="block-editor-block-lock-modal__options-all"
+								label={ __( 'Lock all' ) }
+								checked={ isAllChecked }
+								indeterminate={ isMixed }
+								onChange={ ( newValue ) =>
+									setLock( {
+										move: newValue,
+										remove: newValue,
+										...( allowsEditLocking
+											? { edit: newValue }
+											: {} ),
+									} )
 								}
 							/>
-							<Icon
-								className="block-editor-block-lock-modal__lock-icon"
-								icon={ lock.move ? lockIcon : unlockIcon }
-							/>
-						</li>
-						<li className="block-editor-block-lock-modal__checklist-item">
-							<CheckboxControl
-								__nextHasNoMarginBottom
-								label={ __( 'Prevent removal' ) }
-								checked={ lock.remove }
-								onChange={ ( remove ) =>
-									setLock( ( prevLock ) => ( {
-										...prevLock,
-										remove,
-									} ) )
-								}
-							/>
-							<Icon
-								className="block-editor-block-lock-modal__lock-icon"
-								icon={ lock.remove ? lockIcon : unlockIcon }
-							/>
+							<ul
+								role="list"
+								className="block-editor-block-lock-modal__checklist"
+							>
+								{ allowsEditLocking && (
+									<li className="block-editor-block-lock-modal__checklist-item">
+										<CheckboxControl
+											__nextHasNoMarginBottom
+											label={ __( 'Restrict editing' ) }
+											checked={ !! lock.edit }
+											onChange={ ( edit ) =>
+												setLock( ( prevLock ) => ( {
+													...prevLock,
+													edit,
+												} ) )
+											}
+										/>
+										<Icon
+											className="block-editor-block-lock-modal__lock-icon"
+											icon={
+												lock.edit
+													? lockIcon
+													: unlockIcon
+											}
+										/>
+									</li>
+								) }
+								<li className="block-editor-block-lock-modal__checklist-item">
+									<CheckboxControl
+										__nextHasNoMarginBottom
+										label={ __( 'Disable movement' ) }
+										checked={ lock.move }
+										onChange={ ( move ) =>
+											setLock( ( prevLock ) => ( {
+												...prevLock,
+												move,
+											} ) )
+										}
+									/>
+									<Icon
+										className="block-editor-block-lock-modal__lock-icon"
+										icon={
+											lock.move ? lockIcon : unlockIcon
+										}
+									/>
+								</li>
+								<li className="block-editor-block-lock-modal__checklist-item">
+									<CheckboxControl
+										__nextHasNoMarginBottom
+										label={ __( 'Prevent removal' ) }
+										checked={ lock.remove }
+										onChange={ ( remove ) =>
+											setLock( ( prevLock ) => ( {
+												...prevLock,
+												remove,
+											} ) )
+										}
+									/>
+									<Icon
+										className="block-editor-block-lock-modal__lock-icon"
+										icon={
+											lock.remove ? lockIcon : unlockIcon
+										}
+									/>
+								</li>
+							</ul>
 						</li>
 					</ul>
+					{ /* eslint-enable jsx-a11y/no-redundant-roles */ }
 					{ hasTemplateLock && (
 						<ToggleControl
 							__nextHasNoMarginBottom
@@ -197,7 +210,7 @@ export default function BlockLockModal( { clientId, onClose } ) {
 							}
 						/>
 					) }
-				</div>
+				</fieldset>
 				<Flex
 					className="block-editor-block-lock-modal__actions"
 					justify="flex-end"
diff --git a/packages/block-editor/src/components/block-lock/style.scss b/packages/block-editor/src/components/block-lock/style.scss
index b61447dcf9dc92..0a43985a83847b 100644
--- a/packages/block-editor/src/components/block-lock/style.scss
+++ b/packages/block-editor/src/components/block-lock/style.scss
@@ -8,17 +8,27 @@
 	}
 }
 
+.block-editor-block-lock-modal__options {
+	margin-top: $grid-unit-20;
+
+	legend {
+		margin-bottom: $grid-unit-20;
+		padding: 0;
+	}
+}
+
 .block-editor-block-lock-modal__checklist {
 	margin: 0;
 }
 
-.block-editor-block-lock-modal__options-title {
+.block-editor-block-lock-modal__options-all {
 	padding: $grid-unit-15 0;
 
 	.components-checkbox-control__label {
 		font-weight: 600;
 	}
 }
+
 .block-editor-block-lock-modal__checklist-item {
 	display: flex;
 	justify-content: space-between;