From 146f2994deea54fd2e33fbd06dd335086641f050 Mon Sep 17 00:00:00 2001
From: Javier Eguiluz <javier.eguiluz@gmail.com>
Date: Sat, 28 Oct 2023 14:57:16 +0200
Subject: [PATCH] Fixes and updates about field labels

---
 doc/actions.rst                               |  2 +-
 doc/fields.rst                                | 32 ++++++--
 src/Factory/MenuFactory.php                   |  2 +-
 .../Configurator/CommonPostConfigurator.php   |  4 +
 .../Configurator/CommonPreConfigurator.php    |  2 +-
 src/Resources/views/crud/detail.html.twig     | 29 ++++---
 src/Resources/views/crud/form_theme.html.twig |  8 +-
 .../FormFieldHelpControllerTest.php           |  2 +-
 .../FormFieldLabelControllerTest.php          | 75 +++++++++++++++++++
 .../Controller/FormFieldLabelController.php   | 37 +++++++++
 tests/TestApplication/src/Entity/User.php     |  5 ++
 11 files changed, 172 insertions(+), 26 deletions(-)
 create mode 100644 tests/Controller/FormFieldLabelControllerTest.php
 create mode 100644 tests/TestApplication/src/Controller/FormFieldLabelController.php

diff --git a/doc/actions.rst b/doc/actions.rst
index b752cc699f..430bc1a027 100644
--- a/doc/actions.rst
+++ b/doc/actions.rst
@@ -265,7 +265,7 @@ the ``Action`` class constructor::
     // that the label is autogenerated from the name (e.g. 'viewInvoice' -> 'View Invoice')
     $viewInvoice = Action::new('viewInvoice', null);
     // set the label to FALSE to not display any label for this action (but make sure
-    // to display an icon for the action; otherwise users won't see it)
+    // to display an icon for the action; otherwise users can't see or click on the action)
     $viewInvoice = Action::new('viewInvoice', false);
 
     // the third optional argument is the full CSS class of a FontAwesome icon
diff --git a/doc/fields.rst b/doc/fields.rst
index f4908896b1..f264177215 100644
--- a/doc/fields.rst
+++ b/doc/fields.rst
@@ -641,19 +641,35 @@ some fields define additional config options, as shown in the
 Label Options
 ~~~~~~~~~~~~~
 
-The second optional argument of the field constructors is the label::
-
-    // not defining the label explicitly or setting it to NULL means
-    // that the label is autogenerated (e.g. 'firstName' -> 'First Name')
+The second optional argument of the field constructors is the label, which can
+take many different values:
+
+* If you **don't set the label** explicitly, EasyAdmin generates the label
+  automatically based on the field name (e.g. 'firstName' -> 'First Name');
+* **null**: EasyAdmin generates the label automatically based on the field name
+  (e.g. 'firstName' -> 'First Name');
+* **An empty string**: the field doesn't display any label, but and empty
+  ``<label>`` element is rendered to not mess with the form layout;
+* **false**: the field doesn't display any label and no ``<label>`` element is
+  rendered either. This is useful to display special full-width fields such as
+  maps or wide tables created with custom field templates;
+* If you **set the label** explicitly, EasyAdmin will use that value; the contents
+  can include HTML tags and they will be rendered, not escaped. Also, you can use
+  ``Translatable`` contents (e.g. ``t('admin.form.labels.user')``)
+
+Here are some examples of field labels in action:
+
+    // label not defined: generate it automatically (label = 'First Name')
     TextField::new('firstName'),
+    // label is null: generate it automatically (label = 'First Name')
     TextField::new('firstName', null),
 
-    // set the label explicitly to display exactly that label
-    TextField::new('firstName', 'Name'),
-
-    // set the label to FALSE to not display any label for this field
+    // label is false: no label is displayed and no <label> element is rendered
     TextField::new('firstName', false),
 
+    // label is set explicitly: render its contents, including HTML tags
+    TextField::new('firstName', 'Customer <b>Name</b>'),
+
 Design Options
 ~~~~~~~~~~~~~~
 
diff --git a/src/Factory/MenuFactory.php b/src/Factory/MenuFactory.php
index e27e342281..f0c703e80c 100644
--- a/src/Factory/MenuFactory.php
+++ b/src/Factory/MenuFactory.php
@@ -136,7 +136,7 @@ private function generateMenuItemUrl(MenuItemDto $menuItemDto): string
             // 1. if CRUD controller is defined, use it...
             if (null !== $crudControllerFqcn) {
                 $this->adminUrlGenerator->setController($crudControllerFqcn);
-                // 2. ...otherwise, find the CRUD controller from the entityFqcn
+            // 2. ...otherwise, find the CRUD controller from the entityFqcn
             } else {
                 $crudControllers = $this->adminContextProvider->getContext()?->getCrudControllers();
                 if (null === $controllerFqcn = $crudControllers->findCrudFqcnByEntityFqcn($entityFqcn)) {
diff --git a/src/Field/Configurator/CommonPostConfigurator.php b/src/Field/Configurator/CommonPostConfigurator.php
index ef02a23ab2..22ae40dde2 100644
--- a/src/Field/Configurator/CommonPostConfigurator.php
+++ b/src/Field/Configurator/CommonPostConfigurator.php
@@ -33,6 +33,10 @@ public function supports(FieldDto $field, EntityDto $entityDto): bool
 
     public function configure(FieldDto $field, EntityDto $entityDto, AdminContext $context): void
     {
+        // EasyAdmin by default allows using HTML contents in labels, help messages, etc.
+        // so we must enable the 'label_html' form option unless a field has configured it explicitly
+        $field->setFormTypeOptionIfNotSet('label_html', true);
+
         if (\in_array($context->getCrud()->getCurrentPage(), [Crud::PAGE_INDEX, Crud::PAGE_DETAIL], true)) {
             $formattedValue = $this->buildFormattedValueOption($field->getFormattedValue(), $field, $entityDto);
             $field->setFormattedValue($formattedValue);
diff --git a/src/Field/Configurator/CommonPreConfigurator.php b/src/Field/Configurator/CommonPreConfigurator.php
index ae868b81d9..62d20c4a7b 100644
--- a/src/Field/Configurator/CommonPreConfigurator.php
+++ b/src/Field/Configurator/CommonPreConfigurator.php
@@ -131,7 +131,7 @@ private function buildLabelOption(FieldDto $field, string $translationDomain, ?s
             $label = $this->humanizeString($field->getProperty());
         }
 
-        if ('' === $label) {
+        if ('' === $label || false === $label) {
             return $label;
         }
 
diff --git a/src/Resources/views/crud/detail.html.twig b/src/Resources/views/crud/detail.html.twig
index 9e982936f6..a234177aa2 100644
--- a/src/Resources/views/crud/detail.html.twig
+++ b/src/Resources/views/crud/detail.html.twig
@@ -70,19 +70,24 @@
 
 {% macro render_field_contents(entity, field) %}
     <div class="field-group {{ field.cssClass }}">
-        <div class="field-label">
-            {%- set label_html_attributes -%}
-                {%- if field.help is not empty -%}
-                    data-bs-toggle="tooltip" data-bs-placement="auto" data-bs-animation="false"
-                    data-bs-html="true" data-bs-custom-class="ea-detail-label-tooltip"
-                    data-bs-title="{{ field.help|trans|e('html_attr') }}"
-                {%- endif -%}
-            {%- endset -%}
-
-            <div {{ label_html_attributes }}>
-                {{ field.label|trans|raw }}
+        {% if field.label is same as (false) %}
+            {# a FALSE label value means that the field doesn't even display the <label> element;
+               use an empty string to not display a label but keep the <label> element to not mess with the layout #}
+        {% else %}
+            <div class="field-label">
+                {%- set label_html_attributes -%}
+                    {%- if field.help is not empty -%}
+                        data-bs-toggle="tooltip" data-bs-placement="auto" data-bs-animation="false"
+                        data-bs-html="true" data-bs-custom-class="ea-detail-label-tooltip"
+                        data-bs-title="{{ field.help|trans|e('html_attr') }}"
+                    {%- endif -%}
+                {%- endset -%}
+
+                <div {{ label_html_attributes }}>
+                    {{ field.label|trans|raw }}
+                </div>
             </div>
-        </div>
+        {% endif %}
 
         <div class="field-value">
             {{ include(field.templatePath, { field: field, entity: entity }, with_context = false) }}
diff --git a/src/Resources/views/crud/form_theme.html.twig b/src/Resources/views/crud/form_theme.html.twig
index 00170de46c..fee1058d5c 100644
--- a/src/Resources/views/crud/form_theme.html.twig
+++ b/src/Resources/views/crud/form_theme.html.twig
@@ -245,7 +245,8 @@
 
 {% block form_label -%}
     {% if label is same as(false) -%}
-        <label>{# the empty <label> is needed to not break the form design #}</label>
+        {# don't display anything, not even an empty <label> element; if you want to not display
+           any label contents but keep the form layout, use an empty string as the field label #}
     {%- else -%}
         {%- if compound is defined and compound -%}
             {%- set element = 'legend' -%}
@@ -256,7 +257,10 @@
         {% if required -%}
             {% set label_attr = label_attr|merge({class: (label_attr.class|default('') ~ ' required')|trim}) %}
         {%- endif -%}
-        {% if label is empty -%}
+        {% if label == '' -%}
+            {# don't process the label; this is used to not display any label content
+               but render an empty <label> element keep the form layout #}
+        {%- elseif label is null -%}
             {%- if label_format is not empty -%}
                 {% set label = label_format|replace({
                     '%name%': name,
diff --git a/tests/Controller/FormFieldHelpControllerTest.php b/tests/Controller/FormFieldHelpControllerTest.php
index a158c1a678..3c2e279782 100644
--- a/tests/Controller/FormFieldHelpControllerTest.php
+++ b/tests/Controller/FormFieldHelpControllerTest.php
@@ -30,7 +30,7 @@ protected function getDashboardFqcn(): string
         return DashboardController::class;
     }
 
-    public function testFieldsWithoutHelp()
+    public function testFieldsHelpMessages()
     {
         $crawler = $this->client->request('GET', $this->generateNewFormUrl());
 
diff --git a/tests/Controller/FormFieldLabelControllerTest.php b/tests/Controller/FormFieldLabelControllerTest.php
new file mode 100644
index 0000000000..b4350befce
--- /dev/null
+++ b/tests/Controller/FormFieldLabelControllerTest.php
@@ -0,0 +1,75 @@
+<?php
+
+namespace EasyCorp\Bundle\EasyAdminBundle\Tests\Controller;
+
+use Doctrine\ORM\EntityRepository;
+use EasyCorp\Bundle\EasyAdminBundle\Test\AbstractCrudTestCase;
+use EasyCorp\Bundle\EasyAdminBundle\Tests\TestApplication\Controller\DashboardController;
+use EasyCorp\Bundle\EasyAdminBundle\Tests\TestApplication\Controller\FormFieldLabelController;
+use EasyCorp\Bundle\EasyAdminBundle\Tests\TestApplication\Entity\BlogPost;
+
+class FormFieldLabelControllerTest extends AbstractCrudTestCase
+{
+    protected EntityRepository $blogPosts;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+        $this->client->followRedirects();
+
+        $this->blogPosts = $this->entityManager->getRepository(BlogPost::class);
+    }
+
+    protected function getControllerFqcn(): string
+    {
+        return FormFieldLabelController::class;
+    }
+
+    protected function getDashboardFqcn(): string
+    {
+        return DashboardController::class;
+    }
+
+    public function testFieldsLabelsInForms()
+    {
+        $crawler = $this->client->request('GET', $this->generateNewFormUrl());
+
+        static::assertSelectorTextSame('label[for="BlogPost_id"]', 'ID', 'The "id" field does not define a label explicitly, so it is generated automatically based on the field name. Edge-case: fields named "id" are labeled as uppercase "ID" instead of the default title-case "Id" of the rest of the fields.');
+
+        static::assertSelectorTextSame('label[for="BlogPost_title"]', 'Title', 'The "title" field uses NULL as the value of the label, which tells EasyAdmin to generate a label automatically as the title-case value of the field name.');
+
+        static::assertSelectorNotExists('.form-group.field-slug label', 'The "slug" field uses FALSE as the value of the label, which means that no <label> element should be rendered for that field.');
+
+        static::assertSelectorTextSame('label[for="BlogPost_content"]', '', 'The "content" field defines an empty string as the label, so it renders a <label> element to keep the design layout but without any contents inside.');
+
+        static::assertSelectorTextSame('label[for="BlogPost_createdAt"]', 'Lorem Ipsum 1', 'The "createdAt" field defines a regular text string as its label (no HTML contents).');
+
+        static::assertSelectorTextSame('label[for="BlogPost_publishedAt"]', 'Lorem Ipsum 2', 'The "publishedAt" field defines its label as a translatable string with regular text (no HTML contents).');
+
+        static::assertSame($crawler->filter('label[for="BlogPost_author"]')->html(), 'Lorem <a href="https://example.com">Ipsum</a> <b>3</b>', 'The "author" field defines its label as regular string with HTML contents, which must be rendered instead of escaped (HTML is allowed in field labels).');
+
+        static::assertSame($crawler->filter('label[for="BlogPost_publisher"]')->html(), 'Lorem <a href="https://example.com">Ipsum</a> <b>4</b>', 'The "publisher" field defines its label as translatable string with HTML contents, which must be rendered instead of escaped (HTML is allowed in field labels).');
+    }
+
+    public function testFieldsLabelsInDetailPage()
+    {
+        $blogPost = $this->blogPosts->findOneBy([]);
+        $crawler = $this->client->request('GET', $this->generateDetailUrl($blogPost->getId()));
+
+        static::assertSelectorTextSame('.field-group.field-id .field-label > div', 'ID', 'The "id" field does not define a label explicitly, so it is generated automatically based on the field name. Edge-case: fields named "id" are labeled as uppercase "ID" instead of the default title-case "Id" of the rest of the fields.');
+
+        static::assertSelectorTextSame('.field-group.field-title .field-label > div', 'Title', 'The "title" field uses NULL as the value of the label, which tells EasyAdmin to generate a label automatically as the title-case value of the field name.');
+
+        static::assertSelectorNotExists('.field-group.field-slug .field-label', 'The "slug" field uses FALSE as the value of the label, which means that no <label> element should be rendered for that field.');
+
+        static::assertSelectorTextSame('.field-group.field-content .field-label > div', '', 'The "content" field defines an empty string as the label, so it renders a <label> element to keep the design layout but without any contents inside.');
+
+        static::assertSelectorTextSame('.field-group.field-created-at .field-label > div', 'Lorem Ipsum 1', 'The "createdAt" field defines a regular text string as its label (no HTML contents).');
+
+        static::assertSelectorTextSame('.field-group.field-published-at .field-label > div', 'Lorem Ipsum 2', 'The "publishedAt" field defines its label as a translatable string with regular text (no HTML contents).');
+
+        static::assertSame(trim($crawler->filter('.field-group.field-author .field-label > div')->html()), 'Lorem <a href="https://example.com">Ipsum</a> <b>3</b>', 'The "author" field defines its label as regular string with HTML contents, which must be rendered instead of escaped (HTML is allowed in field labels).');
+
+        static::assertSame(trim($crawler->filter('.field-group.field-publisher .field-label > div')->html()), 'Lorem <a href="https://example.com">Ipsum</a> <b>4</b>', 'The "publisher" field defines its label as translatable string with HTML contents, which must be rendered instead of escaped (HTML is allowed in field labels).');
+    }
+}
diff --git a/tests/TestApplication/src/Controller/FormFieldLabelController.php b/tests/TestApplication/src/Controller/FormFieldLabelController.php
new file mode 100644
index 0000000000..05ebcc4aa2
--- /dev/null
+++ b/tests/TestApplication/src/Controller/FormFieldLabelController.php
@@ -0,0 +1,37 @@
+<?php
+
+namespace EasyCorp\Bundle\EasyAdminBundle\Tests\TestApplication\Controller;
+
+use EasyCorp\Bundle\EasyAdminBundle\Controller\AbstractCrudController;
+use EasyCorp\Bundle\EasyAdminBundle\Field\AssociationField;
+use EasyCorp\Bundle\EasyAdminBundle\Field\DateTimeField;
+use EasyCorp\Bundle\EasyAdminBundle\Field\IdField;
+use EasyCorp\Bundle\EasyAdminBundle\Field\TextField;
+use EasyCorp\Bundle\EasyAdminBundle\Tests\TestApplication\Entity\BlogPost;
+use function Symfony\Component\Translation\t;
+
+/**
+ * Used to test the different types of labels that fields can configure.
+ */
+class FormFieldLabelController extends AbstractCrudController
+{
+    public static function getEntityFqcn(): string
+    {
+        return BlogPost::class;
+    }
+
+    public function configureFields(string $pageName): iterable
+    {
+        return [
+            // the custom CSS classes are needed so we can select the fields in tests
+            IdField::new('id')->addCssClass('field-id'),
+            TextField::new('title', null)->addCssClass('field-title'),
+            TextField::new('slug', false)->addCssClass('field-slug'),
+            TextField::new('content', '')->addCssClass('field-content'),
+            DateTimeField::new('createdAt', 'Lorem Ipsum 1')->addCssClass('field-created-at'),
+            DateTimeField::new('publishedAt', t('Lorem Ipsum 2'))->addCssClass('field-published-at'),
+            AssociationField::new('author', 'Lorem <a href="https://example.com">Ipsum</a> <b>3</b>')->addCssClass('field-author'),
+            AssociationField::new('publisher', t('Lorem <a href="https://example.com">Ipsum</a> <b>4</b>'))->addCssClass('field-publisher'),
+        ];
+    }
+}
diff --git a/tests/TestApplication/src/Entity/User.php b/tests/TestApplication/src/Entity/User.php
index c20b061114..ce322f445e 100644
--- a/tests/TestApplication/src/Entity/User.php
+++ b/tests/TestApplication/src/Entity/User.php
@@ -29,6 +29,11 @@ public function __construct()
         $this->blogPosts = new ArrayCollection();
     }
 
+    public function __toString(): string
+    {
+        return $this->name;
+    }
+
     public function getId(): ?int
     {
         return $this->id;