Drag-and-drop reordering doesn't work if the region content is nested in other elements

Is this something that could be tested with a functional JS test?

This is great, thank you @pinkdexo
It helps me to save the use of the layout of the TheMag theme -a great work-.
In my case Drupal 8.8.4 I used the #4 patch.
Success to get back the useful drag and drop !

As Patch is created for above issue i changed the status need work to need review.

This patch (tested the 8.9.x) works flawlessly for me.

Patch #10 works for me, too.

Glad it's working! Now just for the automated test coverage...

I am working on this

check the drupal standards

Re-rolled patch #18, Attached interdiff for same.

Fixed custom command failure

I tried writing tests for this and boy was it hard! I have a partial test in this patch. I don't know how to make it any better.

The problems I had were:

1. SortableTestTrait::sortableTo() only simulates a drag and does actually have the browser perform drag and drop.
2. For some reason I don't understand, NodeElement::dragTo() wasn't working for LB blocks, even with the default templates.

This is just a new fail test, so no interdiff.

Sorry...Here's another.

That failed exactly as expected. Clicking the body block did not trigger any sort of "sortable" behavior. Here's the fail patch with the fix from #10 added.

NOTE: Ignore that interdiff. The correct one is attached to #26.

Ignore that interdiff. Oof.

Here's the real interdiff, which is simply the fix from #10.

Aha! Here's why my attempts to use NodeElement::dragTo() weren't working. Per the SortableTestTrait:

* Selenium uses ChromeDriver for FunctionalJavascript tests, but it does not
* currently support HTML5 drag and drop. These methods manipulate the DOM.
* This trait should be deprecated when the Chromium bug is fixed.

This is tracked in an issue: #3078152: Follow-up to #3064049 Deprecate sortable Trait.

I'm pretty sure we can't make a good test for this issue until Chromium supports html5 drag and drop. I say that because the following test runs flawlessly on 9.3.x.

/**
   * Tests the Layout Builder UI with extra wrapper divs in template.
   */
  public function testLayoutBuilderCustomTemplate() {
    // Override the layout templates.
    \Drupal::service('module_installer')->install(['layout_builder_custom_template_test']);
    \Drupal::service('plugin.manager.core.layout')->clearCachedDefinitions();
    \Drupal::service('theme_handler')->refreshInfo();
    $this->testLayoutBuilderUi();
  }

That is, big existing test does not fail with the custom templates featuring extra wrapping divs.

I think the wisest move would be to postpone this issue until the Chromedriver bug is fixed. To me it looks like there's no existing robust test coverage of LB drag-and-drop, which makes changes really risky!

That said, anyone running into this bug will probably be ok with the patch in #10.

The fail patch from #23 will be easy to update when we can do real dragging, so that's a good start.

Thank you thank you thank you!

Patch 25 works for me just fine! I can RTBC!

This still needs tests, so back to NW.

We are seeing this same bug with Drupal core 9.4.8 and following contrib modules:
- Field group 8.x-3.3
- UI Patterns 8.x-1.3.

We have a content display mode with multiple regions

Regions are something like:
- Header
- Main content
- Footer

When I create a field group inside the region, the drag breaks completely.

There is also a related issue in Field group: https://www.drupal.org/project/field_group/issues/3085858.

I have tried the patches from issues 3089151, 3087608 and this one. It seems it gets a little bit better and with trial and error I can get the field to stay in a correct region. But sometimes it just resets. Here is a attached video recording of what happens when I drag multiple times and then save.

Without the patches, the dragging is completely broken and you cannot even stop the drag. The field you are dragging just keeps following you. With the patch from issue 3087608 the dragging works a bit better but it let's me let go of dragging. But after saving the form, the region is still reset.

Without the patches the functionality seemed to be that if I try to drag the field to a region, it gets assigned to the first available region instead of the one I would like to drag the field to. I have tested this with Claro, Gin and Seven admin themes.

Rerolls the patch to make it compatible with 9.5

Looked like the reroll was missing the tests that were in the patch from #25 so pushed those to both the 9.5 and 10 MRs

Fixed for drupal/core:9.5.3

Patch rerolled for 9.5.x
Added reroll diff.

Was previously tagged for tests which still need to happen.

Also issue summary could use some updating.

Thanks.

Rerolled for D10.1.x

I can confirm #44 patch applies to D10.1.8

@weseze - Please provide interdiff
Also we need a re-roll for 11.x

Patches here aren't working for me with a setup like this:

<div class="layout-builder__region js-layout-builder-region layout__region">
  <div class="js-layout-builder-block layout-builder-block">Block 1</div>
  <div class="js-layout-builder-block layout-builder-block">Block 2</div>
  <div class="js-layout-builder-block layout-builder-block">Block 3</div>
  <div class="my-custom-wrapper-here">
    <div class="js-layout-builder-block layout-builder-block">Block 4</div>
    <div class="js-layout-builder-block layout-builder-block">Block 5</div>
  </div>
</div>

It sounds like I should expect to be able to drag and drop 'Block 4' and 'Block 5' around after applying some of the patches here, however I am unable to. My custom wrapper breaks the layout builder drag+drop functionality.

From the original post:

The Drag-and-drop reordering doesn't work because the selector takes .js-layout-builder-region > .js-layout-builder-block instead of .js-layout-builder-region .js-layout-builder-block.

Is something incorrect with my formatting perhaps? Seems like what I have should work?

Hi,

Thanks for all the work done on this issue.

Patch from comment 44 applies on Core 10.3.1 and fixes the issue for me. I have a layout with multiple levels of nesting mixed with other sections with "standards" layouts and now it works perfectly.

https://git.drupalcode.org/project/ui_suite_bootstrap/-/blob/5.0.x/templ...

Where:
- <div{{ attributes.addClass('accordion-item') }}> is the region div.
- {{ content }} is the region content

#44 was not working in my case, made change suggested in #47, works for me (attached the patch)

Patch #44 no longer applies to D11.2
Patch #49 does not work for us, it seems te create a direct parent-child requirement between the regions and the blocks (which is what we set out to avoid?)

Rerolled patch #44 to work against D11.2 and attached it here.

UPDATE:

So patch #44 was previously working for us. (Drupal 10.1.x through 10.3.x)
Since updating to Drupal 11 (11.2.5 in our case), patch no longer applies.
I tried patch from #49, it did apply, but wasn't working for us. Could not initiate the dragging.

My updated patch from #50 did apply and drag-and-drop was visually working, but the order was not saved.

So, here is an updated patch that actually works for us now.
The trick is to pass the direct parent to the sortable plugin (which is technically not correct, but can be handled in the "dragged" function.

Regarding the comment in #47: I don't think that can ever work correctly since Sortable assumes direct children for the sortable items. Having items in 2 different levels assumes nested sortables. That't something much more complex to implement according to the documentation... (I could be wrong)