Replace #ajax['wrapper'] with #ajax['wrapper_selector']

Removing issue credit for @Rajender Rajan since these patches are incorrect and do not advance the issue.

@Rajender Rajan, please post an explanation in the comment field and an interdiff when you update core patches, so that your contributions help move the Drupal core issues forward. You can also get help in IRC if you are not sure how to contribute. Thanks for your interest in working on Drupal core!

I am unable to understand error ! I thought i have done wrong changes that why its been showing error with each patch of mine !!!

My patches invalid in what sense ? Can u please explain bit more ?

diff --git a/core/core.api.php b/core/core.api.php
index 7c4ebf4..d79ba36 100644
--- a/core/core.api.php
+++ b/core/core.api.php
@@ -2361,16 +2361,16 @@ function hook_validation_constraint_alter(array &$definitions) {
  *   Ajax event. More information on callbacks is below in @ref sub_callback.
  * - wrapper: The HTML 'id' attribute of the area where the content returned by
  *   the callback should be placed. Note that callbacks have a choice of
- *   returning content or JavaScript commands; 'wrapper' is used for content
+ *   returning content or JavaScript commands; 'wrapper_selector' is used for content
  *   returns.

Docs aren't fully updated. Property 'wrapper' is still listed as an option and 'wrapper_selector' isn't.

diff --git a/core/modules/simpletest/src/WebTestBase.php b/core/modules/simpletest/src/WebTestBase.php
index 975d95b..9fa39ff 100644
--- a/core/modules/simpletest/src/WebTestBase.php
+++ b/core/modules/simpletest/src/WebTestBase.php
@@ -1306,10 +1306,16 @@ protected function drupalProcessAjaxResponse($content, array $ajax_response, arr
 
         case 'insert':
           $wrapperNode = NULL;
-          // When a command doesn't specify a selector, use the
-          // #ajax['wrapper'] which is always an HTML ID.
+          // When a command doesn't specify a selector, use #ajax['wrapper_selector'].
           if (!isset($command['selector'])) {
-            $wrapperNode = $xpath->query('//*[@id="' . $ajax_settings['wrapper'] . '"]')->item(0);
+            $selector_type = substr($ajax_settings['wrapper_selector'], 0, 1);
+            $selector = substr($ajax_settings['wrapper_selector'], 1);
+            if ($selector_type === '#') {
+              $wrapperNode = $xpath->query('//*[@id="' . $selector . '"]')->item(0);
+            }
+            else {
+              $wrapperNode = $xpath->query('//*[contains(@class, ' . $selector . ')]')->item(0);
+            }
           }

XPath query would fail in other case. E.g. if selector is 'textarea' or '[type="radio"]' or even ':empty'.

@@ -164,7 +164,7 @@ function views_ui_add_ajax_wrapper($element, FormStateInterface $form_state) {
 
   // The HTML ID that AJAX expects was also stored in a property on the
   // element, so use that information to insert the wrapper <div> here.
-  $id = $element['#views_ui_ajax_data']['wrapper'];
+  $id = substr($element['#views_ui_ajax_data']['wrapper_selector'], 1);
   $refresh_element += [
     '#prefix' => '',
     '#suffix' => '',

Can end up in invalid id because not only class (.) and id (#) selectors are valid 'wrapper_selector' values.

@@ -327,6 +327,12 @@ public static function preRenderAjaxForm($element) {
 
       $settings = $element['#ajax'];
 
+      // @todo Remove support for 'wrapper' and solely use 'wrapper_selector'.
+      if (isset($settings['wrapper'])) {
+        $settings['wrapper_selector'] = '#' . $settings['wrapper'];
+        unset($settings['wrapper']);
+      }
+

It should be if (isset($settings['wrapper_selector'])) and not if (isset($settings['wrapper'])) .

     if (this.wrapper) {
 
       /**
+       * @deprecated Use this.wrapper_selector.
+       *
        * @type {string}
        */
-      this.wrapper = '#' + this.wrapper;
+      this.wrapper = null;
+
+      /**
+       * @type {string}
+       */
+      this.wrapper_selector = '#' + this.wrapper;
     }

if (this.wrapper) should be replaced by if (this.wrapper_selector).
this.wrapper = null ----> this line should be removed

Nice, however 'wrapper_selector' is a horrible name... :/

Seems like the collision anticipated in #4 didn't turn out to exist / be a problem, otherwise eschewing to 'wrapper_selector' would have fixed at least one of the #3 test failures.

Let's see if patch #41, with 'wrapper_selector' renamed back to 'selector', tests green as well. No other changes in here.

You were right, and the tests failed, too. Please ignore #42.

Let's however figure out if there's another way to have a concise property key.

I weighed #ajax['target'] as an alternative, but in the end, 'selector' is better, plus is used in AJAX commands, so should be used here, too. Let's see if we can fix that collision differently.

OK, unsubscribing.

It would be good to get sign off from a framework manager on this API change since changing 'wrapper' into 'wrapper_selector' is conceptually a big change. Using id as a selector ensures that the wrapper element is always unique. Since that is not the case after this change, I assume part of the scope is to allow returning content into multiple different wrapper elements from a single #ajax callback. We should document that in the issue summary explicitly since it is not a minor change. We should also add test coverage specifically for that.

I tagged this for FM review because I accidentally thought there was no subsystem maintainer for ajax system. Moving this to the subsystem maintainer queue instead for addressing feedback in #50.

Improved the problem/motivation section (I hope).

Discussed this a bit with @tim.plunkett in Slack, trying to figure out what to do about the "multiple targeted elements" question. I felt that we should hard-code the system to only work on the first targeted element (.eq(0)), at least for now; Tim felt that we should allow multiple targets, and add test coverage. Since he is one of the maintainers of this subsystem, I think his opinion probably carries more weight than mine :) Tim also pointed out that nothing in core is currently taking advantage of multiple targeting, so things will continue to work the way they already do. In other words, although this change will expand the AJAX system's capabilities, it won't break existing code.

So, okay. Multiple targeting it is. I'm tagging this issue for more tests and kicking it back for that.

One reason IMHO is that ids are, at least in certain places, dynamically altered, and if you're not extra careful, then ajax replacements no longer work the second time.

What I always wondered is why we added those data-drupal-selector attributes but afaik not use it anywhere (there might be an issue somewhere). You could also use the new selector to target these, probably just a bit unwieldy.

One reason you would want to use this is if you are replacing the entire form on AJAX. You cannot keep replacing the form unless you ensure a stable ID.

Tested #62 on 9.1, Replace #ajax['wrapper'] with #ajax['wrapper_selector'] successfully applied

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 think this issue is a good idea. If you want to embed a form in a block, and that block occurs more than one time on the same page, then you have an ID conflict due to hardcoding the wrapper ID. I don't know of any workaround as-is.

Patch #62, no longer applies to 10.1.x. I have re-rolled the patch #62, for 10.1.x. Please review

There is size difference with the patch #62, as es6.js files are removed from the current patch.

Fixed CCF.

Fixed CCF, attached interdiff with #74.

Deprecation needs updating.

Also this was tagged for issue summary update 4 years ago. is #55 good enough?

Proposed solution could be cleaned up I think.

We are on a path to deprecate Ajax API. New functionality is now out of scope, however the new subsystem accepts any valid CSS selector by design. See #3404409: [Plan] Gradually replace Drupal's AJAX system with HTMX.

Usually for issues like this that we're hoping to make irrelevant, I move them to postponed on the issue/meta that will do that. That way there's a way to come back and point people to the new way when it's done, while still discouraging work on something that's unlikely to get reviewed/committed before it's removed anyway.

Marking postponed on #3404409: [Plan] Gradually replace Drupal's AJAX system with HTMX for now