Refactor the draggable component to avoid passing around unnecessary props

docs/designers-developers/developers/data/data-core-block-editor.md

@@ -756,6 +756,18 @@ _Returns_
 
 -   `boolean`: Whether the caret is within formatted text.
 
+<a name="isDraggingBlocks" href="#isDraggingBlocks">#</a> **isDraggingBlocks**
+
+Returns true if the user is dragging blocks, or false otherwise.
+
+_Parameters_
+
+-   _state_ `Object`: Global application state.
+
+_Returns_
+
+-   `boolean`: Whether user is dragging blocks.
+
 <a name="isFirstMultiSelectedBlock" href="#isFirstMultiSelectedBlock">#</a> **isFirstMultiSelectedBlock**
 
 Returns true if a multi-selection exists, and the block corresponding to the
@@ -1173,6 +1185,14 @@ _Returns_
 
 -   `Object`: Action object.
 
+<a name="startDraggingBlocks" href="#startDraggingBlocks">#</a> **startDraggingBlocks**
+
+Returns an action object used in signalling that the user has begun to drag blocks.
+
+_Returns_
+
+-   `Object`: Action object.
+
 <a name="startMultiSelect" href="#startMultiSelect">#</a> **startMultiSelect**
 
 Returns an action object used in signalling that a block multi-selection has started.
@@ -1189,6 +1209,14 @@ _Returns_
 
 -   `Object`: Action object.
 
+<a name="stopDraggingBlocks" href="#stopDraggingBlocks">#</a> **stopDraggingBlocks**
+
+Returns an action object used in signalling that the user has stopped dragging blocks.
+
+_Returns_
+
+-   `Object`: Action object.
+
 <a name="stopMultiSelect" href="#stopMultiSelect">#</a> **stopMultiSelect**
 
 Returns an action object used in signalling that block multi-selection stopped.

packages/block-editor/src/components/block-draggable/index.js

@@ -1,23 +1,73 @@
+/**
+ * External dependencies
+ */
+import { castArray } from 'lodash';
+
 /**
  * WordPress dependencies
  */
 import { Draggable } from '@wordpress/components';
-import { withSelect } from '@wordpress/data';
+import { useSelect, useDispatch } from '@wordpress/data';
+import { useEffect, useRef } from '@wordpress/element';
+
+const BlockDraggable = ( { children, clientIds } ) => {
+	const {
+		srcRootClientId,
+		index,
+		isDraggable,
+	} = useSelect( ( select ) => {
+		const {
+			getBlockIndex,
+			getBlockRootClientId,
+			getTemplateLock,
+		} = select( 'core/block-editor' );
+		const normalizedClientIds = castArray( clientIds );
+		const rootClientId = normalizedClientIds.length === 1 ? getBlockRootClientId( normalizedClientIds[ 0 ] ) : null;
+		const templateLock = rootClientId ? getTemplateLock( rootClientId ) : null;
+
+		return {
+			index: getBlockIndex( normalizedClientIds[ 0 ], rootClientId ),
+			srcRootClientId: rootClientId,
+			isDraggable: normalizedClientIds.length === 1 && 'all' !== templateLock,
+		};
+	}, [ clientIds ] );
+	const isDragging = useRef( false );
+	const { startDraggingBlocks, stopDraggingBlocks } = useDispatch( 'core/block-editor' );
 
-const BlockDraggable = ( { children, clientId, rootClientId, blockElementId, index, onDragStart, onDragEnd } ) => {
+	// Stop dragging blocks if the block draggable is unmounted
+	useEffect( () => {
+		return () => {
+			if ( isDragging.current ) {
+				stopDraggingBlocks();
+			}
+		};
+	}, [] );
+
+	if ( ! isDraggable ) {
+		return null;
+	}
+
+	const normalizedClientIds = castArray( clientIds );
+	const blockElementId = `block-${ normalizedClientIds[ 0 ] }`;
 	const transferData = {
 		type: 'block',
 		srcIndex: index,
-		srcRootClientId: rootClientId,
-		srcClientId: clientId,
+		srcClientId: normalizedClientIds[ 0 ],
+		srcRootClientId,
 	};
 
 	return (
 		<Draggable
 			elementId={ blockElementId }
 			transferData={ transferData }
-			onDragStart={ onDragStart }
-			onDragEnd={ onDragEnd }
+			onDragStart={ () => {
+				startDraggingBlocks();
+				isDragging.current = true;
+			} }
+			onDragEnd={ () => {
+				stopDraggingBlocks();
+				isDragging.current = false;
+			} }
 		>
 			{
 				( { onDraggableStart, onDraggableEnd } ) => {
@@ -31,11 +81,4 @@ const BlockDraggable = ( { children, clientId, rootClientId, blockElementId, ind
 	);
 };
 
-export default withSelect( ( select, { clientId } ) => {
-	const { getBlockIndex, getBlockRootClientId } = select( 'core/block-editor' );
-	const rootClientId = getBlockRootClientId( clientId );
-	return {
-		index: getBlockIndex( clientId, rootClientId ),
-		rootClientId,
-	};
-} )( BlockDraggable );
+export default BlockDraggable;

packages/block-editor/src/components/block-list/block.js

@@ -27,6 +27,7 @@ import { __, sprintf } from '@wordpress/i18n';
 import {
 	withDispatch,
 	withSelect,
+	useSelect,
 } from '@wordpress/data';
 import { withViewportMatch } from '@wordpress/viewport';
 import { compose, pure, ifCondition } from '@wordpress/compose';
@@ -76,9 +77,7 @@ function BlockListBlock( {
 	isTypingWithinBlock,
 	isCaretWithinFormattedText,
 	isEmptyDefaultBlock,
-	isMovable,
 	isParentOfSelectedBlock,
-	isDraggable,
 	isSelectionEnabled,
 	className,
 	name,
@@ -103,6 +102,14 @@ function BlockListBlock( {
 	setNavigationMode,
 	isMultiSelecting,
 } ) {
+	// In addition to withSelect, we should favor using useSelect in this component going forward
+	// to avoid leaking new props to the public API (editor.BlockListBlock filter)
+	const { isDraggingBlocks } = useSelect( ( select ) => {
+		return {
+			isDraggingBlocks: select( 'core/block-editor' ).isDraggingBlocks(),
+		};
+	} );
+
 	// Random state used to rerender the component if needed, ideally we don't need this
 	const [ , updateRerenderState ] = useState( {} );
 	const rerender = () => updateRerenderState( {} );
@@ -168,15 +175,6 @@ function BlockListBlock( {
 		}
 	} );
 
-	// Handling the dragging state
-	const [ isDragging, setBlockDraggingState ] = useState( false );
-	const onDragStart = () => {
-		setBlockDraggingState( true );
-	};
-	const onDragEnd = () => {
-		setBlockDraggingState( false );
-	};
-
 	// Handling the error state
 	const [ hasError, setErrorState ] = useState( false );
 	const onBlockError = () => setErrorState( true );
@@ -457,6 +455,7 @@ function BlockListBlock( {
 	);
 
 	const shouldRenderDropzone = shouldShowInsertionPoint;
+	const isDragging = isDraggingBlocks && ( isSelected || isPartOfMultiSelection );
 
 	// The wp-block className is important for editor styles.
 	// Generate the wrapper class names handling the different states of the block.
@@ -489,14 +488,7 @@ function BlockListBlock( {
 	const blockMover = (
 		<BlockMover
 			clientIds={ clientId }
-			blockElementId={ blockElementId }
 			isHidden={ ! isSelected }
-			isDraggable={
-				isDraggable !== false &&
-				( ! isPartOfMultiSelection && isMovable )
-			}
-			onDragStart={ onDragStart }
-			onDragEnd={ onDragEnd }
 			__experimentalOrientation={ moverDirection }
 		/>
 	);
@@ -701,7 +693,6 @@ const applyWithSelect = withSelect(
 			initialPosition: isSelected ? getSelectedBlocksInitialCaretPosition() : null,
 			isEmptyDefaultBlock:
 				name && isUnmodifiedDefaultBlock( { name, attributes } ),
-			isMovable: 'all' !== templateLock,
 			isLocked: !! templateLock,
 			isFocusMode: focusMode && isLargeViewport,
 			hasFixedToolbar: hasFixedToolbar && isLargeViewport,

packages/block-editor/src/components/block-list/index.js

@@ -218,7 +218,6 @@ class BlockList extends Component {
 			blockClientIds,
 			rootClientId,
 			__experimentalMoverDirection: moverDirection = 'vertical',
-			isDraggable,
 			selectedBlockClientId,
 			multiSelectedBlockClientIds,
 			hasMultiSelection,
@@ -250,7 +249,6 @@ class BlockList extends Component {
 								rootClientId={ rootClientId }
 								clientId={ clientId }
 								onSelectionStart={ this.onSelectionStart }
-								isDraggable={ isDraggable }
 								moverDirection={ moverDirection }
 								isMultiSelecting={ isMultiSelecting }
 								// This prop is explicitely computed and passed down

packages/block-editor/src/components/block-mover/index.js

@@ -19,7 +19,7 @@ import { withInstanceId, compose } from '@wordpress/compose';
  */
 import { getBlockMoverDescription } from './mover-description';
 import { leftArrow, rightArrow, upArrow, downArrow, dragHandle } from './icons';
-import { IconDragHandle } from './drag-handle';
+import BlockDraggable from '../block-draggable';
 
 export class BlockMover extends Component {
 	constructor() {
@@ -44,7 +44,7 @@ export class BlockMover extends Component {
 	}
 
 	render() {
-		const { onMoveUp, onMoveDown, __experimentalOrientation: orientation, isRTL, isFirst, isLast, isDraggable, onDragStart, onDragEnd, clientIds, blockElementId, blockType, firstIndex, isLocked, instanceId, isHidden, rootClientId } = this.props;
+		const { onMoveUp, onMoveDown, __experimentalOrientation: orientation, isRTL, isFirst, isLast, clientIds, blockType, firstIndex, isLocked, instanceId, isHidden, rootClientId } = this.props;
 		const { isFocused } = this.state;
 		const blocksCount = castArray( clientIds ).length;
 		if ( isLocked || ( isFirst && isLast && ! rootClientId ) ) {
@@ -98,15 +98,20 @@ export class BlockMover extends Component {
 					onFocus={ this.onFocus }
 					onBlur={ this.onBlur }
 				/>
-				<IconDragHandle
-					className="editor-block-mover__control block-editor-block-mover__control"
-					icon={ dragHandle }
-					clientId={ clientIds }
-					blockElementId={ blockElementId }
-					isVisible={ isDraggable }
-					onDragStart={ onDragStart }
-					onDragEnd={ onDragEnd }
-				/>
+
+				<BlockDraggable clientIds={ clientIds }>
+					{ ( { onDraggableStart, onDraggableEnd } ) => (
+						<IconButton
+							icon={ dragHandle }
+							className="block-editor-block-mover__control-drag-handle editor-block-mover__control block-editor-block-mover__control"
+							aria-hidden="true"
+							onDragStart={ onDraggableStart }
+							onDragEnd={ onDraggableEnd }
+							draggable
+						/>
+					) }
+				</BlockDraggable>
+
 				<IconButton
 					className="editor-block-mover__control block-editor-block-mover__control"
 					onClick={ isLast ? null : onMoveDown }