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

Drop zone: rewrite with hooks (useDropZone) #19514

Merged
merged 14 commits into from
Jan 10, 2020
62 changes: 45 additions & 17 deletions packages/block-editor/src/components/block-drop-zone/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
findTransform,
} from '@wordpress/blocks';
import { useDispatch, useSelect } from '@wordpress/data';
import { useEffect, useState } from '@wordpress/element';

const parseDropEvent = ( event ) => {
let result = {
Expand All @@ -30,7 +31,10 @@ const parseDropEvent = ( event ) => {
return result;
};

export default function useBlockDropZone( { element, clientId, rootClientId } ) {
export default function useBlockDropZone( { element, rootClientId } ) {
// const [ inserterElement, setInserterElement ] = useState( null );
const [ clientId, setInserterClientId ] = useState( null );

function selector( select ) {
const {
getBlockIndex,
Expand All @@ -40,6 +44,7 @@ export default function useBlockDropZone( { element, clientId, rootClientId } )
} = select( 'core/block-editor' );
return {
getBlockIndex,
blockIndex: getBlockIndex( clientId, rootClientId ),
getClientIdsOfDescendants,
hasUploadPermissions: !! getSettings().mediaUpload,
isLockedAll: getTemplateLock( rootClientId ) === 'all',
Expand All @@ -48,24 +53,18 @@ export default function useBlockDropZone( { element, clientId, rootClientId } )

const {
getBlockIndex,
blockIndex,
getClientIdsOfDescendants,
hasUploadPermissions,
isLockedAll,
} = useSelect( selector, [ rootClientId ] );
} = useSelect( selector, [ rootClientId, clientId ] );
const {
insertBlocks,
updateBlockAttributes,
moveBlockToPosition,
} = useDispatch( 'core/block-editor' );

function getInsertIndex( position ) {
if ( clientId !== undefined ) {
const index = getBlockIndex( clientId, rootClientId );
return ( position && position.y === 'top' ) ? index : index + 1;
}
}

function onFilesDrop( files, position ) {
function onFilesDrop( files ) {
if ( ! hasUploadPermissions ) {
return;
}
Expand All @@ -76,21 +75,20 @@ export default function useBlockDropZone( { element, clientId, rootClientId } )
);

if ( transformation ) {
const insertIndex = getInsertIndex( position );
const blocks = transformation.transform( files, updateBlockAttributes );
insertBlocks( blocks, insertIndex, rootClientId );
insertBlocks( blocks, blockIndex, rootClientId );
}
}

function onHTMLDrop( HTML, position ) {
function onHTMLDrop( HTML ) {
const blocks = pasteHandler( { HTML, mode: 'BLOCKS' } );

if ( blocks.length ) {
insertBlocks( blocks, getInsertIndex( position ), rootClientId );
insertBlocks( blocks, blockIndex, rootClientId );
}
}

function onDrop( event, position ) {
function onDrop( event ) {
const { srcRootClientId, srcClientId, srcIndex, type } = parseDropEvent( event );

const isBlockDropType = ( dropType ) => dropType === 'block';
Expand All @@ -109,18 +107,48 @@ export default function useBlockDropZone( { element, clientId, rootClientId } )
}

const dstIndex = clientId ? getBlockIndex( clientId, rootClientId ) : undefined;
const positionIndex = getInsertIndex( position );
const positionIndex = blockIndex;
// If the block is kept at the same level and moved downwards,
// subtract to account for blocks shifting upward to occupy its old position.
const insertIndex = dstIndex && srcIndex < dstIndex && isSameLevel( srcRootClientId, rootClientId ) ? positionIndex - 1 : positionIndex;
moveBlockToPosition( srcClientId, srcRootClientId, rootClientId, insertIndex );
}

return useDropZone( {
const { position } = useDropZone( {
element,
onFilesDrop,
onHTMLDrop,
onDrop,
isDisabled: isLockedAll,
withExactPosition: true,
} );

useEffect( () => {
if ( position ) {
const { y } = position;
const rect = element.current.getBoundingClientRect();

const offset = y - rect.top;
const target = Array.from( element.current.children ).find( ( blockEl ) => {
return blockEl.offsetTop + ( blockEl.offsetHeight / 2 ) > offset;
} );
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to take into consideration the blocks that are laid out horizontally (I'm not sure it was the case before)

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not possible in master right now, so I'd prefer to tackle this in a separate PR to keep this small and contained.


if ( ! target ) {
return;
}

const targetClientId = target.id.slice( 'block-'.length );

if ( ! targetClientId ) {
return;
}

// setInserterElement( target );
setInserterClientId( targetClientId );
}
} );

if ( position ) {
return clientId;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,18 @@
* External dependencies
*/
import { last } from 'lodash';
import classnames from 'classnames';

/**
* WordPress dependencies
*/
import { withSelect } from '@wordpress/data';
import { getDefaultBlockName } from '@wordpress/blocks';
import { useRef } from '@wordpress/element';

/**
* Internal dependencies
*/
import DefaultBlockAppender from '../default-block-appender';
import ButtonBlockAppender from '../button-block-appender';
import useBlockDropZone from '../block-drop-zone';

function stopPropagation( event ) {
event.stopPropagation();
Expand All @@ -29,12 +26,6 @@ function BlockListAppender( {
isLocked,
renderAppender: CustomAppender,
} ) {
const ref = useRef();
const { isDraggingOverElement } = useBlockDropZone( {
element: ref,
rootClientId,
} );

if ( isLocked || CustomAppender === false ) {
return null;
}
Expand Down Expand Up @@ -77,10 +68,7 @@ function BlockListAppender( {
// Prevent the block from being selected when the appender is
// clicked.
onFocus={ stopPropagation }
ref={ ref }
className={ classnames( 'block-list-appender', {
'is-dragging-over': isDraggingOverElement,
} ) }
className="block-list-appender"
>
{ appender }
</div>
Expand Down
9 changes: 0 additions & 9 deletions packages/block-editor/src/components/block-list/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import { useShortcut } from '@wordpress/keyboard-shortcuts';
* Internal dependencies
*/
import BlockEdit from '../block-edit';
import useBlockDropZone from '../block-drop-zone';
import BlockInvalidWarning from './block-invalid-warning';
import BlockCrashWarning from './block-crash-warning';
import BlockCrashBoundary from './block-crash-boundary';
Expand Down Expand Up @@ -286,12 +285,6 @@ function BlockListBlock( {

const isDragging = isDraggingBlocks && ( isSelected || isPartOfMultiSelection );

const { position } = useBlockDropZone( {
element: wrapper,
clientId,
rootClientId,
} );

// The wp-block className is important for editor styles.
// Generate the wrapper class names handling the different states of the block.
const wrapperClassName = classnames(
Expand All @@ -309,8 +302,6 @@ function BlockListBlock( {
'is-focus-mode': isFocusMode,
'has-child-selected': isAncestorOfSelectedBlock,
'has-toolbar-captured': hasAncestorCapturingToolbars,
'is-dragging-close-to-top': position && position.y === 'top',
'is-dragging-close-to-bottom': position && position.y === 'bottom',
},
className
);
Expand Down
11 changes: 11 additions & 0 deletions packages/block-editor/src/components/block-list/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import classnames from 'classnames';
* WordPress dependencies
*/
import { AsyncModeProvider, useSelect } from '@wordpress/data';
import { useRef } from '@wordpress/element';

/**
* Internal dependencies
Expand All @@ -15,6 +16,7 @@ import BlockListBlock from './block';
import BlockListAppender from '../block-list-appender';
import __experimentalBlockListFooter from '../block-list-footer';
import RootContainer from './root-container';
import useBlockDropZone from '../block-drop-zone';

/**
* If the block count exceeds the threshold, we disable the reordering animation
Expand Down Expand Up @@ -79,8 +81,16 @@ function BlockList( {

const Container = rootClientId ? 'div' : RootContainer;

const ref = useRef();

const targetClientId = useBlockDropZone( {
element: ref,
rootClientId,
} );

return (
<Container
ref={ ref }
className={ classnames(
'block-editor-block-list__layout',
className
Expand All @@ -106,6 +116,7 @@ function BlockList( {
enableAnimation={ enableAnimation }
hasSelectedUI={ uiParts.hasSelectedUI }
hasMovers={ uiParts.hasMovers }
className={ clientId === targetClientId ? 'is-drop-target' : undefined }
/>
</AsyncModeProvider>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* WordPress dependencies
*/
import { useRef, createContext } from '@wordpress/element';
import { createContext, forwardRef } from '@wordpress/element';
import { useSelect, useDispatch } from '@wordpress/data';

/**
Expand Down Expand Up @@ -44,8 +44,7 @@ function onDragStart( event ) {
}
}

export default function RootContainer( { children, className } ) {
const ref = useRef();
function RootContainer( { children, className }, ref ) {
const {
selectedBlockClientId,
hasMultiSelection,
Expand Down Expand Up @@ -92,3 +91,5 @@ export default function RootContainer( { children, className } ) {
</InsertionPoint>
);
}

export default forwardRef( RootContainer );
6 changes: 1 addition & 5 deletions packages/block-editor/src/components/block-list/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,9 @@
}
}

&.is-dragging-close-to-top::before {
&.is-drop-target::before {
border-top: 3px solid theme(primary);
}

&.is-dragging-close-to-bottom::before {
border-bottom: 3px solid theme(primary);
}
}


Expand Down
12 changes: 10 additions & 2 deletions packages/components/src/drop-zone/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,14 @@ import { useContext, useEffect, useState, useRef } from '@wordpress/element';
import Dashicon from '../dashicon';
import { DropZoneConsumer, Context } from './provider';

export function useDropZone( { element, onFilesDrop, onHTMLDrop, onDrop, isDisabled } ) {
export function useDropZone( {
element,
onFilesDrop,
onHTMLDrop,
onDrop,
isDisabled,
withExactPosition,
} ) {
const { addDropZone, removeDropZone } = useContext( Context );
const [ state, setState ] = useState( {
isDraggingOverDocument: false,
Expand All @@ -32,13 +39,14 @@ export function useDropZone( { element, onFilesDrop, onHTMLDrop, onDrop, isDisab
onFilesDrop,
onHTMLDrop,
setState,
withExactPosition,
};
addDropZone( dropZone );
return () => {
removeDropZone( dropZone );
};
}
}, [ isDisabled ] );
}, [ isDisabled, onDrop, onFilesDrop, onHTMLDrop, withExactPosition ] );

return state;
}
Expand Down
13 changes: 9 additions & 4 deletions packages/components/src/drop-zone/provider.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,15 @@ class DropZoneProvider extends Component {
if ( hoveredDropZone ) {
const rect = hoveredDropZone.element.current.getBoundingClientRect();

position = {
x: detail.clientX - rect.left < rect.right - detail.clientX ? 'left' : 'right',
y: detail.clientY - rect.top < rect.bottom - detail.clientY ? 'top' : 'bottom',
};
let x = detail.clientX;
let y = detail.clientY;

if ( ! hoveredDropZone.withExactPosition ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

should we just use another property like "coordinates" instead of hacking two different types in the same property?

Copy link
Member Author

Choose a reason for hiding this comment

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

I personally wouldn't, because it will make the component rerender much more, but I'm fine with it either way. We could remove the top/bottom calculation since they were specific for blocks.

Copy link
Member Author

Choose a reason for hiding this comment

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

Although is-close-to-top is part of DropZone's API. Wouldn't mind removing it if it's ok, nothing in Gutenberg is now using these classes.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't mind dropping it too if not used. (dev note)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I removed them. :)

x = detail.clientX - rect.left < rect.right - detail.clientX ? 'left' : 'right';
y = detail.clientY - rect.top < rect.bottom - detail.clientY ? 'top' : 'bottom';
}

position = { x, y };
}

// Optimisation: Only update the changed dropzones
Expand Down