Add an implementation of EntityDisplayBase::label()

There is @todo left in \Drupal\layout_builder\Entity\LayoutBuilderEntityViewDisplay::label

+++ b/core/lib/Drupal/Core/Entity/EntityDisplayBase.php
@@ -247,6 +248,16 @@ public function id() {
+    return new TranslatableMarkup('@bundle @label', ['@bundle' => $bundle_label, '@label' => $target_entity_type->getPluralLabel()]);

Why can't we just concatenate these strings and not use TranslatableMarkup?

Patch no longer applies.

re-rolled the patch

1. +++ b/core/lib/Drupal/Core/Entity/EntityDisplayBase.php
   @@ -606,4 +607,14 @@ protected function getLogger() {
   +    $bundle_info = \Drupal::service('entity_type.bundle.info')->getBundleInfo($this->getTargetEntityTypeId());
   +    $bundle_label = $bundle_info[$this->getTargetBundle()]['label'];
   +    $target_entity_type = $this->entityTypeManager()->getDefinition($this->getTargetEntityTypeId());
   

   Call $this->entityTypeBundleInfo() intead of \Drupal::service('entity_type.bundle.info')

   Furthermore, the first two lines seem unnecessary, call $target_entity_type->getBundleLabel() directly.

2. +++ b/core/lib/Drupal/Core/Entity/EntityDisplayBase.php
   @@ -606,4 +607,14 @@ protected function getLogger() {
   +    return new TranslatableMarkup('@bundle @label', ['@bundle' => $bundle_label, '@label' => $target_entity_type->getPluralLabel()]);
   

   do not understand why calling $target_entity_type->getPluralLabel() here, should it be $target_entity_type->getLabel() ?

3. Implement ::label(), returning a string indicating which entity type, bundle, and view mode is represented.

   -- from IS

   Where is the mode in the label?

Addressed, #12.1,2,3

Updating the patch first, interdiff ignored as the patch is small. screenshots coming soon.

1. +++ b/core/lib/Drupal/Core/Entity/EntityDisplayBase.php
   @@ -606,4 +607,20 @@ protected function getLogger() {
   +    if ($target_entity_type->getBundleOf() === NULL) {
   

   Tested that can not use getBundleOf() to distinguish whether it has bundles.

2. +++ b/core/lib/Drupal/Core/Entity/EntityDisplayBase.php
   @@ -606,4 +607,20 @@ protected function getLogger() {
   +      '@bundle' => $target_entity_type->getBundleLabel(),
   

   And can not get bundle label by calling getBundleLabel() too

Before

• user with default view mode
  
• node article with default view mode
  
• node article with teaser view mode
  

After

• user with default view mode

  

• node article with default view mode

  

• node article with teaser view mode

  

BTW, the display mode in use is its machine name, if it's necessary to display the label, entity_display.repository is the right service to continue with.

+1 to use the label of display mode instead of its machine name

the path did not apply success on Drupal 9.1, see screenshot.

+++ b/core/modules/layout_builder/src/Entity/LayoutBuilderEntityViewDisplay.php
@@ -365,18 +365,6 @@ protected function getRuntimeSections(FieldableEntityInterface $entity) {
-  /**
-   * {@inheritdoc}
-   *
-   * @todo Move this upstream in https://www.drupal.org/node/2939931.
-   */
-  public function label() {
-    $bundle_info = \Drupal::service('entity_type.bundle.info')->getBundleInfo($this->getTargetEntityTypeId());
-    $bundle_label = $bundle_info[$this->getTargetBundle()]['label'];
-    $target_entity_type = $this->entityTypeManager()->getDefinition($this->getTargetEntityTypeId());
-    return new TranslatableMarkup('@bundle @label', ['@bundle' => $bundle_label, '@label' => $target_entity_type->getPluralLabel()]);
-  }
-

this chunk was removed in the latest patch.

Hi neclimdul,

Removed unused code, Kindly check.

Working fine for me.
For my side, it's ready to RTBC +1

BTW, this is not RTBC ready, There is a Needs usability review tag still.

Sorry, reverting the unintentional change made to IS

Rerolled a patch from #15 directly, as #25 has one coding standard violation.

Assigning to myself and making this issue reviewable for #3135889: Drupal Usability Meeting 2020-05-19.

Changing from "content items" back to "Content" is probably not ideal. Nor is using machine names in UI text.

Using display mode label, instead of the machine name.

Updating IS

Hide files

Updating before-after image in IS

First, thanks everybody for your work! We reviewed this during today's weekly UX meeting and after a long discussion we've decided that this issue would ideally just add the view mode in parenthesis after the title. So this should ideally be:

Bundle title + current plural label + (View mode name in parenthesis)

So it would be like: Article content items (Teaser) or Category taxonomy terms (Default).

The reason behind it is that adding "Content:" before article has 2 backsides. First, it moves away the real item/title that makes reference to what we're editing (Bundle is more relevant here than the Entity type). Second, it makes confusing for someone without experience understanding the meaning of the label "Content" there. And adding "Entity type: Content" is too long.

Also, the _ideal_ solution should go through adding a new element below the title to add additional information. Like a subtitle or a tag. Here's a really really quick exploration that probably would need another issue + proper discussion:

Closed #3099612: EntityFormDisplay and EntityViewDisplay should implement label() as duplicate, assigning credit for folks over there

This makes a nice UX improvement, thanks!

1. +++ b/core/lib/Drupal/Core/Entity/EntityDisplayBase.php
   @@ -570,4 +571,39 @@ protected function getLogger() {
   +    // The target entity type has no bundles, such as the user entity.
   +    if ($target_entity_type->getBundleEntityType() === NULL) {
   

   Unfortunately, this isn't true.

   Entities can have bundles without having an entity bundle type, there's hook_entity_bundle_info and a corresponding alter hook.

   You need to use the entity_type.bundle.info service to ascertain if the entity supports bundles.

2. +++ b/core/lib/Drupal/Core/Entity/EntityDisplayBase.php
   @@ -570,4 +571,39 @@ protected function getLogger() {
   +    $bundle_info = $this->entityTypeBundleInfo()->getBundleInfo($target_entity_type_id);
   

   oh! and we already have that here too

There's a fair bit going on in that method now, so I think we need some test coverage - probably simpler to do as a kernel test

@larowlan do we consider changing the things that mentioned in #33

@nitesh624, yes, #33 must be addressed which is the result of Usability review during the meeting #3135889: Drupal Usability Meeting 2020-05-19

I was really confused catching up trying to remember what I said until I realized there was another n name in the discussion talking about 33. Lol

Sorry, @neclimdul, I meant to reply @nitesh624. Blame me and dreditor :p hitting TAB after typing @n, dreditor autocompleted with your username. in #42. corrected/edited.

Changes done as per Drupal Usability Meeting 2020-05-19 in #33

Layout form for user

Layout form for article content type in default view mode

Layout form for article content type in teaser view mode

Addressed, #39.1
Please check. #39 2 please suggest me for a test case in which kernel file or need to create new kernel test file?

looks like unrelated failure.

patch #49 Applied on 9.2.x

Addressed #54.

fixed the failed testcase.

- Adding a few test cases to validate the title.
For the regular fieldable entity, I couldn't add it to testLayoutBuilderUitest case as it was interfering other test assertions because we have to put the page title block.

+++ b/core/modules/layout_builder/tests/src/Functional/LayoutBuilderTest.php
@@ -1235,6 +1236,26 @@ public function testBreadcrumb() {
+  public function testLayoutBuilderPageTitle() {
+    $this->drupalPlaceBlock('page_title_block');
+    $assert_session = $this->assertSession();
+
+    $this->drupalLogin($this->drupalCreateUser([
+      'configure any layout',
+      'administer node display',
+      'administer node fields',
+    ]));
+
+    // From the manage display page, go to manage the layout.
+    $this->drupalGet("admin/structure/types/manage/bundle_with_section_field/display/default");

- InstallUninstallTest::testInstallUninstall test case seems to be passing on local. It just takes a while to run all the test cases as there are approx 2700 assertions. Let's see how it goes.

Thanks for the tests. Reviewed the interdiff.

1. +++ b/core/modules/layout_builder/tests/src/Functional/LayoutBuilderTest.php
   @@ -1235,6 +1236,26 @@ public function testBreadcrumb() {
   +   * Test to ensure that appropriate title exists on manage layout page.
   

   Nitpick: Consider slight rewording:

   Tests that the appropriate title exists on the manage layout page.

2. +++ b/core/modules/layout_builder/tests/src/Functional/LayoutBuilderTest.php
   @@ -1235,6 +1236,26 @@ public function testBreadcrumb() {
   +    $this->drupalGet("admin/structure/types/manage/bundle_with_section_field/display/default");
   

   Nitpick: Single quotes can be used instead of double quotes.

3. +++ b/core/modules/layout_builder/tests/src/Functional/LayoutBuilderTest.php
   @@ -1235,6 +1236,26 @@ public function testBreadcrumb() {
   +    $assert_session->elementTextContains('css', '.page-title', 'Edit layout for Bundle with section field content items (Default)');
   

   This one seems a bit odd to me... we can only check via the CSS? I do see two examples (below) of something similar in other code but it feels awkward to me.

   KristenBackupMBP:drupal-9.2.x-dev admin$ grep -r elementTextContains . | grep css | grep page-title
   ./core/tests/Drupal/FunctionalTests/Entity/DeleteMultipleFormTest.php:    $assert->elementTextContains('css', '.page-title', 'Are you sure you want to delete these test entity - revisions, data table, and published interface entities?');
   ./core/tests/Drupal/FunctionalTests/Entity/DeleteMultipleFormTest.php:    $assert->elementTextContains('css', '.page-title', 'Are you sure you want to delete these test entity - revisions entities?');
   
   

Moving back to needs work for consideration of #59.

@Kristen Pol regarding 3rd point from #59 as you have posted the examples for it, i think we can use it for now until all of them will be removed. What do you think?

Thanks for the update. If we know of a better way to test it, then it would be nice to do it now and perhaps make a follow-up issue for those other places. But I'm not an expert on tests, so maybe someone else can chime in here.

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

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