From 111bd3e8a13de5b989919235f35b1acac46b3691 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 19 Aug 2020 12:28:17 +0800 Subject: [PATCH 1/6] Refactor block drop event handlers into a single hook --- .../components/use-block-drop-zone/index.js | 183 ++------------ .../src/components/use-on-block-drop/index.js | 231 ++++++++++++++++++ 2 files changed, 249 insertions(+), 165 deletions(-) create mode 100644 packages/block-editor/src/components/use-on-block-drop/index.js diff --git a/packages/block-editor/src/components/use-block-drop-zone/index.js b/packages/block-editor/src/components/use-block-drop-zone/index.js index 6e9d05d7f0d72d..e04c2cc5bd90a5 100644 --- a/packages/block-editor/src/components/use-block-drop-zone/index.js +++ b/packages/block-editor/src/components/use-block-drop-zone/index.js @@ -7,13 +7,13 @@ import { difference } from 'lodash'; * WordPress dependencies */ import { __unstableUseDropZone as useDropZone } from '@wordpress/components'; -import { - pasteHandler, - getBlockTransforms, - findTransform, -} from '@wordpress/blocks'; -import { useDispatch, useSelect } from '@wordpress/data'; -import { useEffect, useCallback, useState } from '@wordpress/element'; +import { useSelect } from '@wordpress/data'; +import { useEffect, useState } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import useOnBlockDrop from '../use-on-block-drop'; /** @typedef {import('@wordpress/element').WPSyntheticEvent} WPSyntheticEvent */ @@ -23,6 +23,8 @@ import { useEffect, useCallback, useState } from '@wordpress/element'; * @property {number} y The vertical position of the block being dragged. */ +/** @typedef {import('@wordpress/dom').WPPoint} WPPoint */ + /** * The orientation of a block list. * @@ -126,36 +128,6 @@ export function getNearestBlockIndex( elements, position, orientation ) { return candidateIndex; } -/** - * Retrieve the data for a block drop event. - * - * @param {WPSyntheticEvent} event The drop event. - * - * @return {Object} An object with block drag and drop data. - */ -function parseDropEvent( event ) { - let result = { - srcRootClientId: null, - srcClientIds: 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; -} - /** * @typedef {Object} WPBlockDropZoneConfig * @property {Object} element A React ref object pointing to the block list's DOM element. @@ -179,149 +151,30 @@ export default function useBlockDropZone( { } ) { const [ targetBlockIndex, setTargetBlockIndex ] = useState( null ); - const { - getClientIdsOfDescendants, - getBlockIndex, - hasUploadPermissions, - isLockedAll, - orientation, - } = useSelect( + const { isLockedAll, orientation } = useSelect( ( select ) => { - const { - getBlockListSettings, - getClientIdsOfDescendants: _getClientIdsOfDescendants, - getBlockIndex: _getBlockIndex, - getSettings, - getTemplateLock, - } = select( 'core/block-editor' ); + const { getBlockListSettings, getTemplateLock } = select( + 'core/block-editor' + ); return { + isLockedAll: getTemplateLock( targetRootClientId ) === 'all', orientation: getBlockListSettings( targetRootClientId ) ?.orientation, - getClientIdsOfDescendants: _getClientIdsOfDescendants, - getBlockIndex: _getBlockIndex, - hasUploadPermissions: !! getSettings().mediaUpload, - isLockedAll: getTemplateLock( targetRootClientId ) === 'all', }; }, [ targetRootClientId ] ); - const { - insertBlocks, - updateBlockAttributes, - moveBlocksToPosition, - } = useDispatch( 'core/block-editor' ); - const onFilesDrop = useCallback( - ( files ) => { - if ( ! hasUploadPermissions ) { - return; - } - - const transformation = findTransform( - getBlockTransforms( 'from' ), - ( transform ) => - transform.type === 'files' && transform.isMatch( files ) - ); - - if ( transformation ) { - const blocks = transformation.transform( - files, - updateBlockAttributes - ); - insertBlocks( blocks, targetBlockIndex, targetRootClientId ); - } - }, - [ - hasUploadPermissions, - updateBlockAttributes, - insertBlocks, - targetBlockIndex, - targetRootClientId, - ] - ); - - const onHTMLDrop = useCallback( - ( HTML ) => { - const blocks = pasteHandler( { HTML, mode: 'BLOCKS' } ); - - if ( blocks.length ) { - insertBlocks( blocks, targetBlockIndex, targetRootClientId ); - } - }, - [ insertBlocks, targetBlockIndex, targetRootClientId ] - ); - - const onDrop = useCallback( - ( event ) => { - const { - srcRootClientId: sourceRootClientId, - srcClientIds: sourceClientIds, - type: dropType, - } = parseDropEvent( event ); - - // If the user isn't dropping a block, return early. - if ( dropType !== 'block' ) { - return; - } - - const sourceBlockIndex = getBlockIndex( - sourceClientIds[ 0 ], - sourceRootClientId - ); - - // If the user is dropping to the same position, return early. - if ( - sourceRootClientId === targetRootClientId && - sourceBlockIndex === targetBlockIndex - ) { - return; - } - - // If the user is attempting to drop a block within its own - // nested blocks, return early as this would create infinite - // recursion. - if ( - sourceClientIds.includes( targetRootClientId ) || - getClientIdsOfDescendants( sourceClientIds ).some( - ( id ) => id === targetRootClientId - ) - ) { - return; - } - - // If the block is kept at the same level and moved downwards, - // subtract to take into account that the blocks being dragged - // were removed from the block list. - const isAtSameLevel = sourceRootClientId === targetRootClientId; - const draggedBlockCount = sourceClientIds.length; - const insertIndex = - isAtSameLevel && sourceBlockIndex < targetBlockIndex - ? targetBlockIndex - draggedBlockCount - : targetBlockIndex; - - moveBlocksToPosition( - sourceClientIds, - sourceRootClientId, - targetRootClientId, - insertIndex - ); - }, - [ - getClientIdsOfDescendants, - getBlockIndex, - targetBlockIndex, - moveBlocksToPosition, - targetRootClientId, - ] + const dropEventHandlers = useOnBlockDrop( + targetRootClientId, + targetBlockIndex ); const { position } = useDropZone( { element, - onFilesDrop, - onHTMLDrop, - onDrop, isDisabled: isLockedAll, withPosition: true, + ...dropEventHandlers, } ); useEffect( () => { diff --git a/packages/block-editor/src/components/use-on-block-drop/index.js b/packages/block-editor/src/components/use-on-block-drop/index.js new file mode 100644 index 00000000000000..456bde88ad138e --- /dev/null +++ b/packages/block-editor/src/components/use-on-block-drop/index.js @@ -0,0 +1,231 @@ +/** + * WordPress dependencies + */ +import { + findTransform, + getBlockTransforms, + pasteHandler, +} from '@wordpress/blocks'; +import { useDispatch, useSelect } from '@wordpress/data'; + +/** @typedef {import('@wordpress/element').WPSyntheticEvent} WPSyntheticEvent */ + +/** + * Retrieve the data for a block drop event. + * + * @param {WPSyntheticEvent} event The drop event. + * + * @return {Object} An object with block drag and drop data. + */ +function parseDropEvent( event ) { + let result = { + srcRootClientId: null, + srcClientIds: 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; +} + +/** + * A function that returns an event handler function for block drop events. + * + * @param {string} targetRootClientId The root client id where the block(s) will be inserted. + * @param {number} targetBlockIndex The index where the block(s) will be inserted. + * @param {Function} getBlockIndex A function that gets the index of a block. + * @param {Function} getClientIdsOfDescendants A function that gets the client ids of descendant blocks. + * @param {Function} moveBlocksToPosition A function that moves blocks. + * + * @return {Function} The event handler for a block drop event. + */ +function onBlockDrop( + targetRootClientId, + targetBlockIndex, + getBlockIndex, + getClientIdsOfDescendants, + moveBlocksToPosition +) { + return ( event ) => { + const { + srcRootClientId: sourceRootClientId, + srcClientIds: sourceClientIds, + type: dropType, + } = parseDropEvent( event ); + + // If the user isn't dropping a block, return early. + if ( dropType !== 'block' ) { + return; + } + + const sourceBlockIndex = getBlockIndex( + sourceClientIds[ 0 ], + sourceRootClientId + ); + + // If the user is dropping to the same position, return early. + if ( + sourceRootClientId === targetRootClientId && + sourceBlockIndex === targetBlockIndex + ) { + return; + } + + // If the user is attempting to drop a block within its own + // nested blocks, return early as this would create infinite + // recursion. + if ( + sourceClientIds.includes( targetRootClientId ) || + getClientIdsOfDescendants( sourceClientIds ).some( + ( id ) => id === targetRootClientId + ) + ) { + return; + } + + const isAtSameLevel = sourceRootClientId === targetRootClientId; + const draggedBlockCount = sourceClientIds.length; + + // If the block is kept at the same level and moved downwards, + // subtract to take into account that the blocks being dragged + // were removed from the block list above the insertion point. + const insertIndex = + isAtSameLevel && sourceBlockIndex < targetBlockIndex + ? targetBlockIndex - draggedBlockCount + : targetBlockIndex; + + moveBlocksToPosition( + sourceClientIds, + sourceRootClientId, + targetRootClientId, + insertIndex + ); + }; +} + +/** + * A function that returns an event handler function for block-related file drop events. + * + * @param {string} targetRootClientId The root client id where the block(s) will be inserted. + * @param {number} targetBlockIndex The index where the block(s) will be inserted. + * @param {boolean} hasUploadPermissions Whether the user has upload permissions. + * @param {Function} updateBlockAttributes A function that updates a block's attributes. + * @param {Function} insertBlocks A function that inserts blocks. + * + * @return {Function} The event handler for a block-related file drop event. + */ +function onFileDrop( + targetRootClientId, + targetBlockIndex, + hasUploadPermissions, + updateBlockAttributes, + insertBlocks +) { + return ( files ) => { + if ( ! hasUploadPermissions ) { + return; + } + + const transformation = findTransform( + getBlockTransforms( 'from' ), + ( transform ) => + transform.type === 'files' && transform.isMatch( files ) + ); + + if ( transformation ) { + const blocks = transformation.transform( + files, + updateBlockAttributes + ); + insertBlocks( blocks, targetBlockIndex, targetRootClientId ); + } + }; +} + +/** + * A function that returns an event handler function for block-related HTML drop events. + * + * @param {string} targetRootClientId The root client id where the block(s) will be inserted. + * @param {number} targetBlockIndex The index where the block(s) will be inserted. + * @param {Function} insertBlocks A function that inserts blocks. + * + * @return {Function} The event handler for a block-related HTML drop event. + */ +function onHTMLDrop( targetRootClientId, targetBlockIndex, insertBlocks ) { + return ( HTML ) => { + const blocks = pasteHandler( { HTML, mode: 'BLOCKS' } ); + + if ( blocks.length ) { + insertBlocks( blocks, targetBlockIndex, targetRootClientId ); + } + }; +} + +/** + * A React hook for handling block drop events. + * + * @param {string} targetRootClientId The root client id where the block(s) will be inserted. + * @param {number} targetBlockIndex The index where the block(s) will be inserted. + * + * @return {Function[]} An object that contains the event handlers `onDrop`, `onFileDrop` and `onHTMLDrop`. + */ +export default function useOnBlockDrop( targetRootClientId, targetBlockIndex ) { + const { + getBlockIndex, + getClientIdsOfDescendants, + hasUploadPermissions, + } = useSelect( ( select ) => { + const { + getBlockIndex: _getBlockIndex, + getClientIdsOfDescendants: _getClientIdsOfDescendants, + getSettings, + } = select( 'core/block-editor' ); + + return { + getBlockIndex: _getBlockIndex, + getClientIdsOfDescendants: _getClientIdsOfDescendants, + hasUploadPermissions: getSettings().mediaUpload, + }; + }, [] ); + + const { + insertBlocks, + moveBlocksToPosition, + updateBlockAttributes, + } = useDispatch( 'core/block-editor' ); + + return { + onDrop: onBlockDrop( + targetRootClientId, + targetBlockIndex, + getBlockIndex, + getClientIdsOfDescendants, + moveBlocksToPosition + ), + onFileDrop: onFileDrop( + targetRootClientId, + targetBlockIndex, + hasUploadPermissions, + updateBlockAttributes, + insertBlocks + ), + onHTMLDrop: onHTMLDrop( + targetRootClientId, + targetBlockIndex, + insertBlocks + ), + }; +} From e8867dc4a86a4635cff7d50c6152019839a47d31 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 19 Aug 2020 14:33:49 +0800 Subject: [PATCH 2/6] Add tests --- .../src/components/use-on-block-drop/index.js | 12 +- .../use-on-block-drop/test/index.js | 416 ++++++++++++++++++ 2 files changed, 424 insertions(+), 4 deletions(-) create mode 100644 packages/block-editor/src/components/use-on-block-drop/test/index.js diff --git a/packages/block-editor/src/components/use-on-block-drop/index.js b/packages/block-editor/src/components/use-on-block-drop/index.js index 456bde88ad138e..c0f6cda3000d7c 100644 --- a/packages/block-editor/src/components/use-on-block-drop/index.js +++ b/packages/block-editor/src/components/use-on-block-drop/index.js @@ -17,7 +17,7 @@ import { useDispatch, useSelect } from '@wordpress/data'; * * @return {Object} An object with block drag and drop data. */ -function parseDropEvent( event ) { +export function parseDropEvent( event ) { let result = { srcRootClientId: null, srcClientIds: null, @@ -52,7 +52,7 @@ function parseDropEvent( event ) { * * @return {Function} The event handler for a block drop event. */ -function onBlockDrop( +export function onBlockDrop( targetRootClientId, targetBlockIndex, getBlockIndex, @@ -127,7 +127,7 @@ function onBlockDrop( * * @return {Function} The event handler for a block-related file drop event. */ -function onFileDrop( +export function onFileDrop( targetRootClientId, targetBlockIndex, hasUploadPermissions, @@ -164,7 +164,11 @@ function onFileDrop( * * @return {Function} The event handler for a block-related HTML drop event. */ -function onHTMLDrop( targetRootClientId, targetBlockIndex, insertBlocks ) { +export function onHTMLDrop( + targetRootClientId, + targetBlockIndex, + insertBlocks +) { return ( HTML ) => { const blocks = pasteHandler( { HTML, mode: 'BLOCKS' } ); diff --git a/packages/block-editor/src/components/use-on-block-drop/test/index.js b/packages/block-editor/src/components/use-on-block-drop/test/index.js new file mode 100644 index 00000000000000..9118e13d40131e --- /dev/null +++ b/packages/block-editor/src/components/use-on-block-drop/test/index.js @@ -0,0 +1,416 @@ +/** + * Internal dependencies + */ +import { parseDropEvent, onFileDrop, onHTMLDrop, onBlockDrop } from '..'; +/** + * WordPress dependencies + */ +import { findTransform, pasteHandler } from '@wordpress/blocks'; + +const noop = () => {}; + +jest.mock( '@wordpress/blocks/src/api/factory', () => ( { + findTransform: jest.fn(), + getBlockTransforms: jest.fn(), +} ) ); + +jest.mock( '@wordpress/blocks/src/api/raw-handling', () => ( { + pasteHandler: jest.fn(), +} ) ); + +describe( 'parseDropEvent', () => { + it( 'converts an event dataTransfer property from JSON to an object', () => { + const rawDataTransfer = { + srcRootClientId: '123', + srcClientIds: [ 'abc' ], + srcIndex: 1, + type: 'block', + }; + const event = { + dataTransfer: { + getData() { + return JSON.stringify( rawDataTransfer ); + }, + }, + }; + + expect( parseDropEvent( event ) ).toEqual( rawDataTransfer ); + } ); + + it( 'defaults any missing values to null', () => { + const rawDataTransfer = { + srcClientIds: [ 'abc' ], + type: 'block', + }; + const event = { + dataTransfer: { + getData() { + return JSON.stringify( rawDataTransfer ); + }, + }, + }; + + expect( parseDropEvent( event ) ).toEqual( { + srcRootClientId: null, + srcIndex: null, + ...rawDataTransfer, + } ); + } ); + + it( 'returns an object with null values if the event dataTransfer can not be parsed', () => { + const expected = { + srcRootClientId: null, + srcClientIds: null, + srcIndex: null, + type: null, + }; + const event = { + dataTransfer: { + getData() { + return '{ something_that_cannot_be_parsed_as_json }'; + }, + }, + }; + + expect( parseDropEvent( event ) ).toEqual( expected ); + } ); + + it( 'returns an object with null values if the event has no dataTransfer property', () => { + const expected = { + srcRootClientId: null, + srcClientIds: null, + srcIndex: null, + type: null, + }; + const event = {}; + + expect( parseDropEvent( event ) ).toEqual( expected ); + } ); +} ); + +describe( 'onBlockDrop', () => { + it( 'does nothing if the event is not a block type drop', () => { + const targetRootClientId = '1'; + const targetBlockIndex = 0; + const getBlockIndex = noop; + const getClientIdsOfDescendants = noop; + const moveBlocksToPosition = jest.fn(); + + const event = { + dataTransfer: { + getData() { + return JSON.stringify( { + type: 'not-a-block-type-drop', + } ); + }, + }, + }; + + const eventHandler = onBlockDrop( + targetRootClientId, + targetBlockIndex, + getBlockIndex, + getClientIdsOfDescendants, + moveBlocksToPosition + ); + eventHandler( event ); + + expect( moveBlocksToPosition ).not.toHaveBeenCalled(); + } ); + + it( 'does nothing if the block is dropped to the same place it was dragged from', () => { + const targetRootClientId = '1'; + const targetBlockIndex = 0; + // Target and source block index is the same. + const getBlockIndex = jest.fn( () => targetBlockIndex ); + const getClientIdsOfDescendants = noop; + const moveBlocksToPosition = jest.fn(); + + const event = { + dataTransfer: { + getData() { + return JSON.stringify( { + type: 'block', + // Target and source root client id is the same. + srcRootClientId: targetRootClientId, + srcClientIds: [ '2' ], + } ); + }, + }, + }; + + const eventHandler = onBlockDrop( + targetRootClientId, + targetBlockIndex, + getBlockIndex, + getClientIdsOfDescendants, + moveBlocksToPosition + ); + eventHandler( event ); + + expect( moveBlocksToPosition ).not.toHaveBeenCalled(); + } ); + + it( 'does nothing if the block is dropped as a child of itself', () => { + const targetRootClientId = '1'; + const targetBlockIndex = 0; + const getBlockIndex = jest.fn( () => 6 ); + const getClientIdsOfDescendants = noop; + const moveBlocksToPosition = jest.fn(); + + const event = { + dataTransfer: { + getData() { + return JSON.stringify( { + type: 'block', + srcRootClientId: '0', + // The dragged block is being dropped as a child of itself. + srcClientIds: [ targetRootClientId ], + } ); + }, + }, + }; + + const eventHandler = onBlockDrop( + targetRootClientId, + targetBlockIndex, + getBlockIndex, + getClientIdsOfDescendants, + moveBlocksToPosition + ); + eventHandler( event ); + + expect( moveBlocksToPosition ).not.toHaveBeenCalled(); + } ); + + it( 'does nothing if the block is dropped as a descendant of itself', () => { + const targetRootClientId = '1'; + const targetBlockIndex = 0; + const getBlockIndex = jest.fn( () => 1 ); + // Dragged block is being dropped as a descendant of itself. + const getClientIdsOfDescendants = jest.fn( () => [ + targetRootClientId, + ] ); + const moveBlocksToPosition = jest.fn(); + + const event = { + dataTransfer: { + getData() { + return JSON.stringify( { + type: 'block', + srcRootClientId: '0', + // The dragged block is being dropped as a child of itself. + srcClientIds: [ '5' ], + } ); + }, + }, + }; + + const eventHandler = onBlockDrop( + targetRootClientId, + targetBlockIndex, + getBlockIndex, + getClientIdsOfDescendants, + moveBlocksToPosition + ); + eventHandler( event ); + + expect( moveBlocksToPosition ).not.toHaveBeenCalled(); + } ); + + it( 'inserts blocks if the drop is valid', () => { + const sourceClientIds = [ '5' ]; + const sourceRootClientId = '0'; + const targetRootClientId = '1'; + const targetBlockIndex = 0; + const getBlockIndex = jest.fn( () => 1 ); + // Dragged block is being dropped as a descendant of itself. + const getClientIdsOfDescendants = () => []; + const moveBlocksToPosition = jest.fn(); + + const event = { + dataTransfer: { + getData() { + return JSON.stringify( { + type: 'block', + srcRootClientId: sourceRootClientId, + // The dragged block is being dropped as a child of itself. + srcClientIds: sourceClientIds, + } ); + }, + }, + }; + + const eventHandler = onBlockDrop( + targetRootClientId, + targetBlockIndex, + getBlockIndex, + getClientIdsOfDescendants, + moveBlocksToPosition + ); + eventHandler( event ); + + expect( moveBlocksToPosition ).toHaveBeenCalledWith( + sourceClientIds, + sourceRootClientId, + targetRootClientId, + targetBlockIndex + ); + } ); + + it( 'adjusts the index blocks are dropped at when moved down under the same root block', () => { + const sourceClientIds = [ '5', '6', '7' ]; + const sourceRootClientId = '0'; + const targetRootClientId = sourceRootClientId; + const targetBlockIndex = 5; + const getBlockIndex = jest.fn( () => 1 ); + // Dragged block is being dropped as a descendant of itself. + const getClientIdsOfDescendants = () => []; + const moveBlocksToPosition = jest.fn(); + + const event = { + dataTransfer: { + getData() { + return JSON.stringify( { + type: 'block', + srcRootClientId: sourceRootClientId, + // The dragged block is being dropped as a child of itself. + srcClientIds: sourceClientIds, + } ); + }, + }, + }; + + const insertIndex = targetBlockIndex - sourceClientIds.length; + + const eventHandler = onBlockDrop( + targetRootClientId, + targetBlockIndex, + getBlockIndex, + getClientIdsOfDescendants, + moveBlocksToPosition + ); + eventHandler( event ); + + expect( moveBlocksToPosition ).toHaveBeenCalledWith( + sourceClientIds, + sourceRootClientId, + targetRootClientId, + insertIndex + ); + } ); +} ); + +describe( 'onFileDrop', () => { + it( 'does nothing if hasUploadPermissions is false', () => { + const updateBlockAttributes = jest.fn(); + const insertBlocks = jest.fn(); + const targetRootClientId = '1'; + const targetBlockIndex = 0; + const uploadPermissions = false; + + const onFileDropHandler = onFileDrop( + targetRootClientId, + targetBlockIndex, + uploadPermissions, + updateBlockAttributes, + insertBlocks + ); + onFileDropHandler(); + + expect( findTransform ).not.toHaveBeenCalled(); + expect( insertBlocks ).not.toHaveBeenCalled(); + } ); + + it( 'does nothing if the block has no matching file transforms', () => { + findTransform.mockImplementation( () => {} ); + const updateBlockAttributes = noop; + const insertBlocks = jest.fn(); + const targetRootClientId = '1'; + const targetBlockIndex = 0; + const uploadPermissions = true; + + const onFileDropHandler = onFileDrop( + targetRootClientId, + targetBlockIndex, + uploadPermissions, + updateBlockAttributes, + insertBlocks + ); + onFileDropHandler(); + + expect( findTransform ).toHaveBeenCalled(); + expect( insertBlocks ).not.toHaveBeenCalled(); + } ); + + it( 'inserts blocks if a valid transform can be found', () => { + const blocks = 'blocks'; + const transformation = { transform: jest.fn( () => blocks ) }; + findTransform.mockImplementation( () => transformation ); + const updateBlockAttributes = noop; + const insertBlocks = jest.fn(); + const targetRootClientId = '1'; + const targetBlockIndex = 0; + const uploadPermissions = true; + + const onFileDropHandler = onFileDrop( + targetRootClientId, + targetBlockIndex, + uploadPermissions, + updateBlockAttributes, + insertBlocks + ); + const files = 'test'; + onFileDropHandler( files ); + + expect( findTransform ).toHaveBeenCalled(); + expect( transformation.transform ).toHaveBeenCalledWith( + files, + updateBlockAttributes + ); + expect( insertBlocks ).toHaveBeenCalledWith( + blocks, + targetBlockIndex, + targetRootClientId + ); + } ); +} ); + +describe( 'onHTMLDrop', () => { + it( 'does nothing if the HTML cannot be converted into blocks', () => { + pasteHandler.mockImplementation( () => [] ); + const targetRootClientId = '1'; + const targetBlockIndex = 0; + const insertBlocks = jest.fn(); + + const eventHandler = onHTMLDrop( + targetRootClientId, + targetBlockIndex, + insertBlocks + ); + eventHandler(); + + expect( insertBlocks ).not.toHaveBeenCalled(); + } ); + + it( 'inserts blocks if the HTML can be converted into blocks', () => { + const blocks = [ 'blocks' ]; + pasteHandler.mockImplementation( () => blocks ); + const targetRootClientId = '1'; + const targetBlockIndex = 0; + const insertBlocks = jest.fn(); + + const eventHandler = onHTMLDrop( + targetRootClientId, + targetBlockIndex, + insertBlocks + ); + eventHandler(); + + expect( insertBlocks ).toHaveBeenCalledWith( + blocks, + targetBlockIndex, + targetRootClientId + ); + } ); +} ); From 62f283f537380e99c132184fa545ea55ac11b20c Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 19 Aug 2020 14:47:47 +0800 Subject: [PATCH 3/6] Fix copy/pasta --- .../src/components/use-on-block-drop/test/index.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/block-editor/src/components/use-on-block-drop/test/index.js b/packages/block-editor/src/components/use-on-block-drop/test/index.js index 9118e13d40131e..161368a9c4d5fe 100644 --- a/packages/block-editor/src/components/use-on-block-drop/test/index.js +++ b/packages/block-editor/src/components/use-on-block-drop/test/index.js @@ -199,7 +199,6 @@ describe( 'onBlockDrop', () => { return JSON.stringify( { type: 'block', srcRootClientId: '0', - // The dragged block is being dropped as a child of itself. srcClientIds: [ '5' ], } ); }, @@ -224,7 +223,6 @@ describe( 'onBlockDrop', () => { const targetRootClientId = '1'; const targetBlockIndex = 0; const getBlockIndex = jest.fn( () => 1 ); - // Dragged block is being dropped as a descendant of itself. const getClientIdsOfDescendants = () => []; const moveBlocksToPosition = jest.fn(); @@ -234,7 +232,6 @@ describe( 'onBlockDrop', () => { return JSON.stringify( { type: 'block', srcRootClientId: sourceRootClientId, - // The dragged block is being dropped as a child of itself. srcClientIds: sourceClientIds, } ); }, From 971c0ea2a3b466970eee5351871bc7441f6b0edf Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 19 Aug 2020 16:09:51 +0800 Subject: [PATCH 4/6] Fix doc blocks --- .../src/components/use-on-block-drop/index.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/block-editor/src/components/use-on-block-drop/index.js b/packages/block-editor/src/components/use-on-block-drop/index.js index c0f6cda3000d7c..22c88e03889bd6 100644 --- a/packages/block-editor/src/components/use-on-block-drop/index.js +++ b/packages/block-editor/src/components/use-on-block-drop/index.js @@ -158,9 +158,9 @@ export function onFileDrop( /** * A function that returns an event handler function for block-related HTML drop events. * - * @param {string} targetRootClientId The root client id where the block(s) will be inserted. - * @param {number} targetBlockIndex The index where the block(s) will be inserted. - * @param {Function} insertBlocks A function that inserts blocks. + * @param {string} targetRootClientId The root client id where the block(s) will be inserted. + * @param {number} targetBlockIndex The index where the block(s) will be inserted. + * @param {Function} insertBlocks A function that inserts blocks. * * @return {Function} The event handler for a block-related HTML drop event. */ @@ -181,10 +181,10 @@ export function onHTMLDrop( /** * A React hook for handling block drop events. * - * @param {string} targetRootClientId The root client id where the block(s) will be inserted. - * @param {number} targetBlockIndex The index where the block(s) will be inserted. + * @param {string} targetRootClientId The root client id where the block(s) will be inserted. + * @param {number} targetBlockIndex The index where the block(s) will be inserted. * - * @return {Function[]} An object that contains the event handlers `onDrop`, `onFileDrop` and `onHTMLDrop`. + * @return {Object} An object that contains the event handlers `onDrop`, `onFileDrop` and `onHTMLDrop`. */ export default function useOnBlockDrop( targetRootClientId, targetBlockIndex ) { const { From b010d3b2e2612a834e2bcf2e62d9a1e996509a52 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 20 Aug 2020 14:27:34 +0800 Subject: [PATCH 5/6] Fix typo with onFilesDrop --- .../src/components/use-on-block-drop/index.js | 6 +++--- .../src/components/use-on-block-drop/test/index.js | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/block-editor/src/components/use-on-block-drop/index.js b/packages/block-editor/src/components/use-on-block-drop/index.js index 22c88e03889bd6..eda98177563ef9 100644 --- a/packages/block-editor/src/components/use-on-block-drop/index.js +++ b/packages/block-editor/src/components/use-on-block-drop/index.js @@ -127,7 +127,7 @@ export function onBlockDrop( * * @return {Function} The event handler for a block-related file drop event. */ -export function onFileDrop( +export function onFilesDrop( targetRootClientId, targetBlockIndex, hasUploadPermissions, @@ -184,7 +184,7 @@ export function onHTMLDrop( * @param {string} targetRootClientId The root client id where the block(s) will be inserted. * @param {number} targetBlockIndex The index where the block(s) will be inserted. * - * @return {Object} An object that contains the event handlers `onDrop`, `onFileDrop` and `onHTMLDrop`. + * @return {Object} An object that contains the event handlers `onDrop`, `onFilesDrop` and `onHTMLDrop`. */ export default function useOnBlockDrop( targetRootClientId, targetBlockIndex ) { const { @@ -219,7 +219,7 @@ export default function useOnBlockDrop( targetRootClientId, targetBlockIndex ) { getClientIdsOfDescendants, moveBlocksToPosition ), - onFileDrop: onFileDrop( + onFilesDrop: onFilesDrop( targetRootClientId, targetBlockIndex, hasUploadPermissions, diff --git a/packages/block-editor/src/components/use-on-block-drop/test/index.js b/packages/block-editor/src/components/use-on-block-drop/test/index.js index 161368a9c4d5fe..2fb33e10d5159f 100644 --- a/packages/block-editor/src/components/use-on-block-drop/test/index.js +++ b/packages/block-editor/src/components/use-on-block-drop/test/index.js @@ -1,7 +1,7 @@ /** * Internal dependencies */ -import { parseDropEvent, onFileDrop, onHTMLDrop, onBlockDrop } from '..'; +import { parseDropEvent, onFilesDrop, onHTMLDrop, onBlockDrop } from '..'; /** * WordPress dependencies */ @@ -298,7 +298,7 @@ describe( 'onBlockDrop', () => { } ); } ); -describe( 'onFileDrop', () => { +describe( 'onFilesDrop', () => { it( 'does nothing if hasUploadPermissions is false', () => { const updateBlockAttributes = jest.fn(); const insertBlocks = jest.fn(); @@ -306,7 +306,7 @@ describe( 'onFileDrop', () => { const targetBlockIndex = 0; const uploadPermissions = false; - const onFileDropHandler = onFileDrop( + const onFileDropHandler = onFilesDrop( targetRootClientId, targetBlockIndex, uploadPermissions, @@ -327,7 +327,7 @@ describe( 'onFileDrop', () => { const targetBlockIndex = 0; const uploadPermissions = true; - const onFileDropHandler = onFileDrop( + const onFileDropHandler = onFilesDrop( targetRootClientId, targetBlockIndex, uploadPermissions, @@ -350,7 +350,7 @@ describe( 'onFileDrop', () => { const targetBlockIndex = 0; const uploadPermissions = true; - const onFileDropHandler = onFileDrop( + const onFileDropHandler = onFilesDrop( targetRootClientId, targetBlockIndex, uploadPermissions, From 227a6e9b299f5d5c4a2c1699e38c66683c9920b5 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 20 Aug 2020 15:37:29 +0800 Subject: [PATCH 6/6] Use noop and add some helper comments --- .../src/components/use-on-block-drop/test/index.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/use-on-block-drop/test/index.js b/packages/block-editor/src/components/use-on-block-drop/test/index.js index 2fb33e10d5159f..beae8362fbfcb3 100644 --- a/packages/block-editor/src/components/use-on-block-drop/test/index.js +++ b/packages/block-editor/src/components/use-on-block-drop/test/index.js @@ -320,7 +320,9 @@ describe( 'onFilesDrop', () => { } ); it( 'does nothing if the block has no matching file transforms', () => { - findTransform.mockImplementation( () => {} ); + // Test the scenario of 'no transforms' by mocking findTransform + // to have no return value. + findTransform.mockImplementation( noop ); const updateBlockAttributes = noop; const insertBlocks = jest.fn(); const targetRootClientId = '1'; @@ -341,6 +343,9 @@ describe( 'onFilesDrop', () => { } ); it( 'inserts blocks if a valid transform can be found', () => { + // Mock findTransform to return a valid transform. The implementation + // of the transform isn't important just that there is a callable 'transform' + // function that returns a value. const blocks = 'blocks'; const transformation = { transform: jest.fn( () => blocks ) }; findTransform.mockImplementation( () => transformation );