Contrib.social

Constraints on paragraph field highlight all fields in paragraph

Open on Drupal.org →
Created on 26 December 2018, almost 6 years ago
Updated 6 August 2024, 4 months ago

Problem/Motivation

I recently attempted to add a constraint on a paragraph field so that a user could not select the same dropdown value twice. The constraint works but highlights the whole paragraph item rather than the field the error is on. This is misleading and possibly confusing for the end user.

Here is the content setup:

Content Type

  • Entity Reference field to paragraph

Paragraph

  • text list
  • Entity reference to content

Actual:

Expected:

I found that field error handling is passed up to the widget and that InlineParagraphsWidget and ParagraphsWidget handle errorElement differently. For my use case, I want to drill down to the actual field causing the error so I started using "Paragraphs EXPERIMENTAL". While testing, I came to the understanding that FormState::setError expects the #parents key of the element to contain the correct property path to the element. Quick tangent, what constraints consider property paths and formState considers $name are identical except property paths expect "." to separate the field names and $name expects "][".

property path: field_quote_detail_page_componen.1.subform.field_component_name_wrapper
name: field_quote_detail_page_componen][1][subform][field_component_name_wrapper

Here is where the problem exists, (WidgetBase::flagErrors()):

foreach ($delta_violations as $violation) {
            // @todo: Pass $violation->arrayPropertyPath as property path.
            $error_element = $this->errorElement($delta_element, $violation, $form, $form_state);
            if ($error_element !== FALSE) {
              $form_state->setError($error_element, $violation->getMessage());
            }
          }

$this->errorElement will call ParagraphsWidget::errorElement which in terns calls NestedArray::getValue($element, $error->arrayPropertyPath). The returned element, $error_element, will have a parent key as follows:

result = {array} [4]
 0 = "field_quote_detail_page_componen"
 1 = 1
 2 = "subform"
 3 = "field_component_name_wrapper"

When FormState::setError is called it uses the above array to build at the $name arg for FormState::setErrorByName, field_quote_detail_page_componen][1][subform][field_component_name_wrapper. While we cannot assign an error to a wrapper element the error message will show but there is no error styling to alert the user where the error is.

Proposed resolution

Escalate the todo in WidgetBase::flagErrors():

// @todo: Pass $violation->arrayPropertyPath as property path.

Come up with workaround

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

🐛 Bug report
Status

Postponed: needs info

Version

1.0

Component

Code

Created by

🇺🇸United States robpowell Boston

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.

  • Comment over 1 year ago →
    🇨🇭Switzerland tcrawford

    ParagraphsWidget::errorElement() attempts to get the sub element when the path is valid. It currently checks for empty($this->arrayPropertyPath) where arrayPropertyPath is got via magic on Drupal\Core\Field\InternalViolation, but InternalViolation does not implement __isset(). Therefore the code on errorElement never returns $sub_element, even if there is a valid sub element found by NestedArray::getValue(). We could open an issue in core for InternalViolation to implement __isset(), but likely that won't move forward as accessing arrayPropertyPath via magic is deprecated. Therefore, I would propose the following solution (patch to follow):

    ?>
/**
   * {@inheritdoc}
   */
  public function errorElement(array $element, ConstraintViolationInterface $error, array $form, FormStateInterface $form_state) {
    // Validation errors might be a about a specific (behavior) form element
    // attempt to find a matching element.
    // A temporary variable needed as arrayPropertyPath is accessed via magic
    // and Drupal\Core\Field\InternalViolation does not implement __isset.
    // Therefore, empty($error->arrayPropertyPath) returns true even when the
    // magic getter returns a value.
    $arrayPropertyPath = $error->arrayPropertyPath;
    if (!empty($arrayPropertyPath) && $sub_element = NestedArray::getValue($element, $error->arrayPropertyPath)) {
      return $sub_element;
    }
    return $element;
  }
  • Comment over 1 year ago →
    🇨🇭Switzerland tcrawford

    Patch attached.

  • Status changed to Needs review over 1 year ago
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update over 1 year ago
    180 pass
  • Comment over 1 year ago →
    🇨🇭Switzerland tcrawford
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.1 & MariaDB 10.3.22
    last update over 1 year ago
    159 pass, 12 fail
  • Comment about 1 year ago →
    🇧🇪Belgium herved

    Hi @robpowell, @tcrawford,

    It would be great if you could check out the patch in 📌 Stop using $error->arrayPropertyPath Needs review which should fix both issues (this one + the deprecation of arrayPropertyPath), so we could close this as duplicate.

    Thanks

  • Status changed to Postponed: needs info 4 months ago
  • Comment 4 months ago →
    🇨🇭Switzerland berdir Switzerland

    The referenced issue is now committed. Could still use this to add test coverage.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024