For layout builder to properly allow drag/drop, you need to be printing region_attributes per region, in all of the Bootstrap layout templates, it instead is printing the region itself with it's own attributes.

Instead of using regions as their own variables like:

  {% if right.content %}
  <{{ right.wrapper }}{{ right.attributes }}>
    {{ right.content }}
  </{{ right.wrapper }}>
  {% endif %}

It should instead append the right attributes to the region_attributes.right, and then print like this:

  {% if content.right %}
    <div {{ region_attributes.right }}>
      {{ content.right }}
    </div>
  {% endif %}
CommentFileSizeAuthor
#6 bootstrap_layouts-3018633-6.patch1.06 KBjustin2pin
#2 3018633-draggable.patch974 bytesb_sharpe

Comments

b_sharpe created an issue. See original summary.

b_sharpe’s picture

b_sharpe commented
Status: Active » Needs review
StatusFileSize
new 3018633-draggable.patch974 bytes

Patch merges the attributes with the region attributes, not sure of why it needs to be separate like this, but this seems like the least invasive way to get these attributes into the variables.

chertzog’s picture

chertzog
he/him
Primary language English
Location Harrisburg, PA, USA
commented
Status: Needs review » Reviewed & tested by the community

Works for me +1 RTBC

markhalliwell’s picture

markhalliwell
Primary language English
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/bootstrap_layouts.module
@@ -153,7 +153,9 @@ function _bootstrap_layouts_preprocess_layout(array &$variables) {
-    $region_attributes = new Attribute(_bootstrap_layouts_parse_attributes($region_settings['attributes'], $tokens));
+    $merged_attributes = NestedArray::mergeDeep(_bootstrap_layouts_parse_attributes($region_settings['attributes'], $tokens), $variables['region_attributes'][$region]);

NestedArray::mergeDeep has, historically, never worked very well when applied to a whole attributes array. It can cause duplicate classes.

I'm almost tempted to say maybe _bootstrap_layouts_parse_attributes should take another argument so $variables['region_attributes'] can be handled by it as well.

asterovim’s picture

asterovim commented

The patch 3018633-draggable.patch works for me. thanks you.

justin2pin’s picture

justin2pin commented
Status: Needs work » Needs review
StatusFileSize
new bootstrap_layouts-3018633-6.patch1.06 KB

This worked great for me with one small issue: in cases where region attributes don't exist for a specific region, errors are thrown. Attached patch checks to make sure attributes actually exist.

markhalliwell’s picture

markhalliwell
Primary language English
commented
Status: Needs review » Needs work

See #4.

b_sharpe’s picture

b_sharpe commented
Status: Needs work » Needs review

@markcarver Can you comment on the original post?

I want to move this forward, and the patch was made to be non-invasive to any other functionality as I didn't have a lot of time to dig in, but wondering why we need something like `<{{ right.wrapper }}{{ right.attributes }}>` and `{{ right.content }}` in the first place?

It seems as if there was specific effort to NOT use the standard:

  {% if content.right %}
    <div {{ region_attributes.right }}>
      {{ content.right }}
    </div>
  {% endif %}

I'm happy to contribute, but want to stray away from core as little as possible

dariogcode’s picture

dariogcode commented

#6 works for me.

phily’s picture

phily
he/him
Primary language French
Location 🇪🇺🇫🇷 Paris, France
commented

Patch #6 seems to be working fine with Drupal 8.7.8 and Bootstrap Layouts 8.x-5.1
Thanks

shane birley’s picture

shane birley commented

@PhilY, did you do anything special outside of applying the patch? I am testing this module and the interface is still blowing apart.

rajab natshah’s picture

rajab natshah
he/him
commented

#6 is working without any change to old TWIG files. or any overridden Twig templates in custom themes.
Thank you :)
Looking forward for a better fix as listed in #3

rajab natshah’s picture

rajab natshah
he/him
commented
Status: Needs review » Reviewed & tested by the community
dubs’s picture

dubs commented

+1 - works for me. Thanks for investigating and providing a patch :-)

rajendar reddy’s picture

rajendar reddy commented

#6 works for me.

komlenic’s picture

komlenic commented

+1 Patch in #6 is confirmed working for me also.

rajab natshah’s picture

rajab natshah
he/him
commented
johanf’s picture

johanf commented

Patch #6 works like a charm ( drupal 8.9.2, bootstrap_layouts 5.2, with both theme bootstrap4 2.0 and radix 4.8 )

alexmoreno’s picture

alexmoreno commented

this is still breaking for me in Drupal 9

b_sharpe’s picture

b_sharpe commented

@alexmoreno I'd switch to https://www.drupal.org/project/bootstrap_layout_builder for D9 if you have the option, far better support/options

alexmoreno’s picture

alexmoreno commented

oh, great Bryan, I'll give it a go

alexmoreno’s picture

alexmoreno commented

actually this was not the issue, but some html tags that were breaking javascript with this message.

Uncaught TypeError: Cannot read property 'permissionsHash' of undefined(anonymous function

See https://www.drupal.org/project/drupal/issues/2780777#comment-14258750

joseph.olstad’s picture

joseph.olstad
Primary language French
commented

There is a workaround to the incompatibility issue with layout_builder, install this module:

https://www.drupal.org/project/layout_builder_admin_theme

rajab natshah’s picture

rajab natshah
he/him
commented
Status: Reviewed & tested by the community » Fixed

Committed ... Thank you.

rajab natshah’s picture

rajab natshah
he/him
commented
Issue tags: +bootstrap_layouts-8.x-5.3

✅ Released bootstrap_layouts-8.x-5.3

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.