-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Enable drag and drop in List View, fix performance issues #33320
Changes from all commits
ec5daeb
22f841f
639de98
2b30ac4
8486668
18b967a
1649c9c
e8af5b5
6ecbaea
1d0fcbe
8da5546
c4a970d
9299188
1c629aa
a6bdb3f
d1ab212
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,18 +32,11 @@ const ListViewBlockContents = forwardRef( | |
}, | ||
ref | ||
) => { | ||
const { | ||
__experimentalFeatures, | ||
blockDropTarget = {}, | ||
} = useListViewContext(); | ||
const { __experimentalFeatures } = useListViewContext(); | ||
|
||
const { clientId } = block; | ||
|
||
const { | ||
rootClientId, | ||
blockMovingClientId, | ||
selectedBlockInBlockEditor, | ||
} = useSelect( | ||
const { blockMovingClientId, selectedBlockInBlockEditor } = useSelect( | ||
( select ) => { | ||
const { | ||
getBlockRootClientId, | ||
|
@@ -62,34 +55,12 @@ const ListViewBlockContents = forwardRef( | |
const isBlockMoveTarget = | ||
blockMovingClientId && selectedBlockInBlockEditor === clientId; | ||
|
||
const { | ||
rootClientId: dropTargetRootClientId, | ||
clientId: dropTargetClientId, | ||
dropPosition, | ||
} = blockDropTarget; | ||
|
||
const isDroppingBefore = | ||
dropTargetRootClientId === rootClientId && | ||
dropTargetClientId === clientId && | ||
dropPosition === 'top'; | ||
const isDroppingAfter = | ||
dropTargetRootClientId === rootClientId && | ||
dropTargetClientId === clientId && | ||
dropPosition === 'bottom'; | ||
const isDroppingToInnerBlocks = | ||
dropTargetRootClientId === clientId && dropPosition === 'inside'; | ||
|
||
const className = classnames( 'block-editor-list-view-block-contents', { | ||
'is-dropping-before': isDroppingBefore || isBlockMoveTarget, | ||
'is-dropping-after': isDroppingAfter, | ||
'is-dropping-to-inner-blocks': isDroppingToInnerBlocks, | ||
'is-dropping-before': isBlockMoveTarget, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TODO - remove this and use the new drop indicator for the 'Move To' option. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, I'm going to make this a follow up too. I think Move To needs some refactoring. It currently changes the selected block to determine where it should be inserted, but it should just use the |
||
} ); | ||
|
||
return ( | ||
<BlockDraggable | ||
clientIds={ [ block.clientId ] } | ||
elementId={ `list-view-block-${ block.clientId }` } | ||
> | ||
<BlockDraggable clientIds={ [ block.clientId ] }> | ||
{ ( { draggable, onDragStart, onDragEnd } ) => | ||
__experimentalFeatures ? ( | ||
<ListViewBlockSlot | ||
|
@@ -117,7 +88,7 @@ const ListViewBlockContents = forwardRef( | |
position={ position } | ||
siblingBlockCount={ siblingBlockCount } | ||
level={ level } | ||
draggable={ draggable && __experimentalFeatures } | ||
draggable={ draggable } | ||
onDragStart={ onDragStart } | ||
onDragEnd={ onDragEnd } | ||
{ ...props } | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,125 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { Popover } from '@wordpress/components'; | ||
import { useCallback, useMemo } from '@wordpress/element'; | ||
|
||
export default function ListViewDropIndicator( { | ||
listViewRef, | ||
blockDropTarget, | ||
} ) { | ||
const { rootClientId, clientId, dropPosition } = blockDropTarget || {}; | ||
|
||
const [ rootBlockElement, blockElement ] = useMemo( () => { | ||
if ( ! listViewRef.current ) { | ||
return []; | ||
} | ||
|
||
// The rootClientId will be defined whenever dropping into inner | ||
// block lists, but is undefined when dropping at the root level. | ||
const _rootBlockElement = rootClientId | ||
? listViewRef.current.querySelector( | ||
`[data-block="${ rootClientId }"]` | ||
) | ||
: undefined; | ||
|
||
// The clientId represents the sibling block, the dragged block will | ||
// usually be inserted adjacent to it. It will be undefined when | ||
// dropping a block into an empty block list. | ||
const _blockElement = clientId | ||
? listViewRef.current.querySelector( | ||
`[data-block="${ clientId }"]` | ||
) | ||
: undefined; | ||
|
||
return [ _rootBlockElement, _blockElement ]; | ||
}, [ rootClientId, clientId ] ); | ||
|
||
// The targetElement is the element that the drop indicator will appear | ||
// before or after. When dropping into an empty block list, blockElement | ||
// is undefined, so the indicator will appear after the rootBlockElement. | ||
const targetElement = blockElement || rootBlockElement; | ||
|
||
const getDropIndicatorIndent = useCallback( () => { | ||
if ( ! rootBlockElement ) { | ||
return 0; | ||
} | ||
|
||
// Calculate the indent using the block icon of the root block. | ||
// Using a classname selector here might be flaky and could be | ||
// improved. | ||
const targetElementRect = targetElement.getBoundingClientRect(); | ||
const rootBlockIconElement = rootBlockElement.querySelector( | ||
'.block-editor-block-icon' | ||
); | ||
const rootBlockIconRect = rootBlockIconElement.getBoundingClientRect(); | ||
return rootBlockIconRect.right - targetElementRect.left; | ||
}, [ rootBlockElement, targetElement ] ); | ||
|
||
const style = useMemo( () => { | ||
if ( ! targetElement ) { | ||
return {}; | ||
} | ||
|
||
const indent = getDropIndicatorIndent(); | ||
|
||
return { | ||
width: targetElement.offsetWidth - indent, | ||
}; | ||
}, [ getDropIndicatorIndent, targetElement ] ); | ||
|
||
const getAnchorRect = useCallback( () => { | ||
if ( ! targetElement ) { | ||
return {}; | ||
} | ||
|
||
const ownerDocument = targetElement.ownerDocument; | ||
const rect = targetElement.getBoundingClientRect(); | ||
const indent = getDropIndicatorIndent(); | ||
|
||
const anchorRect = { | ||
left: rect.left + indent, | ||
right: rect.right, | ||
width: 0, | ||
height: rect.height, | ||
ownerDocument, | ||
}; | ||
|
||
if ( dropPosition === 'top' ) { | ||
return { | ||
...anchorRect, | ||
top: rect.top, | ||
bottom: rect.top, | ||
}; | ||
} | ||
|
||
if ( dropPosition === 'bottom' || dropPosition === 'inside' ) { | ||
return { | ||
...anchorRect, | ||
top: rect.bottom, | ||
bottom: rect.bottom, | ||
}; | ||
} | ||
|
||
return {}; | ||
}, [ targetElement, dropPosition, getDropIndicatorIndent ] ); | ||
|
||
if ( ! targetElement ) { | ||
return null; | ||
} | ||
|
||
return ( | ||
<Popover | ||
noArrow | ||
animate={ false } | ||
getAnchorRect={ getAnchorRect } | ||
focusOnMount={ false } | ||
className="block-editor-list-view-drop-indicator" | ||
> | ||
<div | ||
style={ style } | ||
className="block-editor-list-view-drop-indicator__line" | ||
/> | ||
</Popover> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
* WordPress dependencies | ||
*/ | ||
|
||
import { useMergeRefs } from '@wordpress/compose'; | ||
import { __experimentalTreeGrid as TreeGrid } from '@wordpress/components'; | ||
import { useDispatch } from '@wordpress/data'; | ||
import { | ||
|
@@ -18,6 +19,7 @@ import { __ } from '@wordpress/i18n'; | |
*/ | ||
import ListViewBranch from './branch'; | ||
import { ListViewContext } from './context'; | ||
import ListViewDropIndicator from './drop-indicator'; | ||
import useListViewClientIds from './use-list-view-client-ids'; | ||
import useListViewDropZone from './use-list-view-drop-zone'; | ||
import { store as blockEditorStore } from '../../store'; | ||
|
@@ -70,17 +72,15 @@ export default function ListView( { | |
); | ||
const [ expandedState, setExpandedState ] = useReducer( expanded, {} ); | ||
|
||
let { ref: treeGridRef, target: blockDropTarget } = useListViewDropZone(); | ||
const { ref: dropZoneRef, target: blockDropTarget } = useListViewDropZone(); | ||
const elementRef = useRef(); | ||
const treeGridRef = useMergeRefs( [ elementRef, dropZoneRef ] ); | ||
|
||
const isMounted = useRef( false ); | ||
useEffect( () => { | ||
isMounted.current = true; | ||
}, [] ); | ||
|
||
if ( ! __experimentalFeatures ) { | ||
blockDropTarget = undefined; | ||
} | ||
|
||
const expand = ( clientId ) => { | ||
if ( ! clientId ) { | ||
return; | ||
|
@@ -104,7 +104,6 @@ export default function ListView( { | |
() => ( { | ||
__experimentalFeatures, | ||
__experimentalPersistentListViewFeatures, | ||
blockDropTarget, | ||
isTreeGridMounted: isMounted.current, | ||
expandedState, | ||
expand, | ||
|
@@ -113,7 +112,6 @@ export default function ListView( { | |
[ | ||
__experimentalFeatures, | ||
__experimentalPersistentListViewFeatures, | ||
blockDropTarget, | ||
isMounted.current, | ||
expandedState, | ||
expand, | ||
|
@@ -122,21 +120,27 @@ export default function ListView( { | |
); | ||
|
||
return ( | ||
<TreeGrid | ||
className="block-editor-list-view-tree" | ||
aria-label={ __( 'Block navigation structure' ) } | ||
ref={ treeGridRef } | ||
onCollapseRow={ collapseRow } | ||
onExpandRow={ expandRow } | ||
> | ||
<ListViewContext.Provider value={ contextValue }> | ||
<ListViewBranch | ||
blocks={ clientIdsTree } | ||
selectBlock={ selectEditorBlock } | ||
selectedBlockClientIds={ selectedClientIds } | ||
{ ...props } | ||
/> | ||
</ListViewContext.Provider> | ||
</TreeGrid> | ||
<> | ||
<ListViewDropIndicator | ||
listViewRef={ elementRef } | ||
blockDropTarget={ blockDropTarget } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TODO - try making List View use the Insertion Point state so that the indicators appear in both the main and list view block lists. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tried this out, but it does slow things down a bit, will push my explorations to a separate PR. |
||
/> | ||
<TreeGrid | ||
className="block-editor-list-view-tree" | ||
aria-label={ __( 'Block navigation structure' ) } | ||
ref={ treeGridRef } | ||
onCollapseRow={ collapseRow } | ||
onExpandRow={ expandRow } | ||
> | ||
<ListViewContext.Provider value={ contextValue }> | ||
<ListViewBranch | ||
blocks={ clientIdsTree } | ||
selectBlock={ selectEditorBlock } | ||
selectedBlockClientIds={ selectedClientIds } | ||
{ ...props } | ||
/> | ||
</ListViewContext.Provider> | ||
</TreeGrid> | ||
</> | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
elementId
prop seems to be dead code,Draggable
ignores it when__experimentalDragComponent
is defined, andBlockDraggable
has that prop hard-coded just below this line.