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

Allow dragging-and-dropping images from the inserter to image blocks #49673

Merged
merged 18 commits into from
May 4, 2023
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
2 changes: 2 additions & 0 deletions packages/block-editor/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Unreleased

- `MediaPlaceholder`: Remove the undocumented `onHTMLDrop` prop ([#49673](https://github.com/WordPress/gutenberg/pull/49673)).

## 12.0.0 (2023-04-26)

### Breaking Changes
Expand Down
16 changes: 10 additions & 6 deletions packages/block-editor/src/components/block-draggable/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,18 @@ const BlockDraggable = ( {
__experimentalTransferDataType="wp-blocks"
transferData={ transferData }
onDragStart={ ( event ) => {
startDraggingBlocks( clientIds );
isDragging.current = true;
// Defer hiding the dragged source element to the next
// frame to enable dragging.
window.requestAnimationFrame( () => {
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 is to retain the former behavior of asynchronous onDragStart before. I also changed setTimeout to requestAnimationFrame to better describe the intent.

startDraggingBlocks( clientIds );
isDragging.current = true;

startScrolling( event );
startScrolling( event );

if ( onDragStart ) {
onDragStart();
}
if ( onDragStart ) {
onDragStart();
}
} );
} }
onDragOver={ scrollOnDragOver }
onDragEnd={ () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* WordPress dependencies
*/
import { Draggable } from '@wordpress/components';
import { serialize } from '@wordpress/blocks';
/**
* Internal dependencies
*/
Expand All @@ -23,6 +24,9 @@ const InserterDraggableBlocks = ( {
<Draggable
__experimentalTransferDataType="wp-blocks"
transferData={ transferData }
onDragStart={ ( event ) => {
event.dataTransfer.setData( 'text/html', serialize( blocks ) );
kevin940726 marked this conversation as resolved.
Show resolved Hide resolved
} }
__experimentalDragComponent={
<BlockDraggableChip
count={ blocks.length }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ 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';

/**
* Internal dependencies
Expand Down Expand Up @@ -74,12 +76,19 @@ export function MediaPlaceholder( {
onToggleFeaturedImage,
onDoubleClick,
onFilesPreUpload = noop,
onHTMLDrop = noop,
kevin940726 marked this conversation as resolved.
Show resolved Hide resolved
onHTMLDrop: deprecatedOnHTMLDrop,
children,
mediaLibraryButton,
placeholder,
style,
} ) {
if ( deprecatedOnHTMLDrop ) {
deprecated( 'wp.blockEditor.MediaPlaceholder onHTMLDrop prop', {
since: '6.2',
version: '6.4',
} );
}

const mediaUpload = useSelect( ( select ) => {
const { getSettings } = select( blockEditorStore );
return getSettings().mediaUpload;
Expand Down Expand Up @@ -177,6 +186,70 @@ export function MediaPlaceholder( {
} );
};

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

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

const mediaBlocks = recursivelyFindMediaFromBlocks( blocks );

if ( ! mediaBlocks.length ) {
return;
}

const uploadedMediaList = await Promise.all(
mediaBlocks.map( ( block ) =>
block.attributes.id
? block.attributes
: new Promise( ( resolve, reject ) => {
window
.fetch( block.attributes.url )
.then( ( response ) => response.blob() )
.then( ( blob ) =>
mediaUpload( {
filesList: [ blob ],
additionalData: {
title: block.attributes.title,
alt_text: block.attributes.alt,
caption: block.attributes.caption,
},
onFileChange: ( [ media ] ) => {
if ( media.id ) {
resolve( media );
}
},
allowedTypes,
onError: reject,
} )
)
.catch( () => resolve( block.attributes.url ) );
} )
)
).catch( ( err ) => onError( err ) );

if ( multiple ) {
onSelect( uploadedMediaList );
} else {
onSelect( uploadedMediaList[ 0 ] );
}
}

async function onHTMLDrop( HTML ) {
const blocks = pasteHandler( { HTML } );
return await handleBlocksDrop( blocks );
}

const onUpload = ( event ) => {
onFilesUpload( event.target.files );
};
Expand Down
6 changes: 0 additions & 6 deletions packages/block-library/src/gallery/editor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,6 @@ figure.wp-block-gallery {
// See https://github.com/WordPress/gutenberg/pull/10358

display: block;
&.has-nested-images {
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 have some unexpected side effects, unfortunately. Namely it will no longer be possible to drag between two image blocks when the gap is zero. We'll have to determine which is the better trade-off, and what potential other options we have to solve this.

.components-drop-zone {
display: none;
pointer-events: none;
}
}

> .blocks-gallery-caption {
flex: 0 0 100%;
Expand Down
6 changes: 5 additions & 1 deletion packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,16 @@

- `CheckboxControl`, `CustomGradientPicker`, `FormToggle`, : Refactor and correct the focus style for consistency ([#50127](https://github.com/WordPress/gutenberg/pull/50127)).

### Breaking Changes

- `onDragStart` in `<Draggable>` is now a synchronous function to allow setting additional data for `event.dataTransfer` ([#49673](https://github.com/WordPress/gutenberg/pull/49673)).
Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, this is a breaking change, but I think it actually fixes a bug so it might not be that impactful. c.c. @ciampo if you think this is acceptable :).

Copy link
Contributor

@ciampo ciampo May 8, 2023

Choose a reason for hiding this comment

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

Sorry, still catching up through the notifications pile. Sounds good to me!

Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably add a dev note for this change, though. I've added the GH label


### Internal

- `NavigableContainer`: Convert to TypeScript ([#49377](https://github.com/WordPress/gutenberg/pull/49377)).
- `ToolbarItem`: Convert to TypeScript ([#49190](https://github.com/WordPress/gutenberg/pull/49190)).
- Move rich-text related types to the rich-text package ([#49651](https://github.com/WordPress/gutenberg/pull/49651)).
- `SlotFill`: simplified the implementation and removed unused code ([#50098](https://github.com/WordPress/gutenberg/pull/50098) and [#50133](https://github.com/WordPress/gutenberg/pull/50133))
- `SlotFill`: simplified the implementation and removed unused code ([#50098](https://github.com/WordPress/gutenberg/pull/50098) and [#50133](https://github.com/WordPress/gutenberg/pull/50133)).

### Documentation

Expand Down
10 changes: 1 addition & 9 deletions packages/components/src/draggable/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -212,14 +212,8 @@ export function Draggable( {
// Update cursor to 'grabbing', document wide.
ownerDocument.body.classList.add( bodyClass );

// Allow the Synthetic Event to be accessed from asynchronous code.
// https://reactjs.org/docs/events.html#event-pooling
event.persist();
kevin940726 marked this conversation as resolved.
Show resolved Hide resolved

let timerId: number | undefined;

if ( onDragStart ) {
timerId = setTimeout( () => onDragStart( event ) );
onDragStart( event );
kevin940726 marked this conversation as resolved.
Show resolved Hide resolved
}

cleanup.current = () => {
Expand All @@ -236,8 +230,6 @@ export function Draggable( {
ownerDocument.body.classList.remove( bodyClass );

ownerDocument.removeEventListener( 'dragover', throttledDragOver );

clearTimeout( timerId );
};
}

Expand Down
146 changes: 146 additions & 0 deletions test/e2e/specs/editor/blocks/image.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,152 @@ test.describe( 'Image', () => {
<figure class="wp-block-image"><img alt=""/></figure>
<!-- /wp:image -->` );
} );

test( 'can be replaced by dragging-and-dropping images from the inserter', async ( {
page,
editor,
} ) => {
await editor.insertBlock( { name: 'core/image' } );
const imageBlock = page.getByRole( 'document', {
name: 'Block: Image',
} );
const blockLibrary = page.getByRole( 'region', {
name: 'Block Library',
} );

async function openMediaTab() {
await page
.getByRole( 'button', { name: 'Toggle block inserter' } )
.click();

await blockLibrary.getByRole( 'tab', { name: 'Media' } ).click();

await blockLibrary
.getByRole( 'tabpanel', { name: 'Media' } )
.getByRole( 'button', { name: 'Openverse' } )
.click();
}

await openMediaTab();

// Drag the first image from the media library into the image block.
await blockLibrary
.getByRole( 'listbox', { name: 'Media List' } )
.getByRole( 'option' )
.first()
.dragTo( imageBlock );

await expect( async () => {
const blocks = await editor.getBlocks();
expect( blocks ).toHaveLength( 1 );

const [
{
attributes: { url },
},
] = blocks;
expect(
await imageBlock.getByRole( 'img' ).getAttribute( 'src' )
).toBe( url );
expect(
new URL( url ).host,
'should be updated to the media library'
).toBe( new URL( page.url() ).host );
}, 'should update the image from the media inserter' ).toPass();
const [
{
attributes: { url: firstUrl },
},
] = await editor.getBlocks();

await openMediaTab();

// Drag the second image from the media library into the image block.
await blockLibrary
.getByRole( 'listbox', { name: 'Media List' } )
.getByRole( 'option' )
.nth( 1 )
.dragTo( imageBlock );

await expect( async () => {
const blocks = await editor.getBlocks();
expect( blocks ).toHaveLength( 1 );

const [
{
attributes: { url },
},
] = blocks;
expect( url ).not.toBe( firstUrl );
expect(
await imageBlock.getByRole( 'img' ).getAttribute( 'src' )
).toBe( url );
expect(
new URL( url ).host,
'should be updated to the media library'
).toBe( new URL( page.url() ).host );
}, 'should replace the original image with the second image' ).toPass();
} );

test( 'should allow dragging and dropping HTML to media placeholder', async ( {
page,
editor,
} ) => {
await editor.insertBlock( { name: 'core/image' } );
const imageBlock = page.getByRole( 'document', {
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 ) => {
const dummy = document.createElement( 'div' );
dummy.style.width = '10px';
dummy.style.height = '10px';
dummy.style.zIndex = 99999;
dummy.style.position = 'fixed';
dummy.style.top = 0;
dummy.style.left = 0;
dummy.draggable = 'true';
dummy.addEventListener( 'dragstart', ( event ) => {
event.dataTransfer.setData( 'text/html', _html );
setTimeout( () => {
dummy.remove();
}, 0 );
} );
document.body.appendChild( dummy );
}, html );

await page.mouse.move( 0, 0 );
await page.mouse.down();
await imageBlock.hover();
await page.mouse.up();

const host = new URL( page.url() ).host;

await expect.poll( editor.getBlocks ).toMatchObject( [
{
name: 'core/image',
attributes: {
link: expect.stringContaining( host ),
url: expect.stringContaining( host ),
id: expect.any( Number ),
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 url = ( await editor.getBlocks() )[ 0 ].attributes.url;
await expect( imageBlock.getByRole( 'img' ) ).toHaveAttribute(
'src',
url
);
} );
} );

class ImageBlockUtils {
Expand Down