#description_display doesn't work with multiple value tableselect field

Created on 17 April 2023, about 2 years ago

Problem/Motivation

The "#description_display" property should tell the Form API how to position a form field description.

The values "#before" (and possibly "#invisible" as well) are ignored by multiple value tableselect fields, and the #description display is not changed.

Steps to reproduce

Proposed resolution

#description_display property should be supported by all elements that support #description

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

πŸ› Bug report
Status

Active

Version

10.1 ✨

Component
FormΒ  β†’

Last updated about 3 hours ago

Created by

πŸ‡ΊπŸ‡ΈUnited States AaronBauman Philadelphia

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

Comments & Activities

  • Issue created by @AaronBauman
  • πŸ‡ΊπŸ‡ΈUnited States AaronBauman Philadelphia
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update about 2 years ago
    Custom Commands Failed
  • πŸ‡ΊπŸ‡ΈUnited States AaronBauman Philadelphia
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update about 2 years ago
    29,203 pass, 2 fail
  • πŸ‡ΊπŸ‡ΈUnited States jrb Raleigh-Durham Area, NC, USA

    The usual fixes for this type of problem are done by adding something like this to a template file:

    {% if title %}
      <h4{{ title_attributes.addClass(title_classes) }}>{{ title }}</h4>
    {% endif %}
    {% if description_display == 'before' and description %}
      <div{{ description_attributes.addClass(description_classes) }}>
        {{ description }}
      </div>
    {% endif %}
    

    But that won't work here because the markup for the description text needs to be added inside a th in the table. For this, we'd need to update template_preprocess_field_multiple_value_form() in core's theme.incand edit the code that creates the table header.

    The diff would look something like this:

    diff --git a/core/includes/theme.inc b/core/includes/theme.inc
    index fbf78cf66b..ddb97d05a5 100644
    --- a/core/includes/theme.inc
    +++ b/core/includes/theme.inc
    @@ -1603,14 +1603,38 @@ function template_preprocess_field_multiple_value_form(&$variables) {
           $header_attributes['class'][] = 'js-form-required';
           $header_attributes['class'][] = 'form-required';
         }
    +
    +    $header_label = [
    +      'label' => [
    +        '#type' => 'html_tag',
    +        '#tag' => 'h4',
    +        '#value' => $element['#title'],
    +        '#attributes' => $header_attributes,
    +      ],
    +    ];
    +
    +    $description_id = NULL;
    +    if (!empty($element['#description'])) {
    +      $description_id = $element['#attributes']['aria-describedby'];
    +      $description_attributes['id'] = $description_id;
    +      $variables['description']['attributes'] = new Attribute($description_attributes);
    +      if ($element['#description_display'] == 'before') {
    +        $variables['description']['attributes']->addClass('description');
    +        $header_label['description'] = [
    +          '#type' => 'html_tag',
    +          '#tag' => 'div',
    +          '#value' => $element['#description'],
    +          '#attributes' => $variables['description']['attributes'],
    +        ];
    +      }
    +      else {
    +        $variables['description']['content'] = $element['#description'];
    +      }
    +    }
    +
         $header = [
           [
    -        'data' => [
    -          '#type' => 'html_tag',
    -          '#tag' => 'h4',
    -          '#value' => $element['#title'],
    -          '#attributes' => $header_attributes,
    -        ],
    +        'data' => $header_label,
             'colspan' => 2,
             'class' => ['field-label'],
           ],
    @@ -1678,14 +1702,9 @@ function template_preprocess_field_multiple_value_form(&$variables) {
           ],
         ];
    
    -    if (!empty($element['#description'])) {
    -      $description_id = $element['#attributes']['aria-describedby'];
    -      $description_attributes['id'] = $description_id;
    -      $variables['description']['attributes'] = new Attribute($description_attributes);
    -      $variables['description']['content'] = $element['#description'];
    -
    +    if ($description_id) {
           // Add the description's id to the table aria attributes.
    -      $variables['table']['#attributes']['aria-describedby'] = $element['#attributes']['aria-describedby'];
    +      $variables['table']['#attributes']['aria-describedby'] = $description_id;
         }
       }
       else {
    

    But, in order for that to look right, CSS would be need to be updated, too. Without that, the description text would end up being bold, too, because it's in a table header. Something like this would need to be added to themes:

    .field-multiple-table th.field-label div.description {
      font-weight: normal;
    }
    

    Also, Claro and other themes generally add their own class to the description text to format it so it's smaller. Each theme that needed that would have to implement a hook like this:

    /**
     * Implements hook_preprocess_HOOK().
     */
    function claro_preprocess_field_multiple_value_form(&$variables) {
      if (!empty($variables['table']['#header'][0]['data']['description'])) {
        $variables['table']['#header'][0]['data']['description']['#attributes']['class'][] = 'form-item__description';
      }
    }
    

    I'm also not quite sure where $element['#description_display'] = 'before' could be set since template_preprocess_field_multiple_value_form() runs so early.

    Does this seem like the right path to go down? If so, I can created a patch / MR that does this.

    There's also the Top Description β†’ module that I created that handles this in its own implementation of hook_preprocess_field_multiple_value_form().

Production build 0.71.5 2024