Merge lp:~rharding/juju-gui/deploy-quicksearch into lp:juju-gui/experimental
- deploy-quicksearch
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 1166 |
Proposed branch: | lp:~rharding/juju-gui/deploy-quicksearch |
Merge into: | lp:juju-gui/experimental |
Diff against target: |
716 lines (+345/-30) 16 files modified
app/app.js (+2/-2) app/subapps/browser/browser.js (+20/-6) app/subapps/browser/views/charm.js (+1/-1) app/subapps/browser/views/entity-base.js (+22/-6) app/subapps/browser/views/view.js (+51/-2) app/templates/bundle-token.handlebars (+7/-0) app/templates/charm-token.handlebars (+10/-0) app/views/utils.js (+1/-1) app/widgets/charm-search.js (+94/-8) app/widgets/token.js (+10/-0) lib/views/browser/bws-searchbox.less (+2/-1) lib/views/browser/token.less (+15/-0) test/test_browser_app.js (+38/-2) test/test_browser_charm_details.js (+1/-1) test/test_browser_search_widget.js (+48/-0) test/test_token.js (+23/-0) |
To merge this branch: | bzr merge lp:~rharding/juju-gui/deploy-quicksearch |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju GUI Hackers | Pending | ||
Review via email: mp+193059@code.launchpad.net |
Commit message
Description of the change
Provide a 'deploy' button in quicksearch results.
- Add the control to the tokens for both charms and bundles
- Wire up a new event that the search widget can fire EVT_DEPLOY
- Make views that extend view.js (fullscree/sidebar) watch for that event and
build a proper deploy command.
- Make sure those views get access to the deploy and bundleDeploy helpers from
the browser.js
- Deploying does not actual perform a selection so no search or details takes
place. It's an alternate behavior for quicksearch.
Several drive-by fixes for things. Documented in the reviewer comments.
QA:
The button is feature flagged due to the fact that this quick deploy doesn't
involve the charm cache and causes the inspector to 'hang' for a sec when you
hit the deploy button. It's not an ideal experience.
http://
Search for apa and then click on the + next to apache2. It should deploy to
the canvas. No text is in the quicksearch box, no charm highlighted, etc.
You can also QA this with a bundle.
http://
Now enter "TestB" and get the TestBundle result. Press the + to deploy the
bundle.
Richard Harding (rharding) wrote : | # |
Richard Harding (rharding) wrote : | # |
Review comments
https:/
File app/subapps/
https:/
app/subapps/
this.get(
Only set one deploy method on the details view. They can share the same
code for this. Just the deploy method needs to be different.
https:/
app/subapps/
this.get('deploy');
pass the deploy methods into the sidebar/fullscreen views so that they
can use quicksearch to perform a deploy (vs the typical path from
details view or drag/drop)
https:/
app/subapps/
this.get(
See above
https:/
app/subapps/
document all the things!
https:/
File app/subapps/
https:/
app/subapps/
this.get(
so this changes to just deploy vs a diff method
https:/
File app/subapps/
https:/
app/subapps/
no idea why there were two different "if search" checks. Block them into
one.
https:/
app/subapps/
this is the new event the search-widget grew to handle the deploy
selection.
https:/
app/subapps/
mostly just sharing what the details views for bundles/charms do.
https:/
File app/templates/
https:/
app/templates/
the token only displays this when requested. The search widget passes
this in for use and other token uses don't care.
https:/
app/templates/
we only feature flag the UX since it's a UX driven feature. Simpler than
FF'ing all the code.
https:/
File app/views/utils.js (right):
https:/
app/views/
Gary Poster (gary) wrote : | # |
QA thought: what is the expected/desired behavior of the search dropdown
when the deploy button is clicked? I'm expecting that it should close.
Richard Harding (rharding) wrote : | # |
On 2013/10/29 18:41:03, gary.poster wrote:
> QA thought: what is the expected/desired behavior of the search
dropdown when
> the deploy button is clicked? I'm expecting that it should close.
It should close when you click the deploy button. If it's not then I'd
be curious what browser/
QA here.
Be aware that the input box is still 'live' so if you click into it,
mouse over it, you might get it to pop back?
Gary Poster (gary) wrote : | # |
LGTM with comments; thank you!
Gary
https:/
File app/subapps/
https:/
app/subapps/
On 2013/10/29 18:31:54, rharding wrote:
> document all the things!
:-) cool
https:/
File app/subapps/
https:/
app/subapps/
this.get(
On 2013/10/29 18:31:54, rharding wrote:
> so this changes to just deploy vs a diff method
The only downside to the new approach is that sometimes we separate the
two names (deploy vs deployBundle) and sometimes we merge them. That
makes it just a bit harder to figure out what's going on while reading.
In your shoes, I'd be tempted to rename the charm-only deploy function
to "deployCharm" so as to make a parallel with "deployBundle" and a
contrast with the more mutable "deploy". I am sure there are other
solutions, some better than this one. Moreover, this is just a
suggestion. What you have done is OK with me as is, and an improvement.
https:/
File app/subapps/
https:/
app/subapps/
On 2013/10/29 18:31:54, rharding wrote:
> no idea why there were two different "if search" checks. Block them
into one.
Heh. cool.
https:/
app/subapps/
So this is the one I'd call deployCharm.
https:/
File app/templates/
https:/
app/templates/
On 2013/10/29 18:31:54, rharding wrote:
> we only feature flag the UX since it's a UX driven feature. Simpler
than FF'ing
> all the code.
Cool.
https:/
File app/views/utils.js (right):
https:/
app/views/
On 2013/10/29 18:31:54, rharding wrote:
> Document the real usage.
Uh-oh, the doc string is becoming dangerously close to being helpful.
;-)
https:/
File app/widgets/
https:/
app/widgets/
This comment, or a variation of it, deserves to be in the code, IMO.
Gary Poster (gary) wrote : | # |
On 2013/10/29 18:42:45, rharding wrote:
> On 2013/10/29 18:41:03, gary.poster wrote:
> > QA thought: what is the expected/desired behavior of the search
dropdown when
> > the deploy button is clicked? I'm expecting that it should close.
> It should close when you click the deploy button. If it's not then I'd
be
> curious what browser/
here.
> Be aware that the input box is still 'live' so if you click into it,
mouse over
> it, you might get it to pop back?
Sounds good. I hadn't qa'd it--you said Jeff would do that. I was
thinking through expectations before reviewing and hoping you agreed
with me. After I reviewed the code I saw you agreed with me. Thanks!
Jeff Pihach (hatch) wrote : | # |
Thanks for this feature! I'll be doing the QA now, just wanted to get my
comments landed so we can discuss.
https:/
File app/subapps/
https:/
app/subapps/
-1) {
Because everywhere else that uses 'deploy' is using it for only charms
and 'deployBundle' for bundles, I feel that this adds in a level of
complexity that isn't easy to follow. I don't see any benefit to this
change.
https:/
app/subapps/
Thanks for adding these in!
https:/
File app/widgets/
https:/
app/widgets/
We should subclass the ac widget vs doing this monkeypatch. otherwise it
will be difficult to debug if the api changes down the road when we
upgrade YUI.
https:/
app/widgets/
You have probably looked into this already, but just in case...Does this
shut down any pending ac requests to avoid the result list from popping
up in the future? I'm just a little concerned that we are hiding the
list element but keeping the ac functional in the background since we
are short circuiting the real code.
https:/
app/widgets/
Instead of simply copying these methods a better approach would simply
be:
this._ac.
or if you subclass it (I think):
MyNewAutocomple
This way any changes to the parent method won't require us to update
this part of the conditional.
https:/
app/widgets/
Thanks for this fix!
https:/
File test/test_
https:/
test/test_
Y.juju.
Shouldn't we also have tests here(and below) for the v3 api?
Jeff Pihach (hatch) wrote : | # |
QA OK
Love this functionality, works great - just wish the button was a little
more clear as to its purpose and that it would be bigger so it's easier
to click.
Jeff Pihach (hatch) wrote : | # |
LGTM as per our discussion on IRC. Thanks for this branch, great
functionality.
- 1167. By Richard Harding
-
Replace deploy with specific calls
- 1168. By Richard Harding
-
Code review tweaks
Richard Harding (rharding) wrote : | # |
Please take a look.
https:/
File app/widgets/
https:/
app/widgets/
(ev.target.
On 2013/10/29 19:09:46, gary.poster wrote:
> I'm guessing we can't have an event registration for
li.search_
> whatever that simply catches the event first because it is a more
specific
> selector, and then calls ev.halt()?
> If not, I suggest changing the body of this block to something like
the
> following.
> this.hide();
> this.handleDepl
> Then handleDeploy can look at the data and do the right thing (that
is, the code
> you have below) within its own code. That feels like a readability
improvement
> to me.
> Alternatively, you could expand this to divide up on the basis of
bundle vs
> charm.
Agreed and in Jeff's review and discussion we agreed that this version
of 'autocomplete' has gotten complex enough to be it's own module.
Three's a card to clean this up. For a start I've moved the logic into
_onDeploy(ev) and called that. It's what we'll have when moved to its
own module.
https:/
app/widgets/
On 2013/10/29 19:21:51, jeff.pihach wrote:
> You have probably looked into this already, but just in case...Does
this shut
> down any pending ac requests to avoid the result list from popping up
in the
> future? I'm just a little concerned that we are hiding the list
element but
> keeping the ac functional in the background since we are short
circuiting the
> real code.
No, that'll have to be a new feature. From what I can tell, there's not
currently request tracking/killing in AC.
https:/
File test/test_
https:/
test/test_
Y.juju.
On 2013/10/29 19:09:46, gary.poster wrote:
> v2? expected v3, but doubt it matters.
True, it's a fakestore never called so it didn't matter. It would make
for one fewer instance to change during the conversion to making v3 live
though so I'll update.
- 1169. By Richard Harding
-
QA on laptop, sync with trunk
- 1170. By Richard Harding
-
Merge with trunk
Richard Harding (rharding) wrote : | # |
*** Submitted:
Provide a 'deploy' button in quicksearch results.
- Add the control to the tokens for both charms and bundles
- Wire up a new event that the search widget can fire EVT_DEPLOY
- Make views that extend view.js (fullscree/sidebar) watch for that
event and
build a proper deploy command.
- Make sure those views get access to the deploy and bundleDeploy
helpers from
the browser.js
- Deploying does not actual perform a selection so no search or details
takes
place. It's an alternate behavior for quicksearch.
Several drive-by fixes for things. Documented in the reviewer comments.
QA:
The button is feature flagged due to the fact that this quick deploy
doesn't
involve the charm cache and causes the inspector to 'hang' for a sec
when you
hit the deploy button. It's not an ideal experience.
http://
Search for apa and then click on the + next to apache2. It should deploy
to
the canvas. No text is in the quicksearch box, no charm highlighted,
etc.
You can also QA this with a bundle.
http://
Now enter "TestB" and get the TestBundle result. Press the + to deploy
the
bundle.
R=gary.poster, jeff.pihach
CC=
https:/
Preview Diff
1 | === modified file 'app/app.js' |
2 | --- app/app.js 2013-10-22 00:24:31 +0000 |
3 | +++ app/app.js 2013-10-30 13:23:35 +0000 |
4 | @@ -572,7 +572,7 @@ |
5 | |
6 | // To use the new service Inspector use the deploy method |
7 | // from the Y.juju.GhostDeployer extension |
8 | - cfg.deploy = Y.bind(this.deployService, this); |
9 | + cfg.deployService = Y.bind(this.deployService, this); |
10 | |
11 | cfg.deployBundle = this.deployBundle.bind(this); |
12 | |
13 | @@ -595,7 +595,7 @@ |
14 | // When someone wants a charm to be deployed they fire an event and we |
15 | // show the charm panel to configure/deploy the service. |
16 | Y.on('initiateDeploy', function(charm, ghostAttributes) { |
17 | - cfg.deploy(charm, ghostAttributes); |
18 | + cfg.deployService(charm, ghostAttributes); |
19 | }, this); |
20 | }, |
21 | |
22 | |
23 | === added file 'app/assets/images/search_add_to_canvas.png' |
24 | Binary files app/assets/images/search_add_to_canvas.png 1970-01-01 00:00:00 +0000 and app/assets/images/search_add_to_canvas.png 2013-10-30 13:23:35 +0000 differ |
25 | === modified file 'app/subapps/browser/browser.js' |
26 | --- app/subapps/browser/browser.js 2013-10-23 00:12:47 +0000 |
27 | +++ app/subapps/browser/browser.js 2013-10-30 13:23:35 +0000 |
28 | @@ -536,10 +536,11 @@ |
29 | activeTab: this._viewState.hash, |
30 | entityId: entityId, |
31 | container: Y.Node.create('<div class="charmview"/>'), |
32 | - deploy: this.get('deploy'), |
33 | - deployBundle: this.get('deployBundle') |
34 | + deployBundle: this.get('deployBundle'), |
35 | + deployService: this.get('deployService') |
36 | }; |
37 | |
38 | + |
39 | // If the only thing that changed was the hash, then don't redraw. It's |
40 | // just someone clicking a tab in the UI. |
41 | if (this._details && this._hasStateChanged('hash') && |
42 | @@ -696,7 +697,10 @@ |
43 | } |
44 | |
45 | if (this._hasStateChanged('viewmode') || forceFullscreen) { |
46 | - var extraCfg = {}; |
47 | + var extraCfg = { |
48 | + deployService: this.get('deployService'), |
49 | + deployBundle: this.get('deployBundle') |
50 | + }; |
51 | if (this._viewState.search || this._viewState.charmID) { |
52 | extraCfg.withHome = true; |
53 | } |
54 | @@ -789,7 +793,9 @@ |
55 | if (this._hasStateChanged('viewmode') || forceSidebar) { |
56 | this._sidebar = new views.Sidebar( |
57 | this._getViewCfg({ |
58 | - container: this.get('container') |
59 | + container: this.get('container'), |
60 | + deployService: this.get('deployService'), |
61 | + deployBundle: this.get('deployBundle') |
62 | })); |
63 | this._sidebar.render(); |
64 | this._sidebar.addTarget(this); |
65 | @@ -1142,11 +1148,19 @@ |
66 | The "deploy" function prompts the user for service configuration and |
67 | deploys a service. |
68 | |
69 | - @attribute deploy |
70 | + @attribute deployService |
71 | @default undefined |
72 | @type {Function} |
73 | */ |
74 | - deploy: {}, |
75 | + deployService: {}, |
76 | + |
77 | + /** |
78 | + * @attribute deployBundle |
79 | + * @default undefined |
80 | + * @type {Function} |
81 | + * |
82 | + */ |
83 | + deployBundle: {}, |
84 | |
85 | /** |
86 | The default viewmode |
87 | |
88 | === modified file 'app/subapps/browser/views/charm.js' |
89 | --- app/subapps/browser/views/charm.js 2013-10-29 22:51:19 +0000 |
90 | +++ app/subapps/browser/views/charm.js 2013-10-30 13:23:35 +0000 |
91 | @@ -93,7 +93,7 @@ |
92 | ghostAttributes = { |
93 | icon: this.get('store').iconpath(charm.get('storeId')) |
94 | }; |
95 | - this.get('deploy').call(null, charm, ghostAttributes); |
96 | + this.get('deployService').call(null, charm, ghostAttributes); |
97 | }, |
98 | |
99 | /** |
100 | |
101 | === modified file 'app/subapps/browser/views/entity-base.js' |
102 | --- app/subapps/browser/views/entity-base.js 2013-10-29 22:51:19 +0000 |
103 | +++ app/subapps/browser/views/entity-base.js 2013-10-30 13:23:35 +0000 |
104 | @@ -526,12 +526,28 @@ |
105 | * The "deploy" function prompts the user for service configuration and |
106 | * deploys a service. |
107 | * |
108 | - * @attribute deploy |
109 | - * @default undefined |
110 | - * @type {Function} |
111 | - * |
112 | - */ |
113 | - deploy: {} |
114 | + * The proper deploy function is provided from the browser subapp. |
115 | + * |
116 | + * @attribute deployService |
117 | + * @default undefined |
118 | + * @type {Function} |
119 | + * |
120 | + */ |
121 | + deployService: {}, |
122 | + |
123 | + /** |
124 | + * The "deploy" function prompts the user for service configuration and |
125 | + * deploys a service. |
126 | + * |
127 | + * The proper deploy function is provided from the browser subapp. |
128 | + * |
129 | + * @attribute deployBundle |
130 | + * @default undefined |
131 | + * @type {Function} |
132 | + * |
133 | + */ |
134 | + deployBundle: {} |
135 | + |
136 | }; |
137 | |
138 | ns.EntityBaseView = EntityBaseView; |
139 | |
140 | === modified file 'app/subapps/browser/views/view.js' |
141 | --- app/subapps/browser/views/view.js 2013-09-26 21:14:50 +0000 |
142 | +++ app/subapps/browser/views/view.js 2013-10-30 13:23:35 +0000 |
143 | @@ -91,14 +91,17 @@ |
144 | this.search.on( |
145 | this.search.EVT_SEARCH_CHANGED, this._searchChanged, this) |
146 | ); |
147 | - } |
148 | |
149 | - if (this.search) { |
150 | this.addEvent( |
151 | this.search.on( |
152 | this.search.EVT_SEARCH_GOHOME, this._goHome, this) |
153 | ); |
154 | |
155 | + this.addEvent( |
156 | + this.search.on( |
157 | + this.search.EVT_DEPLOY, this._deployEntity, this) |
158 | + ); |
159 | + |
160 | // If the showHome attribute is changed, update our html by adding the |
161 | // with-home class to the widget. |
162 | this.after('withHomeChange', function(ev) { |
163 | @@ -123,10 +126,39 @@ |
164 | } |
165 | }, this); |
166 | } |
167 | + |
168 | this._bindViewmodeControls(this.controls); |
169 | }, |
170 | |
171 | /** |
172 | + * Deploy either a bundle or charm given by the quicksearch widget. |
173 | + * |
174 | + * @method _deployEntity |
175 | + * @param {Y.Event} ev the event object from the widget. |
176 | + * |
177 | + */ |
178 | + _deployEntity: function(ev) { |
179 | + var entityType = ev.entityType, |
180 | + entity = ev.data, |
181 | + entityId = ev.id, |
182 | + deployer; |
183 | + |
184 | + if (entityType === 'bundle') { |
185 | + deployer = this.get('deployBundle'); |
186 | + var bundle = new models.Bundle(entity); |
187 | + deployer(bundle.get('data')); |
188 | + } else { |
189 | + deployer = this.get('deployService'); |
190 | + var charm = new models.Charm(entity); |
191 | + var ghostAttributes; |
192 | + ghostAttributes = { |
193 | + icon: this.get('store').iconpath(charm.get('storeId')) |
194 | + }; |
195 | + deployer.call(null, charm, ghostAttributes); |
196 | + } |
197 | + }, |
198 | + |
199 | + /** |
200 | * Force a navigate event when the search widget says "Home" was clicked. |
201 | * |
202 | * @method _goHome |
203 | @@ -278,6 +310,22 @@ |
204 | charmID: {}, |
205 | |
206 | /** |
207 | + * @attribute deployService |
208 | + * @default undefined |
209 | + * @type {Function} |
210 | + * |
211 | + */ |
212 | + deployService: {}, |
213 | + |
214 | + /** |
215 | + * @attribute deployBundle |
216 | + * @default undefined |
217 | + * @type {Function} |
218 | + * |
219 | + */ |
220 | + deployBundle: {}, |
221 | + |
222 | + /** |
223 | The list of filters to be used in the rendering of the view. |
224 | |
225 | This is always handed down from the subapp, but default to something |
226 | @@ -339,6 +387,7 @@ |
227 | 'juju-charm-store', |
228 | 'juju-browser-models', |
229 | 'juju-models', |
230 | + 'juju-bundle-models', |
231 | 'querystring-stringify', |
232 | 'view', |
233 | 'viewmode-controls' |
234 | |
235 | === modified file 'app/templates/bundle-token.handlebars' |
236 | --- app/templates/bundle-token.handlebars 2013-10-30 03:33:36 +0000 |
237 | +++ app/templates/bundle-token.handlebars 2013-10-30 13:23:35 +0000 |
238 | @@ -1,6 +1,13 @@ |
239 | {{! The token class is used by the deploy method in |
240 | test/test_charm_running.py. If you change this name, change the other |
241 | one too! }} |
242 | +{{#if deployButton}} |
243 | + {{#ifFlag 'searchDeploy'}} |
244 | + <a class="deployButton bundle" href=""> |
245 | + <i class="sprite search_add_to_canvas" data-bundleid="{{id}}" title="Deploy bundle"></i> |
246 | + </a> |
247 | + {{/ifFlag}} |
248 | +{{/if}} |
249 | <a href="/bundle/{{id}}" |
250 | class="token {{size}}" |
251 | data-charmid="/bundle/{{id}}"> |
252 | |
253 | === modified file 'app/templates/charm-token.handlebars' |
254 | --- app/templates/charm-token.handlebars 2013-10-30 03:33:36 +0000 |
255 | +++ app/templates/charm-token.handlebars 2013-10-30 13:23:35 +0000 |
256 | @@ -1,6 +1,14 @@ |
257 | {{! The token class is used by the deploy method in |
258 | test/test_charm_running.py. If you change this name, change the other |
259 | one too! }} |
260 | + |
261 | +{{#if deployButton}} |
262 | + {{#ifFlag 'searchDeploy'}} |
263 | + <a class="deployButton" href=""> |
264 | + <i class="sprite search_add_to_canvas" data-charmid="{{storeId}}" title="Deploy service"></i> |
265 | + </a> |
266 | + {{/ifFlag}} |
267 | +{{/if}} |
268 | <a href="/{{storeId}}" |
269 | class="token {{size}}" |
270 | data-charmid="{{storeId}}"> |
271 | @@ -21,6 +29,8 @@ |
272 | <span class="title"> |
273 | {{ name }} |
274 | </span> |
275 | + |
276 | + |
277 | {{#unless_eq size 'tiny'}} |
278 | <span class="metadata"> |
279 | <div><strong>Deployed {{downloads}} {{pluralize 'time' downloads}}</strong></div> |
280 | |
281 | === modified file 'app/views/utils.js' |
282 | --- app/views/utils.js 2013-10-25 15:33:34 +0000 |
283 | +++ app/views/utils.js 2013-10-30 13:23:35 +0000 |
284 | @@ -1564,7 +1564,7 @@ |
285 | /* |
286 | * Check if a flag is set. |
287 | * |
288 | - * {{ifFlag 'flag_name'}} |
289 | + * {{#ifFlag 'flag_name'}} {{/ifFlag}} |
290 | * |
291 | */ |
292 | Y.Handlebars.registerHelper('ifFlag', function(flag, options) { |
293 | |
294 | === modified file 'app/widgets/charm-search.js' |
295 | --- app/widgets/charm-search.js 2013-09-26 21:14:50 +0000 |
296 | +++ app/widgets/charm-search.js 2013-10-30 13:23:35 +0000 |
297 | @@ -50,6 +50,7 @@ |
298 | EVT_CLEAR_SEARCH: 'clear_search', |
299 | EVT_SEARCH_CHANGED: 'search_changed', |
300 | EVT_SEARCH_GOHOME: 'go_home', |
301 | + EVT_DEPLOY: 'charm_deploy', |
302 | |
303 | TEMPLATE: templates['browser-search'], |
304 | |
305 | @@ -207,7 +208,8 @@ |
306 | // be false. |
307 | var tokenAttrs = Y.merge(charm.getAttrs(), { |
308 | size: 'tiny', |
309 | - is_approved: false |
310 | + is_approved: false, |
311 | + deployButton: isCategory(charm.get('id')) ? false : true |
312 | }); |
313 | var token = new ns.Token(tokenAttrs); |
314 | var html = Y.Node.create(token.TEMPLATE(token.getAttrs())); |
315 | @@ -245,6 +247,7 @@ |
316 | * |
317 | */ |
318 | _setupAutocomplete: function() { |
319 | + var self = this; |
320 | // Bind out helpers to the current objects context, not the auto |
321 | // complete widget context.. |
322 | var fetchSuggestions = Y.bind(this._fetchSuggestions, this); |
323 | @@ -266,6 +269,77 @@ |
324 | }, |
325 | source: fetchSuggestions |
326 | }); |
327 | + |
328 | + // Holder for the deploy logic the AC uses when clicking on a deploy |
329 | + // icon from a result item. |
330 | + this.ac._onDeploy = function(ev) { |
331 | + var isBundle = false, |
332 | + data, |
333 | + found, |
334 | + id; |
335 | + |
336 | + // Fire an event up to the View with the charm information so that |
337 | + // it can proceed to build/send the deploy information out to |
338 | + // the environment. |
339 | + id = ev.target.getData('charmId'); |
340 | + |
341 | + if (!id) { |
342 | + // try to see if this is a bundle clicked on. |
343 | + id = ev.target.getData('bundleId'); |
344 | + isBundle = true; |
345 | + } |
346 | + // Find the charm data for the selected item from the set of |
347 | + // results. |
348 | + found = this.get('results').filter(function(result) { |
349 | + if (isBundle) { |
350 | + if (result.raw.bundle.id === id) { |
351 | + return result; |
352 | + } |
353 | + } else { |
354 | + if (result.raw.charm.id === id) { |
355 | + return result; |
356 | + } |
357 | + } |
358 | + }); |
359 | + |
360 | + // Make sure that we've found a result before returning. |
361 | + if (found.length === 0) { |
362 | + console.error( |
363 | + 'Clicked deploy on an item we could not find in results.'); |
364 | + } else { |
365 | + if (isBundle) { |
366 | + data = found[0].raw.bundle; |
367 | + } else { |
368 | + data = found[0].raw.charm; |
369 | + } |
370 | + |
371 | + self.fire(self.EVT_DEPLOY, { |
372 | + id: id, |
373 | + data: data, |
374 | + entityType: isBundle ? 'bundle' : 'charm' |
375 | + }); |
376 | + |
377 | + } |
378 | + |
379 | + }; |
380 | + |
381 | + this.ac._onItemClick = function(ev) { |
382 | + // If the selection is coming from the deployButton then we kind of |
383 | + // ignore the way autocomplete works. It's more of a 'quick search' |
384 | + // with a deploy option. No search is really performed after the |
385 | + // deploy button is selected. |
386 | + if (ev.target.hasClass('search_add_to_canvas')) { |
387 | + // Hide the autocomplete widget. You've selected something |
388 | + // that's not really a suggestion, but it should still go away. |
389 | + this.hide(); |
390 | + this._onDeploy(ev); |
391 | + } else { |
392 | + var itemNode = ev.currentTarget; |
393 | + this.set('active_item', itemNode); |
394 | + this.selectItem(itemNode, ev); |
395 | + } |
396 | + }; |
397 | + |
398 | this.ac.render(); |
399 | |
400 | this.ac.get('inputNode').on('focus', function(ev) { |
401 | @@ -281,6 +355,7 @@ |
402 | this.get('boundingBox').delegate('click', function(ev) { |
403 | ev.halt(); |
404 | }, 'a', this); |
405 | + |
406 | }, |
407 | |
408 | /** |
409 | @@ -293,14 +368,25 @@ |
410 | */ |
411 | _suggestionSelected: function(ev) { |
412 | ev.halt(); |
413 | + |
414 | var change, |
415 | - newVal, |
416 | - charmid = ev.result.raw.charm.id, |
417 | - form = this.get('boundingBox').one('form'); |
418 | - |
419 | - if (charmid.substr(0, 4) === 'cat:') { |
420 | + form = this.get('boundingBox').one('form'), |
421 | + id, |
422 | + isBundle = false, |
423 | + newVal; |
424 | + |
425 | + if (ev.result.raw.charm) { |
426 | + id = ev.result.raw.charm.id; |
427 | + } else { |
428 | + // Currently we have to pretend to be a charm. |
429 | + // XXX: We should support the idea of a bundle separate from charm? Go |
430 | + // with entity? Something to clean up. |
431 | + id = '/bundle/' + ev.result.raw.bundle.id; |
432 | + } |
433 | + |
434 | + if (id.substr(0, 4) === 'cat:') { |
435 | form.one('input').set('value', ''); |
436 | - var category = charmid.match(/([^\/]+)-\d\/?/); |
437 | + var category = id.match(/([^\/]+)-\d\/?/); |
438 | change = { |
439 | charmID: null, |
440 | search: true, |
441 | @@ -318,7 +404,7 @@ |
442 | form.one('input').set('value', ev.result.text); |
443 | newVal = ev.result.text; |
444 | change = { |
445 | - charmID: charmid, |
446 | + charmID: id, |
447 | filter: { |
448 | categories: [], |
449 | text: newVal, |
450 | |
451 | === modified file 'app/widgets/token.js' |
452 | --- app/widgets/token.js 2013-10-09 22:20:53 +0000 |
453 | +++ app/widgets/token.js 2013-10-30 13:23:35 +0000 |
454 | @@ -216,6 +216,16 @@ |
455 | charmIcons: { |
456 | setter: '_charmIconsSetter' |
457 | }, |
458 | + |
459 | + /** |
460 | + * @attribute deployButton |
461 | + * @default false |
462 | + * @type {Boolean} |
463 | + */ |
464 | + deployButton: { |
465 | + value: false |
466 | + }, |
467 | + |
468 | /** |
469 | * @attribute downloads |
470 | * @default undefined |
471 | |
472 | === modified file 'lib/views/browser/bws-searchbox.less' |
473 | --- lib/views/browser/bws-searchbox.less 2013-10-29 03:07:12 +0000 |
474 | +++ lib/views/browser/bws-searchbox.less 2013-10-30 13:23:35 +0000 |
475 | @@ -66,11 +66,12 @@ |
476 | } |
477 | } |
478 | |
479 | - .token { |
480 | + .token.tiny { |
481 | padding: 8px 0; |
482 | |
483 | .title { |
484 | .type15; |
485 | + margin-top: 2px; |
486 | } |
487 | |
488 | .icon, |
489 | |
490 | === modified file 'lib/views/browser/token.less' |
491 | --- lib/views/browser/token.less 2013-10-30 03:33:36 +0000 |
492 | +++ lib/views/browser/token.less 2013-10-30 13:23:35 +0000 |
493 | @@ -2,6 +2,21 @@ |
494 | .border-box; |
495 | position: relative; |
496 | |
497 | + .deployButton { |
498 | + display: block; |
499 | + float: right; |
500 | + margin-top: 19px; |
501 | + |
502 | + /* Bundle tokens are taller due to the charm icons inside. */ |
503 | + &.bundle { |
504 | + margin-top: 34px; |
505 | + } |
506 | + |
507 | + i.sprite { |
508 | + display: block; |
509 | + } |
510 | + } |
511 | + |
512 | .token { |
513 | display: block; |
514 | // Clear floats: |
515 | |
516 | === modified file 'test/test_browser_app.js' |
517 | --- test/test_browser_app.js 2013-10-29 19:38:27 +0000 |
518 | +++ test/test_browser_app.js 2013-10-30 13:23:35 +0000 |
519 | @@ -34,7 +34,7 @@ |
520 | |
521 | (function() { |
522 | describe('browser fullscreen view', function() { |
523 | - var container, FullScreen, view, views, Y; |
524 | + var container, FullScreen, utils, view, views, Y; |
525 | |
526 | before(function(done) { |
527 | Y = YUI(GlobalConfig).use( |
528 | @@ -43,6 +43,7 @@ |
529 | 'juju-tests-utils', |
530 | 'subapp-browser-fullscreen', function(Y) { |
531 | views = Y.namespace('juju.browser.views'); |
532 | + utils = Y.namespace('juju-tests.utils'); |
533 | FullScreen = views.FullScreen; |
534 | done(); |
535 | }); |
536 | @@ -176,6 +177,23 @@ |
537 | }); |
538 | }); |
539 | |
540 | + it('picks up the search widget deploy event', function(done) { |
541 | + var container = utils.makeContainer('subapp-browser'), |
542 | + fakeStore = new Y.juju.charmworld.APIv2({}); |
543 | + view = new FullScreen({ |
544 | + charmID: 'precise/jenkins-13', |
545 | + store: fakeStore |
546 | + }); |
547 | + |
548 | + view._deployEntity = function() { |
549 | + container.remove(true); |
550 | + done(); |
551 | + }; |
552 | + |
553 | + view.render(container); |
554 | + view.search.fire(view.search.EVT_DEPLOY); |
555 | + }); |
556 | + |
557 | }); |
558 | })(); |
559 | |
560 | @@ -228,7 +246,7 @@ |
561 | |
562 | (function() { |
563 | describe('browser sidebar view', function() { |
564 | - var Y, container, view, views, Sidebar; |
565 | + var Y, container, utils, view, views, Sidebar; |
566 | |
567 | before(function(done) { |
568 | Y = YUI(GlobalConfig).use( |
569 | @@ -240,6 +258,7 @@ |
570 | 'subapp-browser-sidebar', |
571 | function(Y) { |
572 | views = Y.namespace('juju.browser.views'); |
573 | + utils = Y.namespace('juju-tests.utils'); |
574 | Sidebar = views.Sidebar; |
575 | done(); |
576 | }); |
577 | @@ -371,6 +390,23 @@ |
578 | }); |
579 | }); |
580 | |
581 | + it('picks up the search widget deploy event', function(done) { |
582 | + var container = utils.makeContainer('subapp-browser'), |
583 | + fakeStore = new Y.juju.charmworld.APIv2({}); |
584 | + view = new Sidebar({ |
585 | + charmID: 'precise/jenkins-13', |
586 | + store: fakeStore |
587 | + }); |
588 | + |
589 | + view._deployEntity = function() { |
590 | + container.remove(true); |
591 | + done(); |
592 | + }; |
593 | + |
594 | + view.render(container); |
595 | + view.search.fire(view.search.EVT_DEPLOY); |
596 | + }); |
597 | + |
598 | }); |
599 | })(); |
600 | |
601 | |
602 | === modified file 'test/test_browser_charm_details.js' |
603 | --- test/test_browser_charm_details.js 2013-10-29 22:51:19 +0000 |
604 | +++ test/test_browser_charm_details.js 2013-10-30 13:23:35 +0000 |
605 | @@ -357,7 +357,7 @@ |
606 | container: utils.makeContainer(), |
607 | store: fakeStore |
608 | }); |
609 | - view.set('deploy', function(charm, serviceAttrs) { |
610 | + view.set('deployService', function(charm, serviceAttrs) { |
611 | var serviceCharm = view.get('entity'); |
612 | assert.deepEqual(charm, serviceCharm); |
613 | assert.equal(charm.get('id'), 'cs:precise/ceph-9'); |
614 | |
615 | === modified file 'test/test_browser_search_widget.js' |
616 | --- test/test_browser_search_widget.js 2013-09-26 21:14:50 +0000 |
617 | +++ test/test_browser_search_widget.js 2013-10-30 13:23:35 +0000 |
618 | @@ -155,6 +155,24 @@ |
619 | }); |
620 | }); |
621 | |
622 | + it('generates a bundle change event when bundle selected', function(done) { |
623 | + search.on(search.EVT_SEARCH_CHANGED, function(ev) { |
624 | + assert.equal(ev.newVal, 'TestBundle'); |
625 | + // The bundle id gets prefixed with the /bundle to help routing. |
626 | + assert.equal(ev.change.charmID, '/bundle/~hatch/wiki/7/TestBundle'); |
627 | + assert.equal(ev.change.filter.categories.length, 0); |
628 | + done(); |
629 | + }); |
630 | + |
631 | + search._suggestionSelected({ |
632 | + halt: function() {}, |
633 | + result: { |
634 | + raw: {bundle: {id: '~hatch/wiki/7/TestBundle'}}, |
635 | + text: 'TestBundle' |
636 | + } |
637 | + }); |
638 | + }); |
639 | + |
640 | it('supports an onHome event', function(done) { |
641 | search.on(search.EVT_SEARCH_GOHOME, function() { |
642 | done(); |
643 | @@ -188,6 +206,8 @@ |
644 | utils = Y.namespace('juju-tests.utils'); |
645 | data = utils.loadFixture('data/autocomplete.json'); |
646 | |
647 | + cleanIconHelper = utils.stubCharmIconPath(); |
648 | + |
649 | // Need the handlebars helper for the token to render. |
650 | Y.Handlebars.registerHelper( |
651 | 'charmFilePath', |
652 | @@ -307,4 +327,32 @@ |
653 | }); |
654 | }); |
655 | |
656 | + it('fires deploy event when the deploy button is selected', function(done) { |
657 | + window.flags.searchDeploy = true; |
658 | + // This is heading into the private, non-publicized events of the AC |
659 | + // widget in an effort to hit the html on render after results come |
660 | + // back. |
661 | + search.ac.after('resultsChange', function(ev) { |
662 | + //Find one of the deployable results and trigger click event. |
663 | + var chosenOne = container.one('.search_add_to_canvas'); |
664 | + chosenOne.simulate('click'); |
665 | + }); |
666 | + |
667 | + search.on(search.EVT_DEPLOY, function(ev) { |
668 | + // Verify that the event was called with the correct payload to deploy. |
669 | + assert.equal(ev.entityType, 'charm'); |
670 | + assert.equal(ev.id, 'precise/apache2-passenger-3'); |
671 | + assert.equal(ev.data.url, 'cs:precise/apache2-passenger-3'); |
672 | + window.flags = {}; |
673 | + done(); |
674 | + }); |
675 | + |
676 | + // hack into the ac widget to simulate the valueChange event |
677 | + search.ac._afterValueChange({ |
678 | + newVal: 'a', |
679 | + src: 'ui' |
680 | + }); |
681 | + |
682 | + }); |
683 | + |
684 | }); |
685 | |
686 | === modified file 'test/test_token.js' |
687 | --- test/test_token.js 2013-10-10 04:28:39 +0000 |
688 | +++ test/test_token.js 2013-10-30 13:23:35 +0000 |
689 | @@ -246,4 +246,27 @@ |
690 | assert.equal(token_container.one('span.charms').all('img').size(), 4); |
691 | }); |
692 | |
693 | + it('renders the deployer button when asked to', function() { |
694 | + window.flags.searchDeploy = true; |
695 | + var token = new Token({ |
696 | + size: 'tiny', |
697 | + storeId: 'test', |
698 | + deployButton: true, |
699 | + description: 'some description', |
700 | + mainCategory: 'app-servers', |
701 | + iconUrl: 'http://localhost.svg' |
702 | + }); |
703 | + |
704 | + token.render(token_container); |
705 | + assert.notEqual( |
706 | + token_container.get('innerHTML').indexOf('deployButton'), |
707 | + -1 |
708 | + ); |
709 | + |
710 | + var button = token_container.one('.deployButton'); |
711 | + var sprite = button.one('i'); |
712 | + assert.equal(sprite.getAttribute('data-charmid'), 'test'); |
713 | + window.flags = {}; |
714 | + }); |
715 | + |
716 | }); |
Reviewers: mp+193059_ code.launchpad. net,
Message:
Please take a look.
Description:
Provide a 'deploy' button in quicksearch results.
- Add the control to the tokens for both charms and bundles
- Wire up a new event that the search widget can fire EVT_DEPLOY
- Make views that extend view.js (fullscree/sidebar) watch for that
event and
build a proper deploy command.
- Make sure those views get access to the deploy and bundleDeploy
helpers from
the browser.js
- Deploying does not actual perform a selection so no search or details
takes
place. It's an alternate behavior for quicksearch.
Several drive-by fixes for things. Documented in the reviewer comments.
QA:
The button is feature flagged due to the fact that this quick deploy
doesn't
involve the charm cache and causes the inspector to 'hang' for a sec
when you
hit the deploy button. It's not an ideal experience.
http:// gui:8888/ :flags: /searchDeploy
Search for apa and then click on the + next to apache2. It should deploy
to
the canvas. No text is in the quicksearch box, no charm highlighted,
etc.
You can also QA this with a bundle.
http:// gui:8888/ :flags: /searchDeploy/ charmworldv3/
Now enter "TestB" and get the TestBundle result. Press the + to deploy
the
bundle.
https:/ /code.launchpad .net/~rharding/ juju-gui/ deploy- quicksearch/ +merge/ 193059
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/18920045/
Affected files (+320, -20 lines): images/ search_ add_to_ canvas. png browser/ browser. js browser/ views/bundle. js browser/ views/entity- base.js browser/ views/view. js bundle- token.handlebar s charm-token. handlebars charm-search. js token.js browser/ bws-searchbox. less browser/ token.less browser_ app.js browser_ search_ widget. js bundle_ details_ view.js
A [revision details]
A app/assets/
M app/subapps/
M app/subapps/
M app/subapps/
M app/subapps/
M app/templates/
M app/templates/
M app/views/utils.js
M app/widgets/
M app/widgets/
M lib/views/
M lib/views/
M test/test_
M test/test_
M test/test_
M test/test_token.js