(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.

#5 edit_metadata_json_attachments_ajax-1980744-5.patch14.33 KBWim Leers
PASSED: [[SimpleTest]]: [MySQL] 56,063 pass(es). View
Members fund testing for the Drupal project. Drupal Association Learn more


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
14.33 KB
PASSED: [[SimpleTest]]: [MySQL] 56,063 pass(es). View

As promised in #4.

effulgentsia’s picture

Status: Needs review » Reviewed & tested by the community


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.