States API doesn't work with multiple select fields

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.

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

#144 doesn't work on 9.4.11

#142 does work for me in 9.5.3 with php 7.4

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

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.

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.

added updated patch address #149

try to fixed CCF #150

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.

Addressed #149, attached interdiff for same. please review

Fixed build issue.

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

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

Switching to MR.

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

Improve patch based on #160 comments

By the way, it works fine for me.

Seems tests have been lost.

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.

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

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

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

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"?

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.

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.

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

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.

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

@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 :).

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

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.

Is there any solution for 10.1.5 ?

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

could you, please, check?
thanks

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

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

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.

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.

Assigning to javascript submaintainer.

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

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.

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

There is a todo left in the tests code

nod_ → closed merge request !5188

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

whoot, thanks!

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