Merge lp:~bcsaller/juju-gui/hotkey-ext into lp:juju-gui/experimental
- hotkey-ext
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 688 |
Proposed branch: | lp:~bcsaller/juju-gui/hotkey-ext |
Merge into: | lp:juju-gui/experimental |
Diff against target: |
260 lines (+94/-21) 5 files modified
app/app.js (+61/-15) app/config-debug.js (+2/-1) app/store/env/simulator.js (+17/-0) app/views/topology/panzoom.js (+7/-5) lib/views/stylesheet.less (+7/-0) |
To merge this branch: | bzr merge lp:~bcsaller/juju-gui/hotkey-ext |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju GUI Hackers | Pending | ||
Review via email:
|
Commit message
Description of the change
Hotkey extensions
Keybindings can declare more behavior
Toggle simulator
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Benjamin Saller (bcsaller) wrote : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Jeff Pihach (hatch) wrote : | # |
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?
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Gary Poster (gary) wrote : | # |
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:/
File app/app.js (right):
https:/
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:/
app/app.js:207: '+': {
cool :-)
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Benjamin Saller (bcsaller) wrote : | # |
*** Submitted:
Hotkey extensions
Keybindings can declare more behavior
Toggle simulator
R=jeff.pihach, gary.poster
CC=
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Benjamin Saller (bcsaller) wrote : | # |
Thanks for the reviews, changed it to always create the simulator, to
start on config or hotkey and to clean up in the destructor.
Preview Diff
1 | === modified file 'app/app.js' | |||
2 | --- app/app.js 2013-05-22 22:30:29 +0000 | |||
3 | +++ app/app.js 2013-05-23 16:07:28 +0000 | |||
4 | @@ -154,6 +154,9 @@ | |||
5 | 154 | * target: {String} CSS selector of one element | 154 | * target: {String} CSS selector of one element |
6 | 155 | * focus: {Boolean} Focus the element. | 155 | * focus: {Boolean} Focus the element. |
7 | 156 | * toggle: {Boolean} Toggle element visibility. | 156 | * toggle: {Boolean} Toggle element visibility. |
8 | 157 | * fire: {String} Event to fire when triggered. (XXX: Target is topology) | ||
9 | 158 | * condition: {Function} returns Boolean, should method be added to | ||
10 | 159 | * keybindings. | ||
11 | 157 | * callback: {Function} Taking (event, target). | 160 | * callback: {Function} Taking (event, target). |
12 | 158 | * help: {String} Help text to display in popup. | 161 | * help: {String} Help text to display in popup. |
13 | 159 | * | 162 | * |
14 | @@ -167,17 +170,10 @@ | |||
15 | 167 | focus: true, | 170 | focus: true, |
16 | 168 | help: 'Select the charm Search' | 171 | help: 'Select the charm Search' |
17 | 169 | }, | 172 | }, |
29 | 170 | 'S-d': { | 173 | '/': { |
30 | 171 | callback: function(evt) { | 174 | target: '#charm-search-field', |
31 | 172 | /* global saveAs: false */ | 175 | focus: true, |
32 | 173 | this.env.exportEnvironment(function(r) { | 176 | help: 'Select the charm Search' |
22 | 174 | var exportData = JSON.stringify(r.result, undefined, 2); | ||
23 | 175 | var exportBlob = new Blob([exportData], | ||
24 | 176 | {type: 'application/json;charset=utf-8'}); | ||
25 | 177 | saveAs(exportBlob, 'export.json'); | ||
26 | 178 | }); | ||
27 | 179 | }, | ||
28 | 180 | help: 'Export the environment' | ||
33 | 181 | }, | 177 | }, |
34 | 182 | 'S-/': { | 178 | 'S-/': { |
35 | 183 | target: '#shortcut-help', | 179 | target: '#shortcut-help', |
36 | @@ -187,10 +183,15 @@ | |||
37 | 187 | if (target && !target.getHTML().length) { | 183 | if (target && !target.getHTML().length) { |
38 | 188 | var bindings = []; | 184 | var bindings = []; |
39 | 189 | Y.each(this.keybindings, function(v, k) { | 185 | Y.each(this.keybindings, function(v, k) { |
41 | 190 | if (v.help) { | 186 | if (v.help && (v.condition === undefined || |
42 | 187 | v.condition.call(this) === true)) { | ||
43 | 188 | // TODO: translate keybindings to | ||
44 | 189 | // human <Alt> m | ||
45 | 190 | // <Control> <Shift> N (note caps) | ||
46 | 191 | // also 'g then i' style | ||
47 | 191 | bindings.push({key: k, help: v.help}); | 192 | bindings.push({key: k, help: v.help}); |
48 | 192 | } | 193 | } |
50 | 193 | }); | 194 | }, this); |
51 | 194 | target.setHTML( | 195 | target.setHTML( |
52 | 195 | views.Templates.shortcuts({bindings: bindings})); | 196 | views.Templates.shortcuts({bindings: bindings})); |
53 | 196 | } | 197 | } |
54 | @@ -203,13 +204,46 @@ | |||
55 | 203 | }, | 204 | }, |
56 | 204 | help: 'Navigate to the Environment overview.' | 205 | help: 'Navigate to the Environment overview.' |
57 | 205 | }, | 206 | }, |
58 | 207 | '+': { | ||
59 | 208 | fire: 'zoom_in', | ||
60 | 209 | help: 'Zoom In' | ||
61 | 210 | }, | ||
62 | 211 | '-': { | ||
63 | 212 | fire: 'zoom_out', | ||
64 | 213 | help: 'Zoom Out' | ||
65 | 214 | }, | ||
66 | 206 | 'esc': { | 215 | 'esc': { |
67 | 216 | fire: 'clearState', | ||
68 | 207 | callback: function() { | 217 | callback: function() { |
69 | 208 | // Explicitly hide anything we might care about. | 218 | // Explicitly hide anything we might care about. |
70 | 209 | Y.one('#shortcut-help').hide(); | 219 | Y.one('#shortcut-help').hide(); |
71 | 210 | }, | 220 | }, |
72 | 211 | help: 'Cancel current action' | 221 | help: 'Cancel current action' |
73 | 222 | }, | ||
74 | 223 | |||
75 | 224 | 'C-s': { | ||
76 | 225 | 'condition': function() { | ||
77 | 226 | return this._simulator !== undefined; | ||
78 | 227 | }, | ||
79 | 228 | callback: function() { | ||
80 | 229 | this._simulator.toggle(); | ||
81 | 230 | }, | ||
82 | 231 | help: 'Toggle the simulator' | ||
83 | 232 | }, | ||
84 | 233 | |||
85 | 234 | 'S-d': { | ||
86 | 235 | callback: function(evt) { | ||
87 | 236 | /* global saveAs: false */ | ||
88 | 237 | this.env.exportEnvironment(function(r) { | ||
89 | 238 | var exportData = JSON.stringify(r.result, undefined, 2); | ||
90 | 239 | var exportBlob = new Blob([exportData], | ||
91 | 240 | {type: 'application/json;charset=utf-8'}); | ||
92 | 241 | saveAs(exportBlob, 'export.json'); | ||
93 | 242 | }); | ||
94 | 243 | }, | ||
95 | 244 | help: 'Export the environment' | ||
96 | 212 | } | 245 | } |
97 | 246 | |||
98 | 213 | }, | 247 | }, |
99 | 214 | 248 | ||
100 | 215 | /** | 249 | /** |
101 | @@ -248,14 +282,14 @@ | |||
102 | 248 | */ | 282 | */ |
103 | 249 | activateHotkeys: function() { | 283 | activateHotkeys: function() { |
104 | 250 | var key_map = { | 284 | var key_map = { |
106 | 251 | '/': 191, '?': 63, | 285 | '/': 191, '?': 63, '+': 187, '-': 189, |
107 | 252 | enter: 13, esc: 27, backspace: 8, | 286 | enter: 13, esc: 27, backspace: 8, |
108 | 253 | tab: 9, pageup: 33, pagedown: 34}; | 287 | tab: 9, pageup: 33, pagedown: 34}; |
109 | 254 | var code_map = {}; | 288 | var code_map = {}; |
110 | 255 | Y.each(key_map, function(v, k) { | 289 | Y.each(key_map, function(v, k) { |
111 | 256 | code_map[v] = k; | 290 | code_map[v] = k; |
112 | 257 | }); | 291 | }); |
114 | 258 | Y.one(window).on('keydown', function(evt) { | 292 | this._keybindings = Y.one(window).on('keydown', function(evt) { |
115 | 259 | //Normalize key-code | 293 | //Normalize key-code |
116 | 260 | var symbolic = []; | 294 | var symbolic = []; |
117 | 261 | if (evt.ctrlKey) { symbolic.push('C');} | 295 | if (evt.ctrlKey) { symbolic.push('C');} |
118 | @@ -266,12 +300,21 @@ | |||
119 | 266 | var trigger = symbolic.join('-'); | 300 | var trigger = symbolic.join('-'); |
120 | 267 | var spec = this.keybindings[trigger]; | 301 | var spec = this.keybindings[trigger]; |
121 | 268 | if (spec) { | 302 | if (spec) { |
122 | 303 | if (spec.condition && !spec.condition.call(this)) { | ||
123 | 304 | // Note that when a condition check fails | ||
124 | 305 | // the event still propagates. | ||
125 | 306 | return; | ||
126 | 307 | } | ||
127 | 269 | var target = Y.one(spec.target); | 308 | var target = Y.one(spec.target); |
128 | 270 | if (target) { | 309 | if (target) { |
129 | 271 | if (spec.toggle) { target.toggleView(); } | 310 | if (spec.toggle) { target.toggleView(); } |
130 | 272 | if (spec.focus) { target.focus(); } | 311 | if (spec.focus) { target.focus(); } |
131 | 273 | } | 312 | } |
132 | 274 | if (spec.callback) { spec.callback.call(this, evt, target); } | 313 | if (spec.callback) { spec.callback.call(this, evt, target); } |
133 | 314 | // HACK w/o context/view restriction but right direction | ||
134 | 315 | if (spec.fire) { | ||
135 | 316 | this.views.environment.instance.topo.fire(spec.fire); | ||
136 | 317 | } | ||
137 | 275 | // If we handled the event nothing else has to. | 318 | // If we handled the event nothing else has to. |
138 | 276 | evt.stopPropagation(); | 319 | evt.stopPropagation(); |
139 | 277 | evt.preventDefault(); | 320 | evt.preventDefault(); |
140 | @@ -521,6 +564,9 @@ | |||
141 | 521 | @method destructor | 564 | @method destructor |
142 | 522 | */ | 565 | */ |
143 | 523 | destructor: function() { | 566 | destructor: function() { |
144 | 567 | if (this._keybindings) { | ||
145 | 568 | this._keybindings.detach(); | ||
146 | 569 | } | ||
147 | 524 | Y.each( | 570 | Y.each( |
148 | 525 | [this.env, this.db, this.charm_store, this.notifications, | 571 | [this.env, this.db, this.charm_store, this.notifications, |
149 | 526 | this.landscape, this.endpointsController], | 572 | this.landscape, this.endpointsController], |
150 | 527 | 573 | ||
151 | === modified file 'app/config-debug.js' | |||
152 | --- app/config-debug.js 2013-05-21 01:16:15 +0000 | |||
153 | +++ app/config-debug.js 2013-05-23 16:07:28 +0000 | |||
154 | @@ -45,7 +45,8 @@ | |||
155 | 45 | sandbox: true, | 45 | sandbox: true, |
156 | 46 | // When in sandbox mode should we create events to simulate a live env. | 46 | // When in sandbox mode should we create events to simulate a live env. |
157 | 47 | // You can also use the :flags:/simulateEvents feature flag. | 47 | // You can also use the :flags:/simulateEvents feature flag. |
159 | 48 | simulateEvents: false, | 48 | // There is also a hotkey to toggle the simulator. |
160 | 49 | simulateEvents: true, | ||
161 | 49 | readOnly: false, | 50 | readOnly: false, |
162 | 50 | login_help: 'For this demonstration, use the password "admin" to connect.' | 51 | login_help: 'For this demonstration, use the password "admin" to connect.' |
163 | 51 | }; | 52 | }; |
164 | 52 | 53 | ||
165 | === modified file 'app/store/env/simulator.js' | |||
166 | --- app/store/env/simulator.js 2013-05-21 12:52:33 +0000 | |||
167 | +++ app/store/env/simulator.js 2013-05-23 16:07:28 +0000 | |||
168 | @@ -484,6 +484,7 @@ | |||
169 | 484 | 484 | ||
170 | 485 | // Invoke on start as well. | 485 | // Invoke on start as well. |
171 | 486 | tick(); | 486 | tick(); |
172 | 487 | console.log('Simulator started'); | ||
173 | 487 | return this; | 488 | return this; |
174 | 488 | 489 | ||
175 | 489 | }, | 490 | }, |
176 | @@ -497,6 +498,22 @@ | |||
177 | 497 | if (this._scheduler) { | 498 | if (this._scheduler) { |
178 | 498 | this._scheduler.cancel(); | 499 | this._scheduler.cancel(); |
179 | 499 | this._scheduler = null; | 500 | this._scheduler = null; |
180 | 501 | console.log('Simulator stopped'); | ||
181 | 502 | } | ||
182 | 503 | return this; | ||
183 | 504 | }, | ||
184 | 505 | |||
185 | 506 | /** | ||
186 | 507 | Toggle the running state of the simulator. | ||
187 | 508 | |||
188 | 509 | @method toggle | ||
189 | 510 | @chainable | ||
190 | 511 | */ | ||
191 | 512 | toggle: function() { | ||
192 | 513 | if (this._scheduler) { | ||
193 | 514 | this.stop(); | ||
194 | 515 | } else { | ||
195 | 516 | this.start(); | ||
196 | 500 | } | 517 | } |
197 | 501 | return this; | 518 | return this; |
198 | 502 | } | 519 | } |
199 | 503 | 520 | ||
200 | === modified file 'app/views/topology/panzoom.js' | |||
201 | --- app/views/topology/panzoom.js 2013-05-17 14:51:05 +0000 | |||
202 | +++ app/views/topology/panzoom.js 2013-05-23 16:07:28 +0000 | |||
203 | @@ -59,7 +59,9 @@ | |||
204 | 59 | 59 | ||
205 | 60 | @event rendered | 60 | @event rendered |
206 | 61 | */ | 61 | */ |
208 | 62 | rendered: 'renderedHandler' | 62 | rendered: 'renderedHandler', |
209 | 63 | zoom_in: 'zoom_in', | ||
210 | 64 | zoom_out: 'zoom_out' | ||
211 | 63 | } | 65 | } |
212 | 64 | }, | 66 | }, |
213 | 65 | 67 | ||
214 | @@ -130,8 +132,8 @@ | |||
215 | 130 | * | 132 | * |
216 | 131 | * @method zoom_out | 133 | * @method zoom_out |
217 | 132 | */ | 134 | */ |
220 | 133 | zoom_out: function(data, context) { | 135 | zoom_out: function(data, self) { |
221 | 134 | var slider = context.slider, | 136 | var slider = this.slider || self.slider, |
222 | 135 | val = slider.get('value'); | 137 | val = slider.get('value'); |
223 | 136 | slider.set('value', val - 25); | 138 | slider.set('value', val - 25); |
224 | 137 | }, | 139 | }, |
225 | @@ -141,8 +143,8 @@ | |||
226 | 141 | * | 143 | * |
227 | 142 | * @method zoom_in | 144 | * @method zoom_in |
228 | 143 | */ | 145 | */ |
231 | 144 | zoom_in: function(data, context) { | 146 | zoom_in: function(data, self) { |
232 | 145 | var slider = context.slider, | 147 | var slider = this.slider || self.slider, |
233 | 146 | val = slider.get('value'); | 148 | val = slider.get('value'); |
234 | 147 | slider.set('value', val + 25); | 149 | slider.set('value', val + 25); |
235 | 148 | }, | 150 | }, |
236 | 149 | 151 | ||
237 | === modified file 'lib/views/stylesheet.less' | |||
238 | --- lib/views/stylesheet.less 2013-05-22 22:30:29 +0000 | |||
239 | +++ lib/views/stylesheet.less 2013-05-23 16:07:28 +0000 | |||
240 | @@ -117,6 +117,7 @@ | |||
241 | 117 | background: url(/juju-ui/assets/images/top_bar.png) repeat-x; | 117 | background: url(/juju-ui/assets/images/top_bar.png) repeat-x; |
242 | 118 | padding: 0; | 118 | padding: 0; |
243 | 119 | position: relative; | 119 | position: relative; |
244 | 120 | overflow: hidden; | ||
245 | 120 | color: @navbar-color; | 121 | color: @navbar-color; |
246 | 121 | .brand { | 122 | .brand { |
247 | 122 | margin: 12px 26px 6px 10px; | 123 | margin: 12px 26px 6px 10px; |
248 | @@ -1725,6 +1726,12 @@ | |||
249 | 1725 | letter-spacing: normal; | 1726 | letter-spacing: normal; |
250 | 1726 | } | 1727 | } |
251 | 1727 | 1728 | ||
252 | 1729 | .yui3-slider-thumb { | ||
253 | 1730 | &:focus { | ||
254 | 1731 | outline: none; | ||
255 | 1732 | } | ||
256 | 1733 | } | ||
257 | 1734 | |||
258 | 1728 | /** Browser Setup **/ | 1735 | /** Browser Setup **/ |
259 | 1729 | @import "browser/reset.less"; | 1736 | @import "browser/reset.less"; |
260 | 1730 | @import "browser/vars.less"; | 1737 | @import "browser/vars.less"; |
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: env/simulator. js topology/ panzoom. js stylesheet. less
A [revision details]
M app/app.js
M app/config-debug.js
M app/store/
M app/views/
M lib/views/