Add option to allow HTML in labels

Created on 8 January 2019, almost 6 years ago
Updated 9 September 2024, 3 months ago

Hi,

It will be great and so usefull if HTML is allowed inside the field group label so "fontawesome icons" or "images" could replace the simple "text".

For example, the field group label can contain:

<i class="fa fa-tint" aria-hidden="true"></i>
or
url(image1.png)

This issue here with its patch ( Use HTML in Fieldset label ) it is a great start but unfortunately, it is for the drupal 7 version and only for the "Fieldset" type.

Also, this module here ( Field Image label ) gives a good idea if it can be implemented within the "field group" module.

Finally, the field group tabs will be possible to look something like the below image (as an example):

Thank you,

Feature request
Status

Fixed

Version

3.0

Component

Code

Created by

🇱🇧Lebanon C.E.A

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Status changed to Needs work almost 2 years ago
  • 🇩🇪Germany Anybody Porta Westfalica
  • First commit to issue fork.
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    17 pass
  • @grevil opened merge request.
  • 🇩🇪Germany Grevil

    Before we manually rebase the code again and again, I created an issue fork and applied patch #55 by @smustgrave, for automatic rebasing and easier in Browser code reviews.

  • 🇩🇪Germany Anybody Porta Westfalica

    Thank you @Grevil!

    So finally I think this needs test copies with
    'fieldset_label_html' => TRUE,
    and the expected results and checking #57.

    If possible #57 should simply also be covered by a test, so we can ensure it works as expected.

  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    17 pass
  • 🇩🇪Germany Grevil

    Could not reproduce behaviour stated in #58, I tested the patch and it worked for (mostly) all field group widgets!

    The icon was displayed cleanly for Details, Tab, Details Sitebar, Accordion Item and Fieldset. I don't have too much experience with the module, so I was unable to properly use the "Accordion" and "Tabs" Widget types. I am also unsure, if it should work with "HTML Element" as the icon will get wrapped inside other incompatible elements, leading to break the icon.

    Somebody with a bit more knowledge about this module should decide if the option should also exist for "Accordion", "Tabs" and "HTML Element".

    Here are screenshots, testing the "Tab" Widget:
    Form Display Settings:

    Form:

    As of #64, this still needs automated tests.

  • 🇩🇪Germany Anybody Porta Westfalica

    Thanks @Grevil, perfect! :)

    Somebody with a bit more knowledge about this module should decide if the option should also exist for "Accordion", "Tabs" and "HTML Element".

    Thanks for the question, no that wouldn't make sense, as they are only wrappers with no label. HTML Element is special, I'd vote not to touch it here. It's a type itself and has special handling.

    So only tests left to get this in! Whao!!

  • 🇩🇪Germany Grevil

    // By default tabs don't have titles but you can override it in the theme.

    Just found that in code, I guess we shouldn't touch that definition either then.

  • 🇩🇪Germany Grevil

    OK, I could replicate the behaviour of #58 now! It seems to be specific to multiple "Tab" Elements being rendered inside a "Tabs" Element. And ONLY if the Tabs are rendered vertically. horizontally works as expected.

  • 🇩🇪Germany Grevil

    Alright, the issue with the vertically rendered tabs is an upstream core issue, which we currently can't do anything about it. The core "vertical-tabs.js" uses "textContent" to render the tab labels instead of "innerHTML".

    Element.innerHTML returns HTML, as its name indicates. Sometimes people use innerHTML to retrieve or write text inside an element, but textContent has better performance because its value is not parsed as HTML.

    See https://developer.mozilla.org/en-US/docs/Web/API/Node/textContent?retire...

    Additionally, the HTML gets escaped somewhere else (but that should be resolved in the core issue).

    I will create a follow-up issue for this and link the core issue once created.

    Here is a screenshot, where the tabs are rendered as details elements with the correct title for a split second:

    And here the final rendered vertical tabs render element with the escaped and removed HTML in the title:

    So only tests remain here!

  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    23 pass
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    23 pass
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    23 pass
  • Status changed to Needs review over 1 year ago
  • 🇩🇪Germany Grevil

    Alright, I added appropriate tests, please review!

  • Status changed to Needs work over 1 year ago
  • 🇩🇪Germany Anybody Porta Westfalica

    WHAO! So happy to see the progress here and the required tests being added!

    I just reviewed the code, the tests and implementation LGTM!
    I left some final comments, most of them are just around wording and it would be nice, if a native speaker (EN) could pick these up.

    @Grevil: Perhaps you'd like to have a final look at the label-else-part comment and try if it works without the additional handling there?

    Once these minor comments are solved, RTBC from my side! Yay! Thank you so much @Grevil!

  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    23 pass
  • 🇩🇪Germany Anybody Porta Westfalica
  • Status changed to Needs review over 1 year ago
  • 🇩🇪Germany Grevil

    @Anybody:

    I guess the additional handling isn't needed in the ELSE part. That could just stay as before? Drupal / Twig will do the escaping? I couldn't find a place where we're changing the output logic, so it should still work?

    The "Markup::create()" was originally suggested in #12, but it was just a workaround for "&" being escaped because of the "Html::escape" in the original patch 6.

    I guess, before this patch, A lot of HTML elements were actually allowed, BUT not for horizontal tabs. Markup::create(Html::escape($this->getLabel())); will ensure, no HTML is allowed, when the checkbox isn't checked.

  • 🇩🇪Germany Anybody Porta Westfalica

    Re #74 thanks for the explanation @Grevil! That makes sense, so let's keep this fix for #12 to also fix such potential double escapings. I don't think it will do any harm.

  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    23 pass
  • 🇩🇪Germany Grevil

    OK, let's see how the test turn out!

  • 🇩🇪Germany Grevil

    All green!

  • Status changed to RTBC over 1 year ago
  • 🇩🇪Germany Anybody Porta Westfalica

    Wohoo! RTBC! :)

  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 5.7
    last update about 1 year ago
    23 pass
  • Status changed to Fixed about 1 year ago
  • 🇩🇪Germany Anybody Porta Westfalica
  • Automatically closed - issue fixed for 2 weeks with no activity.

  • Status changed to Fixed 3 months ago
  • 🇩🇪Germany Grevil

    This introduced a regression, when rendering a horizontal "tabs" field group in which we render a "tab" field group which uses HTML in the label AND "label_as_html" TRUE. It will "destroy" the tabs output.

    Without HTML in the label AND "label_as_html" TRUE:

    With HTML in the label AND "label_as_html" TRUE:

  • 🇩🇪Germany Grevil

    This should have been caught by my tests already, but it seems I implemented them quite poorly. They had some unexpected html output and it seems I didn't investigate any further, my apologies.

    I created a follow-up issue regarding this problem here: 🐛 Horizontal Tabs render broken, if child allows HTML Active .

  • 🇩🇪Germany Grevil

    Update: Ok, the problem was a recently merged issue: Add classes to JS rewriting in horizontal tabs Fixed .

    But the tests are still not very good.

Production build 0.71.5 2024