From 04837c47b2f290abf18a729bd6e0bbc73fcd30ac Mon Sep 17 00:00:00 2001
From: Dave Smith <getdavemail@gmail.com>
Date: Tue, 31 Jan 2023 15:11:41 +0000
Subject: [PATCH] Nav offcanvas restore click to edit (#47608)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Default to select block on click

* Remove dedicated “edit” button

* Only allow editing where valid

* Remove redundant component

* Allow all blocks to be edited

* Remove `selectBlockInCanvas` prop as no longer required
---
 .../off-canvas-editor/block-edit-button.js    | 27 -----------
 .../off-canvas-editor/block-select-button.js  | 13 +++++-
 .../src/components/off-canvas-editor/block.js | 45 ++-----------------
 .../components/off-canvas-editor/branch.js    |  3 --
 .../src/components/off-canvas-editor/index.js | 25 ++++-------
 .../edit/menu-inspector-controls.js           |  1 -
 .../navigation-inspector/navigation-menu.js   |  4 +-
 7 files changed, 24 insertions(+), 94 deletions(-)
 delete mode 100644 packages/block-editor/src/components/off-canvas-editor/block-edit-button.js

diff --git a/packages/block-editor/src/components/off-canvas-editor/block-edit-button.js b/packages/block-editor/src/components/off-canvas-editor/block-edit-button.js
deleted file mode 100644
index 6281134e8c95e3..00000000000000
--- a/packages/block-editor/src/components/off-canvas-editor/block-edit-button.js
+++ /dev/null
@@ -1,27 +0,0 @@
-/**
- * WordPress dependencies
- */
-import { edit } from '@wordpress/icons';
-import { Button } from '@wordpress/components';
-import { useDispatch } from '@wordpress/data';
-import { forwardRef } from '@wordpress/element';
-
-/**
- * Internal dependencies
- */
-import { store as blockEditorStore } from '../../store';
-
-export default forwardRef( function BlockEditButton(
-	{ clientId, ...props },
-	ref
-) {
-	const { selectBlock } = useDispatch( blockEditorStore );
-
-	const onClick = () => {
-		selectBlock( clientId );
-	};
-
-	return (
-		<Button { ...props } ref={ ref } icon={ edit } onClick={ onClick } />
-	);
-} );
diff --git a/packages/block-editor/src/components/off-canvas-editor/block-select-button.js b/packages/block-editor/src/components/off-canvas-editor/block-select-button.js
index 543823ab1ada25..51404f6a39dbc5 100644
--- a/packages/block-editor/src/components/off-canvas-editor/block-select-button.js
+++ b/packages/block-editor/src/components/off-canvas-editor/block-select-button.js
@@ -14,6 +14,7 @@ import {
 import { forwardRef } from '@wordpress/element';
 import { Icon, lockSmall as lock } from '@wordpress/icons';
 import { SPACE, ENTER } from '@wordpress/keycodes';
+import { sprintf, __ } from '@wordpress/i18n';
 
 /**
  * Internal dependencies
@@ -27,7 +28,7 @@ import { useBlockLock } from '../block-lock';
 function ListViewBlockSelectButton(
 	{
 		className,
-		block: { clientId },
+		block,
 		onClick,
 		onToggleExpanded,
 		tabIndex,
@@ -38,6 +39,7 @@ function ListViewBlockSelectButton(
 	},
 	ref
 ) {
+	const { clientId } = block;
 	const blockInformation = useBlockDisplayInformation( clientId );
 	const blockTitle = useBlockDisplayTitle( {
 		clientId,
@@ -60,6 +62,14 @@ function ListViewBlockSelectButton(
 		}
 	}
 
+	const editAriaLabel = blockInformation
+		? sprintf(
+				// translators: %s: The title of the block.
+				__( 'Edit %s block' ),
+				blockInformation.title
+		  )
+		: __( 'Edit' );
+
 	return (
 		<>
 			<Button
@@ -77,6 +87,7 @@ function ListViewBlockSelectButton(
 				draggable={ draggable }
 				href={ `#block-${ clientId }` }
 				aria-hidden={ true }
+				title={ editAriaLabel }
 			>
 				<ListViewExpander onClick={ onToggleExpanded } />
 				<BlockIcon
diff --git a/packages/block-editor/src/components/off-canvas-editor/block.js b/packages/block-editor/src/components/off-canvas-editor/block.js
index 2aafee3a6fa687..aa47b9d8bebea6 100644
--- a/packages/block-editor/src/components/off-canvas-editor/block.js
+++ b/packages/block-editor/src/components/off-canvas-editor/block.js
@@ -33,7 +33,6 @@ import {
 } from '../block-mover/button';
 import ListViewBlockContents from './block-contents';
 import BlockSettingsDropdown from '../block-settings-menu/block-settings-dropdown';
-import BlockEditButton from './block-edit-button';
 import { useListViewContext } from './context';
 import { getBlockPositionDescription } from './utils';
 import { store as blockEditorStore } from '../../store';
@@ -55,7 +54,6 @@ function ListViewBlock( {
 	isExpanded,
 	selectedClientIds,
 	preventAnnouncement,
-	selectBlockInCanvas,
 } ) {
 	const cellRef = useRef( null );
 	const [ isHovered, setIsHovered ] = useState( false );
@@ -186,15 +184,6 @@ function ListViewBlock( {
 		  )
 		: __( 'Options' );
 
-	const editAriaLabel = blockInformation
-		? sprintf(
-				// translators: %s: The title of the block.
-				__( 'Edit %s block' ),
-				blockInformation.title
-		  )
-		: __( 'Edit' );
-
-	const isEditable = !! block && block.name !== 'core/page-list-item';
 	const hasSiblings = siblingBlockCount > 0;
 	const hasRenderedMovers = showBlockMovers && hasSiblings;
 	const moverCellClassName = classnames(
@@ -207,17 +196,11 @@ function ListViewBlock( {
 		{ 'is-visible': isHovered || isFirstSelectedBlock }
 	);
 
-	const listViewBlockEditClassName = classnames(
-		'block-editor-list-view-block__menu-cell',
-		'block-editor-list-view-block__menu-edit',
-		{ 'is-visible': isHovered || isFirstSelectedBlock }
-	);
-
 	let colSpan;
 	if ( hasRenderedMovers ) {
-		colSpan = 2;
+		colSpan = 1;
 	} else if ( ! showBlockActions ) {
-		colSpan = 3;
+		colSpan = 2;
 	}
 
 	const classes = classnames( {
@@ -270,13 +253,7 @@ function ListViewBlock( {
 					<div className="block-editor-list-view-block__contents-container">
 						<ListViewBlockContents
 							block={ block }
-							onClick={
-								selectBlockInCanvas
-									? selectEditorBlock
-									: ( event ) => {
-											event.preventDefault();
-									  }
-							}
+							onClick={ selectEditorBlock }
 							onToggleExpanded={ toggleExpanded }
 							isSelected={ isSelected }
 							position={ position }
@@ -332,22 +309,6 @@ function ListViewBlock( {
 
 			{ showBlockActions && (
 				<>
-					<TreeGridCell
-						className={ listViewBlockEditClassName }
-						aria-selected={
-							!! isSelected || forceSelectionContentLock
-						}
-					>
-						{ ( props ) =>
-							isEditable && (
-								<BlockEditButton
-									{ ...props }
-									label={ editAriaLabel }
-									clientId={ clientId }
-								/>
-							)
-						}
-					</TreeGridCell>
 					<TreeGridCell
 						className={ listViewBlockSettingsClassName }
 						aria-selected={
diff --git a/packages/block-editor/src/components/off-canvas-editor/branch.js b/packages/block-editor/src/components/off-canvas-editor/branch.js
index 1c3b24133d917b..7652b204898ad9 100644
--- a/packages/block-editor/src/components/off-canvas-editor/branch.js
+++ b/packages/block-editor/src/components/off-canvas-editor/branch.js
@@ -97,7 +97,6 @@ function ListViewBranch( props ) {
 		isExpanded,
 		parentId,
 		shouldShowInnerBlocks = true,
-		selectBlockInCanvas,
 	} = props;
 
 	const isContentLocked = useSelect(
@@ -186,7 +185,6 @@ function ListViewBranch( props ) {
 								isExpanded={ shouldExpand }
 								listPosition={ nextPosition }
 								selectedClientIds={ selectedClientIds }
-								selectBlockInCanvas={ selectBlockInCanvas }
 							/>
 						) }
 						{ ! showBlock && (
@@ -207,7 +205,6 @@ function ListViewBranch( props ) {
 								isBranchSelected={ isSelectedBranch }
 								selectedClientIds={ selectedClientIds }
 								isExpanded={ isExpanded }
-								selectBlockInCanvas={ selectBlockInCanvas }
 							/>
 						) }
 					</AsyncModeProvider>
diff --git a/packages/block-editor/src/components/off-canvas-editor/index.js b/packages/block-editor/src/components/off-canvas-editor/index.js
index 6f1c5322696971..267357411a3eae 100644
--- a/packages/block-editor/src/components/off-canvas-editor/index.js
+++ b/packages/block-editor/src/components/off-canvas-editor/index.js
@@ -54,24 +54,16 @@ export const BLOCK_LIST_ITEM_HEIGHT = 36;
 /**
  * Show a hierarchical list of blocks.
  *
- * @param {Object}  props                     Components props.
- * @param {string}  props.id                  An HTML element id for the root element of ListView.
- * @param {Array}   props.blocks              Custom subset of block client IDs to be used instead of the default hierarchy.
- * @param {boolean} props.showBlockMovers     Flag to enable block movers
- * @param {boolean} props.isExpanded          Flag to determine whether nested levels are expanded by default.
- * @param {boolean} props.selectBlockInCanvas Flag to determine whether the list view should be a block selection mechanism.
- * @param {Object}  props.LeafMoreMenu        Optional more menu substitution.
- * @param {Object}  ref                       Forwarded ref
+ * @param {Object}  props                 Components props.
+ * @param {string}  props.id              An HTML element id for the root element of ListView.
+ * @param {Array}   props.blocks          Custom subset of block client IDs to be used instead of the default hierarchy.
+ * @param {boolean} props.showBlockMovers Flag to enable block movers
+ * @param {boolean} props.isExpanded      Flag to determine whether nested levels are expanded by default.
+ * @param {Object}  props.LeafMoreMenu    Optional more menu substitution.
+ * @param {Object}  ref                   Forwarded ref
  */
 function OffCanvasEditor(
-	{
-		id,
-		blocks,
-		showBlockMovers = false,
-		isExpanded = false,
-		selectBlockInCanvas = true,
-		LeafMoreMenu,
-	},
+	{ id, blocks, showBlockMovers = false, isExpanded = false, LeafMoreMenu },
 	ref
 ) {
 	const { clientIdsTree, draggedClientIds, selectedClientIds } =
@@ -227,7 +219,6 @@ function OffCanvasEditor(
 							selectedClientIds={ selectedClientIds }
 							isExpanded={ isExpanded }
 							shouldShowInnerBlocks={ shouldShowInnerBlocks }
-							selectBlockInCanvas={ selectBlockInCanvas }
 						/>
 						<TreeGridRow
 							level={ 1 }
diff --git a/packages/block-library/src/navigation/edit/menu-inspector-controls.js b/packages/block-library/src/navigation/edit/menu-inspector-controls.js
index edff6acb7928c7..7a12569b12e54b 100644
--- a/packages/block-library/src/navigation/edit/menu-inspector-controls.js
+++ b/packages/block-library/src/navigation/edit/menu-inspector-controls.js
@@ -54,7 +54,6 @@ const MainContent = ( {
 		<OffCanvasEditor
 			blocks={ clientIdsTree }
 			isExpanded={ true }
-			selectBlockInCanvas={ false }
 			LeafMoreMenu={ LeafMoreMenu }
 		/>
 	);
diff --git a/packages/edit-site/src/components/navigation-inspector/navigation-menu.js b/packages/edit-site/src/components/navigation-inspector/navigation-menu.js
index bb463de56dc7c5..cf70394e194bcd 100644
--- a/packages/edit-site/src/components/navigation-inspector/navigation-menu.js
+++ b/packages/edit-site/src/components/navigation-inspector/navigation-menu.js
@@ -56,7 +56,5 @@ export default function NavigationMenu( { innerBlocks } ) {
 		} );
 	}, [ updateBlockListSettings, innerBlocks ] );
 
-	return (
-		<OffCanvasEditor blocks={ innerBlocks } selectBlockInCanvas={ false } />
-	);
+	return <OffCanvasEditor blocks={ innerBlocks } />;
 }