#states not working correctly when built from a logical combination of multliple fields

Hi Ashley George,
Please use this type of coding after working for me visible functionality .
shared coding format:

$form['select1'] = [
'#type' => 'select',
'#title' => 'Select 1',
'#options' => [0 => 0, 1 => 1],
'#default_value' => 0,
];

$form['select2'] = [
'#type' => 'select',
'#title' => 'Select 2',
'#options' => [0 => 0, 1 => 1],
'#default_value' => 0,
];

$form['select3'] = [
'#type' => 'select',
'#title' => 'Select 3',
'#options' => [0 => 0, 1 => 1],
'#states' => [
'visible' => [
[':input[name="select1"]' => ['value' => 0]],
'and',
[':input[name="select2"]' => ['value' => 1]],
],
],
];

@Ashley George does that answer your question? If so this can be closed.

@smustgrave unfortunately this doesn't seem to resolve things. After implementing the change suggested by @shiv_yadav the visibility on "select3" is now only linked to "select1". So I beleive there is still a valid issue here.

Actually I'm not quite right with my description of how it's behaving now, but it's still not correct. If someone could see if they can replicate that would be awesome.

I can confirm the issue you are seeing. I'm not entirely sure the cause.

At first I thought maybe states wasn't picking up the default value from select1 but that kinda is disproved by switching select2 to 0 and 1 again. Which triggered select3

Believe this is a valid bug, moving to Active for a patch/tests.

Modules like big_pipe invoke attachBehaviors that results in multiple attachment of behaviors for a given context. So maintain semaphore to ensure that the behavior is attached only once.

If the attachBehavior is called multiple times, then in states multiple states.Dependent objects are created for the same elements. However the initialise is not called in Trigger function for all the states.Dependent object since it checks if it was already initialised for an element. Therefore it's best to avoid multiple invocation of attachBehaviors to ensure that the behviors work as intended.

This fix ensures that attachBehavior is called only once for a given context

Will need a test to show the bug.

Also issue summary appears incomplete, recommend using standard issue template

phthlaap → changed the visibility of the branch 3386191-states-not-working to hidden.

phthlaap → changed the visibility of the branch 3386191-states-not-working to active.

phthlaap → changed the visibility of the branch 3386191-9x-states-not-working to hidden.

@phthlaap the fix that you have provided is very specific to #states. The duplicates loading of attachBehavior can cause problems for any other module or custom module. So it's best to fix the root cause rather than in the affected module. Enclosed in a patch with tests for the same using #states for testing.

If a different approach is being proposed can a separate MR be opened for reviewing. Issue summary will eventually have to be updated to match.

The approach in patch #26 will affect some tests because I believe Ajax should reattach on form submission once again.

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.

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.

phthlaap → changed the visibility of the branch 3386191-states-not-working-drupaljs to hidden.

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.

phthlaap → closed merge request !7100

As suggestion of @nod_ , I use Once library to fix the issue. I think it is better.
I also changed the variable name $states because it is no longer a jQuery instance. but, it will be identical to the state variable defined at the top of the file, so I named it element.

I have tested the last changes and I can confirm it's working.
I'm also adding a patch for the 10.1.6 core version.

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.

I fixed this issue modifying only the $states variable to

const $states = once('states', '[data-drupal-states]', context);

@ozie can you try with tests though. Would have to be an MR

@smustgrave

The merge request above are already using same solution once library and also have a tests https://git.drupalcode.org/project/drupal/-/merge_requests/7045/diffs

MR also has additional changes besides that one line

As comment #40 I has explained, the variable name $states is no longer a jQuery instance. but, it will be identical to the state variable defined at the top of the file, so I named it element.

Removing the 2nd approach to avoid confusion

1) Drupal\FunctionalJavascriptTests\Core\Form\JavascriptStatesTest::testJavascriptStates
Failed asserting that false is true.
/builds/issue/drupal-3386191/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:121
/builds/issue/drupal-3386191/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:55
/builds/issue/drupal-3386191/core/tests/Drupal/FunctionalJavascriptTests/Core/Form/JavascriptStatesTest.php:454
/builds/issue/drupal-3386191/core/tests/Drupal/FunctionalJavascriptTests/Core/Form/JavascriptStatesTest.php:71
/builds/issue/drupal-3386191/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
FAILURES!
Tests: 1, Assertions: 203, Failures: 1.

Shows test coverage

Believe the change makes sense using once()

Hi folks
Can we get an issue summary update that details why using once is the solution here.
Its not evident to me why that makes things different or why bigpipe is part of the issue.
Fine to self RTBC afterwards.

Thanks

@Iarowllan the analysis was provided in #14. Copying the same below for convenience.

Modules like big_pipe invoke attachBehaviors that results in multiple attachment of behaviors for a given context. So maintain semaphore to ensure that the behavior is attached only once.

If the attachBehavior is called multiple times, then in states multiple states.Dependent objects are created for the same elements. However the initialise is not called in Trigger function for all the states.Dependent object since it checks if it was already initialised for an element. Therefore it's best to avoid multiple invocation of attachBehaviors to ensure that the behviors work as intended.