Merge lp:~rharding/juju-gui/deploy-quicksearch into lp:juju-gui/experimental

Proposed by Richard Harding
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
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+193059@code.launchpad.net

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://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://codereview.appspot.com/18920045/

To post a comment you must log in.
Revision history for this message
Richard Harding (rharding) wrote :

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):
   A [revision details]
   A app/assets/images/search_add_to_canvas.png
   M app/subapps/browser/browser.js
   M app/subapps/browser/views/bundle.js
   M app/subapps/browser/views/entity-base.js
   M app/subapps/browser/views/view.js
   M app/templates/bundle-token.handlebars
   M app/templates/charm-token.handlebars
   M app/views/utils.js
   M app/widgets/charm-search.js
   M app/widgets/token.js
   M lib/views/browser/bws-searchbox.less
   M lib/views/browser/token.less
   M test/test_browser_app.js
   M test/test_browser_search_widget.js
   M test/test_bundle_details_view.js
   M test/test_token.js

Revision history for this message
Richard Harding (rharding) wrote :
Download full text (5.3 KiB)

Review comments

https://codereview.appspot.com/18920045/diff/20001/app/subapps/browser/browser.js
File app/subapps/browser/browser.js (right):

https://codereview.appspot.com/18920045/diff/20001/app/subapps/browser/browser.js#newcode542
app/subapps/browser/browser.js:542: extraCfg.deploy =
this.get('deployBundle');
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://codereview.appspot.com/18920045/diff/20001/app/subapps/browser/browser.js#newcode708
app/subapps/browser/browser.js:708: extraCfg.deploy =
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://codereview.appspot.com/18920045/diff/20001/app/subapps/browser/browser.js#newcode800
app/subapps/browser/browser.js:800: deployBundle:
this.get('deployBundle')
See above

https://codereview.appspot.com/18920045/diff/20001/app/subapps/browser/browser.js#newcode1165
app/subapps/browser/browser.js:1165: deployBundle: {},
document all the things!

https://codereview.appspot.com/18920045/diff/20001/app/subapps/browser/views/bundle.js
File app/subapps/browser/views/bundle.js (right):

https://codereview.appspot.com/18920045/diff/20001/app/subapps/browser/views/bundle.js#newcode76
app/subapps/browser/views/bundle.js:76:
this.get('deploy')(bundle.get('data'));
so this changes to just deploy vs a diff method

https://codereview.appspot.com/18920045/diff/20001/app/subapps/browser/views/view.js
File app/subapps/browser/views/view.js (right):

https://codereview.appspot.com/18920045/diff/20001/app/subapps/browser/views/view.js#newcode93
app/subapps/browser/views/view.js:93: );
no idea why there were two different "if search" checks. Block them into
one.

https://codereview.appspot.com/18920045/diff/20001/app/subapps/browser/views/view.js#newcode101
app/subapps/browser/views/view.js:101: this.search.on(
this is the new event the search-widget grew to handle the deploy
selection.

https://codereview.appspot.com/18920045/diff/20001/app/subapps/browser/views/view.js#newcode140
app/subapps/browser/views/view.js:140: _deployEntity: function(ev) {
mostly just sharing what the details views for bundles/charms do.

https://codereview.appspot.com/18920045/diff/20001/app/templates/bundle-token.handlebars
File app/templates/bundle-token.handlebars (right):

https://codereview.appspot.com/18920045/diff/20001/app/templates/bundle-token.handlebars#newcode4
app/templates/bundle-token.handlebars:4: {{#if deployButton}}
the token only displays this when requested. The search widget passes
this in for use and other token uses don't care.

https://codereview.appspot.com/18920045/diff/20001/app/templates/bundle-token.handlebars#newcode5
app/templates/bundle-token.handlebars:5: {{#ifFlag 'searchDeploy'}}
we only feature flag the UX since it's a UX driven feature. Simpler than
FF'ing all the code.

https://codereview.appspot.com/18920045/diff/20001/app/views/utils.js
File app/views/utils.js (right):

https://codereview.appspot.com/18920045/diff/20001/app/views/utils.js#newcode1567
app/views/utils.js...

Read more...

Revision history for this message
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.

https://codereview.appspot.com/18920045/

Revision history for this message
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/search/selection you used. It does close in my
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?

https://codereview.appspot.com/18920045/

Revision history for this message
Gary Poster (gary) wrote :
Download full text (4.6 KiB)

LGTM with comments; thank you!

Gary

https://codereview.appspot.com/18920045/diff/20001/app/subapps/browser/browser.js
File app/subapps/browser/browser.js (right):

https://codereview.appspot.com/18920045/diff/20001/app/subapps/browser/browser.js#newcode1165
app/subapps/browser/browser.js:1165: deployBundle: {},
On 2013/10/29 18:31:54, rharding wrote:
> document all the things!

:-) cool

https://codereview.appspot.com/18920045/diff/20001/app/subapps/browser/views/bundle.js
File app/subapps/browser/views/bundle.js (right):

https://codereview.appspot.com/18920045/diff/20001/app/subapps/browser/views/bundle.js#newcode76
app/subapps/browser/views/bundle.js:76:
this.get('deploy')(bundle.get('data'));
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://codereview.appspot.com/18920045/diff/20001/app/subapps/browser/views/view.js
File app/subapps/browser/views/view.js (right):

https://codereview.appspot.com/18920045/diff/20001/app/subapps/browser/views/view.js#newcode93
app/subapps/browser/views/view.js:93: );
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://codereview.appspot.com/18920045/diff/20001/app/subapps/browser/views/view.js#newcode318
app/subapps/browser/views/view.js:318: deploy: {},
So this is the one I'd call deployCharm.

https://codereview.appspot.com/18920045/diff/20001/app/templates/bundle-token.handlebars
File app/templates/bundle-token.handlebars (right):

https://codereview.appspot.com/18920045/diff/20001/app/templates/bundle-token.handlebars#newcode5
app/templates/bundle-token.handlebars:5: {{#ifFlag 'searchDeploy'}}
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://codereview.appspot.com/18920045/diff/20001/app/views/utils.js
File app/views/utils.js (right):

https://codereview.appspot.com/18920045/diff/20001/app/views/utils.js#newcode1567
app/views/utils.js:1567: * {{#ifFlag 'flag_name'}} {{/ifFlag}}
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://codereview.appspot.com/18920045/diff/20001/app/widgets/charm-search.js
File app/widgets/charm-search.js (right):

https://codereview.appspot.com/18920045/diff/20001/app/widgets/charm-search.js#newcode273
app/widgets/charm-search.js:273: this.ac._onItemClick = function(ev) {
This comment, or a variation of it, deserves to be in the code, IMO.

https://codereview.appspot.com/18920045/diff/20001...

Read more...

Revision history for this message
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/search/selection you used. It does close in my 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?

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!

https://codereview.appspot.com/18920045/

Revision history for this message
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://codereview.appspot.com/18920045/diff/20001/app/subapps/browser/browser.js
File app/subapps/browser/browser.js (right):

https://codereview.appspot.com/18920045/diff/20001/app/subapps/browser/browser.js#newcode541
app/subapps/browser/browser.js:541: if (entityId.indexOf('bundle') !==
-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://codereview.appspot.com/18920045/diff/20001/app/subapps/browser/browser.js#newcode1165
app/subapps/browser/browser.js:1165: deployBundle: {},
Thanks for adding these in!

https://codereview.appspot.com/18920045/diff/20001/app/widgets/charm-search.js
File app/widgets/charm-search.js (right):

https://codereview.appspot.com/18920045/diff/20001/app/widgets/charm-search.js#newcode273
app/widgets/charm-search.js:273: this.ac._onItemClick = function(ev) {
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://codereview.appspot.com/18920045/diff/20001/app/widgets/charm-search.js#newcode327
app/widgets/charm-search.js:327: this.hide();
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://codereview.appspot.com/18920045/diff/20001/app/widgets/charm-search.js#newcode331
app/widgets/charm-search.js:331: var itemNode = ev.currentTarget;
Instead of simply copying these methods a better approach would simply
be:

this._ac.prototype._onItemClick(ev);

or if you subclass it (I think):

MyNewAutocomplete.superclass.constructor._onItemClick(ev).bind(this);

This way any changes to the parent method won't require us to update
this part of the conditional.

https://codereview.appspot.com/18920045/diff/20001/app/widgets/charm-search.js#newcode373
app/widgets/charm-search.js:373: id = ev.result.raw.charm.id;
Thanks for this fix!

https://codereview.appspot.com/18920045/diff/20001/test/test_browser_app.js
File test/test_browser_app.js (right):

https://codereview.appspot.com/18920045/diff/20001/test/test_browser_app.js#newcode182
test/test_browser_app.js:182: fakeStore = new
Y.juju.charmworld.APIv2({});
Shouldn't we also have tests here(and below) for the v3 api?

https://codereview.appspot.com/18920045/

Revision history for this message
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.

https://codereview.appspot.com/18920045/

Revision history for this message
Jeff Pihach (hatch) wrote :

LGTM as per our discussion on IRC. Thanks for this branch, great
functionality.

https://codereview.appspot.com/18920045/

1167. By Richard Harding

Replace deploy with specific calls

1168. By Richard Harding

Code review tweaks

Revision history for this message
Richard Harding (rharding) wrote :

Please take a look.

https://codereview.appspot.com/18920045/diff/20001/app/widgets/charm-search.js
File app/widgets/charm-search.js (right):

https://codereview.appspot.com/18920045/diff/20001/app/widgets/charm-search.js#newcode278
app/widgets/charm-search.js:278: if
(ev.target.hasClass('search_add_to_canvas')) {
On 2013/10/29 19:09:46, gary.poster wrote:
> I'm guessing we can't have an event registration for
li.search_add_to_canvas or
> 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.handleDeploy(ev.target.getData());

> 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://codereview.appspot.com/18920045/diff/20001/app/widgets/charm-search.js#newcode327
app/widgets/charm-search.js:327: this.hide();
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://codereview.appspot.com/18920045/diff/20001/test/test_browser_app.js
File test/test_browser_app.js (right):

https://codereview.appspot.com/18920045/diff/20001/test/test_browser_app.js#newcode182
test/test_browser_app.js:182: fakeStore = new
Y.juju.charmworld.APIv2({});
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.

https://codereview.appspot.com/18920045/

1169. By Richard Harding

QA on laptop, sync with trunk

1170. By Richard Harding

Merge with trunk

Revision history for this message
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://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.

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

https://codereview.appspot.com/18920045/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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'
24Binary 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 });

Subscribers

People subscribed via source and target branches