Dialog drupalAutoButtons option should be respected on initial load

#76 works for Drupal 10.0 - thanks @bnjmnm!

Removing the needs follow up as it seems the todo mentioned in #57 is being addressed.

Think the issue summary could use some love though.

Here's the 9.5.x patch only for the project I'm on

Here's a rerolled version for 11.x. Also updated the issue summary.

Tested this using the module provided dialog_renderer_test
Went to /dialog_renderer-test-links (like the test)
Verified Auto buttons default! and Auto buttons false! both contained .ui-dialog-buttonpane without the fix

With the fix only the first link contained .ui-dialog-buttonpane.

This basically looks good, but I see a risk of introducing an intermittent test failure. Lets get that addressed and I bet this is good to go. It's simple enough that if the person who addresses it is comfortable with the change they can self-switch this back to RTBC.

+++ b/core/modules/system/tests/src/FunctionalJavascript/ModalRendererTest.php
@@ -74,6 +74,29 @@ public function testModalRenderer() {
+    $this->clickLink('Auto buttons default!');
+    $session_assert->assertWaitOnAjaxRequest();
+    $session_assert->elementExists('css', '.ui-dialog-buttonpane .ui-dialog-buttonset .js-form-submit');
+
+    // When the drupalAutoButtons option is false, buttons SHOULD NOT be moved
+    // into the 'ui-dialog-buttonpane' container.
+    $this->drupalGet('/dialog_renderer-test-links');
+    $this->clickLink('Auto buttons false!');
+    $session_assert->assertWaitOnAjaxRequest();
+    $session_assert->elementExists('css', '.form-actions');
+    $session_assert->elementNotExists('css', '.ui-dialog-buttonpane');
+
+    // When the drupalAutoButtons option is true, buttons SHOULD be moved
+    // into the 'ui-dialog-buttonpane' container.
+    $this->drupalGet('/dialog_renderer-test-links');
+    $this->clickLink('Auto buttons true!');
+    $session_assert->assertWaitOnAjaxRequest();
+    $session_assert->elementExists('css', '.ui-dialog-buttonpane .ui-dialog-buttonset .js-form-submit');

assertWaitOnAjaxRequest to wait on a modal opening risks intermittent test failures. jQuery UI can continue to manipulate the elements after Drupal AJAX is done. For a simple use like this there isn't that much risk.. but given the number of test runs that happen daily it's best to be cautious. If this can instead waitForEementExists on whatever element needs to appear for testing to continue, then it should be fine.

Rerolled on current 11.x and switched out $session_assert->assertWaitOnAjaxRequest() in favour of $this->assertNotNull($session_assert->waitForElement() as suggested in #86 🐛 Dialog drupalAutoButtons option should be respected on initial load Needs review .

Let's see if tests still succeed.

Grevil → changed the visibility of the branch Anybody-2793343-dialog-drupalautobuttons-option-patch-94463 to hidden.

Grevil → changed the visibility of the branch 2793343-drupalAutoButtons to hidden.

Grevil → changed the visibility of the branch cherry-pick-e3ae816e to hidden.

Grevil → changed the visibility of the branch 9.2.x to hidden.

No idea, why my patch did not pass tests, created another branch and MR with patch #87 applied here: https://git.drupalcode.org/project/drupal/-/merge_requests/6288

Let's see what the tests from the gitlab-ci are saying.

@Grevil

https://www.drupal.org/project/drupal/issues/3401988 🐛 Spell-checking job fails with "Argument list too long" when too many files are changed Active

you need to update fork 11.x

@finnsky thanks! The fork doesn't even have 11.x haha, going to update it ASAP.

Grevil → changed the visibility of the branch 11.x to hidden.

All green!

From #86 🐛 Dialog drupalAutoButtons option should be respected on initial load Needs review :

It's simple enough that if the person who addresses it is comfortable with the change they can self-switch this back to RTBC.

->Done

lauriii → closed merge request !6288

Committed 77888cf and pushed to 11.x. Also cherry-picked to 10.2.x as a non-disruptive bug fix. Thanks!

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