Skip to content

Commit

Permalink
bug #5987 Fixes and updates about field labels (javiereguiluz)
Browse files Browse the repository at this point in the history
This PR was squashed before being merged into the 4.x branch.

Discussion
----------

Fixes and updates about field labels

This PR fixes some issues:

* If label is `false`, we don't have to render any content or any `<label>` element
* We must allow HTML contents in labels by default (we allowed them in DETAIL page but not in FORMS)
* Added tests so this never breaks in the future

Commits
-------

146f299 Fixes and updates about field labels
  • Loading branch information
javiereguiluz committed Oct 28, 2023
2 parents fbb652e + 146f299 commit 7fdd14d
Show file tree
Hide file tree
Showing 11 changed files with 172 additions and 26 deletions.
2 changes: 1 addition & 1 deletion doc/actions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 24 additions & 8 deletions doc/fields.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
~~~~~~~~~~~~~~

Expand Down
2 changes: 1 addition & 1 deletion src/Factory/MenuFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
4 changes: 4 additions & 0 deletions src/Field/Configurator/CommonPostConfigurator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/Field/Configurator/CommonPreConfigurator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
29 changes: 17 additions & 12 deletions src/Resources/views/crud/detail.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -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) }}
Expand Down
8 changes: 6 additions & 2 deletions src/Resources/views/crud/form_theme.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -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' -%}
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion tests/Controller/FormFieldHelpControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ protected function getDashboardFqcn(): string
return DashboardController::class;
}

public function testFieldsWithoutHelp()
public function testFieldsHelpMessages()
{
$crawler = $this->client->request('GET', $this->generateNewFormUrl());

Expand Down
75 changes: 75 additions & 0 deletions tests/Controller/FormFieldLabelControllerTest.php
Original file line number Diff line number Diff line change
@@ -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).');
}
}
37 changes: 37 additions & 0 deletions tests/TestApplication/src/Controller/FormFieldLabelController.php
Original file line number Diff line number Diff line change
@@ -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'),
];
}
}
5 changes: 5 additions & 0 deletions tests/TestApplication/src/Entity/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 7fdd14d

Please sign in to comment.