Error when dragging and dropping inner blocks of the navigation block

[talldan]

Description

Block drag and drop seems to be buggy in the navigation block.

When dropping a block, often the block disappears, and an error is thrown in the console:

The stack trace seems to indicate an error with one of the rich text formats.

Originally reproduce in the navigation editor, but also happens in the post editor.

Step-by-step reproduction instructions

1. Add some inner blocks to a nav block
2. Try re-ordering them using drag and drop

Expected behaviour

It works

Actual behaviour

The dragged block disappears on drop

[gwwar]

I think it's another regression from #29186 I'll see if I can create a small PR for this, to see if that fixes it against trunk.

Eg c19bd61 has the bad drag behavior and 0a91d2b does not.

[ellatrix]

The error seems to only happen when dragging the navigation link block. I don't get the error when dragging the page list block within the navigation block.

[gwwar]

Summarizing a chat with Ella, we found that there's custom dragging logic added in navigation link from #24479. What we can try here is removing this and revert the ability to drag items into submenus.

The breakage here is blocking another round of feedback for Full Site Editing. @jasmussen also noted that #29727 might be the preferred way of handling submenus.

[annezazu]

The breakage here is blocking another round of feedback for Full Site Editing

Just to follow up, I'm going to move forward with the Call For Testing rather than waiting for an additional plugin release but am going to note this issue as a known problem. Currently, I adjusted the steps as well to avoid this action being taken which, isn't ideal, but removes the blocker :) Thank you all for the quick work in digging into this.

[gwwar]

For folks 👀 this issue, if we decide to add back support for dragging items into submenus, one thing to be aware of 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

[talldan]

For folks 👀 this issue, if we decide to add back support for dragging items into submenus, one thing to be aware of is to aim for solving the general case, instead of adding custom code in the block.

I don't think it's unreasonable for it to have been implemented in the way it was. There are other instances where things that seem like they should be handled as part of the block API are first implemented within a block (#28405 is a recent one that comes to mind).

From what I understand, the navigation block is still the only block that has collapsible nested blocks, so it's hard to generalise an API on the basis of a single use case. It's also pretty common for something to be implemented initially in a more custom way which then graduates to a core feature.

Do we have an idea of what the root cause of this issue is? I don't think removing a feature that worked perfectly well at first, but now exploits what seems to be a bug in some unrelated code is the right way of considering this solved.

[talldan]

The problem seems to be related to the use of the isDraggingBlocks selector in the block. Just calling that selector makes the block crash even when the value is unused.

I think I can get the feature working without that selector. PR incoming.

[talldan]

#30219 fixes without removing the feature.

[gwwar]

Thanks for the alternative @talldan!

Do we have an idea of what the root cause of this issue is?

So far, I'm thinking we're re-rendering too many times and calling the selector is somehow triggering additional renders until we hit the queued update limit. We normally would see this type of react error from calling setState too many times inappropriately.

I'm still searching but it might be good to tune this. Folks did notice that typing used to slow down with the list view open in the site-editor while typing in navigation but not in paragraph blocks, so this might be good to optimize regardless.

[talldan]

#30274 makes a small change that takes us closer to the resolving the root cause.

Still unsure why isDraggingBlocks was so problematic, but I'll continue looking in my spare time.

[jasmussen]

I managed to get a crash today, and posting here because it seems relevant to what @gwwar mentions about re-rendering too many times:

What I did was:

• I had an existing navigation block on the page
• I selected it, copied it (⌘C)
• Pressed Enter a few times to create some space, then paste (⌘V)
• Then opening the block inspector

I'm getting in the console this error:

[ellatrix]

@jasmussen Do you get the error with @talldan's PR #30274?

[jasmussen]

I did see that same error trying 30274 just now:

[jasmussen]

I'm seeing a crash with starting in select mode and selecting the navigation block:

[talldan]

Ella's PR #30311 should solve these issues occurring across any block.

The reason why this was still happening after #30274 is that the nav block also had the same issue. I've created #30374 to tidy up.

[jasmussen]

Thanks for following up on this!

[ellatrix]

Great!