Problem/Motivation

While working on #3160238: Media Library widget produces "This value should not be null" error when field is required, we noticed that a fieldset containing an error, is not highlighted.

Steps to reproduce

Create a form, add a fieldset and in that fieldset, add a required field. Submit the form without values.
The fieldset is not marked as required.

Proposed resolution

When a fieldset contains an error, add an error class and mark the fieldset with a red border.

Remaining tasks

Decide if this should be provided specifally for the media library case, or a general approach is necessary.

User interface changes

A fieldset containing an error will be marked with a red border.

CommentFileSizeAuthor
#12 3222368-nr-bot.txt149 bytesneeds-review-queue-bot

Issue fork drupal-3222368

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

JeroenT created an issue. See original summary.

JeroenT’s picture

amanire’s picture

This is a good idea! I had to do this w/custom code on a project using the jQuery validation plugin. Without it, content contributors may become exasperated, having no idea which field in a submission is failing validation.

phenaproxima’s picture

Yeah, +1 for this. To be clear, I don't think that it's something that should automatically be applied to any fieldset which contains an error. But I think it's something that fieldsets should be able to opt into if needed.

JeroenT’s picture

Issue summary: View changes

saranyamariappan made their first commit to this issue’s fork.

JeroenT’s picture

Status: Active » Needs review

I added a red border for a fieldset containing errors in every theme.

About an option to opt-in, the error class is added in \Drupal\Core\Render\Element\RenderElement::setAttributes(), so I was thinking about an option '#skip_error_highlighting'.

For fieldsets, we can set this by default to TRUE like this:

  public function getInfo() {
    $class = static::class;
    return [
      '#process' => [
        [$class, 'processGroup'],
        [$class, 'processAjaxForm'],
      ],
      '#pre_render' => [
        [$class, 'preRenderGroup'],
      ],
      '#value' => NULL,
      '#theme_wrappers' => ['fieldset'],
      '#skip_error_highlighting' => TRUE,
    ];
  }

In setAttributes we can add an extra check if the option was set and equals true, then skip adding the error class.

For media_library, #3160238: Media Library widget produces "This value should not be null" error when field is required we can enable the option in MediaLibraryWidget. But I think we can fix that in a separate issue.

The solution abovemakes it possible to skip the error highlighting for every kind of form element. But I'm not sure if there really is a usecase for that.

Any thoughts?

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

needs-review-queue-bot’s picture

Status: Needs review » Needs work
FileSize
149 bytes

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Greg Boggs’s picture

I gave this patch a test. For our use, this current patch is not sufficient because a required field must be visible on the screen or the error message does not display. Even with this patch, in a closed details element the message still not visible. So, you can't see the html5 error message about the field being required.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.