Fix calculation of new block index on drop

docs/data/data-core-editor.md

@@ -387,6 +387,20 @@ on each call
 
 Post blocks.
 
+### getClientIdsOfDescendants
+
+Returns an array containing the clientIds of all descendants
+of the blocks given.
+
+*Parameters*
+
+ * state: Global application state.
+ * clientIds: Array of blocks to inspect.
+
+*Returns*
+
+ids of descendants.
+
 ### getClientIdsWithDescendants
 
 Returns an array containing the clientIds of the top-level blocks

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

@@ -4,13 +4,12 @@
 import { Draggable } from '@wordpress/components';
 import { withSelect } from '@wordpress/data';
 
-const BlockDraggable = ( { children, clientId, rootClientId, blockElementId, index, layout, onDragStart, onDragEnd } ) => {
+const BlockDraggable = ( { children, clientId, rootClientId, blockElementId, index, onDragStart, onDragEnd } ) => {
 	const transferData = {
 		type: 'block',
-		fromIndex: index,
-		rootClientId,
-		clientId,
-		layout,
+		srcIndex: index,
+		srcRootClientId: rootClientId,
+		srcClientId: clientId,
 	};
 
 	return (

packages/editor/src/components/block-drop-zone/index.js

@@ -18,6 +18,27 @@ import { Component } from '@wordpress/element';
 import { withDispatch, withSelect } from '@wordpress/data';
 import { compose } from '@wordpress/compose';
 
+const parseDropEvent = ( event ) => {
+	let result = {
+		srcRootClientId: null,
+		srcClientId: null,
+		srcIndex: null,
+		type: null,
+	};
+
+	if ( ! event.dataTransfer ) {
+		return result;
+	}
+
+	try {
+		result = Object.assign( result, JSON.parse( event.dataTransfer.getData( 'text' ) ) );
+	} catch ( err ) {
+		return result;
+	}
+
+	return result;
+};
+
 class BlockDropZone extends Component {
 	constructor() {
 		super( ...arguments );
@@ -56,28 +77,29 @@ class BlockDropZone extends Component {
 	}
 
 	onDrop( event, position ) {
-		if ( ! event.dataTransfer ) {
-			return;
-		}
-
-		let clientId, type, rootClientId, fromIndex;
+		const { rootClientId: dstRootClientId, clientId: dstClientId, index: dstIndex, getDescendants } = this.props;
+		const { srcRootClientId, srcClientId, srcIndex, type } = parseDropEvent( event );
+
+		const isBlockDropType = ( dropType ) => dropType === 'block';
+		const isSameLevel = ( srcRoot, dstRoot ) => {
+			// Note that rootClientId of top-level blocks will be undefined OR a void string,
+			// so we also need to account for that case separately.
+			return ( srcRoot === dstRoot ) || ( ! srcRoot === true && ! dstRoot === true );
+		};
+		const isSameBlock = ( src, dst ) => src === dst;
+		const isSrcBlockAnAncestorOfDstBlock = ( src, dst ) => getDescendants( [ src ] ).some( ( id ) => id === dst );
 
-		try {
-			( { clientId, type, rootClientId, fromIndex } = JSON.parse( event.dataTransfer.getData( 'text' ) ) );
-		} catch ( err ) {
+		if ( ! isBlockDropType( type ) ||
+			isSameBlock( srcClientId, dstClientId ) ||
+			isSrcBlockAnAncestorOfDstBlock( srcClientId, dstClientId ) ) {
 			return;
 		}
 
-		if ( type !== 'block' ) {
-			return;
-		}
-		const { index } = this.props;
 		const positionIndex = this.getInsertIndex( position );
-
-		// If the block is kept at the same level and moved downwards, subtract
-		// to account for blocks shifting upward to occupy its old position.
-		const insertIndex = index && fromIndex < index && rootClientId === this.props.rootClientId ? positionIndex - 1 : positionIndex;
-		this.props.moveBlockToPosition( clientId, rootClientId, insertIndex );
+		// If the block is kept at the same level and moved downwards,
+		// subtract to account for blocks shifting upward to occupy its old position.
+		const insertIndex = dstIndex && srcIndex < dstIndex && isSameLevel( srcRootClientId, dstRootClientId ) ? positionIndex - 1 : positionIndex;
+		this.props.moveBlockToPosition( srcClientId, srcRootClientId, insertIndex );
 	}
 
 	render() {
@@ -109,7 +131,7 @@ export default compose(
 		} = dispatch( 'core/editor' );
 
 		return {
-			insertBlocks( blocks, insertIndex ) {
+			insertBlocks( blocks, index ) {
 				const { rootClientId, layout } = ownProps;
 
 				if ( layout ) {
@@ -122,21 +144,22 @@ export default compose(
 					) );
 				}
 
-				insertBlocks( blocks, insertIndex, rootClientId );
+				insertBlocks( blocks, index, rootClientId );
 			},
 			updateBlockAttributes( ...args ) {
 				updateBlockAttributes( ...args );
 			},
-			moveBlockToPosition( clientId, fromRootClientId, index ) {
-				const { rootClientId, layout } = ownProps;
-				moveBlockToPosition( clientId, fromRootClientId, rootClientId, layout, index );
+			moveBlockToPosition( srcClientId, srcRootClientId, dstIndex ) {
+				const { rootClientId: dstRootClientId, layout } = ownProps;
+				moveBlockToPosition( srcClientId, srcRootClientId, dstRootClientId, layout, dstIndex );
 			},
 		};
 	} ),
 	withSelect( ( select, { rootClientId } ) => {
-		const { getTemplateLock } = select( 'core/editor' );
+		const { getDescendants, getTemplateLock } = select( 'core/editor' );
 		return {
 			isLocked: !! getTemplateLock( rootClientId ),
+			getDescendants,
 		};
 	} )
 )( BlockDropZone );

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

@@ -491,14 +491,14 @@ export class BlockListBlock extends Component {
 				) }
 				<BlockDropZone
 					index={ order }
+					clientId={ clientId }
 					rootClientId={ rootClientId }
 					layout={ layout }
 				/>
 				{ shouldRenderMovers && (
 					<BlockMover
 						clientIds={ clientId }
 						blockElementId={ blockElementId }
-						layout={ layout }
 						isFirst={ isFirst }
 						isLast={ isLast }
 						isHidden={ ! ( isHovered || isSelected ) || hoverArea !== 'left' }

packages/editor/src/components/block-mover/drag-handle.js

@@ -8,7 +8,7 @@ import classnames from 'classnames';
  */
 import BlockDraggable from '../block-draggable';
 
-export const IconDragHandle = ( { isVisible, className, icon, onDragStart, onDragEnd, blockElementId, clientId, layout } ) => {
+export const IconDragHandle = ( { isVisible, className, icon, onDragStart, onDragEnd, blockElementId, clientId } ) => {
 	if ( ! isVisible ) {
 		return null;
 	}
@@ -19,7 +19,6 @@ export const IconDragHandle = ( { isVisible, className, icon, onDragStart, onDra
 		<BlockDraggable
 			clientId={ clientId }
 			blockElementId={ blockElementId }
-			layout={ layout }
 			onDragStart={ onDragStart }
 			onDragEnd={ onDragEnd }
 		>

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

@@ -44,7 +44,7 @@ export class BlockMover extends Component {
 	}
 
 	render() {
-		const { onMoveUp, onMoveDown, isFirst, isLast, isDraggable, onDragStart, onDragEnd, clientIds, blockElementId, layout, blockType, firstIndex, isLocked, instanceId, isHidden } = this.props;
+		const { onMoveUp, onMoveDown, isFirst, isLast, isDraggable, onDragStart, onDragEnd, clientIds, blockElementId, blockType, firstIndex, isLocked, instanceId, isHidden } = this.props;
 		const { isFocused } = this.state;
 		const blocksCount = castArray( clientIds ).length;
 		if ( isLocked || ( isFirst && isLast ) ) {
@@ -72,7 +72,6 @@ export class BlockMover extends Component {
 					icon={ dragHandle }
 					clientId={ clientIds }
 					blockElementId={ blockElementId }
-					layout={ layout }
 					isVisible={ isDraggable }
 					onDragStart={ onDragStart }
 					onDragEnd={ onDragEnd }

packages/editor/src/store/selectors.js

@@ -565,6 +565,20 @@ export const getBlocks = createSelector(
 	]
 );
 
+/**
+ * Returns an array containing the clientIds of all descendants
+ * of the blocks given.
+ *
+ * @param {Object} state Global application state.
+ * @param {Array} clientIds Array of blocks to inspect.
+ *
+ * @return {Array} ids of descendants.
+ */
+export const getClientIdsOfDescendants = ( state, clientIds ) => flatMap( clientIds, ( clientId ) => {
+	const descendants = getBlockOrder( state, clientId );
+	return [ ...descendants, ...getClientIdsOfDescendants( state, descendants ) ];
+} );
+
 /**
  * Returns an array containing the clientIds of the top-level blocks
  * and their descendants of any depth (for nested blocks).
@@ -575,12 +589,8 @@ export const getBlocks = createSelector(
  */
 export const getClientIdsWithDescendants = createSelector(
 	( state ) => {
-		const getDescendants = ( clientIds ) => flatMap( clientIds, ( clientId ) => {
-			const descendants = getBlockOrder( state, clientId );
-			return [ ...descendants, ...getDescendants( descendants ) ];
-		} );
 		const topLevelIds = getBlockOrder( state );
-		return [ ...topLevelIds, ...getDescendants( topLevelIds ) ];
+		return [ ...topLevelIds, ...getClientIdsOfDescendants( state, topLevelIds ) ];
 	},
 	( state ) => [
 		state.editor.present.blockOrder,

packages/editor/src/store/test/selectors.js

@@ -55,6 +55,7 @@ const {
 	getBlocks,
 	getBlockCount,
 	getClientIdsWithDescendants,
+	getClientIdsOfDescendants,
 	hasSelectedBlock,
 	getSelectedBlock,
 	getSelectedBlockClientId,
@@ -1651,6 +1652,63 @@ describe( 'selectors', () => {
 		} );
 	} );
 
+	describe( 'getClientIdsOfDescendants', () => {
+		it( 'should return the ids of any descendants, given an array of clientIds', () => {
+			const state = {
+				currentPost: {},
+				editor: {
+					present: {
+						blocksByClientId: {
+							'uuid-2': { clientId: 'uuid-2', name: 'core/image', attributes: {} },
+							'uuid-4': { clientId: 'uuid-4', name: 'core/paragraph', attributes: {} },
+							'uuid-6': { clientId: 'uuid-6', name: 'core/paragraph', attributes: {} },
+							'uuid-8': { clientId: 'uuid-8', name: 'core/block', attributes: {} },
+							'uuid-10': { clientId: 'uuid-10', name: 'core/columns', attributes: {} },
+							'uuid-12': { clientId: 'uuid-12', name: 'core/column', attributes: {} },
+							'uuid-14': { clientId: 'uuid-14', name: 'core/column', attributes: {} },
+							'uuid-16': { clientId: 'uuid-16', name: 'core/quote', attributes: {} },
+							'uuid-18': { clientId: 'uuid-18', name: 'core/block', attributes: {} },
+							'uuid-20': { clientId: 'uuid-20', name: 'core/gallery', attributes: {} },
+							'uuid-22': { clientId: 'uuid-22', name: 'core/block', attributes: {} },
+							'uuid-24': { clientId: 'uuid-24', name: 'core/columns', attributes: {} },
+							'uuid-26': { clientId: 'uuid-26', name: 'core/column', attributes: {} },
+							'uuid-28': { clientId: 'uuid-28', name: 'core/column', attributes: {} },
+							'uuid-30': { clientId: 'uuid-30', name: 'core/paragraph', attributes: {} },
+						},
+						blockOrder: {
+							'': [ 'uuid-6', 'uuid-8', 'uuid-10', 'uuid-22' ],
+							'uuid-2': [ ],
+							'uuid-4': [ ],
+							'uuid-6': [ ],
+							'uuid-8': [ ],
+							'uuid-10': [ 'uuid-12', 'uuid-14' ],
+							'uuid-12': [ 'uuid-16' ],
+							'uuid-14': [ 'uuid-18' ],
+							'uuid-16': [ ],
+							'uuid-18': [ 'uuid-24' ],
+							'uuid-20': [ ],
+							'uuid-22': [ ],
+							'uuid-24': [ 'uuid-26', 'uuid-28' ],
+							'uuid-26': [ ],
+							'uuid-28': [ 'uuid-30' ],
+						},
+						edits: {},
+					},
+				},
+			};
+			expect( getClientIdsOfDescendants( state, [ 'uuid-10' ] ) ).toEqual( [
+				'uuid-12',
+				'uuid-14',
+				'uuid-16',
+				'uuid-18',
+				'uuid-24',
+				'uuid-26',
+				'uuid-28',
+				'uuid-30',
+			] );
+		} );
+	} );
+
 	describe( 'getClientIdsWithDescendants', () => {
 		it( 'should return the ids for top-level blocks and their descendants of any depth (for nested blocks).', () => {
 			const state = {