Support drag-and-drop for submenus of navigation blocks#24479
Support drag-and-drop for submenus of navigation blocks#24479draganescu merged 4 commits intomasterfrom
Conversation
|
Size Change: +3.89 kB (0%) Total Size: 1.16 MB
ℹ️ View Unchanged
|
talldan
left a comment
talldan
left a comment
There was a problem hiding this comment.
Only a partial review, I'll try to get to the rest tomorrow 😄
kevin940726
requested review from
Soean,
ajitbohra,
ellatrix and
youknowriad
as code owners
kevin940726
changed the title
|
It's not ideal that you have to drag the block movers which are so far away form the menu item. Perhaps we need to re-evaluate our use of Nonetheless, I tested this locally and it looks smooth! Nice job. One thing I noticed though is that this only works in the Navigation block when inserted into a post or page and not in the Navigation screen that's in Gutenberg → Navigation (beta). Could we fix that? |
talldan
left a comment
talldan
left a comment
There was a problem hiding this comment.
Looks good, thanks for working on this! Apologies it took a little while to get around to re-reviewing.
Would be great to make a few final changes as per the comments before merging.
|
|
||
| .is-dragging-components-draggable & { | ||
| opacity: 0; | ||
| // Use a minimal duration to delay hiding the element, see hide-during-dragging animation for more details. |
There was a problem hiding this comment.
It might be good to explain in the comment why the toolbar/popover needs to be hidden, mentioning that it obscures drag events if it remains in position overlapping preceding blocks.
| } | ||
|
|
||
| // Hide the popover block editor list while dragging. | ||
| // Using a hacky animation to delay hiding the element so that dragging can still work. |
There was a problem hiding this comment.
Maybe mention that if the element is hidden immediately, dragging won't work, and this is why a delay is required.
| body.is-dragging-components-draggable &.is-dragging-within { | ||
| > .wp-block-navigation__container { | ||
| opacity: 1; | ||
| visibility: visible; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| body.is-dragging-components-draggable .wp-block-navigation-link > .wp-block-navigation__container { |
There was a problem hiding this comment.
Let's drop the body part of the selector if possible, usually just class names are used.
| 'is-focus-mode': isFocusMode, | ||
| 'has-child-selected': isAncestorOfSelectedBlock, | ||
| // Don't select child while dragging. | ||
| 'has-child-selected': isAncestorOfSelectedBlock && ! isDragging, |
There was a problem hiding this comment.
Not sure the comments here add much, I think we could either remove them or add some more detail on why those classes aren't applied when dragging.
@noisysocks I think this is just removing the one line here: I think that can be done in a separate PR. But even then, the nav page still uses a 'top toolbar' style toolbar, we'd have to drop that as otherwise it looks similar to the |
|
Oups! Again, I see green I click. So @kevin940726 said he'll update this in a separate PR to address feedback from @noisysocks and @talldan 🚀 |
…)" This reverts commit 748c632.
4 participants
Description
Close #23874, enable drag-and-drop for submenus in navigation block.
How has this been tested?
Currently only manually testing are available.
Screenshots
Types of changes
New feature
Checklist: