In @Wim Leers' review in #164 of #2753941: [Experimental] Create Outside In module MVP to provide block configuration in Off-Canvas tray and expand site edit mode

All of this would greatly benefit from this being more structured, because state management for this looks like it will be painful. To provide the additional structure, without reinventing the wheel, the Contextual Links JS, Quick Edit JS, CKEditor toolbar configuration UI JS and Toolbar JS have all been written using Backbone models & views.

@@ -0,0 +1,139 @@
+    data.$el.find('.outside-inblock-configure a').click(function () {
+      if (!isActiveMode()) {
+        $('div.contextual-toolbar-tab.toolbar-tab button').click();
+      }
+    });
This code signals very strongly to me that we indeed want Backbone-based code. Because this looks a lot like the unmaintainable JS code of the past.
b/core/modules/outside_in/js/outside_in.js
@@ -0,0 +1,139 @@
+  var isActiveMode = function () {
+    return $('#toolbar-bar').hasClass('js-outside-in-edit-mode');
+  };

And this is the sort of "brittle DOM-based state tracking" that you can avoid by using Backbone.
@drpal(yes not a mis-spelling ;) ) how did most of the JS concured

Yes. I absolutely agree that this work should be encapsulated with Backbone models & views

Let's do it!

Comments

tedbow created an issue. See original summary.

webchick’s picture

Issue tags: +JavaScript, +sprint, +Outside-in
tedbow’s picture

Component: javascript » outside_in.module
tedbow’s picture

Priority: Normal » Major
Issue tags: -Outside-in

This seems important to do sooner rather than later. As it will affect other JS

droplet’s picture

Don't use framework because you just want to use framework :)

http://cgit.drupalcode.org/drupal/tree/core/modules/outside_in/js/outsid...
This is 147 LINE.

If this module has no secrets plan to expand its features;
If you don't use Backbone.Router;
If you don't use Backbone.Model in Backbone way;
If you don't use Backbone.View to do critical frontend rendering;
If you don't use Backbone.Sync to handle data exchange;

All remaining is a plain object + bundle of custom event listeners; (Or a middleware function to handle & dispatch the actions). This is how Backbone works also, doesn't it? Backbone has no reactive & 2-way data binding.

var dataModel = {
 // Done.
}
tedbow’s picture

Issue tags: +off-canvas

@droplet re:

If this module has no secrets plan to expand its features;

No secret plan but #2784443: Move off-canvas functionality from Settings tray module into drupal.dialog.ajax library so that other modules can use it
The idea is the offcanvas tray will be an available dialog type in core like "modal". Then other parts of core and contrib would start to use it. We are already starting to work on the Place Block module to use it: #2784495: Normalize block place and outside-in experiences

I think the offcanvas tray would likely need expand its features for core and contrib uses.

tkoleary’s picture

Status: Active » Closed (won't fix)

#2786459: "Offcanvas" tray should be using the existing dialog system Renders this issue moot because (at least as long as we use jquery dialog) jquery will handle state management.

Wim Leers’s picture

Status: Closed (won't fix) » Active

There's a lot more JS going on than just the dialog-related parts. There's also the reacting to the "Edit" toolbar button etc.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.