Merge lp:~bcsaller/juju-gui/hotkey-ext into lp:juju-gui/experimental

[Benjamin Saller]

Hotkey extensions

Keybindings can declare more behavior
Toggle simulator

https://codereview.appspot.com/9694043/

[Benjamin Saller (bcsaller)]

Reviewers: mp+165428_code.launchpad.net,

Message:
Please take a look.

Description:
Hotkey extensions

Keybindings can declare more behavior
Toggle simulator

https://code.launchpad.net/~bcsaller/juju-gui/hotkey-ext/+merge/165428

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/9694043/

Affected files:
   A [revision details]
   M app/app.js
   M app/config-debug.js
   M app/store/env/simulator.js
   M app/views/topology/panzoom.js
   M lib/views/stylesheet.less

[Jeff Pihach (hatch)]

LGTM - This hotkey stuff is getting to be a considerable number of lines
and looks pretty easy to be able to break out into an app extension to
clean up app.js. Possibly a follow-up?

https://codereview.appspot.com/9694043/

[Gary Poster (gary)]

LGTM. Maybe we should remove the feature flag simulator support? Oh,
and I think we should always instantiate the simulator in sandbox mode,
but only start it if simulateEvents is true in the config?

Thanks!

https://codereview.appspot.com/9694043/diff/1/app/app.js
File app/app.js (right):

https://codereview.appspot.com/9694043/diff/1/app/app.js#newcode173
app/app.js:173: '/': {
+1
...though on reflection it reminds me that we will need to change this
to work with the charm browser when it lands. Can cross the bridge when
we come to it.

https://codereview.appspot.com/9694043/diff/1/app/app.js#newcode207
app/app.js:207: '+': {
cool :-)

https://codereview.appspot.com/9694043/

[Benjamin Saller (bcsaller)]

*** Submitted:

Hotkey extensions

Keybindings can declare more behavior
Toggle simulator

R=jeff.pihach, gary.poster
CC=
https://codereview.appspot.com/9694043

https://codereview.appspot.com/9694043/

[Benjamin Saller (bcsaller)]

Thanks for the reviews, changed it to always create the simulator, to
start on config or hotkey and to clean up in the destructor.

https://codereview.appspot.com/9694043/

=== modified file 'app/app.js'
--- app/app.js	2013-05-22 22:30:29 +0000
+++ app/app.js	2013-05-23 16:07:28 +0000
@@ -154,6 +154,9 @@
      * target: {String} CSS selector of one element
      * focus: {Boolean} Focus the element.
      * toggle: {Boolean} Toggle element visibility.
+     * fire: {String} Event to fire when triggered. (XXX: Target is topology)
+     * condition: {Function} returns Boolean, should method be added to
+     *            keybindings.
      * callback: {Function} Taking (event, target).
      * help: {String} Help text to display in popup.
      *
@@ -167,17 +170,10 @@
         focus: true,
         help: 'Select the charm Search'
       },
-      'S-d': {
-        callback: function(evt) {
-          /* global saveAs: false */
-          this.env.exportEnvironment(function(r) {
-            var exportData = JSON.stringify(r.result, undefined, 2);
-            var exportBlob = new Blob([exportData],
-                                      {type: 'application/json;charset=utf-8'});
-            saveAs(exportBlob, 'export.json');
-          });
-        },
-        help: 'Export the environment'
+      '/': {
+        target: '#charm-search-field',
+        focus: true,
+        help: 'Select the charm Search'
       },
       'S-/': {
         target: '#shortcut-help',
@@ -187,10 +183,15 @@
           if (target && !target.getHTML().length) {
             var bindings = [];
             Y.each(this.keybindings, function(v, k) {
-              if (v.help) {
+              if (v.help && (v.condition === undefined ||
+                             v.condition.call(this) === true)) {
+                // TODO: translate keybindings to
+                // human <Alt> m
+                // <Control> <Shift> N (note caps)
+                // also 'g then i' style
                 bindings.push({key: k, help: v.help});
               }
-            });
+            }, this);
             target.setHTML(
                 views.Templates.shortcuts({bindings: bindings}));
           }
@@ -203,13 +204,46 @@
         },
         help: 'Navigate to the Environment overview.'
       },
+      '+': {
+        fire: 'zoom_in',
+        help: 'Zoom In'
+      },
+      '-': {
+        fire: 'zoom_out',
+        help: 'Zoom Out'
+      },
       'esc': {
+        fire: 'clearState',
         callback: function() {
           // Explicitly hide anything we might care about.
           Y.one('#shortcut-help').hide();
         },
         help: 'Cancel current action'
+      },
+
+      'C-s': {
+        'condition': function() {
+          return this._simulator !== undefined;
+        },
+        callback: function() {
+          this._simulator.toggle();
+        },
+        help: 'Toggle the simulator'
+      },
+
+      'S-d': {
+        callback: function(evt) {
+          /* global saveAs: false */
+          this.env.exportEnvironment(function(r) {
+            var exportData = JSON.stringify(r.result, undefined, 2);
+            var exportBlob = new Blob([exportData],
+                                      {type: 'application/json;charset=utf-8'});
+            saveAs(exportBlob, 'export.json');
+          });
+        },
+        help: 'Export the environment'
       }
+
     },
 
     /**
@@ -248,14 +282,14 @@
      */
     activateHotkeys: function() {
       var key_map = {
-        '/': 191, '?': 63,
+        '/': 191, '?': 63, '+': 187, '-': 189,
         enter: 13, esc: 27, backspace: 8,
         tab: 9, pageup: 33, pagedown: 34};
       var code_map = {};
       Y.each(key_map, function(v, k) {
         code_map[v] = k;
       });
-      Y.one(window).on('keydown', function(evt) {
+      this._keybindings = Y.one(window).on('keydown', function(evt) {
         //Normalize key-code
         var symbolic = [];
         if (evt.ctrlKey) { symbolic.push('C');}
@@ -266,12 +300,21 @@
         var trigger = symbolic.join('-');
         var spec = this.keybindings[trigger];
         if (spec) {
+          if (spec.condition && !spec.condition.call(this)) {
+            // Note that when a condition check fails
+            // the event still propagates.
+            return;
+          }
           var target = Y.one(spec.target);
           if (target) {
             if (spec.toggle) { target.toggleView(); }
             if (spec.focus) { target.focus(); }
           }
           if (spec.callback) { spec.callback.call(this, evt, target); }
+          // HACK w/o context/view restriction but right direction
+          if (spec.fire) {
+            this.views.environment.instance.topo.fire(spec.fire);
+          }
           // If we handled the event nothing else has to.
           evt.stopPropagation();
           evt.preventDefault();
@@ -521,6 +564,9 @@
     @method destructor
     */
     destructor: function() {
+      if (this._keybindings) {
+        this._keybindings.detach();
+      }
       Y.each(
           [this.env, this.db, this.charm_store, this.notifications,
            this.landscape, this.endpointsController],

=== modified file 'app/config-debug.js'
--- app/config-debug.js	2013-05-21 01:16:15 +0000
+++ app/config-debug.js	2013-05-23 16:07:28 +0000
@@ -45,7 +45,8 @@
   sandbox: true,
   // When in sandbox mode should we create events to simulate a live env.
   // You can also use the :flags:/simulateEvents feature flag.
-  simulateEvents: false,
+  // There is also a hotkey to toggle the simulator.
+  simulateEvents: true,
   readOnly: false,
   login_help: 'For this demonstration, use the password "admin" to connect.'
 };

=== modified file 'app/store/env/simulator.js'
--- app/store/env/simulator.js	2013-05-21 12:52:33 +0000
+++ app/store/env/simulator.js	2013-05-23 16:07:28 +0000
@@ -484,6 +484,7 @@
 
       // Invoke on start as well.
       tick();
+      console.log('Simulator started');
       return this;
 
     },
@@ -497,6 +498,22 @@
       if (this._scheduler) {
         this._scheduler.cancel();
         this._scheduler = null;
+        console.log('Simulator stopped');
+      }
+      return this;
+    },
+
+    /**
+     Toggle the running state of the simulator.
+
+     @method toggle
+     @chainable
+     */
+    toggle: function() {
+      if (this._scheduler) {
+        this.stop();
+      } else {
+        this.start();
       }
       return this;
     }

=== modified file 'app/views/topology/panzoom.js'
--- app/views/topology/panzoom.js	2013-05-17 14:51:05 +0000
+++ app/views/topology/panzoom.js	2013-05-23 16:07:28 +0000
@@ -59,7 +59,9 @@
 
           @event rendered
         */
-        rendered: 'renderedHandler'
+        rendered: 'renderedHandler',
+        zoom_in: 'zoom_in',
+        zoom_out: 'zoom_out'
       }
     },
 
@@ -130,8 +132,8 @@
      *
      * @method zoom_out
      */
-    zoom_out: function(data, context) {
-      var slider = context.slider,
+    zoom_out: function(data, self) {
+      var slider = this.slider || self.slider,
           val = slider.get('value');
       slider.set('value', val - 25);
     },
@@ -141,8 +143,8 @@
      *
      * @method zoom_in
      */
-    zoom_in: function(data, context) {
-      var slider = context.slider,
+    zoom_in: function(data, self) {
+      var slider = this.slider || self.slider,
           val = slider.get('value');
       slider.set('value', val + 25);
     },

=== modified file 'lib/views/stylesheet.less'
--- lib/views/stylesheet.less	2013-05-22 22:30:29 +0000
+++ lib/views/stylesheet.less	2013-05-23 16:07:28 +0000
@@ -117,6 +117,7 @@
     background: url(/juju-ui/assets/images/top_bar.png) repeat-x;
     padding: 0;
     position: relative;
+    overflow: hidden;
     color: @navbar-color;
     .brand {
         margin: 12px 26px 6px 10px;
@@ -1725,6 +1726,12 @@
     letter-spacing: normal;
 }
 
+.yui3-slider-thumb {
+  &:focus {
+    outline: none;
+  }
+}
+
 /** Browser Setup **/
 @import "browser/reset.less";
 @import "browser/vars.less";