Problem/Motivation

I recently upgraded from beta 2 to beta 5 and my block classes (for my custom blocks) now have the classes from the sub container added to the block wrapper.

Steps to reproduce

I have a custom block plugin class (Extending BlockBase) which returns a render array like :

return [
  '#type' => 'container',
  '#attributes' => [
    'class' => [ 'foo', 'bar'] 
  ],
  'children' => ....
];

This used to return a block:

<div class="block block-mymodule block-block-mymodule-blockname">
  <div class="foo bar">
	  ....
  </div>
</div>

Since I updated content I get a block like this:

<div class="foo bar block block-mymodule block-block-mymodule-blockname">
  <div class="foo bar">
	  ....
  </div>
</div>

i.e. foo and bar classes in both my block container and the sub container.

Is this expected operation now and is it due to context?

I'm not 100% sure this is due to context but it's all I have upgraded.

Proposed resolution

I've added a workround in my code which just nests the render array in another array which is not ideal.

return [
  [
    '#type' => 'container',
    '#attributes' => [
      'class' => [ 'foo', 'bar'] 
    ],
    'children' => ....
  }
];

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#12 block-class.png35.36 KBdeaom
#9 Screenshot from 2020-10-09 11-47-40.png33.22 KBpaulocs
#5 3174893-5.patch1.19 KBtimotej-pl

Comments

mistergroove created an issue. See original summary.

mistergroove’s picture

mistergroove commented
Issue summary: View changes
mistergroove’s picture

mistergroove commented
Issue summary: View changes
mistergroove’s picture

mistergroove commented
Issue summary: View changes
timotej-pl’s picture

timotej-pl commented
Status: Active » Needs review
StatusFileSize
new 3174893-5.patch1.19 KB

I looked into the code of the module a bit and found that there's a part that merges existing attributes from certain block with classes configured in Context. I'm not sure if this is implemented for any particular reason.
I created a patch that appends only classes from configuration and not from the block itself. On my environment that fixed the case you mentioned above. Hope it helps.
Needs review.

Status: Needs review » Needs work

The last submitted patch, 5: 3174893-5.patch, failed testing. View results

robin.ingelbrecht’s picture

robin.ingelbrecht commented

Patch in #5 works for me.

deaom’s picture

deaom commented

This fix was added here: #3150394: Mix of block classes between blocks, so I assume the code maybe needs updating or that was the intent, but definitely not deleting that part, as it removes classes added else where (for example if you add a class via view). So the patch under #5 although maybe solves the reported issue causes other issues that were already fixed. So it should not be committed.

paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
commented
StatusFileSize
new Screenshot from 2020-10-09 11-47-40.png33.22 KB

@DeaOm is right.
@mistergroove I tried to reproduce the issue using the "simple instagram feed" module that has a block plugin but I was not able to reproduce.

$build = [
      '#unique_id' => $unique_id,
      '#theme' => 'simple_instagram_block',
      '#markup' => $this->t('Simple Instagram Feed'),
      '#attributes' => [
        'class' => ['foo', 'bar']
      ],
      '#attached' => [
        'library' => ['simple_instagram_feed/simple_instagram_block'],
        'drupalSettings' => []
      ],
      '#cache' => [
        'max-age' => 3600,
      ],
    ];
    $build['#attached']['drupalSettings']['simple_instagram_feed'][$unique_id] = $this->buildAttachedSettings();
    $build['#attached']['drupalSettings']['simple_instagram_feed'][$unique_id]['unique_id'] = $unique_id;

    return $build;

I added only 

'#attributes' => [
        'class' => ['foo', 'bar']
      ]

The result is what I was expecting.
See image attached.

Is your block plugin in a available module? If yes, I could test using it or you can provide other steps to reproduce the issue.

deaom’s picture

deaom commented
Status: Needs work » Closed (works as designed)

After a lot of debugging, it seems this is a core issue #2486267: Attributes of a block content are applied to block itself and not a context one. Could not reproduce the issue without the enabled context, and also could not reproduce it with the context enabled/added.
The mentioned solution to wrap everything in an empty array seems to be working and is also a suggestion in the above mentioned issue. So I'm closing this one to works as designed.

mistergroove’s picture

mistergroove commented
Status: Closed (works as designed) » Needs work

Thanks for investigating @DeaOm and @paulocs - and sorry for the delayed response (been one of those weeks) and thanks for your time finding the core issue @DeaOm.

I'll continue to nest everything for now. Looks like going forward it would be best to put any classes needed on the block wrapper in '#wrapper_attributes' in case the proposed patch makes it into core.

deaom’s picture

deaom commented
Status: Needs work » Closed (works as designed)
StatusFileSize
new block-class.png35.36 KB

O.k., so I've tested this one out with the core patch. The last patch from the linked issue is not working as it's also adding the foreach loop, which is causing the problems in the first place, the other one provided is on the right track, but for me it was not applying so just changed it manually [#5 core patch], and is working as desired, with block added via context or not, so nothing needs to be corrected/updated in the context part, just the core. So closing this one out as it's working as designed.

I tested with custom block:

    return [
      '#type' => 'container',
      '#attributes' => [
        'class' => ['foo', 'bar'],
      ],
      '#markup' => 'This is block content',
    ];

working example