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

Fix: Drag & drop allows to move blocks to not allowed destinations #7212

Conversation

jorgefilipecosta
Copy link
Member

@jorgefilipecosta jorgefilipecosta commented Jun 7, 2018

Fixes: #13099
Part of a general polishing to get #6993.

Currently via drag & drop unless there is a template lock we can always move blocks.
This allows a block to be moved inside a parent even if the parent does not support that block. This also allows a child block to be moved outside of the parent.

This PR makes sure a block can only be moved to a different parent if it is possible to insert blocks of that type inside it.

Child Blocks Test:
https://gist.github.com/noisysocks/9b9503bd6489ffa51088d35c7a2a8ba0

Test block (transforms): gist.github.com/jorgefilipecosta/b958239761a24664685d5efc7ab48fa6
Contains a block that only allows nesting of quotes and lists.

How has this been tested?

I registered the test blocks (pasted the code in the browser console).
I inserted Product block (parent) with a "Buy Button" inside. Then I verified we could not move with drag&drop "Buy Button" outside Product.
I used the "Test transforms" block and tried to move a block e.g.: button, inside the test block, checked it was not possible to do that.

@jorgefilipecosta jorgefilipecosta added the [Type] Bug An existing feature does not function as intended label Jun 7, 2018
@jorgefilipecosta jorgefilipecosta self-assigned this Jun 7, 2018
@jorgefilipecosta jorgefilipecosta changed the title Don't allow to move blocks if the block is not allowed in the destination Fix: Drag & drop allows to move a blocks not allowed destinations Jun 8, 2018
@jorgefilipecosta jorgefilipecosta changed the title Fix: Drag & drop allows to move a blocks not allowed destinations Fix: Drag & drop allows to move blocks to not allowed destinations Jun 8, 2018
@jorgefilipecosta jorgefilipecosta added the [Status] Blocked Used to indicate that a current effort isn't able to move forward label Jun 18, 2018
// currently the only constraint to move inside the same parent is locking
// locking was already handled
// it is not possible to use drag & drop if locking is active
if ( rootUID === fromRootUID || canInsertBlockType( blockName, rootUID ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

While this is good for now, I wonder if we should bake a validation mechanism inside DropZone. That way we can change the cursor to not-allowed when we're hovering an area where we can't drop.

@hacknug
Copy link

hacknug commented Feb 19, 2019

Any chance we can get this reviewed (and hopefully merged) any time soon?

@jorgefilipecosta jorgefilipecosta removed the [Status] Blocked Used to indicate that a current effort isn't able to move forward label Feb 19, 2019
canInsertBlockType,
getBlockName,
};
} ),
withDispatch( ( dispatch, ownProps ) => {
Copy link
Member

Choose a reason for hiding this comment

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

Regardless to what you decide in response to DropZone component, I wanted to leave a note about a way how you can avoid passing down selectors as props. A while back we have added 3rd param registry to withDispatch method which allows you to get access to the same selectors without exposing them as props. There is an example added in the docs: https://github.com/WordPress/gutenberg/tree/master/packages/data#withdispatch-mapdispatchtoprops-function--function (2nd example).

In this case, it would be something like the following:

withDispatch( ( dispatch, ownProps, { select } ) => {
    // ...
    moveBlockToPosition( uid, fromRootUID, index ) {
        const { rootUID, layout } = ownProps;
        const { canInsertBlockType, getBlockName } = select( 'core/editor' );
        // and the rest as before
    }
}

@gziolo gziolo added this to the 5.2 (Gutenberg) milestone Feb 20, 2019
@oandregal oandregal self-requested a review February 20, 2019 14:15
@jorgefilipecosta
Copy link
Member Author

Closing in favor of #14003.

@youknowriad youknowriad deleted the update/dont-allow-drag-drop-if-the-block-being-dragged-isnot-allowed branch March 14, 2019 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Drag and Drop Drag and drop functionality when working with blocks [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants