Problem/Motivation

I was creating a View with 2 block displays to use on the Umami term page. One display would show Recipes and the Articles for the term.

Since they are both in the same view their placeholders are both the same.
Layout builder with 2 views placeholders

You can see here that you can tell which display it is by configuring the block but otherwise you can't

Proposed resolution

Add the display name to the placeholder.

Remaining tasks

User interface changes

Screenshot After applying patch

API changes

Data model changes

Release notes snippet

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tedbow created an issue. See original summary.

tim.plunkett’s picture

Component: layout_builder.module » views.module
Issue tags: +Blocks-Layouts, +sprint

This is controlled directly by the Views module

vadim.hirbu’s picture

Add display name to Views block placeholder.

vadim.hirbu’s picture

Status: Active » Needs review

Status: Needs review » Needs work
tim.plunkett’s picture

The change needs to be made in \Drupal\views\Plugin\Block\ViewsBlockBase::getPreviewFallbackString(), not in Layout Builder.

vadim.hirbu’s picture

Status: Needs work » Needs review
FileSize
620 bytes

Add display name to Views block placeholder in \Drupal\views\Plugin\Block\ViewsBlockBase::getPreviewFallbackString()

Eitisha’s picture

Hi @vadim,

The patch applied successfully and looks good.

mark_fullmer’s picture

Assigned: Unassigned » mark_fullmer
Issue tags: +Seattle2019
mark_fullmer’s picture

Assigned: mark_fullmer » Unassigned
Status: Needs review » Reviewed & tested by the community

Confirming that the patch in #7 successfully appends the view display name in the context of the Layout Builder preview.

Lendude’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests, +Needs issue summary update

It would also be nice to update the IS to show the new results in a screenshot to evaluate if this makes things better (I'm sure they do, but nice to be able to see the result).

Also, this needs tests.

vadim.hirbu’s picture

Issue summary: View changes
FileSize
105.39 KB

Updated Issue Summary with a screenshot after patch was applied.

Lendude’s picture

Status: Needs work » Needs review
Issue tags: -Needs issue summary update
FileSize
877 bytes
1.61 KB

@vadim.hirbu here is a sample test for this, this shows a problem in the patch ($this->pluginDefinition["admin_label"] can not be set) and needs a proper check on the actual expected text, but hopefully this can help you get rolling on tests! If you need some more help feel free to ping me on slack.

Status: Needs review » Needs work

The last submitted patch, 13: 3045812-13.patch, failed testing. View results

vadim.hirbu’s picture

Status: Needs work » Needs review
FileSize
1.82 KB
1.38 KB

Thank you @lendude for test example. It helped me to understand the idea of tests.
I've updated the test and the patch itself.

nord102’s picture

Status: Needs review » Reviewed & tested by the community

Confirming that patch #15 successfully appends the view display name in the context of the Layout Builder preview.

webchick’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs tests

This looks like a nice little improvement in clarity, thanks!

Committed and pushed to 8.8.x!

Also note that I expanded the credit, since it looks like this was worked on at the DrupalCon Seattle sprints. Welcome, contributors! :D

  • webchick committed 5c7c261 on 8.8.x
    Issue #3045812 by vadim.hirbu, Lendude, tedbow: Add display name to...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

smustgrave’s picture

Closed https://www.drupal.org/project/drupal/issues/3014099 as a duplicate of this. Moved over credit.