-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Reorder blocks via drag & drop (v1. using React-dnd). #4056
Reorder blocks via drag & drop (v1. using React-dnd). #4056
Conversation
- Updated editor reducers and actions to allow reindexing of blocks given uid of block to be reindexed and the new index position.
…components. - Centralised the dependency on react-dnd to provide custom entry points for the available/required HOCs.
- Updated editor constants to include draggable types, as required to connect drag sources to drop targets in react-dnd.
…g drag sources and drop targets. - Can define and use these components anywhere on the page. - Can be customised via component properties.
…erations. - May not be needed eventually, depending on how we decide to deal with the conflicts between the current dropzone provider and react-dnd.
…to the block list for testing.
components/drop-zone/index.js
Outdated
@@ -62,7 +62,7 @@ class DropZone extends Component { | |||
|
|||
render() { | |||
const { className, label } = this.props; | |||
const { isDraggingOverDocument, isDraggingOverElement, position } = this.state; | |||
const { isDraggingOverDocument, isDraggingOverElement, position, isReorderingInProgress } = this.state; |
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.
Never really mind this flag - I am experimenting with a couple of ideas to sort out the conflicts with the current drop-zones.
import { Component } from '@wordpress/element'; | ||
|
||
/* Testing... */ | ||
const ModifiedBackend = ( ...args ) => { |
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.
Experimenting with a custom provider for react-dnd due to the conflicts created with dropzones.
|
||
|
||
{ true && | ||
<DragAndDropSource |
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.
This is the gist of it, part 1. A component that we can drag.
} | ||
|
||
{ true && | ||
<DragAndDropTarget |
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.
This is the gist of it, part 2. A component that we can drop to.
…y and removed experimental react-dnd provider. - Added dependency to a fork of the HTML5 backend used for react-dnd. This fixes the conflicts with having both react-dnd and custom event listeners on the page.
]; | ||
|
||
const shouldIgnoreTarget = ( target ) => { | ||
return target.className.split(' ').indexOf( 'is-reordering-in-progress' ); |
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.
This logic doesn't do as you might expect. I assume you're checking whether the target has the specified class assigned, but Array#indexOf
returns -1
if the item is not in the array. This is a truthy value; conversely, 0
is a valid return value but is falsy.
Instead, you might consider:
- Comparing against
-1
-1 !== target.className.split( ' ' ).indexOf( 'is-reordering-in-progress' );
- Using Lodash's
_.includes
which is a bit more semantic to what you're trying to testincludes( target.className.split( ' ' ), 'is-reordering-in-progress' );
- Using
Element#classList.contains
which is designed for what you're trying to do here:target.classList.contains( 'is-reordering-in-progress' );
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.
There's a styling issue here: there needs to be a space between the parentheses of split
, i.e. split( ' ' )
You might consider installing an ESLint plugin for your editor to help surface these styling issues more apparently:
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.
Thanks @aduth for your comments and pointers. This part of the code is actually not used anymore, as I have instead found a patch that fixes the issues I was trying to get to here. I will send an update soon, so apologies for any inconvenience.
I may actually refrain from using react-dnd altogether. I am not too happy with all the clutter that is created. I am running a separate experiment right now with the existing drop-zones. I will share soon.
editor/reducer.js
Outdated
return state; | ||
} | ||
|
||
return without( state.reduce( ( acc, elem, i ) => { |
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.
Is the without
necessary here? Couldn't we just not return with the null
in the Array#reduce
accumulator if we don't want it to be included?
if ( blockIndex === i ) {
return acc;
}
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.
That is correct. Thanks for the catch. :-)
…all to 'without', as per Andrew's comment in the PR.
Description
This PR aims to introduce drag & drop functionality for rearranging blocks, and potentially for reordering other elements elsewhere in the editor (e.g. image galleries). Relevant issues: #38, #743, #1511.
This PR is no longer being worked on. It remains here for reference. There is a separate PR (#4115) that takes a somewhat more minimal approach (does not introduce React-dnd) with which I am currently working on.
How Has This Been Tested?
Only on desktop and manually at this point.
Screenshots (jpeg or gifs if applicable):
Not much in the way of design. Following are two paragraph blocks, each with a drag source and a drop target. Grab a source from one ("Drag") and drop to the target of the other ("Drop Here") to rearrange the list.
Types of changes
Most of the work so far has been at the foundational level i.e. getting the necessary pieces together to have something that roughly works. The idea is to introduce general purpose components that will act as the "drag sources" and "drop targets", and which can be plugged anywhere on the page. This is the current setup. You can see an example of how this works in
editor/block-list/block.js
.The components
DragAndDropSource
andDragAndDropTarget
can be parameterised with a "type", "index", "id", and "reindexHandler" properties, along with other experimental properties.A DragAndDropSource can be dragged, and once dropped onto a DragAndDropTarget, it will be updated with that index. This is the general idea. We define the update operation/method as a property on the DragAndDropTarget (so we can apply the same idea on any list and with any type of state management).
type: A DragAndDropTarget can listen to drop events associated with a DragAndDropSource of the same type. So we can use these draggable components as many times as we like on a page, and only same-type ones will respond to each other.
index: The index specified on a DragAndDropTarget is where the DragAndDropSource will end up being moved to.
Known Issues/TODOs:
The branch introduces React-dnd to the repo. There is currently at least some level of conflict with the dropzone provider.
I have not been able to get both to work together on the page. This is my stumbling block right now(Fixed via a patch, although conflicts will still exist if both are mounted at the same time on the page).Design - haven't really put any thought on the design yet. The end goal is to match the UI mocks provided by Joen in issue #38. It will very likely introduce further changes to the overall setup. One step at a time though.
Checklist: