Remove block config ID from being used as an HTML ID or template suggestion

i'll point Snugug to this and he can speak for himself, but we had a conversation on this exact topic (albeit in a SCOTCH context) a couple days ago. the conclusion we came to - the id should not be necessary. including it by default is markup bloat, as it's a poor practice to be targeting individual blocks, anyway.

in our discussion, he suggested that it should, however, be possible for the user to set an id from the interface, and have that printed. the problem is providing the autogenerated one, especially when basing it on the machine name.

Like Sam said, Sam and I had a very similar discussion the other day. I 100% support the removal of autogenerated IDs for blocks (especially with users having the ability to add their own if they choose), but I'd prefer to keep block template suggestions (or an equivalent unique template identifier). The reason for this being that, while per-region markup tightly couples blocks to regions, per-block templates allow for direct targeting of block HTML while still keeping it decoupled from it's display. I find, at times, for this to be very useful while still keeping the block template reusable throughout multiple displays. That's just me, though. I rarely actually use block-specific markup currently, although the concept of block instances in D8 may present more needs for it (still want all of the pre/processing of the block, but marked up differently).

Updated issue summary.

Is there a use case for block id as a fragment, thinking about removing this and looking for alternate use cases for id.

This should have been unpostponed long ago.

So, I'm pretty sure that per instance twig templates should be added via the block module/block entity view_builder. Having instance_id specific twig templates seems a big win and so long as that's contained in the logic of the block entity stuff, I don't see any reason not to do it. Likewise the plugins should have plugin level twig templates that can style anything the plugin is spitting out.

Eclipse

After further discussion, I think I'd prefer to 'won't fix' this.

Eclipse

@EclipseGc: There is no "plugin-level twig template", looking at how the page_manager block page renders blocks right now that it's basically the inner content (exactly what the plugin provides) only. There is no wrapper, no label, etc. right

So it seems like we should either change this to make it unnecessary, we would have to duplicate the whole logic around the block content or otherwise refactor the hook suggestion is added.

Maybe we can just make it optional if it exists?

What about this?

The attached patch adds a #id to the EntityViewBuilder and updates the usage to use that, in line with #plugin_id and so on and then page_manager or layout can define it's own id's for single blocks, or leave it out completely.

Also cleans up the aria code that was looking for $block->subject, which doesn't exist anymore since more than a year. And the template suggestions code did an explode() on '.' still, that no longer exists, so simplifying that too.

Would make more sense if I'd actually remove #block..

There's also a stray addition of this in BlockViewBuilder::viewMultiple, which seems useless ($key is an arbitrary integer AFAIK):

      // @todo Remove after fixing http://drupal.org/node/1989568.
      $build[$key]['#block'] = $entity;

That said, BlockViewBuilder::buildBlock, which is the #pre_render callback, uses #block to do the actual building...

Yeah, didn't test it after removing #block, looks like it's needed internally, but I think that's not a problem, we could also rename it to #entity or something?

We could also add the #block only temporarly and remove it again like in this patch?

Also removed the additional #block, weird.

Fixing that test.

Awesome work @Berdir. This goes a long way to improving block rendering, even if it seems simple.

Okay that was bugging me :)

Committed and pushed to 8.x. Thanks!

One side effect of this change is that lots of frontend themes are no re-loading the block based on the plugin ID in their preprocess functions.

See #3174075: Both olivero_theme_suggestions_block_alter and olivero_preprocess_block should NOT load block by ID,.

Is there any reason why we couldn't reverse this change, so the item is there if themes want it?

+++ b/core/modules/block/src/BlockViewBuilder.php
@@ -122,6 +120,9 @@ public function viewMultiple(array $entities = array(), $view_mode = 'full', $la
+    // Remove the block entity from the render array, to ensure that blocks
+    // can be rendered without the block config entity.
+    unset($build['#block']);