States API doesn't work with multiple select fields

Created on 6 May 2011, over 13 years ago
Updated 24 July 2024, 4 months ago

Problem/Motivation

The States API won't pick up the value of a select field with #multiple = TRUE option.

Steps to reproduce:
- Create two fields in a form with the following code:

  $form['dependee'] = array(
    '#type' => 'select',
    '#options' => array(
      'a' => 'Option A',
      'b' => 'Option B',
      'c' => 'Option C',
    ),
    '#multiple' => TRUE,
  );

  $form['dependent'] = array(
    '#type' => 'textfield',
    '#states' => array(
      'visible' => array(
        'select[name="dependee[]"]' => array('value' => array('a')),
      ),
    ),
  );

The dependent field will stay hidden regardless of the value of the dependee. This happens because the value of a multiple select field is an array, and States tries to compare it with the reference with a === operator, which returns always false.

Proposed resolution

Add a handler for arrays in states.Dependent.comparisons. This works with ANDed values:

'select[name="dependee[]"]' => array('value' => array('a', 'b')),

and with ORs as well (following the syntax proposed in 🐛 FAPI #states: Fix conditionals to allow OR and XOR constructions Fixed ):

'select[name="dependee[]"]' => array(array('value' => array('a')), array('value' => array('c')))

Remaining tasks

  1. Land #2702233: [backport] Add JavaScript tests for Form API #states: required, visible, invisible, expanded, checked, unchecked so we have somewhere to put tests for this.
  2. Expand that test to cover this case.
  3. Upload test-only vs. full patch to verify test and fix.
  4. Further reviews/refinements.
  5. RTBC.
  6. Commit to D9/D8.
  7. Open follow-up issue to backport to D7.

User interface changes

This feature finally works as intended:

Before

After

API changes

N/A

Data model changes

N/A

Release notes snippet

TBD.

🐛 Bug report
Status

Fixed

Version

10.2

Component
Javascript 

Last updated 1 day ago

Created by

🇮🇹Italy peterpoe

Live updates comments and jobs are added and updated live.
  • Needs backport to D7

    After being applied to the 8.x branch, it should be considered for backport to the 7.x branch. Note: This tag should generally remain even after the backport has been written, approved, and committed.

  • Contributed project blocker

    It denotes an issue that prevents porting of a contributed project to the stable version of Drupal due to missing APIs, regressions, and so on.

  • Needs subsystem maintainer review

    It is used to alert the maintainer(s) of a particular core subsystem that an issue significantly impacts their subsystem, and their signoff is needed (see the governance policy draft for more information). Also, if you use this tag, make sure the issue component is set to the correct subsystem. If an issue significantly impacts more than one subsystem, use needs framework manager review instead.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

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

  • 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.

  • 🇮🇳India _utsavsharma

    Tried to fix CCF for 9.5.x in #142.

  • 🇪🇸Spain abelass

    #144 doesn't work on 9.4.11

  • 🇦🇷Argentina abelpzl

    #142 does work for me in 9.5.3 with php 7.4

  • Status changed to Needs review over 1 year ago
  • 🇮🇳India gauravvvv Delhi, India

    I have attached patch for 10.1.x. none of the above patch is applying to 10.1.x. So not attaching interdiff. please review

  • 🇺🇦Ukraine ysamoylenko

    Confirming that https://www.drupal.org/project/drupal/issues/1149078#comment-14271390 🐛 States API doesn't work with multiple select fields Needs review works correctly with empty values on 9.5.3.

  • Status changed to Needs work over 1 year ago
  • 🇫🇷France nod_ Lille

    I don't have a good idea about what should be the expected behavior, and I don't have the time to get into it this weekend. One the other hand, one small review:

    1. +++ b/core/misc/states.js
      @@ -159,6 +159,24 @@
      +      Object.entries(reference).forEach(([key, referenceValue]) => {
      

      key is never used, Object.values() would be more appropriate than entries.

    2. +++ b/core/misc/states.js
      @@ -159,6 +159,24 @@
      +        if (value.indexOf(referenceValue) === -1) {
      +          return false;
      +        }
      +      });
      +      return true;
      

      This could be simplified by using includes()

    Also not sure what that loop is doing, returing true or false inside the foreach does not do anything. I see #142 was using a for loop, in that case returning true/false would make sense. Not with forEach().

    I would definitely expect some tests cases here just to make sure we agree on the behavior.

  • 🇮🇳India Akram Khan Cuttack, Odisha

    added updated patch address #149

  • Status changed to Needs review over 1 year ago
  • 🇮🇳India Akram Khan Cuttack, Odisha

    try to fixed CCF #150

  • Status changed to Needs work over 1 year ago
  • The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

    This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

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

  • Status changed to Needs review over 1 year ago
  • 🇮🇳India gauravvvv Delhi, India

    Addressed #149, attached interdiff for same. please review

  • 🇮🇳India gauravvvv Delhi, India

    Fixed build issue.

  • 🇮🇳India gauravvvv Delhi, India

    Wrong patch uploaded in #153. Updated the patch in #154.

  • 🇮🇳India saurabh-2k17

    #144 did not work in my case. I later used #142 which fixed the problem. My Drupal Core version is 9.5.7

  • First commit to issue fork.
  • @vladimiraus opened merge request.
  • 🇦🇺Australia VladimirAus Brisbane, Australia

    Switching to MR.

  • 🇳🇴Norway neslee canil pinto India

    @VladimirAus Object.values will give referenceValue undefined, inside if we use Object.entries it will work fine with key, value pair

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    29,283 pass
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    29,366 pass
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    29,374 pass
  • 🇫🇷France arnaud-brugnon

    Improve patch based on #160 comments

    By the way, it works fine for me.

  • Status changed to Needs work over 1 year ago
  • 🇺🇸United States smustgrave

    Seems tests have been lost.

  • First commit to issue fork.
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    29,420 pass
  • Status changed to Needs review over 1 year ago
  • 🇷🇴Romania vasike Ramnicu Valcea

    Updated the MR - trying to add the tests from #132

    So:
    - Add tests
    - Change compare - The arrays values should match - it should work also for empty arrays.
    - Fix a test ... i think the OR check there was not right if value2 OR value3 doesn't mean if should be displayed when both value2 and value3 selected.
    For OR case we should something if (value2 and value3) OR (value7 and value32)

    Note: i only used tests to check the "solution" ... i didn't test manually.

  • Status changed to Needs work over 1 year ago
  • 🇺🇸United States dww

    Thanks, great progress!

    About out the door for a few AFK meetings, but I'll try to look more closely when I'm back. Super quick skim finds some nits with the tests (wrong comments, etc). Also, the other #states tests in these files are now starting to do:

    1. Setup initial state + validate
    2. Trigger the change
    3. Check all the changes
    4. Revert the trigger change
    5. Re-verify the initial state is back

    The test in the MR is currently stopping at step 3 (like many, but not all of the other protected helpers)...

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    Custom Commands Failed
  • 🇺🇸United States dww

    Ugh, that's super annoying. 😢 I worked on this locally. To my great surprise, 2 of the "initial state" assertions are failing after I remove the values in the multi-select trigger:

    1. $this->assertTrue($item_visible_no_value->isVisible());
    2. $this->assertFalse($textfield_visible_value2_and_value3->isVisible());

    If I comment those out, the test passes. If either are still present, the test fails. However, when I inspect the HTML output of the failing page, I see it looks like it's working as expected:

    The multi-select indeed has no values, and the only thing visible in the section of tested elements is what $item_visible_no_value points to. The rest of the test is working fine. Manually testing the multi-select trigger from this HTML result page and all the JS seems to be working as expected, too.

    I'm not sure this is actually a bug in the states code -- it could just be a bug in the test. But I'm out of time for today to keep digging.

    Sorry,
    -Derek

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    29,428 pass, 1 fail
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    29,429 pass
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    29,429 pass
  • 🇺🇸United States dww

    Grr, phpcs didn't like how I commented out the failing assertions. 😢

    I fixed that and pushed a commit with the failing assertions included. That run should fail.

    Pushed another where I properly commented out. core/scripts/dev/commit-code-check.sh is now passing locally. 😅

    Also rebased to the latest 10.1.x branch. We probably need to change the MR to point to 11.x for now. However, I just verified that the MR plain diff applies cleanly to the 11.x branch, too (as expected), so I'm not going to mess with trying to get the MR target branch changed (which I don't have permissions to do).

  • 🇷🇴Romania vasike Ramnicu Valcea

    i confirm that the js is working properly ... but it seems the $trigger->setValue([]); doesn't work properly.

    So what should we do ... find another way to "deselect" the multiple selection?

    What about the chasing this "odd case" in a separate issue.
    And try to commit the "current solution"?

  • 🇺🇸United States dww

    Yeah, I hate holding up a fix in the quest for "perfect" tests. Definitely open to moving the last bits to a follow-up...

    However, in more closely inspecting the HTML output of the tested page, the initial state's markup for the item looks like this:

    <div data-drupal-states="{&quot;visible&quot;:{&quot;select[name=\u0022multiple_select_trigger[]\u0022]&quot;:{&quot;value&quot;:[]}}}" id="edit-item-visible-when-multiple-select-trigger-has-no-value" class="js-form-item form-item js-form-type-item form-item-item-visible-when-multiple-select-trigger-has-no-value js-form-item-item-visible-when-multiple-select-trigger-has-no-value">
          <label for="edit-item-visible-when-multiple-select-trigger-has-no-value">Item visible when multiple select trigger has no value</label>
            
            </div>
    

    After the test fails, the final output page has this for the element:

    <div data-drupal-states="{&quot;visible&quot;:{&quot;select[name=\u0022multiple_select_trigger[]\u0022]&quot;:{&quot;value&quot;:[]}}}" id="edit-item-visible-when-multiple-select-trigger-has-no-value" class="js-form-item form-item js-form-type-item form-item-item-visible-when-multiple-select-trigger-has-no-value js-form-item-item-visible-when-multiple-select-trigger-has-no-value" style="">
          <label for="edit-item-visible-when-multiple-select-trigger-has-no-value">Item visible when multiple select trigger has no value</label>
            
            </div>
    

    The only difference is that the one that's failing the test assertion includes this: style="". That shouldn't matter, but maybe it does?

    I'll also note that while inspecting the HTML on the failed test output, I'm getting 5 of these:

    Incorrect use of <label for=FORM_ELEMENT>
    
    The label's for attribute doesn't match any element id. This might prevent the browser from correctly autofilling the form and accessibility tools from working correctly.
    
    To fix this issue, make sure the label's for attribute references the correct id of a form field.
    

    All 5 of the affected nodes are the item elements. That seems completely unrelated to this issue or bug, but wanted to note it here. Probably we need a follow-up to investigate this part, too.

  • Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    Waiting for branch to pass
  • Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    Waiting for branch to pass
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    29,430 pass
  • Status changed to Needs review over 1 year ago
  • 🇺🇸United States dww

    Thanks for the review and fixes. Almost there! I opened 📌 Get all assertions working in JavascriptStatesTest::doMultipleSelectTriggerTests() Active as the follow-up to sort out the final test weirdness, and pushed a commit to add the @see comment pointing to it as part of the @todo.

    I believe all feedback is now addressed, so this is ready for (hopefully final?) review. Removing the "Needs tests" tag, since that's now done.

    Thanks again!
    -Derek

    p.s. Before commit, this issue definitely needs a review of issue credits. There are a bunch of 1-off patches in here that probably shouldn't get credits, and a couple of folks who commented a lot but haven't uploaded anything who might need to be credited. If I have a chance later today, I'll work on this, but if I don't get to it, someone needs to before this is truly ready.

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    30,341 pass
  • 🇺🇸United States DamienMcKenna NH, USA

    The latest merge request in patch format for 9.5.x.

  • Status changed to Needs work over 1 year ago
  • The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

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

  • Status changed to Needs review over 1 year ago
  • Slightly confused here, I am looking for a solution for the scenario 4 in this comment
    https://www.drupal.org/project/drupal/issues/1149078#comment-14590562 🐛 States API doesn't work with multiple select fields Needs review

    'select[name="dependee[]"]'=> [['value' => ['a']], ['value' => ['b']]],
    It will only show the field if I select EXACTLY A or EXACTLY B (if I select A and B it won't show the field)

    I need the field to be displayed if I select A or B . I applied the latest patch and still see the issue. Has this issue been addressed in the latest patch/ am i missing something or is it going to be addressed separately?

    Thanks,
    Sukanya

  • 🇷🇴Romania vasike Ramnicu Valcea

    @sukanya.ramakrishnan maybe at first sight
    For
    'select[name="dependee[]"]'=> [['value' => ['a']], ['value' => ['b']]],
    We expect to work also for both a and b selected
    But it's not true ... because it's about the value(s) of the element.

    And actually we have:
    [a,b] !== [a] and [a,b] !== [b]

    Which means that maybe you need something different like
    'select[name="dependee[]"]'=> [['value' => ['a']], ['value' => ['b'], ['value' => ['a', 'b']]],
    To cover all the Multiple element value(s) scenarios.

    P.S. remember, we're talking about MULTIPLE and we're using SET of Values ... not Parts of Values

    i hope i got it right :).

  • Status changed to Needs work about 1 year ago
  • 🇺🇸United States smustgrave

    Still needs submaintainer review but @dww could the MR be updated for 11.x please?

  • 🇮🇳India mukhtarm

    Any idea on how to fix this issue, I applied the MR (https://git.drupalcode.org/project/drupal/-/merge_requests/3805/diffs) and still doesn't resolve the issue.

    for eg:

    $form['payment_methods'] = [
          '#type' => 'select',
          '#title' => $this->t('Select the payment methods'),
          '#options' => [
            'cvs' => $this->t('convenience stores'),
            'credit' => $this->t('credit card'),
            'payeasy' => $this->t('Pay-easy'),
          ],
          '#multiple' => TRUE,
          '#required' => TRUE,
          '#attributes' => [
            'id' => 'payment_methods',
            'name' => 'payment_methods',
          ],
        ];
    
        $form['cvs'] = array(
          '#type' => 'details',
          '#open' => TRUE,
          '#title' => t('Convenience Store Options'),
          '#description' => $this->t('<b>Convenience Store informations are taken only if the cvs payment method selected.</b>'),
          '#attributes' => [
            'id' => 'cvs_fieldset'
          ],
          '#states' => array(
            'visible' => array(
              ':input[name="configuration[link_type_plus][payment_methods][]"]' => ['value' => 'cvs'],
            ),
          )
        );

    Why i given the name so?: when inspected the element the name for the element displayed so. Also when giving name="payment_methods][]" also not working.

  • 🇪🇸Spain abelass

    Is there any solution for 10.1.5 ?

  • 🇷🇴Romania vasike Ramnicu Valcea

    @MukhtarM i think you're trying "wrong selector"
    ':input[name=..."
    when here it's about select
    'select[name=..."

    could you, please, check?
    thanks

  • Pipeline finished with Success
    about 1 year ago
    Total: 889s
    #41499
  • Status changed to Needs review about 1 year ago
  • 🇷🇴Romania vasike Ramnicu Valcea

    new MR for 11.x available ... all updated and "Green", so i think we're back to Needs Review

  • 🇮🇳India mukhtarm

    Thanks @vasike, The issue is fixed for me after applied the latest MR(https://git.drupalcode.org/project/drupal/-/compare/308e696f76fc33d6a5d8...).

    Exactly was in the section:

    Array(reference, value) {
          // Make sure value is an array.
          if (!Array.isArray(value)) {
            return false;
          }
          // Convert all comparisons to strings for indexOf to work with integers
          // comparing to strings.
          reference = reference.map(String);
          value = value.map(String);
          // We iterate through each value provided in the reference. If all of them
          // exist in value array, we return true. Otherwise return false.
          return Object.entries(reference).every(([key, referenceValue]) =>
            value.includes(referenceValue),
          );
        },

    Correct solution for me is:

       '#states' => array(
            'visible' => array(
              'select[name="configuration[link_type_plus][payment_methods][]"]' =>   [['value' => ['cvs']], ['value' => ['payeasy']]],
            ),
          )

    if that helps anybody in future. This will show and hide the fieldset when either 'cvs' or 'payeasy' is selected

  • 🇳🇱Netherlands ronaldtebrake

    Can confirm for us https://git.drupalcode.org/project/drupal/-/merge_requests/3805 also did the trick (for D10.x)
    I've attached a static patch of the current state of the MR for CI/CD purposes.

  • 🇫🇷France ericbellot

    For me, on Drupal 10.1.6, the patch # 184 does not work completely. I had to go back to version # 183 of states.js.

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update 12 months ago
    29,721 pass, 1 fail
  • Assigned to nod_
  • 🇺🇸United States smustgrave

    Assigning to javascript submaintainer.

  • Pipeline finished with Success
    11 months ago
    Total: 6329158s
    #41502
  • Issue was unassigned.
  • Status changed to RTBC 8 months ago
  • 🇫🇷France nod_ Lille

    Works for me, it's simple enough and does the job advertised. Thanks for the tests too!

  • Status changed to Needs work 8 months ago
  • The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

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

  • Status changed to RTBC 8 months ago
  • 🇷🇴Romania vasike Ramnicu Valcea

    it seems we need to hide the patch from #185
    back to previous Status ... RTBC

  • Status changed to Needs work 8 months ago
  • 🇫🇷France nod_ Lille

    There is a todo left in the tests code

  • Status changed to RTBC 8 months ago
  • 🇫🇷France nod_ Lille
    • nod_ committed f6be8531 on 11.x
      Issue #1149078 by vasike, wuinfo - Bill Wu, dww, Gauravvvv, Marios...
    • nod_ committed 5e5839ab on 10.3.x
      Issue #1149078 by vasike, wuinfo - Bill Wu, dww, Gauravvvv, Marios...
    • nod_ committed a44ba4db on 10.2.x
      Issue #1149078 by vasike, wuinfo - Bill Wu, dww, Gauravvvv, Marios...
  • Status changed to Fixed 8 months ago
  • 🇫🇷France nod_ Lille

    Thanks everyone for keeping this going for so long, it's finally in :)

    Committed f6be853 and pushed to 11.x. Thanks! Backported to 10.2.x and 10.3.x

  • Pipeline finished with Success
    8 months ago
    Total: 647s
    #129706
  • 🇺🇸United States dww

    whoot, thanks!

  • Automatically closed - issue fixed for 2 weeks with no activity.

  • Sorry, but the last version of Drupal 10.3.1 break all my "select" based conditional fields.
    My conditional fields works well on Drupal 10.2.6

    Here an extract from my form where the field_child is visible when the taxonomy term (tid:1234) is selected

    $form['field_child']['#states'] =  [
                'visible' => [
                    ['select[name="field_parent[]"]' => '1234',
                ],
            ];
    
  • I answer to myself. finally i've solved my issue by fixing 2 points:

    The first point was a mistake from my side : the "value" used in the definition must be a "string". In my code, i used an entity reference id defined as an integer.

    The second point was linked to my select field defined as a "multiple" one. To address this point it's necessery to alter the "compare" function of core/misc/states.js. I add this js library into my module

    /**
     * @file
     * Extends core/misc/states.js.
     */
     
    (function($) {
    
        //correction core : can't add a #states on a select with multiple = true
        function _compareCustom(a, b) {
            if (a !== "undefined" && typeof b === 'object' && b instanceof Array) {
                if (b.includes(a)) {
                  return true;
                }
            }
    
            if (a === b) {
                return typeof a === 'undefined' ? a : true;
            }
    
            return typeof a === 'undefined' || typeof b === 'undefined';
        }
    
        //hack : surcharge function compare from core/states.js to use _compareCustom instead of _compare2
        Drupal.states.Dependent.prototype.compare = function compare(reference, selector, state) {
            var value = this.values[selector][state.name];
            if (reference.constructor.name in Drupal.states.Dependent.comparisons) {
              return Drupal.states.Dependent.comparisons[reference.constructor.name](reference, value);
            }
    
            return _compareCustom(reference, value);
          };
    
    })(jQuery);
    

    This solution works with all drupal version from 8.x to 10.3.1.

Production build 0.71.5 2024