Provide placeholders for empty blocks (for example, an empty Views listing)

Perhaps we can solve this problem for all blocks at once.

--- a/core/lib/Drupal/Core/Block/BlockBase.php
+++ b/core/lib/Drupal/Core/Block/BlockBase.php

@@ -252,6 +252,13 @@ public function getMachineNameSuggestion() {
+  public function getPlaceholder() {

--- a/core/lib/Drupal/Core/Block/BlockPluginInterface.php
+++ b/core/lib/Drupal/Core/Block/BlockPluginInterface.php

@@ -153,4 +153,12 @@ public function blockSubmit($form, FormStateInterface $form_state);
+  public function getPlaceholder();

Because BlockBase and BlockPluginInterface have a tight 1:1 coupling, this is not considered a BC break.

Here's an 8.5.x compatible one for testing.
The 8.7.x patch will also work on 8.6.x

Need to switch the way it checks for emptiness... by using the one named isEmpty!

Fixed a test, and renamed the method to be more clear.

+++ b/core/modules/layout_builder/src/Plugin/Block/ExtraFieldBlock.php
@@ -130,13 +130,22 @@ public function build() {
+    $extra_fields = $this->entityFieldManager->getExtraFields($entity->getEntityTypeId(), $entity->bundle());

Hmm, I wish we had something to retrieve a single extra field instead, but its static cached anyhow so its fine.

I think this looks great, but still needs test coverage.

This is working great for me with a Views block.

Confirming that this works great!

Here's what I did:

1 - Created a related articles view block
2 - Added a Content: Has taxonomy term filter
3 - Added a contextual filter: Content: ID with the following settings: When the filter is not available- provide default value: "Content ID from URL"
4 - Attempted to place this block on the article content type, nothing appeared.
5 - Applied the patch from #11, cleared cache
6 - Observed a placeholder for my view -- attaching a screenshot.

Also needs tests.

@sugaroverflow and I paired on this test.

I think this patch is a really, really nice idea. From a UX perspective, it's super-valuable (IMHO) to be able to show people where something will go.

+++ b/core/lib/Drupal/Core/Block/BlockPluginInterface.php
@@ -153,4 +153,15 @@ public function blockSubmit($form, FormStateInterface $form_state);
+  /**
+   * Returns a string to be used as a placeholder.
+   *
+   * This is typically used when the block has no output and must be displayed,
+   * for example during block configuration.
+   *
+   * @return string|\Drupal\Core\StringTranslation\TranslatableMarkup
+   *   A placeholder string for this block.
+   */
+  public function getPlaceholderString();

I wonder if this should be its own interface. It's entirely possible that this functionality might be useful for things that are not blocks, so maybe we could just have a stand-alone \Drupal\Core\Render\PlaceholderInterface and get BlockPluginInterface to extend it. What do you think?

Additionally, I wonder if this should not return a renderable array. That would open the door to using things like placeholder images or other neatness. On the other hand, I'm not sure if it would complicate things too much. Worth mentioning, though!

EDIT: It looks like the core render system already has some code in place for dealing with placeholders -- I believe it is there because of BigPipe. Could be worth looking into that existing code?

I'm fine with a separate interface, that could indeed be useful. I also removed the API addition to BlockPluginInterface and added an instanceof check before calling the method. But I still put it on BlockBase.

+++ b/core/modules/layout_builder/src/EventSubscriber/BlockComponentRenderArray.php
@@ -98,6 +99,9 @@ public function onBuildRender(SectionComponentBuildRenderArrayEvent $event) {
+        $build['content']['#markup'] = $block->getPlaceholderString();

If we switched from a string to a render array, we'd have to make this $build['content']['placeholder'] = $block->getPlaceholder(); to avoid conflicts with any existing #cache keys.

And then all existing implementations will have return ['#markup' => $whatever]; instead of return $whatever;

Not ruling it out, just thinking it through...
It's probably the right thing to do to be future-thinking, but it certainly makes things unwieldy until we need more than a string.

1. +++ b/core/lib/Drupal/Core/Block/BlockBase.php
   @@ -252,6 +253,13 @@ public function getMachineNameSuggestion() {
   +    return $this->t('Placeholder for the "@block" block', ['@block' => $this->getPluginDefinition()['admin_label']]);
   

   Wouldn't we want to use $this->label() here? It will default to admin_label if not set, and I feel like that might be clearer for the users, if they're setting the label in a meaningful way.

2. +++ b/core/modules/layout_builder/tests/src/Kernel/FieldBlockTest.php
   @@ -287,7 +287,7 @@ public function providerTestBuild() {
   -      'Placeholder for the "The Field Label" field',
   +      '',
        ];
        $data['exception, no preview'] = [
          new ThrowPromise(new \Exception('The exception message')),
   @@ -299,7 +299,7 @@ public function providerTestBuild() {
   
   @@ -299,7 +299,7 @@ public function providerTestBuild() {
        $data['exception, preview'] = [
          new ThrowPromise(new \Exception('The exception message')),
          TRUE,
   -      'Placeholder for the "The Field Label" field',
   +      '',
   

   Why was this changed? Not objecting, just asking in case committers do...

1) I thought about this and decided against it at first, but on second thought I agree

2) Previously FieldBlock had it's own built in placeholder system based on the existence of $entity->in_preview. Now that we're removing it, these tests should be streamlined completely.

Okay, I feel pretty good about it. Let's make it happen!

Committed 53f1980 and pushed to 8.7.x.

@tim.plunkett mentioned #3007439: Layout builder renders Book navigation block on non-book pages was a Blocks-Layouts blocker. So I looked at it … and discovered a unknown-to-me PlaceholderInterface, which was introduced here. Quoting my comment at #3007439-10: Layout builder renders Book navigation block on non-book pages.1:

1. +++ b/core/modules/layout_builder/src/EventSubscriber/BlockComponentRenderArray.php
   @@ -90,6 +90,14 @@ public function onBuildRender(SectionComponentBuildRenderArrayEvent $event) {
   +      $is_placeholder_ready = $event->inPreview() && $block instanceof PlaceholderInterface;
   ...
   +
   
   @@ -98,9 +106,9 @@ public function onBuildRender(SectionComponentBuildRenderArrayEvent $event) {
            '#weight' => $event->getComponent()->getWeight(),
   

   TIL \Drupal\Core\Render\PlaceholderInterface is a thing! :O

   Introduced in #2992410: Provide placeholders for empty blocks (for example, an empty Views listing) apparently.

   This "placeholder" concept does not match the one in \Drupal\Core\Render\Placeholder\PlaceholderStrategyInterface … that's … very confusing :( I'm very sorry to raise this only now, but I hadn't seen that issue yet, and there was no CR for it. I'll leave a comment there.

So yeah … I really really hate to reopen this, but I think I have to, to avoid future confusion.

Pre-existing:

• \Drupal\Core\Render\PlaceholderGenerator(Interface)
• \Drupal\Core\Render\PlaceholderingRenderCache
• \Drupal\Core\Render\Placeholder\PlaceholderStrategyInterface
• \Drupal\Core\Render\Placeholder\ChainedPlaceholderStrategy
• \Drupal\Core\Render\Placeholder\SingleFlushStrategy
• \Drupal\big_pipe\Render\Placeholder\BigPipeStrategy

Added in this issue:

• \Drupal\Core\Render\PlaceholderInterface

The pre-existing code/classes/interfaces ones define "placeholder" as an something to be replaced with final content. Used to for example be able to cache the entire /node/42 response except for the comment form which is then cacheable by Dynamic Page Cache; the comment form is present only as a placeholder, and is replaced by \Drupal\Core\Render\Renderer::replacePlaceholders() just before sending a response to the end user. A placeholder looks like this: <drupal-render-placeholder callback="…" arguments="…" token="…" ></drupal-render-placeholder — the end user never gets to see this. That's the SingleFlushStrategy. When BigPipe is installed, the placeholders look different: they become BigPipe placeholders (currently just empty <div>s since they do end up in the end user's DOM, but #2632750: Interface previews/skeleton screens through optional "preview" or "placeholder" templates would change this so that a more meaningful placeholder can be shown: an interface preview, or alternatively, perhaps just a spinner).

The interface added in this issue defines "placeholder" to be something rather different, for a different purpose. It's even explicitly for blocks that commonly render to something that's empty! If that weren't the case, then perhaps it could be used to help solve #2632750: Interface previews/skeleton screens through optional "preview" or "placeholder" templates. But because it's explicitly about "placeholder to make something that may not show up still be visually present, accessible and navigatable", it seems fundamentally different?

Hence my concern here: we now have two very different "placeholder" concepts in the \Drupal\Core\Render namespace.

This was only committed to Drupal 8.7. I think we can rename the interface introduced here without breaking BC.

Sorry Tim! :(

As far as terminology: maybe this could be a "block preview" rather than a "placeholder"?

#28: yep, exactly.

And the more I think about it, the more I think that this needs exactly the same infrastructure as #2632750: Interface previews/skeleton screens through optional "preview" or "placeholder" templates. I was thinking about that issue again today and had forgotten I'd already made this connection a week ago in #7…

It'd be wonderful if we could introduce one "preview interface" and use it for both Layout Builder and BigPipe!

We can do s/getPlaceholderString/getPreviewFallbackString
As that's what it is, a fallback when the preview fails

As that's what it is, a fallback when the preview fails

Makes sense 👍

I think it then also makes sense to rename PlaceholderInterface to PreviewFallbackInterface. What do you think?

Also backported to 8.6.x.

With two commits here, unless we plan to fully revert this then let's close and move to a follow-up.
The backport puts a deadline on that for the next 8.6 minor...

#3013187: Rename PlaceholderInterface to PreviewFallbackInterface