Contextual links of reusable content blocks are not displayed when rendering entities built via Layout Builder

Blocks placed through the Block UI have a 1:1 relationship with their storage, a single config entity solely dedicated to that block.

Blocks placed through Layout Builder are one small piece of a much larger puzzle, and in the case of overrides, are stored as a part of the node itself.
This means it must care for translations, revisions, moderation states, and other concurrent editing done by other users.
There are multiple issues working on solving all of those problems.
It will be very difficult indeed to allow the editing of these custom blocks directly from the view-side of things.

This is somewhat related to #2948828: Layout Builder's Field Blocks do not work with Quick Edit in that way.

Not saying it isn't possible, because we don't know yet. But as it stands, the current situation is by design in order to not exacerbate those scenarios above.

I respectfully propose that this be labelled as a bug as opposed to a feature request as contextual links are a part of Drupal 8 which is expected to work for rendered blocks.

For reusable blocks, yes. I'm not sure about inline blocks though just yet.

I have just started working with layout builder in drupal 8.7 today.

I have a content type, whose teaser view mode is rendered via layout builder.

I then have that teaser display by a view, in a block. And the view's block is then placed on yet another layout for a specific node.

When I view that specific node, I expected to see contextual links for my original content type's teaser.. I get links to the view but not to the node that is rendered in that view's block.. Appears to be the same or at least very similar issue to that described here.

I have seen similar behavior. I think it goes beyond just title_suffix, as #2 suggested.

For example, even in bartik, when placing a menu block, you don't even get a proper 'id' on the block header. You get something like this:

<h2 id="-menu">My block title</h2>

This is because there is no 'attributes' value delivered to the block twig template, so it doesn't have access to things like the block id.

Compare that to another block that I placed via the Block UI in the Bartik sidebar:

<h2 id="block-bartik-tools-menu">Tools</h2>

Of course without the id attribute, it makes styling harder, but more importantly, the moment you place two menu blocks you will have html elements with duplicate ID's.

I think I've found the solution.

The issue is on building block in file core/modules/layout_builder/src/EventSubscriber/BlockComponentRenderArray.php

      $build = [
        // @todo Move this to BlockBase in https://www.drupal.org/node/2931040.
        '#theme' => 'block',
        '#configuration' => $block->getConfiguration(),
        '#plugin_id' => $block->getPluginId(),
        '#base_plugin_id' => $block->getBaseId(),
        '#derivative_plugin_id' => $block->getDerivativeId(),
        '#weight' => $event->getComponent()->getWeight(),
        'content' => $content,
      ];

so, I've added following code after it, and works for me

      // If there is contextual links in content,
      // place it into parent block to be shown on user interface.
      if (!empty($content['#contextual_links'])) {
        $build['#contextual_links'] = $content['#contextual_links'];
      }

Please check the patch.

Here's fixed patch with corrected relative file paths

Fixed patch.

Through manual testing I can confirm #12 fixes the issue with displaying contextual links for block content entities added via layout builder. We do need automated tests though :)

I've just encountered a bug through using this, we shouldn't be rendering contextual links for inline blocks. This lead to me editing an inline block via the normal UI (which sounds like it shouldn't be allowed), leading to some weird behaviour.

Re #14, the ability to edit inline blocks could be useful, given the issues in #2 are addressed. Can you document the "weird behaviour"?

@Chris Burge they are editable via layout builder using the "Configure" link when on the layout page. I've documented why I don't think they should be editable via the UI in #3075308: Inline blocks shouldn't be editable via the normal block content UI

I can confirm that #12 causes contextual links to be rendered for blocks (both re-usable and inline).

The ability to edit inline blocks via a contextual link would be a UX improvement, so long as the issues described in #2 are addressed. (See #3075308-3: Inline blocks shouldn't be editable via the normal block content UI.)

Here are steps to reproduce the "weird behavior" identified in #14:

1. Create a node with a Layout Builder-enabled view mode.
2. On the node's Layout page, add an inline block and save the layout.
3. Use the contextual link on the node's View page to edit the inline block, ensuring that a new block revision is not created.
4. Observe that the inline block is updated as expected.
5. Use the contextual link on the node's View page to edit the inline block, ensuring that a new block revision is created.
6. Observe the inline block is not updated.
7. Navigate to the node's Layout page, edit the inline block, and attempt to save.
8. Observe the following error: "The content has either been modified by another user, or you have already submitted modifications. As a result, your changes cannot be saved."

If I'm reading this right this is a major bug.

While we're figuring out what to do with inline blocks, here's a patch that will just skip any inline blocks contextual links. Same patch as #12, just with the extra check for any blocks with the inline_block base plugin id.

I added my thoughts on allowing inline blocks to be edited outside of LB to: #3075308: Inline blocks shouldn't be editable via the normal block content UI. In summary, I think it's really bad and we should fix that bug as soon as possible.

I added a proposal in there for how contextual links for inline blocks could work:

Why not instead create contextual links that open the layout builder form, then launch the configuration screen for that inline block? Roughly the the same experience, with the added benefit of having all the above problems disappear and you also get layout builders preview facility.

This issue could be repurposed or closed in favour of adding this as a new feature and the issue linked above could address the bug.

FWIW, I've also tentatively closed #3052042: If an inline block has been edited outside of layout builder it can't be edited in layout builder, so fixing this issue without explicitly excluding inline blocks would expose a lot more users to a major bug.

Reroll of #19 for 8.8.0-rc1.

Update patch.
Add contextual links only block_content

Oh, sorry bad patch in #24 comment

This patch works for some views blocks.

I had issues with the view contextual links being hidden by one from the listed items.

This reminds me of the patch in #3077734: Plugin blocks cannot set their own attributes when put in the layout.
Maybe that one (which only handles #attributes, but in the same exact way) should be combined with this?
Also I'm not sure that this needs to be special cased to block_content. If it should, then it should be moved to a new subscriber in that module.

Added '#contextual_links' directly to $build array. Works well for me. Please test.

I'm Working on it.

One thing I've noticed with the last patch is that it adds contextual links for non-resuable, inline LB blocks as well (for Layout Overrides, haven't checked 'default' Layouts). Which I believe is an issue, as if you edit those separately it breaks the link to the layout.

Fixed tests.

+1 for this crucial feature add. Content editors should be able to quickly edit the parts of a Layout Builder layout without having to hunt things down in the Custom Block Library. This feature would sync up expected behaviour from back in the Panels days.

Unrelated to the patch, but I came across this issue while trying to figure out why my Contextual Links were missing on the layout tab for a block type with a template override. The reason was that I wasn't rendering the {{ title_suffix }}, which apparently contains the contextual menu. Just in case anybody else lands here via search for the same reason.

@mrweiner I thankfully stumbled upon your post while researching my several issues with getting Paragraphs to play nice with Layout Builder and vice versa. There were some other things I also needed to resolve, but your post has saved me from going into a dark hole of debugging on this particular issue, so I was able to use my work day to get actual things accomplished. Thanks for posting your findings!

I bet we've all been there. To think how much time we've collectively wasted before realizing `{{ title_suffix }}` actually means contextual links. Oh Drupal.

#38 worked for me
Drupal 9.0.8

I think this needs a test involving reusable content blocks (as per this issue title) that proves the bug is fixed.

#38 worked for me
Drupal 8.9.11

Today I update to 8.9.13 and it stopped working.

#38 was working for me, but one thing I noticed was that when some blocks (maybe only those using paragraphs -- I didn't dig deeply yet) edited through the contextual links while viewing were loosing some attributes resulting in the block being unable to be changed in layout builder. I didn't dig into this deeply, but I'd presume it had something to do with revision status not being updated in layout builder or something. This is very possibly unrelated to this patch directly (and possibly more related to #3047022 (or the like), but I thought I'd add a note about observations so far (in case others find this issue and encounter something similar), but removing this patch seems to reduce the risk of other issues. Sorry for the noise, but I thought it _might_ be helpful to others as they work through a range of lb-related issues.

#38 applied cleanly and did the job for me with standard reusable core blocks and D9.1.4

The MR branch is empty.

#29 is still unaddressed.

#38 worked for me
Drupal 9.1.5

@tim.plunkett
I've merged patches from both issues:
https://www.drupal.org/project/drupal/issues/3020876#comment-13836979
https://www.drupal.org/project/drupal/issues/3077734#comment-13549037

Regarding #29, there is already one change in core/modules/layout_builder/src/EventSubscriber/BlockComponentRenderArray.php file in PR #501

Can you please check if this is what you were expecting?

I've removed the Needs test tag, as the other PR already had a few test cases with the block.

I came here via https://www.drupal.org/project/layout_builder_ibqe

To me #29 is addressed but there is a minor nit in that 'content' is set in the array then overwritten a few lines later.

Just want to ad that #38 worked for me. Not sure what work is remaining, but it does what I need. Thx

Oh man, a HUGE thank you to @mrweiner for mentioning the

{{ title_prefix }}
{{ title_suffix }} 

I couldn't figure out why the contextual link to configure / remove blocks was not appearing in Layout Builder, so I went hunting for patches. I rarely use the block titles so I didn't even think twice about removing these. I put them back in and voila.

FYI for anyone TLDR;

Let's draw some attention. RTBC +1. Works fine for us. :)

Committed and pushed 6d04e573e3 to 9.3.x and bbf39c4341 to 9.2.x. Thanks!

Nice!

This core change has caused a major regression in our Drupal site which uses custom blocks to add classes to our components in the block's $build['#attributes'] array.

This is an example of the build render array that we are using in our block's build() method:

    $build = [
      '#type' => 'design_system_hero_component',
      '#title' => $this->configuration['title'],
      '#body' => $this->configuration['body'],
      '#attributes' => [
        'class' => ['design-system-hero'],
        'hero-title' => $this->configuration['title'],
        'data-gtm' => 'promotional-banner',
      ],
    ];

Before, these #attributes would be passed to our Twig template for this component, but now they have gone missing, and are added to the wrapping block <div> instead, which broke all of our output.

I also don't see a change notice about this.

BlockPluginInterface::build() mentions nothing about $build['#attributes'] being moved, or being owned by the wrapping block markup.

  /**
   * Builds and returns the renderable array for this block plugin.
   *
   * If a block should not be rendered because it has no content, then this
   * method must also ensure to return no content: it must then only return an
   * empty array, or an empty array with #cache set (with cacheability metadata
   * indicating the circumstances for it being empty).
   *
   * @return array
   *   A renderable array representing the content of the block.
   *
   * @see \Drupal\block\BlockViewBuilder
   */
  public function build();

Which now seems at odds with the new comment added here:

      // Place the $content returned by the block plugin into a 'content' child
      // element, as a way to allow the plugin to have complete control of its
      // properties and rendering (for instance, its own #theme) without
      // conflicting with the properties used above, or alternate ones used by
      // alternate block rendering approaches in contributed modules. However,
      // the use of a child element is an implementation detail of this
      // particular block rendering approach. Semantically, the content returned
      // by the block plugin, and in particular, attributes and contextual links
      // are information that belong to the entire block. Therefore, we must
      // move these properties from $content and merge them into the top-level
      // element.
      if (isset($content['#attributes'])) {
        $build['#attributes'] = $content['#attributes'];
        unset($content['#attributes']);
      }

Core blocks that are affected by this change:
\Drupal\language\Plugin\Block\LanguageBlock::build

  public function build() {
    $build = [];
    $route_name = $this->pathMatcher->isFrontPage() ? '<front>' : '<current>';
    $type = $this->getDerivativeId();
    $links = $this->languageManager->getLanguageSwitchLinks($type, Url::fromRoute($route_name));

    if (isset($links->links)) {
      $build = [
        '#theme' => 'links__language_block',
        '#links' => $links->links,
        '#attributes' => [
          'class' => [
            "language-switcher-{$links->method_id}",
          ],
        ],
        '#set_active_class' => TRUE,
      ];
    }
    return $build;
  }

That does sound like a pretty big regression if you where relying on it. After spending some time comparing the effects of the patch on the language block the effect seems like exactly what the fix was trying to do. The issue is about fixing contextual links but the reason contextual links weren't working was that blocks where rendering different if they where in LB versus placed on the page normally.

Language block in a theme region:

And the same block placed in a layout prior to the patch highlighting how the class moved.

You can see how the class moving aligns it with the "standard" rendering which would fix a class of bugs like contextual links but also how disruptive that move would be to heavy layout builder users relying on that class placement like Dave.

From a discussion in Slack:

This is a tricky situation. Because from 8.7 to 9.1, block.module placed blocks worked one way and LB placed blocks worked another. and now in 9.2+, they work the same way. But that switch broke the expectations of any block that was only ever intended to work in LB...

LanguageBlock is and has been broken in the way @Dave Reid observed when placed in the Block UI.
I'd propose a follow-up for fixing LanguageBlock, and perhaps adding docs to BPI::build() at the same time.

This issue should have a CR to explain the break and what to do about it (nesting your render array one level down sidesteps this).

Do we need to create a new issue to fix the regression? Or do we only need to document the change in a Change Records? Any core blocks affected that need a fix?

This has been driving me crazy :). After some digging it turns out this is what's causing the Commerce core tests to fail on Drupal core 9.2 (Created #3247884: Fix the 9.2 test failures today).

Commerce has tests coverage for its Layout builder integration, and tests have been failing since this got committed, due to the markup changing between ajax requests (i.e the "block-field-block-..." wrapper disappearing as well as the content div around the field after a different product variation attributed is selected on add to cart.

This is actually a Commerce release blocker, but looks like I may have to comment out the code that's failing in tests until this gets resolved.

For reference, when landing on the product page, a div with the following class exists on the page:

    $price_field_selector = '.block-field-blockcommerce-product-variationdefaultprice';

Then, the following fails:

    $this->getSession()->getPage()->selectFieldOption('purchased_entity[0][variation]', $second_variation->id());
    $this->assertSession()->assertWaitOnAjaxRequest();

    $this->assertSession()->elementExists('css', $price_field_selector);

The same div no longer exists after the ajax refresh.

Hi, to illustrate what jsacksick said, here is a concrete example of the problem encountered on Commerce core, this behavior can not remain as is in a professional perspective...

https://www.drupal.org/project/commerce/issues/3253323

Updating the patch for Drupal 9.4

Reroll of patch #84 for Drupal 9.5.2

I ran the previous patch diff against the wrong dir. Here's the correct one.

Closed #3077734: Plugin blocks cannot set their own attributes when put in the layout as a duplicate

This patch works with core 9.5.x and 10.2.x.

@petar.gnjidic
this patch from #96 only works if you don't have revision. Moment you check revision checkbox on the block and save it - contextual links are gone again.
Could you try to reproduce it?
Thanks