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

Can't drag content in columns #12831

Closed
mikeh000 opened this issue Dec 13, 2018 · 8 comments
Closed

Can't drag content in columns #12831

mikeh000 opened this issue Dec 13, 2018 · 8 comments
Labels
[Block] Columns Affects the Columns Block [Feature] Drag and Drop Drag and drop functionality when working with blocks Good First Issue An issue that's suitable for someone looking to contribute for the first time

Comments

@mikeh000
Copy link

Wordpress 5.0

When you create a column block and put your initial text paragraph in both columns, you can't move the text in those columns at all because the move icon won't show. Those initial column text blocks are stuck there until you add other text blocks into the columns there.

https://www.screencast.com/t/Z88QaTpLTSQ

@swissspidy swissspidy added Needs Testing Needs further testing to be confirmed. [Feature] Drag and Drop Drag and drop functionality when working with blocks [Block] Columns Affects the Columns Block labels Dec 13, 2018
@tmdesigned
Copy link
Contributor

To expand on this issue, no single block within a parent block will show the mover icons (up, drag, down).

This behavior makes sense for top level items, i.e. a single block has no neighbors to move above or below. So the movers are hidden for a single block, but shown on both items with a second.

a - one paragraph block
b - two paragraph blocks

The same behavior occurs within a parent block -- with respect to that parent block -- even though the situation is not the same. For instance, I may want to drag an item outside of the parent block.

c - one paragraph in a container
d - two paragraph blocks in a container

It looks like the property of note here is isMovable, although I haven't traced it back far enough to see where it's considering its neighbors.

@tmdesigned
Copy link
Contributor

Tracing it back, it's not isMovable, but rather the properties isFirst and isLast. If they are both true, no moving controls are shown.

I am thinking there may need to be a third property, isChild, for which we would at least render the dragger icon even if isFirst and isLast are both true. (isChild may be a confusing name, because in the context of the setter function for these properties it is always a child node).

@youknowriad
Copy link
Contributor

Mmm right, just took a quick look at this and yes it's seems like the drag handle could be always shown in nested contexts.

@youknowriad youknowriad added Good First Issue An issue that's suitable for someone looking to contribute for the first time and removed Needs Testing Needs further testing to be confirmed. labels Apr 17, 2019
@youknowriad
Copy link
Contributor

Hint: The fix should be somewhere in this component https://github.com/WordPress/gutenberg/blob/master/packages/block-editor/src/components/block-mover/index.js

The idea would be to check if we're in a nested block or not (this selector might help getBlockRootClientId)

@tmdesigned
Copy link
Contributor

Thanks for the tip. This code seems to do it, but feels a little awkward. I didn't see another example of using the selectors outside of the compose functions, but here we need access to the selector getBlockRootclientId in the render function.

Starting with line 51 in block-mover\index.js

let rootClientId;

withSelect( ( select, { clientIds } ) => {
	const getBlockRootClientId = select( 'core/block-editor' );
	rootClientId = getBlockRootClientId( first( castArray( clientIds ) ) );
});

if ( isLocked || ( isFirst && isLast && rootClientId ) ) {
	return null;
}

Since getBlockRootClientId returns an empty string for top level blocks, we just have to check its truthiness. Don't show the mover if it's locked, or if it's the first, last, top-level block.

@youknowriad
Copy link
Contributor

you should return { rootClientId } inside the withSelect HoC that wrappes the BlockMover component and you'll receive rootClientId as a prop in the BlockMover component where you'll be able to use it. Don't rely on global variables like here as it doesn't work, you could have multiple block movers on the same page.

@tmdesigned
Copy link
Contributor

Thanks! That is definitely much cleaner (and more accurate). Submitted PR #15025.

@aduth
Copy link
Member

aduth commented Apr 24, 2019

Fixed by #15025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Columns Affects the Columns Block [Feature] Drag and Drop Drag and drop functionality when working with blocks Good First Issue An issue that's suitable for someone looking to contribute for the first time
Projects
None yet
Development

No branches or pull requests

5 participants