(Coming from #1943776-9: In-place editors (Create.js PropertyEditor widgets) should be loaded lazily.)

… because AJAX commands are such a pain to use when they're triggered by the client-side:

+        var id = 'edit-load-metadata';
+        // Create a temporary element to be able to use Drupal.ajax.
+        var $el = jQuery('<div id="' + id + '" class="element-hidden"></div>').appendTo('body');
+        // Create a Drupal.ajax instance to load the form.
+        Drupal.ajax[id] = new Drupal.ajax(id, $el, {
           url: drupalSettings.edit.metadataURL,
+          event: 'edit-internal.edit',
+          submit: { 'fields[]' : _.pluck(remainingFieldsToAnnotate, 'editID') },
+          progress: { type : null } // No progress indicator.
         });
+        // Implement a scoped editMetaData AJAX command: calls the callback.
+        Drupal.ajax[id].commands.editMetadata = function(ajax, response, status) {
+          // Update the metadata cache.
+          _.each(response.data, function(metadata, editID) {
+            Drupal.edit.metadataCache[editID] = metadata;
+          });
+
+          // Annotate the remaining fields based on the updated access cache.
+          _.each(remainingFieldsToAnnotate, annotateField);
+
+          // Find editable fields, make them editable.
+          Drupal.edit.app.findEditableProperties($context);
+
+          // Delete the Drupal.ajax instance that called this very function.
+          delete Drupal.ajax[id];
+
+          // Also delete the temporary element.
+          // $el.remove();
+        };
+        // This will ensure our scoped editMetadata AJAX command gets called.
+        $el.trigger('edit-internal.edit');

That's just… evil. Instead, we should have something like Drupal.loadLibrary() that allows client-side code to decide when to load a library. edit/metadata can then again just return a JSON response, and part of that metadata can indicate which libraries should be loaded.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

effulgentsia’s picture

because AJAX commands are awful for client-side logic

Regardless of this issue, that would be a nice thing to fix: #1533366: Simplify and optimize Drupal.ajax() instantiation and implementation

we should have something like Drupal.loadLibrary()

I wonder if that would make sense as a separate issue.

Turn edit/metadata into a JsonResponse again

Once we have a Drupal.loadLibrary(), then sure, why not :) But I also don't think there's anything wrong with leaving it as AjaxResponse other than #1533366: Simplify and optimize Drupal.ajax() instantiation and implementation.

webchick’s picture

Priority: Minor » Normal
Issue tags: +DX (Developer Experience)

Given this code evoked the response of "majestic PITA" by a so-affected developer, I think it's fine to call this normal rather than minor. :)

Wim Leers’s picture

Wim Leers’s picture

Assigned: Unassigned » Wim Leers
Status: Postponed » Active
Issue tags: +sprint

This needs to happen anyway, because edit/metadata is doing two things: returning metadata *and* loading editors. This against HTTP/REST principles and makes it effectively impossible to do #1872264-6: Minimize metadata HTTP requests triggered by Edit's JS.

Instead of blocking on Drupal.loadLibrary(), which is not possible here after all because an in-place editor can dynamically return different attachments, I'm going to introduce a edit/editors route to load in-place editors via an AJAX command.

Wim Leers’s picture

Title: Turn edit/metadata into a JsonResponse again, because AJAX commands are awful for client-side logic » Turn edit/metadata into a JsonResponse again, to allow contrib to implement client-side caching of metadata
Status: Active » Needs review
Issue tags: +front-end performance
FileSize
14.33 KB

As promised in #4.

effulgentsia’s picture

Status: Needs review » Reviewed & tested by the community

Cool!

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Looks good. Committed to 8.x. Thanks!

Wim Leers’s picture

Issue tags: -sprint

Hurray, thanks! :)

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

YesCT’s picture

Issue tags: -front-end performance +frontend performance

changing to use the more common tag, so the less common one can be deleted, so it does not show up in the auto complete and confuse people.