#states disable property has stopped working for submit button and other elements

Created a quick MR to update the element types that have disabled property set.

Definitely needs tests.

May even be worth a change record so others know they can disable those elements.

For the tests this needs, see:

core/tests/Drupal/FunctionalJavascriptTests/Core/Form/JavascriptStatesTest.php
core/modules/system/tests/modules/form_test/src/Form/JavascriptStatesForm.php

Feel free to ask me (here or Slack) if anyone needs help figuring out how to extend that test.

Thanks!
-Derek

Uploading patch for the current MR.

Thanks for the patch, everything works.
But it didn't apply to the core/misc/states.js file, I downloaded the patch and changed the path manually to the core/misc/states.es6.js

Rerolled #8 for Drupal 9.5.8.

Same problem here with Drupal 9.5.9. Patch #10 solved the problem.

Patch from #10 works for me

Needs reroll for 11.x first before we can consider backporting. Also as stated in #2 and #5 this could do with additional test coverage.

Reroll for 11.x as per #15.

Worked for me on Drupal 9.5.10, thank you

Thank you for the patch it seems working for drupal 10.1.x but as per project requirements I have added one more element "submit" in patch file.

🧐 <submit> isn't a valid HTML element?

https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/disabled

hi @devsoni,

I applied your patch #20 in our project where drupal version is 9.5.10 and its working fine too.

Thanks

Patch #20 cannot be applied to Drupal 10.1.5.

Patch #16 can be applied and fixes the issue.

This patch is an updated version of #16 that also fixed an issue where e.value was undefined when the #states rules are no longer being met, and you are trying to make things enabled. This undefined-ness was leading to different behavior between toggleClass() and prop(), which was preventing the disabled prop not being set when it should have been, even though the form-disabled class was being set.

This very much feels like fixing the symptom and not the cause, but the cause is seemingly buried in the verifyConstraints/checkConstraints code which I unfortunately don't have time to dig into.

#23 didn't work for action submit element, but the MR did.

#16 works for me, #23 doesn't work with checkboxes (at least)

I've created a merge request for version 11.x and added test coverage.

phthlaap → changed the visibility of the branch 3354998-states-disable-elements to hidden.

Could it be documented what element types were discovered not working, IS mentions button but MR has other elements it's testing.

Adding D10.1 compatible patch based on latest MR.

This issue happen with submit button

$form['actions']['submit'] = [ 
  '#type' => 'submit',
  '#value' => $this->t('Submit'),
  '#states' => [
    'disabled' => [':input[name="allow_overwrite"]' => ['checked' => FALSE]],
  ],  
];

Issue summary and title should be updated to reflect that.

The issue summary and solution should line up.

Add additional HTML elements to the find() parameter.

What additional elements were added?

Updated issue summary. I am having this problem for submit buttons.

Tests appear to be added so removing that tag

D9 is done so removing the backport tag.

Looking at the MR though it doesn't match what's proposed in the summary. Can the MR or summary be updated please

Left a small nitpicky but moving to NW for the summary.

I have updated the merge request so that .find() includes all elements that support the disabled attribute. This means that the attributes added match what the summary says need to be added.

I note that the summary does not mention adding the call to .addBack(). Perhaps @phthlaap can add documentation about that.

select, textarea, input are already in the .find(). That list is just the ones being added.

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

How can the add back was needed?

I looked in more detail at the history. .addBack() was in the patch in comment 8 by @jedihe and may be been there when the merge request was first opened in comment 3 by @gapple.

Tested without addBack(tagsSupportDisable), the issue wasn't fixed. This change has been applied since patch #31.

I haven't checked the latest code in the merge request, but if IIRC, the addBack() was necessary because while in most cases the affected element has a wrapper with one of the relevant .js-form-item, .js-form-submit, .js-form-wrapper classes and the find() call will return the proper child node, there are some cases where the class is applied to the element itself.
While closest() will return the element it is called on if it matches the selector, find() does not. addBack() will append the element that was returned by closest() if it matches the list of element types.
Only one of find() or addBack() should return an element.

It should have the same result as this, without needing a variable or conditional block:

elem = eventTarget.closest(wrapperClasses);
if (!elem.is(tagsSupportDisable)) {
  elem = elem.find(tagsSupportDisable)
}

@jedihe was the one who added the addBack() in #7 / #8

https://git.drupalcode.org/project/drupal/-/merge_requests/3844/diffs?co...

Hiding patches for clarity

Ran test-only feature

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

Which shows test coverage

Issue summary appears to have been updated so removing that tag.

Thanks everyone for the digging into the addBack. Seems fine to me now

nod_ → closed merge request !5694

Committed 3d3b2c1 and pushed to 11.x. Thanks!

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

Darren Oh → made their first commit to this issue’s fork.