Revert "Support drag-and-drop for submenus of navigation blocks (#24479)" #30209
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…)" This reverts commit 748c632.
ellatrix
commented
Mar 24, 2021
| @@ -50,12 +50,12 @@ export function useBlockClassNames( clientId ) { | |||
| spotlightEntityBlocks | |||
| ); | |||
| return classnames( 'block-editor-block-list__block', { | |||
| 'is-selected': isSelected && ! isDragging, | |||
There was a problem hiding this comment.
This was a strange change. We shouldn't have to remove this class.
|
Size Change: -475 B (0%) Total Size: 1.41 MB
ℹ️ View Unchanged
|
|
Thanks for the initial effort. It's important to get drag and drop right across the editor, and the more holistically we can do that, the better. That principle remains even if we need to revert this. I'll give this more testing tomorrow, including on the navigation screen, but wanted to share a 👍. |
gwwar
approved these changes
Mar 24, 2021
There was a problem hiding this comment.
Thanks @ellatrix for debugging with me and spinning up the revert 💖 This tests well for me. I verified that we can still add items to the navigation block, add submenus, add items to the submenus, type to edit links and drag items within the navigation block without the block throwing an error or a WSOD for the editor.
For folks 👀 the PR, one thing to keep in mind for a potential second attempt is to aim for solving the general case, instead of adding custom code in the block:
One of the most important aspects of the navigation block is that it helps define child block APIs we can use in other blocks. Any time it deviates or does something custom we lose that advantage and create an extra point of maintenance, code debt, and fragility -- @mtias
Expected updated change in behavior:
We can't drag top level items into submenu, but we can still drag submenu items into the top level.
updatedbehavior.mp4
Horizontal Drag
horizontalmenu.mp4
Vertical Drag
verticaldrag.mp4
verticalsubmenu.mp4
|
I'm fine with fixing it with #30219 and keeping the custom behaviour. Hopefully we can make it part of the block editor some time. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reverts commit 748c632.
Description
Fixes #30177.
Unfortunately we have to revert #24479.
After a refactoring (#29186) the custom drag behaviour of the navigation link block is throwing an infinite loop error.
If we want better drag and drop behaviour in nested contexts, we could implement this in the block editor rather than the block.
How has this been tested?
Dragging a navigation link block within the navigation block should work again.
Screenshots
Types of changes
Checklist: