Template suggestions for custom block view modes

I think this is a nice idea. However, it's certainly a feature request and will need both tests and sign-off from a frontend framework manager.

1. +++ b/core/modules/block_content/block_content.module
   @@ -179,3 +179,24 @@ function _block_content_has_reusable_condition(array $condition, array $tables)
   +    $sanitized_view_mode = strtr($variables['elements']['#configuration']['view_mode'], '.', '_');
   

   This is interesting. Why would a view mode have a period in it? Normally, view mode IDs take the form entity_type_id.view_mode_id, and in this case, the entity type ID will never vary (it will always be "block_content"). So, what might be better is to have $sanitized_view_mode be something like this:

   list(, $sanitized_view_mode) = explode($variables['elements']['#configuration']['view_mode'], '.', 2);
   

2. +++ b/core/modules/block_content/block_content.module
   @@ -179,3 +179,24 @@ function _block_content_has_reusable_condition(array $condition, array $tables)
   +    // View mode suggestions
   

   I don't think this comment adds much. :)

3. +++ b/core/modules/block_content/block_content.module
   @@ -179,3 +179,24 @@ function _block_content_has_reusable_condition(array $condition, array $tables)
   +    $suggestions[] = 'block__' . $variables['elements']['#id'] . '__' . $sanitized_view_mode;
   

   What is $variables['elements']['#id']? What kind of suggestion name will this produce? I'm not sure this one will be very useful, so maybe we should remove it for now.

I added tests.

<!-- FILE NAME SUGGESTIONS:
   * block--block-content--machinename--full.html.twig
   * block--block-content--machinename.html.twig
   * block--block-content--test_block--full.html.twig
   * block--block-content--test_block.html.twig
   * block--block-content--full.html.twig
   * block--machinename.html.twig
   * block--block-content--e81b663b-5755-4daf-889d-7ea1c9857b3d.html.twig
   * block--block-content.html.twig
   * block--block-content.html.twig
   x block.html.twig
-->

1. Duplicated 'block--block-content.html.twig'. I created a separated issue #3062863: Remove duplicate suggestions block--block-content.html.twig.

2. What about 'block--machinename.html.twig' and 'block--block-content--e81b663b-5755-4daf-889d-7ea1c9857b3d.html.twig'? Their order is incorrect.

See How to bring custom block theme suggestions into the right hierachy?.

+++ b/core/modules/block_content/block_content.module
@@ -182,3 +183,27 @@ function _block_content_has_reusable_condition(array $condition, array $tables)
+    $suggestions[] = 'block__block_content__' . $view_mode;
+    $suggestions[] = 'block__block_content__' . $bundle;
...
+      $suggestions[] = 'block__block_content__' . $variables['elements']['#id'];

There's potential for collisions here similar to what has been reported here: #2831144: [policy] Bundle / field name / view mode theme suggestions can conflict. Maybe we should separate these with a prefix to ensure that these don't collide?

I have re-rolled the patch #11 on Drupal 8.9.x.
Still need to resolve comment #14

This patch confirmed working on 8.7.8

However, this does not address block--block-inline--[name]--[view-mode].html.twig naming convention for layout inline blocks.

FILE NAME SUGGESTIONS:
   x block--block-content--[name]--[view-mode].html.twig
   * block--block-content--[name].html.twig
   * block--block-content--[view-mode].html.twig
   * block--inline-block--[name].html.twig
   * block--inline-block.html.twig
   * block--layout-builder.html.twig
   * block.html.twig

patch in #16 worked as expected for me

Confirming RTBC of this important patch and agree as per #16 to handle that point separately. These suggestions are very important and helpful and will replace current workarounds in many many themes which lead to inconsistent naming conventions.

Thank you all very much, I'm very much looking forward to this.

#ignorethis

Alternative would be to postpone [PP-1] this on #2831144: [policy] Bundle / field name / view mode theme suggestions can conflict. It's definitely very important to have consistency over all entity types.

Sorry for the self-reference, I copied the wrong issue id... -.-

Here I have fixing failed tests of patch #15.

As patch #15 didn't pass tests so changing status from RTBC to needs review.

Applied patch #34 and it works fine.After applying the patch the suggestion with view mode is appearing.Adding screenshots below.

Before patch:

After patch:

On new block type

Thanks for everyone's work here. I took #38 and created a gitlab MR for it. I tested on 9.5.x which defaults the frontend theme to Olivero. Olivero has some block theme suggestions so I switched back to bartik to do a bare bones test.

Testing steps

1. created a new block content type Alerts
2. create new Alerts block "alerts are cool"
3. visit page block is on and confirm default theme suggestions
4. apply patch and confirm new twig suggestion block_content_*

screenshots

Marking as needs review and will work on change record.

here is the draft change record https://www.drupal.org/node/3304793. Please review and feel free to make updates.

For failing test

Reran the last commit for 9.5.x

The failure was a random one with the quickedit module. Putting back into NR.

Think at this point we will need to change the MR to point to 10.1.x then we can mark

+1 for RTBC after that.

Made a D10 MR.

Left some comments on the MR, this looks like a good improvement to me.

Fun fact, my first contribution to core was adding these suggestions to node :)

Addressed feedback ready for review again.

From the thread the test should be updated. Should be verified before running.

@larowlan actually that is correct. I updated the variable but one is the block and the other the blockContent.

There are two MRs here, can we get clarification of which one is the correct one? I assume the one with the larger ID?

I will remove the other one.

Ah one is for 9.5, I closed that as this is a feature and 10.1+ only

Made the small tweak.

This looks RTBC in my book

#14 has not been addressed yet. I'm not sure if we should add new suggestions with the known bug because changing these can be quite painful.

A concrete recommendation for tackling this would be to implement the suggestions like this:

• 'block__block_content__view_mode__full'
• 'block__block_content__bundle__test_block'
• 'block__block_content__bundle__test_block__full'
• 'block__block_content__machine_name__machinename'
• 'block__block_content__machine_name__machinename__full'

Thoughts?

Ah sorry for missing that @lauriii - yes I agree avoiding the known collision now is the best approach

Going to take #61 as an FEFM review and remove the tag (please revert if not appropriate)

While I'm slightly off-topic for this particular thread, I just want to sing my praise for everyone's time and effort on this endeavor because otherwise I wouldn't have been able to complete the module I started to write over 1.5 years ago and shelved in frustration, (as I waited for a Change to happen), after much time and effort myself. Thank you all from the bottom of my heart.

I'm using the patch from #34 but it looks like it's basically the same as the MR. I have a block type called mini-nav. Here are the theme suggestions I see:

<!-- FILE NAME SUGGESTIONS:
   * block--block-content--mini-nav--full.html.twig
   * block--block-content--mini-nav.html.twig
   * block--block-content--full.html.twig
   * block--block-content--2c9df2dd-821b-4147-9bd0-adf8ca05474d.html.twig
   * block--block-content.html.twig
   * block--block-content.html.twig
   x block.html.twig
-->

I think it's unexpected and undesirable that block--block-content--2c9df2dd-821b-4147-9bd0-adf8ca05474d.html.twig suggestion is not more powerful. Certainly the uuid is as specific as you can refer to something, right? I think a preferred list of suggestions would be:

<!-- FILE NAME SUGGESTIONS:
   * block--block-content--2c9df2dd-821b-4147-9bd0-adf8ca05474d--full.html.twig
   * block--block-content--2c9df2dd-821b-4147-9bd0-adf8ca05474d.html.twig
   * block--block-content--mini-nav--full.html.twig
   * block--block-content--mini-nav.html.twig
   * block--block-content--full.html.twig
   * block--block-content.html.twig
   x block.html.twig
-->

It seems like this needs to interact more gracefully with the existing theme suggestions provided by the block module. Maybe this should even be block_content_theme_suggestions_block_alter and do some splicing, along the lines of what happens in umami_theme_suggestions_block_alter:

function umami_theme_suggestions_block_alter(array &$suggestions, array $variables) {
  // Block suggestions for custom block bundles.
  if (isset($variables['elements']['content']['#block_content'])) {
    array_splice($suggestions, 1, 0, 'block__bundle__' . $variables['elements']['content']['#block_content']->bundle());
  }
}

It does the splice so that the new suggestion comes before the suggestion that features the uuid.

So looking at this to change the order think the code would have to be added to the block module now is that desired?
@laurii not sure I understand the conflict issue?

So we need to add an extra prefix to address #14 and I think #64 is valid too, the UUID specific one should trump any general suggestions

I think we need to pick one of the two to have a prefix for #14 - either view mode or bundle.

I think the bundle variation will be more common that the view mode, so suggest we prefix the view mode ones with `view_mode_$view_mode` and keep the bundle as is.

Switched to block alter and got

* block--sidebar--id--olivero-testthemes.html.twig
* block--sidebar.html.twig
* block--olivero-testthemes.html.twig
* block--block-content--90a1a6dd-3b88-45dc-9ed3-fe1aa04dcdd4.html.twig
* block--block-content--olivero-testthemes--full.html.twig
* block--block-content--olivero-testthemes.html.twig
* block--block-content--basic--full.html.twig
* block--block-content--basic.html.twig
* block--block-content--full.html.twig
* block--block-content.html.twig
x block.html.twig

Thoughts?

Also updated suggestion, that address #14?

Looks like we have a valid test fail

Also need test coverage but what do you think of the order?

Not sure about the ones with the theme name being more specific - or is that the block ID?

@larowlan:

Not sure about the ones with the theme name being more specific - or is that the block ID?

Yes, looking at the code olivero-testthemes must be the block ID. You can see that at block--sidebar--id--olivero-testthemes.html.twig

So we should come back to the order discussion. As I agree the example might be confusing, it would be best to write down the expected outcome into the issue summary and discuss that (if needed), based on the current example, but with more clear ID's and an explanation perhaps?

@smustgrave would you do that?

This should be ready for review. Updated issue summary too.

This looks good to me.

I like the extra specificity of the `_view_` and `_view_type_` so we don't get collisions, but it would be good for a FEFM to formerly approve that pattern, so tagging as such.

I'll ping @lauriii as he's already looked at this before

The issue summary explains clearly what is being done. Although, the remaining task says that there is a remaining task of 'Agree of suggestions'. Has that been done?

I then read the comments. I do see discussion of order, in particular #64. So, that seems to be done. I also see that @larowlan and @lauriii have been reviewing this.

I then read the MR and have commented there.

I read the CR and made a few changes to make the before/after clearer. The version numbers in the CR need to be updated. Also, the examples need to be checked and possibly updated. They are they same one when it was created in Aug 2022.

Just a few things, so setting back to NW.

To committers: I updated the credit, the ones unchecked need to be reviewed.

Brought this up in #needs-review-queue-initative and @lauriii said he already reviewed and can remove framework tag.

Have addressed the feedback so back to NR.

Everything looks to be good to go now here

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

I think it's safe to remark this myself. Only fix was https://git.drupalcode.org/project/drupal/-/merge_requests/3094/diffs?co... where machinename became machine_name.

Added some questions to the MR.

Feedback addressed.

Added a small nit on the MR otherwise this looks RTBC, all threads have been resolved and code is looking good.

I've updated the CR versions, but not 100% sure if they are accurate.

+1

Thoroughly reviewed and tested; the proposed solution effectively resolves theme suggestions for custom block view modes. Endorsed for community acceptance.

While MR isn't merged, I'm attaching a patch based on the latest MR that can be applied to Drupal 10.2.x.

Hiding patches to reduce confusion.

FYI - you can download an MR diff locally and use that in your composer workflow instead of uploading to an issue. e.g https://git.drupalcode.org/project/drupal/-/merge_requests/3094.diff

@alt.dev
I try to apply patch but getting below message.

Skipped patch 'core/modules/block_content/block_content.module'.
Skipped patch 'core/modules/block_content/tests/src/Kernel/BlockTemplateSuggestionsTest.php'.
Skipped patch 'core/modules/block_content/tests/src/Kernel/BlockTemplateSuggestionsTest.php'.
Skipped patch 'core/modules/block_content/tests/src/Kernel/BlockTemplateSuggestionsTest.php'.
Skipped patch 'core/modules/block_content/tests/src/Kernel/BlockTemplateSuggestionsTest.php'.
Skipped patch 'core/modules/block_content/block_content.module'.
Skipped patch 'core/modules/block_content/block_content.module'.
Skipped patch 'core/modules/block_content/block_content.module'.
Skipped patch 'core/modules/block_content/tests/src/Kernel/BlockTemplateSuggestionsTest.php'.
Skipped patch 'core/modules/block_content/tests/src/Kernel/BlockTemplateSuggestionsTest.php'.
Skipped patch 'core/modules/block_content/tests/src/Kernel/BlockTemplateSuggestionsTest.php'.
Skipped patch 'core/modules/block_content/tests/src/Kernel/BlockTemplateSuggestionsTest.php'.
Skipped patch 'core/modules/block_content/block_content.module'.
Skipped patch 'core/modules/block_content/block_content.module'.

Updated CR with suggestion names and fancy emojis

Committed b050dbf and pushed to 11.x. Thanks!

I opened the following related issue https://www.drupal.org/project/drupal/issues/3468180