Problem/Motivation

Reduce the weight of the contextual JS, since it's used on most pages when user has access to it.

Steps to reproduce

Proposed resolution

change the jquery ajax call with this, seems to be enough

      // Perform an AJAX request to let the server render the contextual links
      // for each of the placeholders.
      if (uncachedIDs.length > 0) {
        const data = new URLSearchParams();
        uncachedIDs.forEach((id) => data.append('ids[]', id));
        uncachedTokens.forEach((id) => data.append('tokens[]', id));

        const req = new Request(Drupal.url('contextual/render'), {
          method: 'POST',
          body: data,
        });

        fetch(req)
          .then((res) => res.json())
          .then((json) =>
            Object.entries(json).forEach(([contextualID, html]) => {
              // Store the metadata.
              storage.setItem(`Drupal.contextual.${contextualID}`, html);
              // If the rendered contextual links are empty, then the current
              // user does not have permission to access the associated links:
              // don't render anything.
              if (html.length > 0) {
                // Update the placeholders to contain its rendered contextual
                // links. Usually there will only be one placeholder, but it's
                // possible for multiple identical placeholders exist on the
                // page (probably because the same content appears more than
                // once).
                $placeholders = $context.find(
                  `[data-contextual-id="${contextualID}"]`,
                );

                // Initialize the contextual links.
                for (let i = 0; i < $placeholders.length; i++) {
                  initContextual($placeholders.eq(i), html);
                }
              }
            }),
          )
          .catch((error) => console.warn(error));
      }

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3548212

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

nod_ created an issue. See original summary.

godotislate made their first commit to this issue’s fork.

godotislate’s picture

Status: Active » Needs work

OK, put up the MR based on the suggestion in the description. I actually tried tweaking the JS a little to remove more jQuery from the snippet, but tests were failing, so decided to leave that for later.

Some tests are failing because of AJAX wait asserts, and those assertions are based on events triggered by jQuery AJAX. I removed some of the lines to see if the tests will pass, but I think it's caused other issues, since I'm guessing there likely needs to be some sort of waiting going on for the requests for finish. Can look at that later.

Also, can't remove core/drupal.ajax as a dependency of contextual/drupal.contextual-links yet, because there's a call to Drupal.ajax.bindAjaxLinks at the bottom of contextual.js.

mstrelan made their first commit to this issue’s fork.

mstrelan’s picture

Some of the tests were random fails, I fixed a couple more, and there are some remaining.

nod_’s picture

Status: Needs work » Needs review

Tests are green, since we never used fetch directly in core before the test framework didn't know how to wait on it.

We should be able to find a way to make the ajax contextual link not a dependency. Does anyone has a example of this being of use? was it for quickedit back in the day maybe?

godotislate’s picture

Does anyone has a example of this being of use?

In non-test modules, layout_builder.links.contextual.yml and settings_tray.links.contextual.yml both have use-ajax links.

godotislate’s picture

Had an idea on how to remove the dependency, pushed a commit to see if tests pass.

godotislate’s picture

OK tests seem to pass and the library dependency was removed. I changed it to attach the library to the links render array if any link had the use-ajax class, then used attachBehaviors in contexual.js on the new DOM element instead of Drupal.ajax.bindAjaxLinks.

nod_’s picture

Oh nice, and the perf test looks great with 27k of JS removed :D

nod_’s picture

Status: Needs review » Needs work

Checked it out more closely, we kind of have nested attachBehaviors call, that isn't ideal.

Instead of that we can just try to call the function "safely". So doing

 $(document).on('drupalContextualLinkAdded', (event, data) => {
    Drupal.ajax?.bindAjaxLinks(data.$el[0]);
  });

and keeping the backend code that conditionaly load core/drupal.ajax should be enough

godotislate’s picture

Status: Needs work » Needs review

Made changes per #12.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Leaning on nod_ review here but his feedback appears to be addressed.

alexpott made their first commit to this issue’s fork.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 8b522ad and pushed to 11.x. Thanks!

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.

  • alexpott committed 8b522adc on 11.x
    Issue #3548212 by nod_, godotislate, mstrelan: Remove dependency on core...
nod_’s picture

exciting :) thanks!

Status: Fixed » Closed (fixed)

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