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 column moved in itself error #13941

Merged
merged 3 commits into from
Feb 19, 2019

Conversation

Naerriel
Copy link
Contributor

@Naerriel Naerriel commented Feb 19, 2019

Description

Closes: #13701
The bug is that empty column has no children and dstClientId was undefined, therefore it could not be recognized as moved block's child.

In solution I assumed that each 2 blocks that can be moved in each other are always separated by a core/column block.

If that assumption is incorrect or unsustainable I can leave:

  isSrcBlockAnAncestorOfDstBlock( srcClientId, dstClientId )

and just add:

  isSrcBlockAnAncestorOfDstBlock( srcClientId, dstRootClientId )

along the previous version.

How has this been tested?

  1. Create new column block.
  2. Drag it to second column.
  3. See that it doesn't crash.

Screenshots

Types of changes

Bug fix

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@gziolo gziolo added [Type] Bug An existing feature does not function as intended [Feature] Drag and Drop Drag and drop functionality when working with blocks [Block] Columns Affects the Columns Block Good First Review A PR that's suitable for someone looking to contribute for the first time by reviewing code labels Feb 19, 2019
@gziolo gziolo added this to the 5.2 (Gutenberg) milestone Feb 19, 2019
Co-Authored-By: Naerriel <naerriel@gmail.com>
Copy link
Contributor

@talldan talldan left a comment

Choose a reason for hiding this comment

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

Gave this a further test, it works well. I tried a few different blocks with inner blocks including third-party ones from the coblocks and stackable plugins. All were prevented from being dragged inside themselves 😄

Also confirmed that normal dragging and dropping still works.

@gziolo
Copy link
Member

gziolo commented Feb 19, 2019

Good job @Naerriel, keep up great work 🥇

@gziolo gziolo merged commit 7858cb2 into WordPress:master Feb 19, 2019
youknowriad pushed a commit that referenced this pull request Mar 6, 2019
* Fix column moved in itself error

* Update packages/editor/src/components/block-drop-zone/index.js

Co-Authored-By: Naerriel <naerriel@gmail.com>
youknowriad pushed a commit that referenced this pull request Mar 6, 2019
* Fix column moved in itself error

* Update packages/editor/src/components/block-drop-zone/index.js

Co-Authored-By: Naerriel <naerriel@gmail.com>
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 Review A PR that's suitable for someone looking to contribute for the first time by reviewing code [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Column block can be moved in itself
3 participants