Create a helper trait for Forms in ajax dialogs

Clarified IS.

I'd be forced to repeat all of the review points that @effulgentsia raised at #2896535: Create a helper trait for Forms in ajax dialogs, especially #2896535: Create a helper trait for Forms in ajax dialogs. Which means we're repeating the same discussion we already had.

Either this should be postponed on #2785047: In Outside In mode, form validation messages should appear in the off-canvas tray, not the main page and #2830584: Use modals for creating, updating, and deleting workflows, with a new DialogFormTrait, or those two issues must be closed in favor of this. I'm assuming the former. If it's the latter, then I'll leave it to @tedbow to mark those two issues as duplicates and reopen this.

This soft-blocks Layout Builder.

#2830584: Use modals for creating, updating, and deleting workflows, with a new DialogFormTrait is just adding a trait to its own namespace currently. What is the plan WRT the layout builder?

Just passing thru, but I couldn't resist...:)

Maybe as a first step we could move \Drupal\layout_builder\Controller\AjaxHelperTrait to core.

Then look at adding items from \Drupal\layout_builder\Form\AjaxFormHelperTrait and #2830584: Use modals for creating, updating, and deleting workflows, with a new DialogFormTrait in a follow up?

Here's a start for that.

I updated the patch in #2830584: Use modals for creating, updating, and deleting workflows, with a new DialogFormTrait so that the trait added there is now just a copy of the two LB traits, which will make the transition easier.

index 072eccab35..aacc75d6b8 100644
--- a/core/modules/layout_builder/src/Controller/AjaxHelperTrait.php

--- a/core/modules/layout_builder/src/Controller/AjaxHelperTrait.php
+++ b/core/lib/Drupal/Core/Ajax/AjaxHelperTrait.php

Please also remove the @todo in this file that points to this issue :)

Lets add AjaxFormHelperTrait to the patch, and update settings tray to use it.

Still todo:
- Make AjaxHelperTrait::isAjax less brittle.

@tedbow suggested

Look at services and figure out all possible drupal_*.* values actually it would anything that uses drupal_dialog.\* or drupal_modal.\* because we can’t add a new data-dialog-type just data-dialog-renderer could vary

Needs work based on the todo in #14 and the change record.

Yesterday I started work on #2949991: Add workspace UI in top dialog and quickly found I needed an isAjax() method, yet again copying the trait from LB. Although this time fully understanding how brittle it is, because I needed to add drupal_dialog.off_canvas_top to the array of wrapper formats.

I've created a change record for this https://www.drupal.org/node/2950404

Here's an update to make isAjax() less brittle.

+++ b/core/lib/Drupal/Core/Ajax/AjaxFormHelperTrait.php
@@ -1,18 +1,13 @@
  * @internal
...
 trait AjaxFormHelperTrait {

+++ b/core/lib/Drupal/Core/Ajax/AjaxHelperTrait.php
@@ -8,8 +8,6 @@
  * @internal
...
 trait AjaxHelperTrait {

Should these still be marked @internal?

The changes to isAjax look good!

@timmillwood points out that while we can always remove the @internal later, if we remove it now we're stuck. Better safe than sorry, and revisit that after issues like #2830584: Use modals for creating, updating, and deleting workflows, with a new DialogFormTrait and #2949991: Add workspace UI in top dialog land.

Committed 9f61972 and pushed to 8.6.x. Thanks!

+1 for leaving internal for now as that does give us a chance to explore what's necessary to make this even more generic in #2830584: Use modals for creating, updating, and deleting workflows, with a new DialogFormTrait. Considering they are internal I think we should hold of publishing the CR until they are not.

diff --git a/core/modules/settings_tray/src/Block/BlockEntitySettingTrayForm.php b/core/modules/settings_tray/src/Block/BlockEntitySettingTrayForm.php
index c55cc4d6a0..243e907ccd 100644
--- a/core/modules/settings_tray/src/Block/BlockEntitySettingTrayForm.php
+++ b/core/modules/settings_tray/src/Block/BlockEntitySettingTrayForm.php
@@ -8,7 +8,6 @@
 use Drupal\Core\Ajax\AjaxFormHelperTrait;
 use Drupal\Core\Ajax\AjaxResponse;
 use Drupal\Core\Ajax\RedirectCommand;
-use Drupal\Core\Ajax\ReplaceCommand;
 use Drupal\Core\Block\BlockPluginInterface;
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\Core\Plugin\PluginWithFormsInterface;
@@ -130,12 +129,12 @@ public function buildForm(array $form, FormStateInterface $form_state) {
     $form['#attached']['library'][] = 'core/drupal.dialog.ajax';
 
     // static::ajaxSubmit() requires data-drupal-selector to be the same between
-    // the various Ajax requests. A bug in
-    // \Drupal\Core\Form\FormBuilder prevents that from happening unless
-    // $form['#id'] is also the same. Normally, #id is set to a unique HTML ID
-    // via Html::getUniqueId(), but here we bypass that in order to work around
-    // the data-drupal-selector bug. This is okay so long as we assume that this
-    // form only ever occurs once on a page.
+    // the various Ajax requests. A bug in \Drupal\Core\Form\FormBuilder
+    // prevents that from happening unless $form['#id'] is also the same.
+    // Normally, #id is set to a unique HTML ID via Html::getUniqueId(), but
+    // here we bypass that in order to work around the data-drupal-selector bug.
+    // This is okay so long as we assume that this form only ever occurs once on
+    // a page.
     // @todo Remove this workaround once https://www.drupal.org/node/2897377 is
     //   fixed.
     $form['#id'] = Html::getId($form_state->getBuildInfo()['form_id']);

There was an used use and a comment that needed properly re-flowing.

publish the change record