From 2a3a168cdc740c24188181ddb186a71576189722 Mon Sep 17 00:00:00 2001 From: Kai Hao Date: Wed, 27 Jul 2022 14:10:09 +0800 Subject: [PATCH 01/14] Allow dropping an image on an empty paragraph block --- package-lock.json | 43 ++++-- package.json | 1 + packages/block-library/CHANGELOG.md | 1 + packages/block-library/src/paragraph/edit.js | 47 ++++++- .../block-library/src/paragraph/editor.scss | 8 ++ packages/components/src/drop-zone/index.tsx | 5 +- packages/components/src/drop-zone/style.scss | 12 ++ .../e2e-test-utils-playwright/package.json | 3 +- .../src/page-utils/drag-files.ts | 131 ++++++++++++++++++ .../src/page-utils/index.ts | 2 + .../e2e/specs/editor/blocks/paragraph.spec.js | 59 ++++++++ 11 files changed, 295 insertions(+), 17 deletions(-) create mode 100644 packages/e2e-test-utils-playwright/src/page-utils/drag-files.ts diff --git a/package-lock.json b/package-lock.json index 86445e18976909..9eb370081bea49 100644 --- a/package-lock.json +++ b/package-lock.json @@ -15509,6 +15509,12 @@ "@types/unist": "*" } }, + "@types/mime": { + "version": "2.0.3", + "resolved": "https://registry.npmjs.org/@types/mime/-/mime-2.0.3.tgz", + "integrity": "sha512-Jus9s4CDbqwocc5pOAnh8ShfrnMcPHuJYzVcSUU7lrh8Ni5HuIqX3oilL86p3dlTrk0LzHRCgA/GQ7uNCw6l2Q==", + "dev": true + }, "@types/minimatch": { "version": "3.0.3", "resolved": "https://registry.npmjs.org/@types/minimatch/-/minimatch-3.0.3.tgz", @@ -16979,7 +16985,16 @@ "@wordpress/keycodes": "file:packages/keycodes", "@wordpress/url": "file:packages/url", "change-case": "^4.1.2", - "form-data": "^4.0.0" + "form-data": "^4.0.0", + "mime": "^3.0.0" + }, + "dependencies": { + "mime": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/mime/-/mime-3.0.0.tgz", + "integrity": "sha512-jSCU7/VB1loIWBZe14aEYHU/+1UMEHoaO7qxCOVJOw9GgH72VAWppxNcjU+x9a2k3GSIBXNKxXQFqRvvZ7vr3A==", + "dev": true + } } }, "@wordpress/e2e-tests": { @@ -18666,7 +18681,7 @@ "app-root-dir": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/app-root-dir/-/app-root-dir-1.0.2.tgz", - "integrity": "sha512-jlpIfsOoNoafl92Sz//64uQHGSyMrD2vYG5d8o2a4qGvyNCvXur7bzIsWtAC/6flI2RYAp3kv8rsfBtaLm7w0g==", + "integrity": "sha1-OBh+wt6nV3//Az/8sSFyaS/24Rg=", "dev": true }, "app-root-path": { @@ -26845,7 +26860,7 @@ "babel-plugin-add-react-displayname": { "version": "0.0.5", "resolved": "https://registry.npmjs.org/babel-plugin-add-react-displayname/-/babel-plugin-add-react-displayname-0.0.5.tgz", - "integrity": "sha512-LY3+Y0XVDYcShHHorshrDbt4KFWL4bSeniCtl4SYZbask+Syngk1uMPCeN9+nSiZo6zX5s0RTq/J9Pnaaf/KHw==", + "integrity": "sha1-M51M3be2X9YtHfnbn+BN4TQSK9U=", "dev": true }, "babel-plugin-apply-mdx-type-prop": { @@ -27268,7 +27283,7 @@ "batch-processor": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/batch-processor/-/batch-processor-1.0.0.tgz", - "integrity": "sha512-xoLQD8gmmR32MeuBHgH0Tzd5PuSZx71ZsbhVxOCRbgktZEPe4SQy7s9Z50uPp0F/f7iw2XmkHN2xkgbMfckMDA==", + "integrity": "sha1-dclcMrdI4IUNEMKxaPa9vpiRrOg=", "dev": true }, "bcrypt-pbkdf": { @@ -30558,7 +30573,7 @@ "css.escape": { "version": "1.5.1", "resolved": "https://registry.npmjs.org/css.escape/-/css.escape-1.5.1.tgz", - "integrity": "sha512-YUifsXXuknHlUsmlgyY0PKzgPOr7/FjCePfHNt0jxm83wHZi44VDMQ7/fGNkjY3/jV1MC+1CmZbaHzugyeRtpg==", + "integrity": "sha1-QuJ9T6BK4y+TGktNQZH6nN3ul8s=", "dev": true }, "cssesc": { @@ -36679,7 +36694,7 @@ "has-glob": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/has-glob/-/has-glob-1.0.0.tgz", - "integrity": "sha512-D+8A457fBShSEI3tFCj65PAbT++5sKiFtdCdOam0gnfBgw9D277OERk+HM9qYJXmdVLZ/znez10SqHN0BBQ50g==", + "integrity": "sha1-mqqe7b/7G6OZCnsAEPtnjuAIEgc=", "dev": true, "requires": { "is-glob": "^3.0.0" @@ -36688,7 +36703,7 @@ "is-glob": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/is-glob/-/is-glob-3.1.0.tgz", - "integrity": "sha512-UFpDDrPgM6qpnFNI+rh/p3bUaq9hKLZN8bMUWzxmcnZVS3omf4IPK+BrewlnWjO1WmUsMYuSjKh4UJuV4+Lqmw==", + "integrity": "sha1-e6WuJCF4BKxwcHuWkiVnSGzD6Eo=", "dev": true, "requires": { "is-extglob": "^2.1.0" @@ -38502,7 +38517,7 @@ "is-window": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/is-window/-/is-window-1.0.2.tgz", - "integrity": "sha512-uj00kdXyZb9t9RcAUAwMZAnkBUwdYGhYlt7djMXhfyhUCzwNba50tIiBKR7q0l7tdoBtFVw/3JmLY6fI3rmZmg==", + "integrity": "sha1-LIlspT25feRdPDMTOmXYyfVjSA0=", "dev": true }, "is-windows": { @@ -41879,7 +41894,7 @@ "js-string-escape": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/js-string-escape/-/js-string-escape-1.0.1.tgz", - "integrity": "sha512-Smw4xcfIQ5LVjAOuJCvN/zIodzA/BBSsluuoSykP+lUvScIi4U6RJLfwHet5cxFnCswUjISV8oAXaqaJDY3chg==", + "integrity": "sha1-4mJbrbwNZ8dTPp7cEGjFh65BN+8=", "dev": true }, "js-tokens": { @@ -43407,7 +43422,7 @@ "lz-string": { "version": "1.4.4", "resolved": "https://registry.npmjs.org/lz-string/-/lz-string-1.4.4.tgz", - "integrity": "sha512-0ckx7ZHRPqb0oUm8zNr+90mtf9DQB60H1wMCjBtfi62Kl3a7JbHob6gA2bC+xRvZoOL+1hzUK8jeuEIQE8svEQ==", + "integrity": "sha1-wNjq82BZ9wV5bh40SBHPTEmNOiY=", "dev": true }, "macos-release": { @@ -46738,7 +46753,7 @@ "num2fraction": { "version": "1.2.2", "resolved": "https://registry.npmjs.org/num2fraction/-/num2fraction-1.2.2.tgz", - "integrity": "sha512-Y1wZESM7VUThYY+4W+X4ySH2maqcA+p7UR+w8VWNWVAd6lwuXXWz/w/Cz43J/dI2I+PS6wD5N+bJUF+gjWvIqg==", + "integrity": "sha1-b2gragJ6Tp3fpFZM0lidHU5mnt4=", "dev": true }, "number-is-nan": { @@ -47817,7 +47832,7 @@ "p-defer": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/p-defer/-/p-defer-1.0.0.tgz", - "integrity": "sha512-wB3wfAxZpk2AzOfUMJNL+d36xothRSyj8EXOa4f6GMqYDN9BJaaSISbsk+wS9abmnebVw95C2Kb5t85UmpCxuw==", + "integrity": "sha1-n26xgvbJqozXQwBKfU+WsZaw+ww=", "dev": true }, "p-event": { @@ -49156,7 +49171,7 @@ "pretty-hrtime": { "version": "1.0.3", "resolved": "https://registry.npmjs.org/pretty-hrtime/-/pretty-hrtime-1.0.3.tgz", - "integrity": "sha512-66hKPCr+72mlfiSjlEB1+45IjXSqvVAIy6mocupoww4tBFE9R9IhwwUGoI4G++Tc9Aq+2rxOt0RFU6gPcrte0A==", + "integrity": "sha1-t+PqQkNaTJsnWdmeDyAesZWALuE=", "dev": true }, "prismjs": { @@ -51388,7 +51403,7 @@ "relateurl": { "version": "0.2.7", "resolved": "https://registry.npmjs.org/relateurl/-/relateurl-0.2.7.tgz", - "integrity": "sha512-G08Dxvm4iDN3MLM0EsP62EDV9IuhXPR6blNz6Utcp7zyV3tr4HVNINt6MpaRWbxoOHT3Q7YN2P+jaHX8vUbgog==", + "integrity": "sha1-VNvzd+UUQKypCkzSdGANP/LYiKk=", "dev": true }, "remark": { diff --git a/package.json b/package.json index 61027290d08af3..2131c0815308c5 100755 --- a/package.json +++ b/package.json @@ -121,6 +121,7 @@ "@types/highlight-words-core": "1.2.1", "@types/istanbul-lib-report": "3.0.0", "@types/lodash": "4.14.172", + "@types/mime": "2.0.3", "@types/npm-package-arg": "6.1.1", "@types/prettier": "2.4.4", "@types/qs": "6.9.7", diff --git a/packages/block-library/CHANGELOG.md b/packages/block-library/CHANGELOG.md index fb8296ad3ee019..1b06c0336e69e2 100644 --- a/packages/block-library/CHANGELOG.md +++ b/packages/block-library/CHANGELOG.md @@ -7,6 +7,7 @@ ### New Feature - Made it possible to import individual blocks ([#42258](https://github.com/WordPress/gutenberg/pull/42258)). Check [README](./README.md#loading-individual-blocks) for more information. +- Paragraph block: You can now drag an image on an empty Paragraph block to create an image block ([#42722](https://github.com/WordPress/gutenberg/pull/42722)). ## 7.13.0 (2022-08-24) diff --git a/packages/block-library/src/paragraph/edit.js b/packages/block-library/src/paragraph/edit.js index dd47ae81466229..3272638d279638 100644 --- a/packages/block-library/src/paragraph/edit.js +++ b/packages/block-library/src/paragraph/edit.js @@ -6,11 +6,14 @@ import classnames from 'classnames'; /** * WordPress dependencies */ +import { useState } from '@wordpress/element'; import { __, _x, isRTL } from '@wordpress/i18n'; import { ToolbarButton, ToggleControl, __experimentalToolsPanelItem as ToolsPanelItem, + DropZone, + Popover, } from '@wordpress/components'; import { AlignmentControl, @@ -19,9 +22,13 @@ import { RichText, useBlockProps, useSetting, + store as blockEditorStore, } from '@wordpress/block-editor'; +import { useDispatch } from '@wordpress/data'; +import { useMergeRefs } from '@wordpress/compose'; import { createBlock } from '@wordpress/blocks'; import { formatLtr } from '@wordpress/icons'; +import { createBlobURL } from '@wordpress/blob'; /** * Internal dependencies @@ -55,14 +62,22 @@ function ParagraphBlock( { } ) { const { align, content, direction, dropCap, placeholder } = attributes; const isDropCapFeatureEnabled = useSetting( 'typography.dropCap' ); + const [ paragraphElement, setParagraphElement ] = useState( null ); + const refCallback = ( element ) => { + setParagraphElement( element ); + }; const blockProps = useBlockProps( { - ref: useOnEnter( { clientId, content } ), + ref: useMergeRefs( [ + useOnEnter( { clientId, content } ), + refCallback, + ] ), className: classnames( { 'has-drop-cap': dropCap, [ `has-text-align-${ align }` ]: align, } ), style: { direction }, } ); + const { replaceBlock } = useDispatch( blockEditorStore ); return ( <> @@ -108,6 +123,36 @@ function ParagraphBlock( { ) } + { ! content && ( + + { + if ( files.length === 1 ) { + replaceBlock( + clientId, + createBlock( 'core/image', { + url: createBlobURL( files[ 0 ] ), + } ) + ); + } + + // TODO: We can handle other file types and sizes here. + } } + /> + + ) } - + =1" diff --git a/packages/e2e-test-utils-playwright/src/page-utils/drag-files.ts b/packages/e2e-test-utils-playwright/src/page-utils/drag-files.ts new file mode 100644 index 00000000000000..e7b44fa1ab2800 --- /dev/null +++ b/packages/e2e-test-utils-playwright/src/page-utils/drag-files.ts @@ -0,0 +1,131 @@ +/** + * External dependencies + */ +import { readFile } from 'fs/promises'; +import { basename } from 'path'; +import { getType } from 'mime'; + +/** + * Internal dependencies + */ +import type { PageUtils } from './index'; + +type FileObject = { + name: string; + mimeType?: string; + buffer: Buffer; +}; + +/** + * Simulate dragging files from outside the current page. + * + * @param this + * @param files The files to be dragged. + * @return The methods of the drag operation. + */ +async function dragFiles( + this: PageUtils, + files: string | string[] | FileObject | FileObject[] +) { + const filesList = Array.isArray( files ) ? files : [ files ]; + const fileObjects = await Promise.all( + filesList.map( async ( filePathOrObject ) => { + if ( typeof filePathOrObject !== 'string' ) { + return { + name: filePathOrObject.name, + mimeType: + filePathOrObject.mimeType || + getType( filePathOrObject.name ), + base64: filePathOrObject.buffer.toString( 'base64' ), + }; + } + const base64 = await readFile( filePathOrObject, 'base64' ); + const name = basename( filePathOrObject ); + return { + name, + mimeType: getType( filePathOrObject ), + base64, + }; + } ) + ); + + const dataTransfer = await this.page.evaluateHandle( + async ( _fileObjects ) => { + const dt = new DataTransfer(); + const fileInstances = await Promise.all( + _fileObjects.map( async ( fileObject ) => { + const blob = await fetch( + `data:${ fileObject.mimeType };base64,${ fileObject.base64 }` + ).then( ( res ) => res.blob() ); + return new File( [ blob ], fileObject.name, { + type: fileObject.mimeType ?? undefined, + } ); + } ) + ); + + fileInstances.forEach( ( file ) => { + dt.items.add( file ); + } ); + + return dt; + }, + fileObjects + ); + + // Simulate dragging over the document. + await this.page.dispatchEvent( 'html', 'dragenter', { dataTransfer } ); + + const position = { + x: 0, + y: 0, + }; + + const getCurrentTopMostElement = async () => { + const elementFromPosition = await this.page.evaluateHandle( + ( point ) => { + const element = document.elementFromPoint( point.x, point.y ); + return element; + }, + position + ); + + return elementFromPosition.asElement(); + }; + + return { + /** + * Move the cursor and drag the files to the specified position. + * + * @param x The X coordinate. + * @param y The Y coordinate. + */ + dragTo: async ( x: number, y: number ) => { + position.x = x; + position.y = y; + + const elementHandle = await getCurrentTopMostElement(); + + if ( ! elementHandle ) { + return; + } + + await elementHandle.dispatchEvent( 'dragenter', { dataTransfer } ); + }, + /** + * Drop the files at the current position. + */ + drop: async () => { + const elementHandle = await getCurrentTopMostElement(); + + if ( ! elementHandle ) { + throw new Error( + `No element at position (${ position.x }, ${ position.y }) to drop on` + ); + } + + await elementHandle.dispatchEvent( 'drop', { dataTransfer } ); + }, + }; +} + +export { dragFiles }; diff --git a/packages/e2e-test-utils-playwright/src/page-utils/index.ts b/packages/e2e-test-utils-playwright/src/page-utils/index.ts index d147365fe5e0e8..95d64a022e22e2 100644 --- a/packages/e2e-test-utils-playwright/src/page-utils/index.ts +++ b/packages/e2e-test-utils-playwright/src/page-utils/index.ts @@ -6,6 +6,7 @@ import type { Browser, Page, BrowserContext } from '@playwright/test'; /** * Internal dependencies */ +import { dragFiles } from './drag-files'; import { isCurrentURL } from './is-current-url'; import { setClipboardData, @@ -29,6 +30,7 @@ class PageUtils { this.browser = this.context.browser()!; } + dragFiles = dragFiles.bind( this ); isCurrentURL = isCurrentURL.bind( this ); pressKeyTimes = pressKeyTimes.bind( this ); pressKeyWithModifier = pressKeyWithModifier.bind( this ); diff --git a/test/e2e/specs/editor/blocks/paragraph.spec.js b/test/e2e/specs/editor/blocks/paragraph.spec.js index d4848234986f2b..d844a1e82970c3 100644 --- a/test/e2e/specs/editor/blocks/paragraph.spec.js +++ b/test/e2e/specs/editor/blocks/paragraph.spec.js @@ -1,13 +1,29 @@ +/** + * External dependencies + */ +const path = require( 'path' ); + /** * WordPress dependencies */ const { test, expect } = require( '@wordpress/e2e-test-utils-playwright' ); test.describe( 'Paragraph', () => { + test.beforeAll( async ( { requestUtils } ) => { + await requestUtils.deleteAllMedia(); + } ); + test.beforeEach( async ( { admin } ) => { await admin.createNewPost(); } ); + test.afterEach( async ( { requestUtils } ) => { + await Promise.all( [ + requestUtils.deleteAllPosts(), + requestUtils.deleteAllMedia(), + ] ); + } ); + test( 'should output unwrapped editable paragraph', async ( { editor, page, @@ -28,4 +44,47 @@ test.describe( 'Paragraph', () => { // style. expect( firstBlockTagName ).toBe( 'P' ); } ); + + test( 'should allow dropping an image on en empty paragraph block', async ( { + editor, + page, + pageUtils, + } ) => { + await editor.insertBlock( { name: 'core/paragraph' } ); + const emptyParagraphBlock = page.locator( + '[data-type="core/paragraph"]' + ); + + const testImageName = '10x10_e2e_test_image_z9T8jK.png'; + const testImagePath = path.join( + __dirname, + '../../../assets', + testImageName + ); + + const { dragTo, drop } = await pageUtils.dragFiles( testImagePath ); + + const { x, y, width, height } = await emptyParagraphBlock.boundingBox(); + const centerPosition = { + x: x + width / 2, + y: y + height / 2, + }; + + await dragTo( centerPosition.x, centerPosition.y ); + + await expect( + page.locator( 'text="Drop files to upload"' ) + ).toBeVisible(); + + await drop(); + + const imageBlock = page.locator( + 'role=document[name="Block: Image"i]' + ); + await expect( imageBlock ).toBeVisible(); + await expect( imageBlock.locator( 'role=img' ) ).toHaveAttribute( + 'src', + new RegExp( testImageName.replace( '.', '\\.' ) ) + ); + } ); } ); From a7cf8e57b0512361e7f38a389677ca2561391516 Mon Sep 17 00:00:00 2001 From: Kai Hao Date: Tue, 13 Sep 2022 14:06:01 +0800 Subject: [PATCH 02/14] Update popover props for best practice --- packages/block-library/src/paragraph/edit.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/block-library/src/paragraph/edit.js b/packages/block-library/src/paragraph/edit.js index 3272638d279638..0b9ab5aeb99700 100644 --- a/packages/block-library/src/paragraph/edit.js +++ b/packages/block-library/src/paragraph/edit.js @@ -127,9 +127,10 @@ function ParagraphBlock( { Date: Thu, 15 Sep 2022 14:28:27 +0800 Subject: [PATCH 03/14] Refactor --- packages/block-editor/src/components/index.js | 1 + .../components/use-block-drop-zone/index.js | 12 +- .../src/components/use-on-block-drop/index.js | 245 +++++++++--------- .../block-library/src/paragraph/drop-zone.js | 72 +++++ packages/block-library/src/paragraph/edit.js | 57 ++-- .../block-library/src/paragraph/editor.scss | 14 +- packages/components/src/index.js | 2 +- 7 files changed, 234 insertions(+), 169 deletions(-) create mode 100644 packages/block-library/src/paragraph/drop-zone.js diff --git a/packages/block-editor/src/components/index.js b/packages/block-editor/src/components/index.js index 6b7a24887b3423..0ad7de536282e2 100644 --- a/packages/block-editor/src/components/index.js +++ b/packages/block-editor/src/components/index.js @@ -155,6 +155,7 @@ export { export { default as __experimentalBlockPatternsList } from './block-patterns-list'; export { default as __experimentalPublishDateTimePicker } from './publish-date-time-picker'; export { default as __experimentalInspectorPopoverHeader } from './inspector-popover-header'; +export { default as __experimentalUseOnBlockDrop } from './use-on-block-drop'; /* * State Related Components 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 ac6a0ea3d1f670..fd72588ad79ee0 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 @@ -130,8 +130,16 @@ export default function useBlockDropZone( { setTargetBlockIndex( targetIndex === undefined ? 0 : targetIndex ); - if ( targetIndex !== null ) { - showInsertionPoint( targetRootClientId, targetIndex ); + const nearestBlock = blockElements[ targetIndex ]; + if ( nearestBlock ) { + const previousBlock = blockElements[ targetIndex - 1 ]; + + if ( + nearestBlock.dataset.empty !== 'true' && + previousBlock?.dataset.empty !== 'true' + ) { + showInsertionPoint( targetRootClientId, targetIndex ); + } } }, [] ), 200 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 449a9fc5dc2a9c..cc6a31e6154df6 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 @@ -52,27 +52,16 @@ export function parseDropEvent( event ) { /** * 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. - * @param {Function} insertBlocks A function that inserts blocks. - * @param {Function} clearSelectedBlock A function that clears block selection. + * @param {Object} options The options object. + * @param {Function} options.addBlocks A function that inserts or replaces blocks. + * @param {Function} options.moveBlocks A function that moves blocks. * @return {Function} The event handler for a block drop event. */ -export function onBlockDrop( - targetRootClientId, - targetBlockIndex, - getBlockIndex, - getClientIdsOfDescendants, - moveBlocksToPosition, - insertBlocks, - clearSelectedBlock -) { +export function useOnBlocksDrop( { addBlocks, moveBlocks } ) { + const { clearSelectedBlock } = useDispatch( blockEditorStore ); + return ( event ) => { const { - srcRootClientId: sourceRootClientId, srcClientIds: sourceClientIds, type: dropType, blocks, @@ -84,56 +73,15 @@ export function onBlockDrop( const blocksToInsert = blocks.map( ( block ) => cloneBlock( block ) ); - insertBlocks( - blocksToInsert, - targetBlockIndex, - targetRootClientId, - true, - null - ); + addBlocks( blocksToInsert, { + updateSelection: true, + initialPosition: null, + } ); } // If the user is moving a block. if ( dropType === 'block' ) { - const sourceBlockIndex = getBlockIndex( sourceClientIds[ 0 ] ); - - // 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 - ); + moveBlocks( sourceClientIds ); } }; } @@ -141,23 +89,18 @@ export function onBlockDrop( /** * 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} canInsertBlockType A function that returns checks whether a block type can be inserted. - * @param {Function} insertBlocks A function that inserts blocks. + * @param {Object} options The options object. + * @param {Function} options.addBlocks A function that insert or replace blocks. * * @return {Function} The event handler for a block-related file drop event. */ -export function onFilesDrop( - targetRootClientId, - targetBlockIndex, - hasUploadPermissions, - updateBlockAttributes, - canInsertBlockType, - insertBlocks -) { +export function useOnFilesDrop( { addBlocks } ) { + const hasUploadPermissions = useSelect( + ( select ) => !! select( blockEditorStore ).getSettings().mediaUpload, + [] + ); + const { updateBlockAttributes } = useDispatch( blockEditorStore ); + return ( files ) => { if ( ! hasUploadPermissions ) { return; @@ -166,9 +109,7 @@ export function onFilesDrop( const transformation = findTransform( getBlockTransforms( 'from' ), ( transform ) => - transform.type === 'files' && - canInsertBlockType( transform.blockName, targetRootClientId ) && - transform.isMatch( files ) + transform.type === 'files' && transform.isMatch( files ) ); if ( transformation ) { @@ -176,7 +117,7 @@ export function onFilesDrop( files, updateBlockAttributes ); - insertBlocks( blocks, targetBlockIndex, targetRootClientId ); + addBlocks( blocks ); } }; } @@ -184,22 +125,17 @@ export function onFilesDrop( /** * 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 {Object} options The options object. + * @param {Function} options.addBlocks A function that add or replace blocks. * * @return {Function} The event handler for a block-related HTML drop event. */ -export function onHTMLDrop( - targetRootClientId, - targetBlockIndex, - insertBlocks -) { +export function useOnHTMLDrop( { addBlocks } ) { return ( HTML ) => { const blocks = pasteHandler( { HTML, mode: 'BLOCKS' } ); if ( blocks.length ) { - insertBlocks( blocks, targetBlockIndex, targetRootClientId ); + addBlocks( blocks ); } }; } @@ -207,47 +143,100 @@ 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. + * @param {boolean} replace Should replace the block at the targetBlockIndex. * * @return {Object} An object that contains the event handlers `onDrop`, `onFilesDrop` and `onHTMLDrop`. */ -export default function useOnBlockDrop( targetRootClientId, targetBlockIndex ) { - const hasUploadPermissions = useSelect( - ( select ) => select( blockEditorStore ).getSettings().mediaUpload, - [] - ); - const { canInsertBlockType, getBlockIndex, getClientIdsOfDescendants } = - useSelect( blockEditorStore ); +export default function useOnBlockDrop( + targetRootClientId, + targetBlockIndex, + replace +) { const { - insertBlocks, - moveBlocksToPosition, - updateBlockAttributes, - clearSelectedBlock, - } = useDispatch( blockEditorStore ); - - const _onDrop = onBlockDrop( - targetRootClientId, - targetBlockIndex, getBlockIndex, getClientIdsOfDescendants, - moveBlocksToPosition, - insertBlocks, - clearSelectedBlock - ); - const _onFilesDrop = onFilesDrop( - targetRootClientId, - targetBlockIndex, - hasUploadPermissions, - updateBlockAttributes, - canInsertBlockType, - insertBlocks - ); - const _onHTMLDrop = onHTMLDrop( - targetRootClientId, - targetBlockIndex, - insertBlocks - ); + getBlockOrder, + getBlockRootClientId, + } = useSelect( blockEditorStore ); + const { insertBlocks, replaceBlocks, moveBlocksToPosition } = + useDispatch( blockEditorStore ); + + const addBlocks = ( + blocks, + { updateSelection = true, initialPosition = 0 } = {} + ) => { + if ( replace ) { + const clientIds = getBlockOrder( targetRootClientId ); + const clientId = clientIds[ targetBlockIndex ]; + + replaceBlocks( clientId, blocks, undefined, initialPosition ); + } else { + insertBlocks( + blocks, + targetBlockIndex, + targetRootClientId, + updateSelection, + initialPosition + ); + } + }; + + const moveBlocks = ( clientIds ) => { + const firstClientId = clientIds[ 0 ]; + const rootClientId = getBlockRootClientId( firstClientId ); + const sourceBlockIndex = getBlockIndex( firstClientId ); + + // If the user is dropping to the same position, return early. + if ( + rootClientId === 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 ( + clientIds.includes( targetRootClientId ) || + getClientIdsOfDescendants( clientIds ).includes( + targetRootClientId + ) + ) { + return; + } + + const isAtSameLevel = rootClientId === targetRootClientId; + const draggedBlockCount = clientIds.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( + clientIds, + rootClientId, + targetRootClientId, + insertIndex + ); + }; + + const onBlocksDrop = useOnBlocksDrop( { + addBlocks, + moveBlocks, + } ); + const onFilesDrop = useOnFilesDrop( { + addBlocks, + } ); + const onHTMLDrop = useOnHTMLDrop( { + addBlocks, + } ); return ( event ) => { const files = getFilesFromDataTransfer( event.dataTransfer ); @@ -258,11 +247,11 @@ export default function useOnBlockDrop( targetRootClientId, targetBlockIndex ) { * The order of the checks is important to recognise the HTML drop. */ if ( html ) { - _onHTMLDrop( html ); + onHTMLDrop( html ); } else if ( files.length ) { - _onFilesDrop( files ); + onFilesDrop( files ); } else { - _onDrop( event ); + onBlocksDrop( event ); } }; } diff --git a/packages/block-library/src/paragraph/drop-zone.js b/packages/block-library/src/paragraph/drop-zone.js new file mode 100644 index 00000000000000..0114f188789e13 --- /dev/null +++ b/packages/block-library/src/paragraph/drop-zone.js @@ -0,0 +1,72 @@ +/** + * WordPress dependencies + */ +import { useState } from '@wordpress/element'; +import { useSelect } from '@wordpress/data'; +import { + __experimentalUseOnBlockDrop as useOnBlockDrop, + store as blockEditorStore, +} from '@wordpress/block-editor'; +import { __experimentalUseDropZone as useDropZone } from '@wordpress/compose'; +import { + Popover, + __unstableMotion as motion, + __unstableAnimatePresence as AnimatePresence, +} from '@wordpress/components'; + +export default function DropZone( { paragraphElement, clientId } ) { + const { rootClientId, blockIndex } = useSelect( + ( select ) => { + const selectors = select( blockEditorStore ); + return { + rootClientId: selectors.getBlockRootClientId( clientId ), + blockIndex: selectors.getBlockIndex( clientId ), + }; + }, + [ clientId ] + ); + const onBlockDrop = useOnBlockDrop( rootClientId, blockIndex, true ); + const [ isVisible, setIsVisible ] = useState( false ); + const dropZoneRef = useDropZone( { + onDrop: onBlockDrop, + onDragEnter: () => { + setIsVisible( true ); + }, + onDragLeave: () => { + setIsVisible( false ); + }, + } ); + + return ( + +
+ + { isVisible ? ( + + ) : null } + +
+ + ); +} diff --git a/packages/block-library/src/paragraph/edit.js b/packages/block-library/src/paragraph/edit.js index 0b9ab5aeb99700..f9ae5390abb2e4 100644 --- a/packages/block-library/src/paragraph/edit.js +++ b/packages/block-library/src/paragraph/edit.js @@ -12,8 +12,6 @@ import { ToolbarButton, ToggleControl, __experimentalToolsPanelItem as ToolsPanelItem, - DropZone, - Popover, } from '@wordpress/components'; import { AlignmentControl, @@ -22,18 +20,19 @@ import { RichText, useBlockProps, useSetting, - store as blockEditorStore, } from '@wordpress/block-editor'; -import { useDispatch } from '@wordpress/data'; -import { useMergeRefs } from '@wordpress/compose'; +import { + useMergeRefs, + __experimentalUseDropZone as useDropZone, +} from '@wordpress/compose'; import { createBlock } from '@wordpress/blocks'; import { formatLtr } from '@wordpress/icons'; -import { createBlobURL } from '@wordpress/blob'; /** * Internal dependencies */ import { useOnEnter } from './use-enter'; +import DropZone from './drop-zone'; const name = 'core/paragraph'; @@ -63,13 +62,23 @@ function ParagraphBlock( { const { align, content, direction, dropCap, placeholder } = attributes; const isDropCapFeatureEnabled = useSetting( 'typography.dropCap' ); const [ paragraphElement, setParagraphElement ] = useState( null ); + const [ isDragging, setIsDragging ] = useState( false ); const refCallback = ( element ) => { setParagraphElement( element ); }; + const draggingRef = useDropZone( { + onDragStart: () => { + setIsDragging( true ); + }, + onDragEnd: () => { + setIsDragging( false ); + }, + } ); const blockProps = useBlockProps( { ref: useMergeRefs( [ useOnEnter( { clientId, content } ), refCallback, + draggingRef, ] ), className: classnames( { 'has-drop-cap': dropCap, @@ -77,7 +86,6 @@ function ParagraphBlock( { } ), style: { direction }, } ); - const { replaceBlock } = useDispatch( blockEditorStore ); return ( <> @@ -123,36 +131,11 @@ function ParagraphBlock( { ) } - { ! content && ( - - { - if ( files.length === 1 ) { - replaceBlock( - clientId, - createBlock( 'core/image', { - url: createBlobURL( files[ 0 ] ), - } ) - ); - } - - // TODO: We can handle other file types and sizes here. - } } - /> - + { ! content && isDragging && ( + ) } Date: Thu, 15 Sep 2022 15:15:24 +0800 Subject: [PATCH 04/14] Take reduced motion into account --- .../block-library/src/paragraph/drop-zone.js | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/packages/block-library/src/paragraph/drop-zone.js b/packages/block-library/src/paragraph/drop-zone.js index 0114f188789e13..204ae833b4938b 100644 --- a/packages/block-library/src/paragraph/drop-zone.js +++ b/packages/block-library/src/paragraph/drop-zone.js @@ -7,7 +7,10 @@ import { __experimentalUseOnBlockDrop as useOnBlockDrop, store as blockEditorStore, } from '@wordpress/block-editor'; -import { __experimentalUseDropZone as useDropZone } from '@wordpress/compose'; +import { + __experimentalUseDropZone as useDropZone, + useReducedMotion, +} from '@wordpress/compose'; import { Popover, __unstableMotion as motion, @@ -36,6 +39,12 @@ export default function DropZone( { paragraphElement, clientId } ) { setIsVisible( false ); }, } ); + const reducedMotion = useReducedMotion(); + + const animate = { + opacity: 1, + scale: 1, + }; return ( ) : null } From b9cc502cf23dc1f92655efd2fc479fc3b9dc1d3e Mon Sep 17 00:00:00 2001 From: Kai Hao Date: Fri, 16 Sep 2022 10:38:46 +0800 Subject: [PATCH 05/14] Update animation to scaleY only --- .../block-library/src/paragraph/drop-zone.js | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/block-library/src/paragraph/drop-zone.js b/packages/block-library/src/paragraph/drop-zone.js index 204ae833b4938b..348200928bec3f 100644 --- a/packages/block-library/src/paragraph/drop-zone.js +++ b/packages/block-library/src/paragraph/drop-zone.js @@ -17,6 +17,12 @@ import { __unstableAnimatePresence as AnimatePresence, } from '@wordpress/components'; +const animateVariants = { + hide: { opacity: 0, scaleY: 0.75 }, + show: { opacity: 1, scaleY: 1 }, + exit: { opacity: 0, scaleY: 0.9 }, +}; + export default function DropZone( { paragraphElement, clientId } ) { const { rootClientId, blockIndex } = useSelect( ( select ) => { @@ -41,11 +47,6 @@ export default function DropZone( { paragraphElement, clientId } ) { } ); const reducedMotion = useReducedMotion(); - const animate = { - opacity: 1, - scale: 1, - }; - return ( From fa4c81b7dcd0effce616465366c3a736b2227c44 Mon Sep 17 00:00:00 2001 From: Kai Hao Date: Tue, 20 Sep 2022 16:17:10 +0800 Subject: [PATCH 06/14] Address code reviews --- .../components/use-block-drop-zone/index.js | 22 ++++- .../src/components/use-on-block-drop/index.js | 88 +++++++++++-------- packages/block-editor/src/store/actions.js | 12 +++ packages/block-editor/src/store/reducer.js | 6 ++ .../block-library/src/paragraph/drop-zone.js | 72 +++++++++------ packages/block-library/src/paragraph/edit.js | 17 +--- packages/components/src/drop-zone/index.tsx | 5 +- packages/components/src/drop-zone/style.scss | 12 --- 8 files changed, 131 insertions(+), 103 deletions(-) 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 fd72588ad79ee0..870085de98dfc3 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 @@ -73,6 +73,20 @@ export function getNearestBlockIndex( elements, position, orientation ) { return candidateIndex; } +/** + * Determine if the element is an empty paragraph block. + * + * @param {?HTMLElement} element The element being tested. + * @return {boolean} True or False. + */ +function isEmptyParagraph( element ) { + return ( + !! element && + element.dataset.type === 'core/paragraph' && + element.dataset.empty === 'true' + ); +} + /** * @typedef {Object} WPBlockDropZoneConfig * @property {string} rootClientId The root client id for the block list. @@ -130,13 +144,13 @@ export default function useBlockDropZone( { setTargetBlockIndex( targetIndex === undefined ? 0 : targetIndex ); - const nearestBlock = blockElements[ targetIndex ]; - if ( nearestBlock ) { + if ( targetIndex !== undefined && targetIndex !== null ) { + const nextBlock = blockElements[ targetIndex ]; const previousBlock = blockElements[ targetIndex - 1 ]; if ( - nearestBlock.dataset.empty !== 'true' && - previousBlock?.dataset.empty !== 'true' + ( ! nextBlock || ! isEmptyParagraph( nextBlock ) ) && + ( ! previousBlock || ! isEmptyParagraph( previousBlock ) ) ) { showInsertionPoint( targetRootClientId, targetIndex ); } 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 cc6a31e6154df6..397d22e0241155 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 @@ -89,14 +89,19 @@ export function useOnBlocksDrop( { addBlocks, moveBlocks } ) { /** * A function that returns an event handler function for block-related file drop events. * - * @param {Object} options The options object. - * @param {Function} options.addBlocks A function that insert or replace blocks. + * @param {Object} options The options object. + * @param {Function} options.addBlocks A function that insert or replace blocks. + * @param {string} options.rootClientId The target root client Id. * * @return {Function} The event handler for a block-related file drop event. */ -export function useOnFilesDrop( { addBlocks } ) { - const hasUploadPermissions = useSelect( - ( select ) => !! select( blockEditorStore ).getSettings().mediaUpload, +export function useOnFilesDrop( { addBlocks, rootClientId } ) { + const { hasUploadPermissions, canInsertBlockType } = useSelect( + ( select ) => ( { + hasUploadPermissions: + !! select( blockEditorStore ).getSettings().mediaUpload, + canInsertBlockType: select( blockEditorStore ).canInsertBlockType, + } ), [] ); const { updateBlockAttributes } = useDispatch( blockEditorStore ); @@ -109,7 +114,9 @@ export function useOnFilesDrop( { addBlocks } ) { const transformation = findTransform( getBlockTransforms( 'from' ), ( transform ) => - transform.type === 'files' && transform.isMatch( files ) + transform.type === 'files' && + canInsertBlockType( transform.blockName, rootClientId ) && + transform.isMatch( files ) ); if ( transformation ) { @@ -143,31 +150,35 @@ export function useOnHTMLDrop( { addBlocks } ) { /** * 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 {boolean} replace Should replace the block at the targetBlockIndex. + * @typedef {'insert'|'replace'} DropAction The type of action to perform on drop. + * + * @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 {Object} options The optional options. + * @param {DropAction} options.action The type of action to perform on drop. Could be `insert` or `replace` for now. * * @return {Object} An object that contains the event handlers `onDrop`, `onFilesDrop` and `onHTMLDrop`. */ export default function useOnBlockDrop( targetRootClientId, targetBlockIndex, - replace + options = {} ) { + const { action = 'insert' } = options; const { getBlockIndex, - getClientIdsOfDescendants, getBlockOrder, getBlockRootClientId, + getBlocksByClientId, } = useSelect( blockEditorStore ); - const { insertBlocks, replaceBlocks, moveBlocksToPosition } = + const { insertBlocks, replaceBlocks, moveBlocksToPosition, removeBlocks } = useDispatch( blockEditorStore ); const addBlocks = ( blocks, { updateSelection = true, initialPosition = 0 } = {} ) => { - if ( replace ) { + if ( action === 'replace' ) { const clientIds = getBlockOrder( targetRootClientId ); const clientId = clientIds[ targetBlockIndex ]; @@ -188,26 +199,6 @@ export default function useOnBlockDrop( const rootClientId = getBlockRootClientId( firstClientId ); const sourceBlockIndex = getBlockIndex( firstClientId ); - // If the user is dropping to the same position, return early. - if ( - rootClientId === 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 ( - clientIds.includes( targetRootClientId ) || - getClientIdsOfDescendants( clientIds ).includes( - targetRootClientId - ) - ) { - return; - } - const isAtSameLevel = rootClientId === targetRootClientId; const draggedBlockCount = clientIds.length; @@ -219,12 +210,30 @@ export default function useOnBlockDrop( ? targetBlockIndex - draggedBlockCount : targetBlockIndex; - moveBlocksToPosition( - clientIds, - rootClientId, - targetRootClientId, - insertIndex - ); + if ( action === 'replace' ) { + const sourceBlocks = getBlocksByClientId( clientIds ); + const targetBlockClientIds = getBlockOrder( targetRootClientId ); + const targetBlockClientId = + targetBlockClientIds[ targetBlockIndex ]; + + // Remove the source blocks and the target block. + removeBlocks( [ ...clientIds, targetBlockClientId ], false ); + // Insert the source blocks back to the target index. + insertBlocks( + sourceBlocks, + insertIndex, + targetRootClientId, + true, + 0 + ); + } else { + moveBlocksToPosition( + clientIds, + rootClientId, + targetRootClientId, + insertIndex + ); + } }; const onBlocksDrop = useOnBlocksDrop( { @@ -233,6 +242,7 @@ export default function useOnBlockDrop( } ); const onFilesDrop = useOnFilesDrop( { addBlocks, + rootClientId: targetRootClientId, } ); const onHTMLDrop = useOnHTMLDrop( { addBlocks, diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index 52202d05ad81da..3284accf85d8f6 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -497,6 +497,18 @@ export const moveBlocksToPosition = if ( ! canInsertBlocks ) { return; } + + // If the user is attempting to drop a block within its own + // nested blocks, return early as this would create infinite + // recursion. + if ( + clientIds.includes( toRootClientId ) || + select + .getClientIdsOfDescendants( clientIds ) + .includes( toRootClientId ) + ) { + return; + } } dispatch( { diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js index 2bdecc15d2a07c..7fc5fd975d1a04 100644 --- a/packages/block-editor/src/store/reducer.js +++ b/packages/block-editor/src/store/reducer.js @@ -974,6 +974,12 @@ export const blocks = flow( if ( fromRootClientId === toRootClientId ) { const subState = state[ toRootClientId ]; const fromIndex = subState.indexOf( clientIds[ 0 ] ); + + // If the block is moved to the same position, return the same state. + if ( fromIndex === index ) { + return state; + } + return { ...state, [ toRootClientId ]: moveTo( diff --git a/packages/block-library/src/paragraph/drop-zone.js b/packages/block-library/src/paragraph/drop-zone.js index 348200928bec3f..696b5143726ed0 100644 --- a/packages/block-library/src/paragraph/drop-zone.js +++ b/packages/block-library/src/paragraph/drop-zone.js @@ -34,8 +34,19 @@ export default function DropZone( { paragraphElement, clientId } ) { }, [ clientId ] ); - const onBlockDrop = useOnBlockDrop( rootClientId, blockIndex, true ); + const onBlockDrop = useOnBlockDrop( rootClientId, blockIndex, { + action: 'replace', + } ); + const [ isDragging, setIsDragging ] = useState( false ); const [ isVisible, setIsVisible ] = useState( false ); + const popoverRef = useDropZone( { + onDragStart: () => { + setIsDragging( true ); + }, + onDragEnd: () => { + setIsDragging( false ); + }, + } ); const dropZoneRef = useDropZone( { onDrop: onBlockDrop, onDragEnter: () => { @@ -56,35 +67,38 @@ export default function DropZone( { paragraphElement, clientId } ) { flip={ false } resize={ false } className="wp-block-paragraph__drop-zone" + ref={ popoverRef } > -
- - { isVisible ? ( - - ) : null } - -
+ { isDragging ? ( +
+ + { isVisible ? ( + + ) : null } + +
+ ) : null } ); } diff --git a/packages/block-library/src/paragraph/edit.js b/packages/block-library/src/paragraph/edit.js index f9ae5390abb2e4..e7a715edcbac8b 100644 --- a/packages/block-library/src/paragraph/edit.js +++ b/packages/block-library/src/paragraph/edit.js @@ -21,10 +21,7 @@ import { useBlockProps, useSetting, } from '@wordpress/block-editor'; -import { - useMergeRefs, - __experimentalUseDropZone as useDropZone, -} from '@wordpress/compose'; +import { useMergeRefs } from '@wordpress/compose'; import { createBlock } from '@wordpress/blocks'; import { formatLtr } from '@wordpress/icons'; @@ -62,23 +59,13 @@ function ParagraphBlock( { const { align, content, direction, dropCap, placeholder } = attributes; const isDropCapFeatureEnabled = useSetting( 'typography.dropCap' ); const [ paragraphElement, setParagraphElement ] = useState( null ); - const [ isDragging, setIsDragging ] = useState( false ); const refCallback = ( element ) => { setParagraphElement( element ); }; - const draggingRef = useDropZone( { - onDragStart: () => { - setIsDragging( true ); - }, - onDragEnd: () => { - setIsDragging( false ); - }, - } ); const blockProps = useBlockProps( { ref: useMergeRefs( [ useOnEnter( { clientId, content } ), refCallback, - draggingRef, ] ), className: classnames( { 'has-drop-cap': dropCap, @@ -131,7 +118,7 @@ function ParagraphBlock( { ) } - { ! content && isDragging && ( + { ! content && ( - + Date: Tue, 20 Sep 2022 17:07:31 +0800 Subject: [PATCH 07/14] Minimize the refactor so that test coverage is still intact --- .../src/components/use-on-block-drop/index.js | 287 +++++++++++------- .../use-on-block-drop/test/index.js | 76 ++--- packages/block-editor/src/store/actions.js | 12 - packages/block-editor/src/store/reducer.js | 6 - 4 files changed, 215 insertions(+), 166 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 397d22e0241155..db21038b0ad9a3 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 @@ -1,6 +1,7 @@ /** * WordPress dependencies */ +import { useCallback } from '@wordpress/element'; import { cloneBlock, findTransform, @@ -52,16 +53,27 @@ export function parseDropEvent( event ) { /** * A function that returns an event handler function for block drop events. * - * @param {Object} options The options object. - * @param {Function} options.addBlocks A function that inserts or replaces blocks. - * @param {Function} options.moveBlocks A function that moves 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} 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} moveBlocks A function that moves blocks. + * @param {Function} insertOrReplaceBlocks A function that inserts or replaces blocks. + * @param {Function} clearSelectedBlock A function that clears block selection. * @return {Function} The event handler for a block drop event. */ -export function useOnBlocksDrop( { addBlocks, moveBlocks } ) { - const { clearSelectedBlock } = useDispatch( blockEditorStore ); - +export function onBlockDrop( + targetRootClientId, + targetBlockIndex, + getBlockIndex, + getClientIdsOfDescendants, + moveBlocks, + insertOrReplaceBlocks, + clearSelectedBlock +) { return ( event ) => { const { + srcRootClientId: sourceRootClientId, srcClientIds: sourceClientIds, type: dropType, blocks, @@ -73,15 +85,45 @@ export function useOnBlocksDrop( { addBlocks, moveBlocks } ) { const blocksToInsert = blocks.map( ( block ) => cloneBlock( block ) ); - addBlocks( blocksToInsert, { - updateSelection: true, - initialPosition: null, - } ); + insertOrReplaceBlocks( blocksToInsert, true, null ); } // If the user is moving a block. if ( dropType === 'block' ) { - moveBlocks( sourceClientIds ); + const sourceBlockIndex = getBlockIndex( sourceClientIds[ 0 ] ); + + // 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; + + moveBlocks( sourceClientIds, sourceRootClientId, insertIndex ); } }; } @@ -89,23 +131,23 @@ export function useOnBlocksDrop( { addBlocks, moveBlocks } ) { /** * A function that returns an event handler function for block-related file drop events. * - * @param {Object} options The options object. - * @param {Function} options.addBlocks A function that insert or replace blocks. - * @param {string} options.rootClientId The target root client Id. + * @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} canInsertBlockType A function that returns checks whether a block type can be inserted. + * @param {Function} insertOrReplaceBlocks A function that inserts or replaces blocks. * * @return {Function} The event handler for a block-related file drop event. */ -export function useOnFilesDrop( { addBlocks, rootClientId } ) { - const { hasUploadPermissions, canInsertBlockType } = useSelect( - ( select ) => ( { - hasUploadPermissions: - !! select( blockEditorStore ).getSettings().mediaUpload, - canInsertBlockType: select( blockEditorStore ).canInsertBlockType, - } ), - [] - ); - const { updateBlockAttributes } = useDispatch( blockEditorStore ); - +export function onFilesDrop( + targetRootClientId, + targetBlockIndex, + hasUploadPermissions, + updateBlockAttributes, + canInsertBlockType, + insertOrReplaceBlocks +) { return ( files ) => { if ( ! hasUploadPermissions ) { return; @@ -115,7 +157,7 @@ export function useOnFilesDrop( { addBlocks, rootClientId } ) { getBlockTransforms( 'from' ), ( transform ) => transform.type === 'files' && - canInsertBlockType( transform.blockName, rootClientId ) && + canInsertBlockType( transform.blockName, targetRootClientId ) && transform.isMatch( files ) ); @@ -124,7 +166,7 @@ export function useOnFilesDrop( { addBlocks, rootClientId } ) { files, updateBlockAttributes ); - addBlocks( blocks ); + insertOrReplaceBlocks( blocks ); } }; } @@ -132,17 +174,22 @@ export function useOnFilesDrop( { addBlocks, rootClientId } ) { /** * A function that returns an event handler function for block-related HTML drop events. * - * @param {Object} options The options object. - * @param {Function} options.addBlocks A function that add or replace 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} insertOrReplaceBlocks A function that inserts or replaces blocks. * * @return {Function} The event handler for a block-related HTML drop event. */ -export function useOnHTMLDrop( { addBlocks } ) { +export function onHTMLDrop( + targetRootClientId, + targetBlockIndex, + insertOrReplaceBlocks +) { return ( HTML ) => { const blocks = pasteHandler( { HTML, mode: 'BLOCKS' } ); if ( blocks.length ) { - addBlocks( blocks ); + insertOrReplaceBlocks( blocks ); } }; } @@ -165,88 +212,118 @@ export default function useOnBlockDrop( options = {} ) { const { action = 'insert' } = options; + const hasUploadPermissions = useSelect( + ( select ) => select( blockEditorStore ).getSettings().mediaUpload, + [] + ); const { + canInsertBlockType, getBlockIndex, + getClientIdsOfDescendants, getBlockOrder, - getBlockRootClientId, getBlocksByClientId, } = useSelect( blockEditorStore ); - const { insertBlocks, replaceBlocks, moveBlocksToPosition, removeBlocks } = - useDispatch( blockEditorStore ); - - const addBlocks = ( - blocks, - { updateSelection = true, initialPosition = 0 } = {} - ) => { - if ( action === 'replace' ) { - const clientIds = getBlockOrder( targetRootClientId ); - const clientId = clientIds[ targetBlockIndex ]; - - replaceBlocks( clientId, blocks, undefined, initialPosition ); - } else { - insertBlocks( - blocks, - targetBlockIndex, - targetRootClientId, - updateSelection, - initialPosition - ); - } - }; - - const moveBlocks = ( clientIds ) => { - const firstClientId = clientIds[ 0 ]; - const rootClientId = getBlockRootClientId( firstClientId ); - const sourceBlockIndex = getBlockIndex( firstClientId ); + const { + insertBlocks, + moveBlocksToPosition, + updateBlockAttributes, + clearSelectedBlock, + replaceBlocks, + removeBlocks, + } = useDispatch( blockEditorStore ); - const isAtSameLevel = rootClientId === targetRootClientId; - const draggedBlockCount = clientIds.length; + const insertOrReplaceBlocks = useCallback( + ( blocks, updateSelection = true, initialPosition = 0 ) => { + if ( action === 'replace' ) { + const clientIds = getBlockOrder( targetRootClientId ); + const clientId = clientIds[ targetBlockIndex ]; - // 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; + replaceBlocks( clientId, blocks, undefined, initialPosition ); + } else { + insertBlocks( + blocks, + targetBlockIndex, + targetRootClientId, + updateSelection, + initialPosition + ); + } + }, + [ + action, + getBlockOrder, + insertBlocks, + replaceBlocks, + targetBlockIndex, + targetRootClientId, + ] + ); - if ( action === 'replace' ) { - const sourceBlocks = getBlocksByClientId( clientIds ); - const targetBlockClientIds = getBlockOrder( targetRootClientId ); - const targetBlockClientId = - targetBlockClientIds[ targetBlockIndex ]; + const moveBlocks = useCallback( + ( sourceClientIds, sourceRootClientId, insertIndex ) => { + if ( action === 'replace' ) { + const sourceBlocks = getBlocksByClientId( sourceClientIds ); + const targetBlockClientIds = + getBlockOrder( targetRootClientId ); + const targetBlockClientId = + targetBlockClientIds[ targetBlockIndex ]; - // Remove the source blocks and the target block. - removeBlocks( [ ...clientIds, targetBlockClientId ], false ); - // Insert the source blocks back to the target index. - insertBlocks( - sourceBlocks, - insertIndex, - targetRootClientId, - true, - 0 - ); - } else { - moveBlocksToPosition( - clientIds, - rootClientId, - targetRootClientId, - insertIndex - ); - } - }; + // Remove the source blocks and the target block. + removeBlocks( + [ ...sourceClientIds, targetBlockClientId ], + false + ); + // Insert the source blocks back to the target index. + insertBlocks( + sourceBlocks, + insertIndex, + targetRootClientId, + true, + 0 + ); + } else { + moveBlocksToPosition( + sourceClientIds, + sourceRootClientId, + targetRootClientId, + insertIndex + ); + } + }, + [ + action, + getBlockOrder, + getBlocksByClientId, + insertBlocks, + moveBlocksToPosition, + removeBlocks, + targetBlockIndex, + targetRootClientId, + ] + ); - const onBlocksDrop = useOnBlocksDrop( { - addBlocks, + const _onDrop = onBlockDrop( + targetRootClientId, + targetBlockIndex, + getBlockIndex, + getClientIdsOfDescendants, moveBlocks, - } ); - const onFilesDrop = useOnFilesDrop( { - addBlocks, - rootClientId: targetRootClientId, - } ); - const onHTMLDrop = useOnHTMLDrop( { - addBlocks, - } ); + insertOrReplaceBlocks, + clearSelectedBlock + ); + const _onFilesDrop = onFilesDrop( + targetRootClientId, + targetBlockIndex, + hasUploadPermissions, + updateBlockAttributes, + canInsertBlockType, + insertOrReplaceBlocks + ); + const _onHTMLDrop = onHTMLDrop( + targetRootClientId, + targetBlockIndex, + insertOrReplaceBlocks + ); return ( event ) => { const files = getFilesFromDataTransfer( event.dataTransfer ); @@ -257,11 +334,11 @@ export default function useOnBlockDrop( * The order of the checks is important to recognise the HTML drop. */ if ( html ) { - onHTMLDrop( html ); + _onHTMLDrop( html ); } else if ( files.length ) { - onFilesDrop( files ); + _onFilesDrop( files ); } else { - onBlocksDrop( event ); + _onDrop( event ); } }; } 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 e2789bb879473e..1b95cc0085a79e 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 @@ -98,7 +98,7 @@ describe( 'onBlockDrop', () => { const targetBlockIndex = 0; const getBlockIndex = noop; const getClientIdsOfDescendants = noop; - const moveBlocksToPosition = jest.fn(); + const moveBlocks = jest.fn(); const event = { dataTransfer: { @@ -115,11 +115,11 @@ describe( 'onBlockDrop', () => { targetBlockIndex, getBlockIndex, getClientIdsOfDescendants, - moveBlocksToPosition + moveBlocks ); eventHandler( event ); - expect( moveBlocksToPosition ).not.toHaveBeenCalled(); + expect( moveBlocks ).not.toHaveBeenCalled(); } ); it( 'does nothing if the block is dropped to the same place it was dragged from', () => { @@ -128,7 +128,7 @@ describe( 'onBlockDrop', () => { // Target and source block index is the same. const getBlockIndex = jest.fn( () => targetBlockIndex ); const getClientIdsOfDescendants = noop; - const moveBlocksToPosition = jest.fn(); + const moveBlocks = jest.fn(); const event = { dataTransfer: { @@ -148,11 +148,11 @@ describe( 'onBlockDrop', () => { targetBlockIndex, getBlockIndex, getClientIdsOfDescendants, - moveBlocksToPosition + moveBlocks ); eventHandler( event ); - expect( moveBlocksToPosition ).not.toHaveBeenCalled(); + expect( moveBlocks ).not.toHaveBeenCalled(); } ); it( 'does nothing if the block is dropped as a child of itself', () => { @@ -160,7 +160,7 @@ describe( 'onBlockDrop', () => { const targetBlockIndex = 0; const getBlockIndex = jest.fn( () => 6 ); const getClientIdsOfDescendants = noop; - const moveBlocksToPosition = jest.fn(); + const moveBlocks = jest.fn(); const event = { dataTransfer: { @@ -180,11 +180,11 @@ describe( 'onBlockDrop', () => { targetBlockIndex, getBlockIndex, getClientIdsOfDescendants, - moveBlocksToPosition + moveBlocks ); eventHandler( event ); - expect( moveBlocksToPosition ).not.toHaveBeenCalled(); + expect( moveBlocks ).not.toHaveBeenCalled(); } ); it( 'does nothing if the block is dropped as a descendant of itself', () => { @@ -195,7 +195,7 @@ describe( 'onBlockDrop', () => { const getClientIdsOfDescendants = jest.fn( () => [ targetRootClientId, ] ); - const moveBlocksToPosition = jest.fn(); + const moveBlocks = jest.fn(); const event = { dataTransfer: { @@ -214,11 +214,11 @@ describe( 'onBlockDrop', () => { targetBlockIndex, getBlockIndex, getClientIdsOfDescendants, - moveBlocksToPosition + moveBlocks ); eventHandler( event ); - expect( moveBlocksToPosition ).not.toHaveBeenCalled(); + expect( moveBlocks ).not.toHaveBeenCalled(); } ); it( 'inserts blocks if the drop is valid', () => { @@ -228,7 +228,7 @@ describe( 'onBlockDrop', () => { const targetBlockIndex = 0; const getBlockIndex = jest.fn( () => 1 ); const getClientIdsOfDescendants = () => []; - const moveBlocksToPosition = jest.fn(); + const moveBlocks = jest.fn(); const event = { dataTransfer: { @@ -247,14 +247,13 @@ describe( 'onBlockDrop', () => { targetBlockIndex, getBlockIndex, getClientIdsOfDescendants, - moveBlocksToPosition + moveBlocks ); eventHandler( event ); - expect( moveBlocksToPosition ).toHaveBeenCalledWith( + expect( moveBlocks ).toHaveBeenCalledWith( sourceClientIds, sourceRootClientId, - targetRootClientId, targetBlockIndex ); } ); @@ -267,7 +266,7 @@ describe( 'onBlockDrop', () => { const getBlockIndex = jest.fn( () => 1 ); // Dragged block is being dropped as a descendant of itself. const getClientIdsOfDescendants = () => []; - const moveBlocksToPosition = jest.fn(); + const moveBlocks = jest.fn(); const event = { dataTransfer: { @@ -289,14 +288,13 @@ describe( 'onBlockDrop', () => { targetBlockIndex, getBlockIndex, getClientIdsOfDescendants, - moveBlocksToPosition + moveBlocks ); eventHandler( event ); - expect( moveBlocksToPosition ).toHaveBeenCalledWith( + expect( moveBlocks ).toHaveBeenCalledWith( sourceClientIds, sourceRootClientId, - targetRootClientId, insertIndex ); } ); @@ -306,7 +304,7 @@ describe( 'onFilesDrop', () => { it( 'does nothing if hasUploadPermissions is false', () => { const updateBlockAttributes = jest.fn(); const canInsertBlockType = noop; - const insertBlocks = jest.fn(); + const insertOrReplaceBlocks = jest.fn(); const targetRootClientId = '1'; const targetBlockIndex = 0; const uploadPermissions = false; @@ -317,12 +315,12 @@ describe( 'onFilesDrop', () => { uploadPermissions, updateBlockAttributes, canInsertBlockType, - insertBlocks + insertOrReplaceBlocks ); onFileDropHandler(); expect( findTransform ).not.toHaveBeenCalled(); - expect( insertBlocks ).not.toHaveBeenCalled(); + expect( insertOrReplaceBlocks ).not.toHaveBeenCalled(); } ); it( 'does nothing if the block has no matching file transforms', () => { @@ -330,7 +328,7 @@ describe( 'onFilesDrop', () => { // to have no return value. findTransform.mockImplementation( noop ); const updateBlockAttributes = noop; - const insertBlocks = jest.fn(); + const insertOrReplaceBlocks = jest.fn(); const canInsertBlockType = noop; const targetRootClientId = '1'; const targetBlockIndex = 0; @@ -342,12 +340,12 @@ describe( 'onFilesDrop', () => { uploadPermissions, updateBlockAttributes, canInsertBlockType, - insertBlocks + insertOrReplaceBlocks ); onFileDropHandler(); expect( findTransform ).toHaveBeenCalled(); - expect( insertBlocks ).not.toHaveBeenCalled(); + expect( insertOrReplaceBlocks ).not.toHaveBeenCalled(); } ); it( 'inserts blocks if a valid transform can be found', () => { @@ -359,7 +357,7 @@ describe( 'onFilesDrop', () => { findTransform.mockImplementation( () => transformation ); const updateBlockAttributes = noop; const canInsertBlockType = noop; - const insertBlocks = jest.fn(); + const insertOrReplaceBlocks = jest.fn(); const targetRootClientId = '1'; const targetBlockIndex = 0; const uploadPermissions = true; @@ -370,7 +368,7 @@ describe( 'onFilesDrop', () => { uploadPermissions, updateBlockAttributes, canInsertBlockType, - insertBlocks + insertOrReplaceBlocks ); const files = 'test'; onFileDropHandler( files ); @@ -380,11 +378,7 @@ describe( 'onFilesDrop', () => { files, updateBlockAttributes ); - expect( insertBlocks ).toHaveBeenCalledWith( - blocks, - targetBlockIndex, - targetRootClientId - ); + expect( insertOrReplaceBlocks ).toHaveBeenCalledWith( blocks ); } ); } ); @@ -393,16 +387,16 @@ describe( 'onHTMLDrop', () => { pasteHandler.mockImplementation( () => [] ); const targetRootClientId = '1'; const targetBlockIndex = 0; - const insertBlocks = jest.fn(); + const insertOrReplaceBlocks = jest.fn(); const eventHandler = onHTMLDrop( targetRootClientId, targetBlockIndex, - insertBlocks + insertOrReplaceBlocks ); eventHandler(); - expect( insertBlocks ).not.toHaveBeenCalled(); + expect( insertOrReplaceBlocks ).not.toHaveBeenCalled(); } ); it( 'inserts blocks if the HTML can be converted into blocks', () => { @@ -410,19 +404,15 @@ describe( 'onHTMLDrop', () => { pasteHandler.mockImplementation( () => blocks ); const targetRootClientId = '1'; const targetBlockIndex = 0; - const insertBlocks = jest.fn(); + const insertOrReplaceBlocks = jest.fn(); const eventHandler = onHTMLDrop( targetRootClientId, targetBlockIndex, - insertBlocks + insertOrReplaceBlocks ); eventHandler(); - expect( insertBlocks ).toHaveBeenCalledWith( - blocks, - targetBlockIndex, - targetRootClientId - ); + expect( insertOrReplaceBlocks ).toHaveBeenCalledWith( blocks ); } ); } ); diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index 3284accf85d8f6..52202d05ad81da 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -497,18 +497,6 @@ export const moveBlocksToPosition = if ( ! canInsertBlocks ) { return; } - - // If the user is attempting to drop a block within its own - // nested blocks, return early as this would create infinite - // recursion. - if ( - clientIds.includes( toRootClientId ) || - select - .getClientIdsOfDescendants( clientIds ) - .includes( toRootClientId ) - ) { - return; - } } dispatch( { diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js index 7fc5fd975d1a04..2bdecc15d2a07c 100644 --- a/packages/block-editor/src/store/reducer.js +++ b/packages/block-editor/src/store/reducer.js @@ -974,12 +974,6 @@ export const blocks = flow( if ( fromRootClientId === toRootClientId ) { const subState = state[ toRootClientId ]; const fromIndex = subState.indexOf( clientIds[ 0 ] ); - - // If the block is moved to the same position, return the same state. - if ( fromIndex === index ) { - return state; - } - return { ...state, [ toRootClientId ]: moveTo( From 7c0b8695675a26651a6ed027f5ad4f88ad4ea2e7 Mon Sep 17 00:00:00 2001 From: Kai Hao Date: Tue, 20 Sep 2022 17:11:11 +0800 Subject: [PATCH 08/14] Address code reviews --- packages/block-library/src/paragraph/edit.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/block-library/src/paragraph/edit.js b/packages/block-library/src/paragraph/edit.js index e7a715edcbac8b..799d2b9d5c462c 100644 --- a/packages/block-library/src/paragraph/edit.js +++ b/packages/block-library/src/paragraph/edit.js @@ -59,13 +59,10 @@ function ParagraphBlock( { const { align, content, direction, dropCap, placeholder } = attributes; const isDropCapFeatureEnabled = useSetting( 'typography.dropCap' ); const [ paragraphElement, setParagraphElement ] = useState( null ); - const refCallback = ( element ) => { - setParagraphElement( element ); - }; const blockProps = useBlockProps( { ref: useMergeRefs( [ useOnEnter( { clientId, content } ), - refCallback, + setParagraphElement, ] ), className: classnames( { 'has-drop-cap': dropCap, From 76eb4031e40b15e5b7c12414cf2111b4f87da103 Mon Sep 17 00:00:00 2001 From: Kai Hao Date: Wed, 21 Sep 2022 11:06:20 +0800 Subject: [PATCH 09/14] Fix the tests and refactor dragFiles util --- .../block-library/src/paragraph/drop-zone.js | 1 + .../compose/src/hooks/use-drop-zone/index.js | 11 +-- .../src/page-utils/drag-files.ts | 86 ++++++++++++------- .../e2e/specs/editor/blocks/paragraph.spec.js | 15 +--- 4 files changed, 65 insertions(+), 48 deletions(-) diff --git a/packages/block-library/src/paragraph/drop-zone.js b/packages/block-library/src/paragraph/drop-zone.js index 696b5143726ed0..e51fb84acf8062 100644 --- a/packages/block-library/src/paragraph/drop-zone.js +++ b/packages/block-library/src/paragraph/drop-zone.js @@ -82,6 +82,7 @@ export default function DropZone( { paragraphElement, clientId } ) { { isVisible ? ( { - onDropRef.current = null; - onDragStartRef.current = null; - onDragEnterRef.current = null; - onDragLeaveRef.current = null; - onDragEndRef.current = null; - onDragOverRef.current = null; delete element.dataset.isDropZone; element.removeEventListener( 'drop', onDrop ); element.removeEventListener( 'dragenter', onDragEnter ); @@ -232,7 +226,10 @@ export default function useDropZone( { element.removeEventListener( 'dragleave', onDragLeave ); ownerDocument.removeEventListener( 'dragend', maybeDragEnd ); ownerDocument.removeEventListener( 'mousemove', maybeDragEnd ); - ownerDocument.addEventListener( 'dragenter', maybeDragStart ); + ownerDocument.removeEventListener( + 'dragenter', + maybeDragStart + ); }; }, [ isDisabled ] diff --git a/packages/e2e-test-utils-playwright/src/page-utils/drag-files.ts b/packages/e2e-test-utils-playwright/src/page-utils/drag-files.ts index e7b44fa1ab2800..f8e237e7e37f1d 100644 --- a/packages/e2e-test-utils-playwright/src/page-utils/drag-files.ts +++ b/packages/e2e-test-utils-playwright/src/page-utils/drag-files.ts @@ -16,6 +16,10 @@ type FileObject = { buffer: Buffer; }; +type Options = { + position?: { x: number; y: number }; +}; + /** * Simulate dragging files from outside the current page. * @@ -72,58 +76,82 @@ async function dragFiles( fileObjects ); - // Simulate dragging over the document. - await this.page.dispatchEvent( 'html', 'dragenter', { dataTransfer } ); + // CDP doesn't actually support dragging files, this is only a _good enough_ + // dummy data so that it will correctly send the relevant events. + const dragData = { + items: fileObjects.map( ( fileObject ) => ( { + mimeType: fileObject.mimeType ?? 'File', + data: fileObject.base64, + } ) ), + files: fileObjects.map( ( fileObject ) => fileObject.name ), + // Copy = 1, Link = 2, Move = 16. + dragOperationsMask: 1, + }; + + const cdpSession = await this.context.newCDPSession( this.page ); const position = { x: 0, y: 0, }; - const getCurrentTopMostElement = async () => { - const elementFromPosition = await this.page.evaluateHandle( - ( point ) => { - const element = document.elementFromPoint( point.x, point.y ); - return element; - }, - position - ); - - return elementFromPosition.asElement(); - }; - return { /** - * Move the cursor and drag the files to the specified position. + * Drag the files over an element (fires `dragenter` and `dragover` events). * - * @param x The X coordinate. - * @param y The Y coordinate. + * @param selector A selector to search for an element. + * @param options The optional options. + * @param options.position A point to use relative to the top-left corner of element padding box. If not specified, uses some visible point of the element. */ - dragTo: async ( x: number, y: number ) => { - position.x = x; - position.y = y; + dragOver: async ( selector: string, options: Options = {} ) => { + const boundingBox = await this.page + .locator( selector ) + .boundingBox(); - const elementHandle = await getCurrentTopMostElement(); - - if ( ! elementHandle ) { - return; + if ( ! boundingBox ) { + throw new Error( + 'Cannot find the element or the element is not visible on the viewport.' + ); } - await elementHandle.dispatchEvent( 'dragenter', { dataTransfer } ); + position.x = + boundingBox.x + + ( options.position?.x ?? boundingBox.width / 2 ); + position.y = + boundingBox.y + + ( options.position?.y ?? boundingBox.height / 2 ); + + await cdpSession.send( 'Input.dispatchDragEvent', { + type: 'dragEnter', + ...position, + data: dragData, + } ); + await cdpSession.send( 'Input.dispatchDragEvent', { + type: 'dragOver', + ...position, + data: dragData, + } ); }, + /** * Drop the files at the current position. */ drop: async () => { - const elementHandle = await getCurrentTopMostElement(); + const topMostElement = await this.page.evaluateHandle( + ( { x, y } ) => { + return document.elementFromPoint( x, y ); + }, + position + ); + const elementHandle = topMostElement.asElement(); if ( ! elementHandle ) { - throw new Error( - `No element at position (${ position.x }, ${ position.y }) to drop on` - ); + throw new Error( 'Element not found.' ); } await elementHandle.dispatchEvent( 'drop', { dataTransfer } ); + + await cdpSession.detach(); }, }; } diff --git a/test/e2e/specs/editor/blocks/paragraph.spec.js b/test/e2e/specs/editor/blocks/paragraph.spec.js index d844a1e82970c3..5f4f892e573e02 100644 --- a/test/e2e/specs/editor/blocks/paragraph.spec.js +++ b/test/e2e/specs/editor/blocks/paragraph.spec.js @@ -51,9 +51,6 @@ test.describe( 'Paragraph', () => { pageUtils, } ) => { await editor.insertBlock( { name: 'core/paragraph' } ); - const emptyParagraphBlock = page.locator( - '[data-type="core/paragraph"]' - ); const testImageName = '10x10_e2e_test_image_z9T8jK.png'; const testImagePath = path.join( @@ -62,18 +59,12 @@ test.describe( 'Paragraph', () => { testImageName ); - const { dragTo, drop } = await pageUtils.dragFiles( testImagePath ); - - const { x, y, width, height } = await emptyParagraphBlock.boundingBox(); - const centerPosition = { - x: x + width / 2, - y: y + height / 2, - }; + const { dragOver, drop } = await pageUtils.dragFiles( testImagePath ); - await dragTo( centerPosition.x, centerPosition.y ); + await dragOver( '[data-type="core/paragraph"]' ); await expect( - page.locator( 'text="Drop files to upload"' ) + page.locator( 'data-testid=empty-paragraph-drop-zone' ) ).toBeVisible(); await drop(); From 86946e2f634a3bace31cc64f0694dc30ea5461f1 Mon Sep 17 00:00:00 2001 From: Kai Hao Date: Wed, 21 Sep 2022 13:09:56 +0800 Subject: [PATCH 10/14] Add more e2e tests --- .../src/components/use-on-block-drop/index.js | 16 +- .../e2e/specs/editor/blocks/paragraph.spec.js | 185 ++++++++++++++---- 2 files changed, 154 insertions(+), 47 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 db21038b0ad9a3..3663aa3195e936 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 @@ -268,17 +268,13 @@ export default function useOnBlockDrop( const targetBlockClientId = targetBlockClientIds[ targetBlockIndex ]; - // Remove the source blocks and the target block. - removeBlocks( - [ ...sourceClientIds, targetBlockClientId ], - false - ); - // Insert the source blocks back to the target index. - insertBlocks( + // Remove the source blocks. + removeBlocks( sourceClientIds, false ); + // Replace the target block with the source blocks. + replaceBlocks( + targetBlockClientId, sourceBlocks, - insertIndex, - targetRootClientId, - true, + undefined, 0 ); } else { diff --git a/test/e2e/specs/editor/blocks/paragraph.spec.js b/test/e2e/specs/editor/blocks/paragraph.spec.js index 5f4f892e573e02..6a043153320d83 100644 --- a/test/e2e/specs/editor/blocks/paragraph.spec.js +++ b/test/e2e/specs/editor/blocks/paragraph.spec.js @@ -9,21 +9,10 @@ const path = require( 'path' ); const { test, expect } = require( '@wordpress/e2e-test-utils-playwright' ); test.describe( 'Paragraph', () => { - test.beforeAll( async ( { requestUtils } ) => { - await requestUtils.deleteAllMedia(); - } ); - test.beforeEach( async ( { admin } ) => { await admin.createNewPost(); } ); - test.afterEach( async ( { requestUtils } ) => { - await Promise.all( [ - requestUtils.deleteAllPosts(), - requestUtils.deleteAllMedia(), - ] ); - } ); - test( 'should output unwrapped editable paragraph', async ( { editor, page, @@ -45,37 +34,159 @@ test.describe( 'Paragraph', () => { expect( firstBlockTagName ).toBe( 'P' ); } ); - test( 'should allow dropping an image on en empty paragraph block', async ( { - editor, - page, - pageUtils, - } ) => { - await editor.insertBlock( { name: 'core/paragraph' } ); + test.describe( 'Empty paragraph', () => { + test.use( { + // Make the viewport large enough so that a scrollbar isn't displayed. + // Otherwise, the page scrolling can interfere with the test runner's + // ability to drop a block in the right location. + viewport: { + width: 960, + height: 1024, + }, + } ); - const testImageName = '10x10_e2e_test_image_z9T8jK.png'; - const testImagePath = path.join( - __dirname, - '../../../assets', - testImageName - ); + test.beforeAll( async ( { requestUtils } ) => { + await requestUtils.deleteAllMedia(); + } ); + + test.afterEach( async ( { requestUtils } ) => { + await requestUtils.deleteAllMedia(); + } ); + + test( 'should allow dropping an image on en empty paragraph block', async ( { + editor, + page, + pageUtils, + } ) => { + await editor.insertBlock( { name: 'core/paragraph' } ); - const { dragOver, drop } = await pageUtils.dragFiles( testImagePath ); + const testImageName = '10x10_e2e_test_image_z9T8jK.png'; + const testImagePath = path.join( + __dirname, + '../../../assets', + testImageName + ); - await dragOver( '[data-type="core/paragraph"]' ); + const { dragOver, drop } = await pageUtils.dragFiles( + testImagePath + ); - await expect( - page.locator( 'data-testid=empty-paragraph-drop-zone' ) - ).toBeVisible(); + await dragOver( '[data-type="core/paragraph"]' ); + + await expect( + page.locator( 'data-testid=empty-paragraph-drop-zone' ) + ).toBeVisible(); + + await drop(); + + const imageBlock = page.locator( + 'role=document[name="Block: Image"i]' + ); + await expect( imageBlock ).toBeVisible(); + await expect( imageBlock.locator( 'role=img' ) ).toHaveAttribute( + 'src', + new RegExp( testImageName.replace( '.', '\\.' ) ) + ); + } ); - await drop(); + test( 'should allow dropping blocks on en empty paragraph block', async ( { + editor, + page, + } ) => { + await editor.insertBlock( { + name: 'core/heading', + attributes: { content: 'My Heading' }, + } ); + await editor.insertBlock( { name: 'core/paragraph' } ); + await page.focus( 'text=My Heading' ); + await editor.showBlockToolbar(); - const imageBlock = page.locator( - 'role=document[name="Block: Image"i]' - ); - await expect( imageBlock ).toBeVisible(); - await expect( imageBlock.locator( 'role=img' ) ).toHaveAttribute( - 'src', - new RegExp( testImageName.replace( '.', '\\.' ) ) - ); + const dragHandle = page.locator( + 'role=toolbar[name="Block tools"i] >> role=button[name="Drag"i][include-hidden]' + ); + await dragHandle.hover(); + await page.mouse.down(); + + const emptyParagraph = page.locator( + '[data-type="core/paragraph"][data-empty="true"]' + ); + const boundingBox = await emptyParagraph.boundingBox(); + // Call the move function twice to make sure the `dragOver` event is sent. + // @see https://github.com/microsoft/playwright/issues/17153 + for ( let i = 0; i < 2; i += 1 ) { + await page.mouse.move( boundingBox.x, boundingBox.y ); + } + + await expect( + page.locator( 'data-testid=empty-paragraph-drop-zone' ) + ).toBeVisible(); + + await page.mouse.up(); + + await expect.poll( editor.getEditedPostContent ) + .toBe( ` +

My Heading

+` ); + } ); + + test( 'should allow dropping HTML on en empty paragraph block', async ( { + editor, + page, + } ) => { + await editor.insertBlock( { name: 'core/paragraph' } ); + + // Insert a dummy draggable element on the page to simulate dragging + // HTML from other places. + await page.evaluate( () => { + const draggable = document.createElement( 'div' ); + draggable.draggable = true; + draggable.style.width = '10px'; + draggable.style.height = '10px'; + // Position it at the top left corner for convenience. + draggable.style.position = 'fixed'; + draggable.style.top = 0; + draggable.style.left = 0; + draggable.style.zIndex = 999999; + + draggable.addEventListener( + 'dragstart', + ( event ) => { + // Set the data transfer to some HTML on dragstart. + event.dataTransfer.setData( + 'text/html', + '

My Heading

' + ); + }, + { once: true } + ); + + document.body.appendChild( draggable ); + } ); + + // This is where the dummy draggable element is at. + await page.mouse.move( 0, 0 ); + await page.mouse.down(); + + const emptyParagraph = page.locator( + '[data-type="core/paragraph"][data-empty="true"]' + ); + const boundingBox = await emptyParagraph.boundingBox(); + // Call the move function twice to make sure the `dragOver` event is sent. + // @see https://github.com/microsoft/playwright/issues/17153 + for ( let i = 0; i < 2; i += 1 ) { + await page.mouse.move( boundingBox.x, boundingBox.y ); + } + + await expect( + page.locator( 'data-testid=empty-paragraph-drop-zone' ) + ).toBeVisible(); + + await page.mouse.up(); + + await expect.poll( editor.getEditedPostContent ) + .toBe( ` +

My Heading

+` ); + } ); } ); } ); From ea314b8b7f5b86d5bde43d5ad3cfdf1d3baa982a Mon Sep 17 00:00:00 2001 From: Kai Hao Date: Wed, 21 Sep 2022 17:42:46 +0800 Subject: [PATCH 11/14] Fix dropzone memory leak --- packages/block-library/CHANGELOG.md | 2 +- packages/compose/src/hooks/use-drop-zone/index.js | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/block-library/CHANGELOG.md b/packages/block-library/CHANGELOG.md index 1b06c0336e69e2..d887afb9fcae2b 100644 --- a/packages/block-library/CHANGELOG.md +++ b/packages/block-library/CHANGELOG.md @@ -7,7 +7,7 @@ ### New Feature - Made it possible to import individual blocks ([#42258](https://github.com/WordPress/gutenberg/pull/42258)). Check [README](./README.md#loading-individual-blocks) for more information. -- Paragraph block: You can now drag an image on an empty Paragraph block to create an image block ([#42722](https://github.com/WordPress/gutenberg/pull/42722)). +- Paragraph block: You can now drop files/blocks/HTML on an empty Paragraph block to transform it into relevant blocks ([#42722](https://github.com/WordPress/gutenberg/pull/42722)). ## 7.13.0 (2022-08-24) diff --git a/packages/compose/src/hooks/use-drop-zone/index.js b/packages/compose/src/hooks/use-drop-zone/index.js index 860e51c83a0991..59d89cb70f5123 100644 --- a/packages/compose/src/hooks/use-drop-zone/index.js +++ b/packages/compose/src/hooks/use-drop-zone/index.js @@ -200,7 +200,10 @@ export default function useDropZone( { isDragging = false; - ownerDocument.addEventListener( 'dragenter', maybeDragStart ); + ownerDocument.removeEventListener( + 'dragenter', + maybeDragStart + ); ownerDocument.removeEventListener( 'dragend', maybeDragEnd ); ownerDocument.removeEventListener( 'mousemove', maybeDragEnd ); From 49387fc6b16a45d80ca9e8da02a02f70e566aa34 Mon Sep 17 00:00:00 2001 From: Kai Hao Date: Thu, 22 Sep 2022 22:47:57 +0800 Subject: [PATCH 12/14] Try to fix stalled listeners issue in useDropZone --- packages/compose/src/hooks/use-drop-zone/index.js | 9 --------- 1 file changed, 9 deletions(-) diff --git a/packages/compose/src/hooks/use-drop-zone/index.js b/packages/compose/src/hooks/use-drop-zone/index.js index 59d89cb70f5123..9b2cc5715f7219 100644 --- a/packages/compose/src/hooks/use-drop-zone/index.js +++ b/packages/compose/src/hooks/use-drop-zone/index.js @@ -107,11 +107,6 @@ export default function useDropZone( { isDragging = true; - ownerDocument.removeEventListener( - 'dragenter', - maybeDragStart - ); - // Note that `dragend` doesn't fire consistently for file and // HTML drag events where the drag origin is outside the browser // window. In Firefox it may also not fire if the originating @@ -200,10 +195,6 @@ export default function useDropZone( { isDragging = false; - ownerDocument.removeEventListener( - 'dragenter', - maybeDragStart - ); ownerDocument.removeEventListener( 'dragend', maybeDragEnd ); ownerDocument.removeEventListener( 'mousemove', maybeDragEnd ); From 4980a3d58b60f57344247a7a78e28e715ef54e47 Mon Sep 17 00:00:00 2001 From: Kai Hao Date: Thu, 22 Sep 2022 22:52:34 +0800 Subject: [PATCH 13/14] Address code reviews --- .../components/use-block-drop-zone/index.js | 9 +++++--- .../src/components/use-on-block-drop/index.js | 23 +++++++++++-------- 2 files changed, 19 insertions(+), 13 deletions(-) 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 870085de98dfc3..b71bed4c2139fd 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 @@ -148,12 +148,15 @@ export default function useBlockDropZone( { const nextBlock = blockElements[ targetIndex ]; const previousBlock = blockElements[ targetIndex - 1 ]; + // Don't show the insertion point when it's near an empty paragraph block. if ( - ( ! nextBlock || ! isEmptyParagraph( nextBlock ) ) && - ( ! previousBlock || ! isEmptyParagraph( previousBlock ) ) + isEmptyParagraph( nextBlock ) || + isEmptyParagraph( previousBlock ) ) { - showInsertionPoint( targetRootClientId, targetIndex ); + return; } + + showInsertionPoint( targetRootClientId, targetIndex ); } }, [] ), 200 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 3663aa3195e936..245f912bcf4da7 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 @@ -8,7 +8,7 @@ import { getBlockTransforms, pasteHandler, } from '@wordpress/blocks'; -import { useDispatch, useSelect } from '@wordpress/data'; +import { useDispatch, useSelect, useRegistry } from '@wordpress/data'; import { getFilesFromDataTransfer } from '@wordpress/dom'; /** @@ -231,6 +231,7 @@ export default function useOnBlockDrop( replaceBlocks, removeBlocks, } = useDispatch( blockEditorStore ); + const registry = useRegistry(); const insertOrReplaceBlocks = useCallback( ( blocks, updateSelection = true, initialPosition = 0 ) => { @@ -268,15 +269,17 @@ export default function useOnBlockDrop( const targetBlockClientId = targetBlockClientIds[ targetBlockIndex ]; - // Remove the source blocks. - removeBlocks( sourceClientIds, false ); - // Replace the target block with the source blocks. - replaceBlocks( - targetBlockClientId, - sourceBlocks, - undefined, - 0 - ); + registry.batch( () => { + // Remove the source blocks. + removeBlocks( sourceClientIds, false ); + // Replace the target block with the source blocks. + replaceBlocks( + targetBlockClientId, + sourceBlocks, + undefined, + 0 + ); + } ); } else { moveBlocksToPosition( sourceClientIds, From db6be4356de20b76cda133b4f7d1a1bccb64e70b Mon Sep 17 00:00:00 2001 From: Kai Hao Date: Thu, 22 Sep 2022 23:03:42 +0800 Subject: [PATCH 14/14] Remove null check for targetIndex --- .../block-editor/src/components/use-block-drop-zone/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 b71bed4c2139fd..3bef31546c711e 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 @@ -144,7 +144,7 @@ export default function useBlockDropZone( { setTargetBlockIndex( targetIndex === undefined ? 0 : targetIndex ); - if ( targetIndex !== undefined && targetIndex !== null ) { + if ( targetIndex !== undefined ) { const nextBlock = blockElements[ targetIndex ]; const previousBlock = blockElements[ targetIndex - 1 ];