Problem/Motivation

The implementation of EntityType::getLowercaseLabel() is simplistic and doesn't offer overriding. That is a problem for some entity type labels like "JW Player preset" which just becomes "jw player preset".

Proposed resolution

Add optional "lowercase_label" key to ConfigEntityType/ContentEntityType annotation?
Or add a $lowercase = FALSE parameter to label() (and label_callback usages)?

Remaining tasks

User interface changes

API changes

Data model changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Arla created an issue. See original summary.

dawehner’s picture

Add optional "lowercase_label" key to ConfigEntityType/ContentEntityType annotation?

That sounds for me like the best, because simplest, solution. Are you sure we need the additional parameter on the label() method?

Arla’s picture

The label() parameter was meant as an alternative solution, in which case a default strtolower implementation could be overridden by entity classes using label_callback.

So I had two separate suggestions.

Arla’s picture

Btw this would also solve the problem mentioned in #2507235: EntityType::getLowercaseLabel() breaks translation. This is possibly a duplicate?

dawehner’s picture

Given that a new entity key also allows you to not break any kind of API, it should be pretty okay for 8.1

Arla’s picture

Issue summary: View changes

Sorry, I got EntityType::getLabel and Entity::label mixed up. Removing the second alternative then :)

Arla’s picture

Here's the general idea.

Btw I also have to take back #4 because this will only solve the translation problem if the lowercase_label is present for a particular entity type (or required and present for all, which would break APIs). Then, with the Block example mentioned in that issue, you could have

 *   label = "Block",
 *   lowercase_label = "block",

Then you would have both strings translatable and German could translate "Block" to "Block" and "block" also to "Block".

Arla’s picture

Status: Active » Needs review
FileSize
2.55 KB

With a test.

Berdir’s picture

Makes sense, but to fix #2507235: EntityType::getLowercaseLabel() breaks translation, we need to add it to all entity types and we need them all to have a 'entity-type-label-lowercase' or similar context so german can translate "content" to uppercase "Inhalt". Maybe we should even enforce that context?

dawehner’s picture

+++ b/core/tests/Drupal/Tests/Core/Entity/EntityTypeTest.php
@@ -299,6 +299,22 @@ public function testGetLabel() {
 
   /**
+   * @covers ::getLowercaseLabel
+   */
+  public function testGetLowercaseLabel() {
+    // The lowercase label should be used if defined.
+    $entity_type = $this->setUpEntityType(['label' => 'Thing ABC']);
+    $this->assertSame('thing abc', $entity_type = $entity_type->getLowercaseLabel());
+    $entity_type = $this->setUpEntityType(['label' => 'Thing ABC', 'lowercase_label' => 'thing ABC']);
+    $this->assertSame('thing ABC', $entity_type = $entity_type->getLowercaseLabel());
+
+    // With markup object.
+    $translatable_lowercase_label = new TranslatableMarkup($this->randomMachineName());
+    $entity_type = $this->setUpEntityType(array('lowercase_label' => $translatable_lowercase_label));
+    $this->assertSame($translatable_lowercase_label, $entity_type->getLabel());
+  }

Ideally you would use two test methods for the two cases.

Arla’s picture

Okay, separated the tests.

But I guess Berdir has a point in that we might want to do all this in the translation issue directly, so we can add things rather than add and then change things. I'll see if I can write up a patch for that other issue too.

Gábor Hojtsy’s picture

Issue tags: +D8MI, +language-ui, +sprint

Patch looks fine. It would be nice to figure out how will this work if/once #1850080: Entity type labels lack plurality, cannot generate UI text based on label if plural is needed is added too. You don't need to resolve it here, just saying that is also in the works.

+++ b/core/modules/rdf/src/Entity/RdfMapping.php
@@ -17,6 +17,7 @@
+ *   lowercase_label = @Translation("RDF mapping"),

I agree it would be important to have a "Lowercase label" context. It needs to be in the annotation for potx to parse it properly, so not much of a chance to automate that at another place.

Gábor Hojtsy’s picture

Berdir’s picture

Status: Needs review » Closed (duplicate)

I guess that makes sense, we can re-open if the direction will change there but I don't think so.

Gábor Hojtsy’s picture

Issue tags: -sprint