Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drag and drop: fix drop zones on block drag #67317

Merged
merged 10 commits into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,9 @@
* WordPress dependencies
*/
import { Draggable } from '@wordpress/components';
import {
createBlock,
serialize,
store as blocksStore,
} from '@wordpress/blocks';
import { createBlock, store as blocksStore } from '@wordpress/blocks';
import { useDispatch, useSelect } from '@wordpress/data';
import { useMemo } from '@wordpress/element';

/**
* Internal dependencies
Expand All @@ -24,20 +21,6 @@ const InserterDraggableBlocks = ( {
children,
pattern,
} ) => {
const transferData = {
type: 'inserter',
blocks,
};

const blocksContainMedia =
blocks.filter(
( block ) =>
( block.name === 'core/image' ||
block.name === 'core/audio' ||
block.name === 'core/video' ) &&
( block.attributes.url || block.attributes.src )
).length > 0;

const blockTypeIcon = useSelect(
( select ) => {
const { getBlockType } = select( blocksStore );
Expand All @@ -52,6 +35,13 @@ const InserterDraggableBlocks = ( {
useDispatch( blockEditorStore )
);

const patternBlock = useMemo( () => {
return pattern?.type === INSERTER_PATTERN_TYPES.user &&
pattern?.syncStatus !== 'unsynced'
? [ createBlock( 'core/block', { ref: pattern.id } ) ]
: undefined;
}, [ pattern?.type, pattern?.syncStatus, pattern?.id ] );

if ( ! isEnabled ) {
return children( {
draggable: false,
Expand All @@ -60,21 +50,21 @@ const InserterDraggableBlocks = ( {
} );
}

const draggableBlocks = patternBlock ?? blocks;
return (
<Draggable
__experimentalTransferDataType="wp-blocks"
transferData={ transferData }
transferData={ { type: 'inserter', blocks: draggableBlocks } }
onDragStart={ ( event ) => {
startDragging();
const parsedBlocks =
pattern?.type === INSERTER_PATTERN_TYPES.user &&
pattern?.syncStatus !== 'unsynced'
? [ createBlock( 'core/block', { ref: pattern.id } ) ]
: blocks;
event.dataTransfer.setData(
blocksContainMedia ? 'default' : 'text/html',
serialize( parsedBlocks )
);
for ( const block of draggableBlocks ) {
const type = `wp-block:${ block.name }`;
// This will fill in the dataTransfer.types array so that
// the drop zone can check if the draggable is eligible.
// Unfortuantely, on drag start, we don't have access to the
// actual data, only the data keys/types.
event.dataTransfer.items.add( '', type );
}
} }
onDragEnd={ () => {
stopDragging();
Expand Down
53 changes: 25 additions & 28 deletions packages/block-editor/src/components/media-placeholder/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import { __ } from '@wordpress/i18n';
import { useState, useEffect } from '@wordpress/element';
import { useSelect } from '@wordpress/data';
import { keyboardReturn } from '@wordpress/icons';
import { pasteHandler } from '@wordpress/blocks';
import deprecated from '@wordpress/deprecated';

/**
Expand All @@ -29,6 +28,7 @@ import MediaUpload from '../media-upload';
import MediaUploadCheck from '../media-upload/check';
import URLPopover from '../url-popover';
import { store as blockEditorStore } from '../../store';
import { parseDropEvent } from '../use-on-block-drop';

const noop = () => {};

Expand Down Expand Up @@ -229,30 +229,15 @@ export function MediaPlaceholder( {
} );
};

async function handleBlocksDrop( blocks ) {
if ( ! blocks || ! Array.isArray( blocks ) ) {
return;
}
async function handleBlocksDrop( event ) {
const { blocks } = parseDropEvent( event );

function recursivelyFindMediaFromBlocks( _blocks ) {
return _blocks.flatMap( ( block ) =>
( block.name === 'core/image' ||
block.name === 'core/audio' ||
block.name === 'core/video' ) &&
( block.attributes.url || block.attributes.src )
? [ block ]
: recursivelyFindMediaFromBlocks( block.innerBlocks )
);
}

const mediaBlocks = recursivelyFindMediaFromBlocks( blocks );

if ( ! mediaBlocks.length ) {
if ( ! blocks?.length ) {
return;
}

const uploadedMediaList = await Promise.all(
mediaBlocks.map( ( block ) => {
blocks.map( ( block ) => {
const blockType = block.name.split( '/' )[ 1 ];
if ( block.attributes.id ) {
block.attributes.type = blockType;
Expand Down Expand Up @@ -292,13 +277,6 @@ export function MediaPlaceholder( {
}
}

async function onDrop( event ) {
const blocks = pasteHandler( {
HTML: event.dataTransfer?.getData( 'default' ),
} );
return await handleBlocksDrop( blocks );
}

const onUpload = ( event ) => {
onFilesUpload( event.target.files );
};
Expand Down Expand Up @@ -385,7 +363,26 @@ export function MediaPlaceholder( {
return null;
}

return <DropZone onFilesDrop={ onFilesUpload } onDrop={ onDrop } />;
return (
<DropZone
onFilesDrop={ onFilesUpload }
onDrop={ handleBlocksDrop }
isEligible={ ( dataTransfer ) => {
const prefix = 'wp-block:core/';
const types = [];
for ( const type of dataTransfer.types ) {
if ( type.startsWith( prefix ) ) {
types.push( type.slice( prefix.length ) );
}
}
return (
types.every( ( type ) =>
allowedTypes.includes( type )
) && ( multiple ? true : types.length === 1 )
);
} }
/>
);
};

const renderCancelLink = () => {
Expand Down
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
- `ColorPicker`: Update sizes of color format select and copy button ([#67093](https://github.com/WordPress/gutenberg/pull/67093)).
- `ComboboxControl`: Update reset button size ([#67215](https://github.com/WordPress/gutenberg/pull/67215)).
- `Autocomplete`: Increase option height ([#67214](https://github.com/WordPress/gutenberg/pull/67214)).
- `DropZone`: Add `isEligible` prop to allow customizing whether the drop zone should activate ([#67317](https://github.com/WordPress/gutenberg/pull/67317)).
- `CircularOptionPicker`: Update `Button` sizes to be ready for 40px default size ([#67285](https://github.com/WordPress/gutenberg/pull/67285)).

### Experimental
Expand Down
45 changes: 21 additions & 24 deletions packages/components/src/drop-zone/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { __experimentalUseDropZone as useDropZone } from '@wordpress/compose';
/**
* Internal dependencies
*/
import type { DropType, DropZoneProps } from './types';
import type { DropZoneProps } from './types';
import type { WordPressComponentProps } from '../context';

/**
Expand Down Expand Up @@ -47,19 +47,22 @@ export function DropZoneComponent( {
onFilesDrop,
onHTMLDrop,
onDrop,
isEligible = () => true,
...restProps
}: WordPressComponentProps< DropZoneProps, 'div', false > ) {
const [ isDraggingOverDocument, setIsDraggingOverDocument ] =
useState< boolean >();
const [ isDraggingOverElement, setIsDraggingOverElement ] =
useState< boolean >();
const [ type, setType ] = useState< DropType >();
const [ isActive, setIsActive ] = useState< boolean >();
const ref = useDropZone( {
onDrop( event ) {
const files = event.dataTransfer
? getFilesFromDataTransfer( event.dataTransfer )
: [];
const html = event.dataTransfer?.getData( 'text/html' );
if ( ! event.dataTransfer ) {
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would the event ever not have a dataTransfer? If so, we're changing the behaviour of this function because currently on trunk onDrop gets called if it exists, regardless of dataTransfer.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will never be the case, it's just to make TypeScript happy. We already had these conditions before


const files = getFilesFromDataTransfer( event.dataTransfer );
const html = event.dataTransfer.getData( 'text/html' );

/**
* From Windows Chrome 96, the `event.dataTransfer` returns both file object and HTML.
Expand All @@ -76,32 +79,31 @@ export function DropZoneComponent( {
onDragStart( event ) {
setIsDraggingOverDocument( true );

let _type: DropType = 'default';
if ( ! event.dataTransfer ) {
return;
}

/**
* From Windows Chrome 96, the `event.dataTransfer` returns both file object and HTML.
* The order of the checks is important to recognize the HTML drop.
*/
if ( event.dataTransfer?.types.includes( 'text/html' ) ) {
_type = 'html';
if ( event.dataTransfer.types.includes( 'text/html' ) ) {
setIsActive( !! onHTMLDrop );
} else if (
// Check for the types because sometimes the files themselves
// are only available on drop.
event.dataTransfer?.types.includes( 'Files' ) ||
( event.dataTransfer
? getFilesFromDataTransfer( event.dataTransfer )
: []
).length > 0
event.dataTransfer.types.includes( 'Files' ) ||
getFilesFromDataTransfer( event.dataTransfer ).length > 0
) {
_type = 'file';
setIsActive( !! onFilesDrop );
} else {
setIsActive( !! onDrop && isEligible( event.dataTransfer ) );
}

setType( _type );
},
onDragEnd() {
setIsDraggingOverElement( false );
setIsDraggingOverDocument( false );
setType( undefined );
setIsActive( undefined );
},
onDragEnter() {
setIsDraggingOverElement( true );
Expand All @@ -112,14 +114,9 @@ export function DropZoneComponent( {
} );

const classes = clsx( 'components-drop-zone', className, {
'is-active':
( isDraggingOverDocument || isDraggingOverElement ) &&
( ( type === 'file' && onFilesDrop ) ||
( type === 'html' && onHTMLDrop ) ||
( type === 'default' && onDrop ) ),
'is-active': isActive,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, this looks a lot better!

'is-dragging-over-document': isDraggingOverDocument,
'is-dragging-over-element': isDraggingOverElement,
[ `is-dragging-${ type }` ]: !! type,
} );

return (
Expand Down
5 changes: 5 additions & 0 deletions packages/components/src/drop-zone/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,9 @@ export type DropZoneProps = {
* It receives the HTML being dropped as an argument.
*/
onHTMLDrop?: ( html: string ) => void;
/**
* A function to determine if the drop zone is eligible to handle the drop
* data transfer items.
*/
isEligible?: ( dataTransfer: DataTransfer ) => boolean;
};
23 changes: 13 additions & 10 deletions test/e2e/specs/editor/blocks/image.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -528,14 +528,13 @@ test.describe( 'Image', () => {
name: 'Block: Image',
} );

const html = `
<figure>
<img src="https://live.staticflickr.com/3894/14962688165_04759a8b03_b.jpg" alt="Cat">
<figcaption>"Cat" by tomhouslay is licensed under <a href="https://creativecommons.org/licenses/by-nc/2.0/?ref=openverse">CC BY-NC 2.0</a>.</figcaption>
</figure>
`;

await page.evaluate( ( _html ) => {
await page.evaluate( () => {
const { createBlock } = window.wp.blocks;
const block = createBlock( 'core/image', {
url: 'https://live.staticflickr.com/3894/14962688165_04759a8b03_b.jpg',
alt: 'Cat',
caption: `"Cat" by tomhouslay is licensed under <a href="https://creativecommons.org/licenses/by-nc/2.0/?ref=openverse">CC BY-NC 2.0</a>.`,
} );
const dummy = document.createElement( 'div' );
dummy.style.width = '10px';
dummy.style.height = '10px';
Expand All @@ -545,13 +544,17 @@ test.describe( 'Image', () => {
dummy.style.left = 0;
dummy.draggable = 'true';
dummy.addEventListener( 'dragstart', ( event ) => {
event.dataTransfer.setData( 'default', _html );
event.dataTransfer.setData(
'wp-blocks',
JSON.stringify( { blocks: [ block ] } )
);
event.dataTransfer.setData( 'wp-block:core/image', '' );
setTimeout( () => {
dummy.remove();
}, 0 );
} );
document.body.appendChild( dummy );
}, html );
} );

await page.mouse.move( 0, 0 );
await page.mouse.down();
Expand Down
Loading