Ajax replace does not refocus element if inside a dialog

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.

I just wanted to make a note here that we're running into the same issue in an off-canvas dialog where the fields don't get properly re-focused after an AJAX callback occurs. It appears to be tied to the focus code inside of dialog.ajax.js rather than the regular ajax.js file referenced in the previous patches. It might be something to consider when doing further testing and development on this.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. 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.

Re-rolled #22 for 10.x

Make it work!

Would be better to mention how it was made to work.

I took a look at the latest patch but didn't see any changes in dialog.ajax.js. I think we'll need some similar changes to happen there if it's going to work inside of the off-canvas dialogs.

Issue summary updated.

Re #31:

I took a look at the latest patch but didn't see any changes in dialog.ajax.js. I think we'll need some similar changes to happen there if it's going to work inside of the off-canvas dialogs.

I expanded the test coverage to prove this solution works with off canvas as well.

Thanks @bjmnm for updating the issue summary. Changes look good.

The last submitted patch, 32: 3020061-32.patch, failed testing. View results →

Looks like another random fail

The patch is introducing jQuery UI dialog dependent code to the Ajax system. I think we should look if we could move that to the code that integrates the jQuery UI dialog with the Ajax system in core/misc/dialog/dialog.ajax.js.

For #37

Also it's directly editing ajax.js when it should be editing ajax.es6.js and compiling ajax.js.

@darvanen We stopped using .es6.js files because all browsers Drupal core supports have support for ES6. See https://www.drupal.org/node/3305487 → for more info. 😊

Oh brilliant! Thanks for the heads-up @laurii

Re #37

The patch is introducing jQuery UI dialog dependent code to the Ajax system. I think we should look if we could move that to the code that integrates the jQuery UI dialog with the Ajax system in core/misc/dialog/dialog.ajax.js.

Took a look at dialog.ajax.js and noticed that what was believed to be a jQuery UI focus management problem was actually something core introduced many years ago with #2113567: Views dialogs don't close via escape. → . Adding a condition to only refocus the dialog if focus has been lost seems to fix the issue reported here. Pushed an MR. Lets see if DrupalCI agrees.

There's also other related issue where the focus is set using different conditions when AJAX updates the dialog compared to when opening a new dialog: 🐛 Dialog focus is returned to dialog instead of first tabbable element on dialog AJAX update Fixed .

Hiding the patches as the fix is in the MR now

Rebased and ran the test-only feature and got

There were 2 failures:
1) Drupal\FunctionalJavascriptTests\Ajax\FormValuesTest::testSimpleAjaxFormValue with data set #1 ('dialog')
The link Open form in dialog was not found on the page.
Failed asserting that an array has the key 0.
/builds/issue/drupal-3020061/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:121
/builds/issue/drupal-3020061/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:55
/builds/issue/drupal-3020061/core/tests/Drupal/Tests/UiHelperTrait.php:414
/builds/issue/drupal-3020061/core/tests/Drupal/FunctionalJavascriptTests/Ajax/FormValuesTest.php:50
/builds/issue/drupal-3020061/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
2) Drupal\FunctionalJavascriptTests\Ajax\FormValuesTest::testSimpleAjaxFormValue with data set #2 ('off canvas dialog')
The link Open form in off canvas dialog was not found on the page.
Failed asserting that an array has the key 0.
/builds/issue/drupal-3020061/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:121
/builds/issue/drupal-3020061/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:55
/builds/issue/drupal-3020061/core/tests/Drupal/Tests/UiHelperTrait.php:414
/builds/issue/drupal-3020061/core/tests/Drupal/FunctionalJavascriptTests/Ajax/FormValuesTest.php:50
/builds/issue/drupal-3020061/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
FAILURES!
Tests: 4, Assertions: 41, Failures: 2.

Think this is good to RTBC again.

There are test failures in CI.

I have tested this, it is working as expected and test coverage looks good to me.
There is one small documentation nit, after which this is good to go.

lauriii → closed merge request !5377

Committed 81088a6 and pushed to 11.x. Also cherry-picked to 10.2.x. Thanks!

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