To optimize outside_in Drupal.behaviors.toggleEditMode

A huge performance impact

Can you quantify this?

+++ b/core/modules/outside_in/js/outside_in.es6.js
@@ -212,21 +213,6 @@
-          instance.options.data.dialogOptions.outsideInActiveEditableId = $(instance.element).parents('.outside-in-editable').attr('id');
-          instance.progress = { type: 'fullscreen' };

These 2 lines are not being replaced anywhere. We still need the behavior so I don't think this block of code can removed at all.

I manually tested and confirm that "active" block is not highlight with this patch. Which means we need a test for that since the patch passed.

1. +++ b/core/modules/outside_in/js/outside_in.es6.js
   @@ -212,21 +213,6 @@
   -        .filter(instance => instance && $(instance.element).attr('data-dialog-renderer') === 'off_canvas')
   

   Can we also check for the existence of instance.options.data.dialogOptions.outsideInActiveEditableId to we can more strictly filter elements to avoid looking at potential duplicates?

2. +++ b/core/modules/outside_in/js/outside_in.es6.js
   @@ -212,21 +213,6 @@
   -          instance.progress = { type: 'fullscreen' };
   

   I think we need these.

Changing to new settings_tray.module component. @drpal thanks for script help! :)

Needs reroll at least for changes in #2803375: Rename Outside-in module to "Settings Tray" for real

I don't think we need the patch. I am not sure there is actually performance problem here.

From the Proposed solution:

2. Ideally, it should only execution after contextual links fully loaded or after Top left Toggle button is clicked.

We can't know when all contextual links are loaded. Ajax could add another part of the page when have new contextual links, correct?

Also the top edit button may never actually be clicked because click "Quick Edit" on any block will invoke edit mode

I don't see how we can not the event drupalContextualLinkAdded

3. Ideally, "fullscreen" should be returned from Ajax. (might be hack Drupal.Ajax to support data-progress-type)

Seems like be a BC break now it would be using the default progress type for jQuery

Alright. I'm not sure this is even needed anymore, but this is mostly a micro optimization.

This function, prepareAjaxLinks() appears to be fired 11 times via contextual.js

The slowest part of this is the jQuery .parents() lookup.

A huge performance impact

vs

this is mostly a micro optimization

If #13 approves the performance I am all for it. It doesn't need some the larger changes that were suggested in the proposed solution in the issue summary. Those other changes would require changes outside of this module I think.

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.