Add support to require Select or Other fields dynamically

Created on 24 April 2023, over 1 year ago
Updated 2 May 2023, over 1 year ago

Problem/Motivation

Right now the only way to reliably require a Select or Other field is to do so from the Field Configuration.

If one wanted to dynamically require a Select or Other field within a form-alter, the select list (or whatever the widget is configured to) will fail validation if empty, but if someone chooses Other and leaves the Other text field empty, then it will pass validation with the empty value.

The main reason this is occurring is right now, the only validation that is validating the Other text element is AllowedValuesConstraintValidator which will only check if the field configuration has it marked as required and does not care about the form array.

This widget should make the Other textfield be required if the field is required and Other has been selected.

Steps to reproduce

  1. Create a Select or Other field/widget.
  2. Add a form alter hook for the entity form.
  3. Configure the form alter hook to require the field $form['field_my_field_name']['widget']['#required'] = TRUE;
  4. Visit the form.
  5. Submit the form without the Select or Other field populated.
  6. We should get a JS validation error about the select list being empty.
  7. Choose Other and leave the Other text field empty.
  8. Submit the form.
  9. The form is submitted with no errors and no value populated in the Select or Other field.

Proposed resolution

We should have validation that occurs when the field is required dynamically.

Preferably this work with/without JS validation enabled ($form['#attributes']['novalidate'] = 'novalidate';)

🐛 Bug report
Status

Needs review

Version

4.0

Component

Code

Created by

🇨🇦Canada mdolnik

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

Comments & Activities

  • Issue created by @mdolnik
  • 🇨🇦Canada mdolnik

    Added a patch to resolve this issue.

    In summary this adds two parts to add validation:

    1. The signature of ElementBase::prepareState() has been updated to accept an array of $states instead of a single string:

    - This ensures that callers of this method can now provide not just the visible state, but visible,required states if the field is required.
    - This causes the Other text element to be required via JS validation whenever its visible AND required.

    2. A new #element_validate callback has been added ElementBase::validateOther()

    - This ensures that if JS Validation is disabled (or somehow bypassed) that the validation will be performed server-side.

    I have manually tested this on:

    - Single-value select lists
    - Single-value buttons (radios)
    - Multi-value buttons (checkboxes)

    With all of the above testing with JS Validation enabled/disabled.

    One caveat is if JS Validation is disabled, the logic in ElementBase::validateOther() does not work for Radios. This is related to and caused by the issue that #3171843 🐛 Multiple-value checkboxes do not render existing values Active is trying to fix and its #6 patch will resolve this. (Radios will still successfully work with the JS Validation with/without #3171843 being resolved)

  • 🇨🇦Canada mdolnik

    This is the same patch file as above, but can be applied to version 4.0.0

  • 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
    12 pass, 4 fail
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update over 1 year ago
    21 pass
  • 🇨🇦Canada mdolnik

    Added a new patch which removes the changes to ElementBase::prepareState().

    While this helped resolve the issues with the logic in ElementBase::validateOther() for Radios, it ended up complicating things when the fields as a whole are required via states by outside logic.

    Removing the modified states code also prevents the need to update relevant tests.

    Since the issue with ElementBase::validateOther() for Radios is caused by the issue that #3171843 is trying to fix, it should be considered to be a requirement for this task to be completed.

Production build 0.71.5 2024