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
=== modified file 'app/app.js'
--- app/app.js 2013-10-22 00:24:31 +0000
+++ app/app.js 2013-10-30 13:23:35 +0000
@@ -572,7 +572,7 @@
572572
573 // To use the new service Inspector use the deploy method573 // To use the new service Inspector use the deploy method
574 // from the Y.juju.GhostDeployer extension574 // from the Y.juju.GhostDeployer extension
575 cfg.deploy = Y.bind(this.deployService, this);575 cfg.deployService = Y.bind(this.deployService, this);
576576
577 cfg.deployBundle = this.deployBundle.bind(this);577 cfg.deployBundle = this.deployBundle.bind(this);
578578
@@ -595,7 +595,7 @@
595 // When someone wants a charm to be deployed they fire an event and we595 // When someone wants a charm to be deployed they fire an event and we
596 // show the charm panel to configure/deploy the service.596 // show the charm panel to configure/deploy the service.
597 Y.on('initiateDeploy', function(charm, ghostAttributes) {597 Y.on('initiateDeploy', function(charm, ghostAttributes) {
598 cfg.deploy(charm, ghostAttributes);598 cfg.deployService(charm, ghostAttributes);
599 }, this);599 }, this);
600 },600 },
601601
602602
=== added file 'app/assets/images/search_add_to_canvas.png'
603Binary 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 differ603Binary 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
=== modified file 'app/subapps/browser/browser.js'
--- app/subapps/browser/browser.js 2013-10-23 00:12:47 +0000
+++ app/subapps/browser/browser.js 2013-10-30 13:23:35 +0000
@@ -536,10 +536,11 @@
536 activeTab: this._viewState.hash,536 activeTab: this._viewState.hash,
537 entityId: entityId,537 entityId: entityId,
538 container: Y.Node.create('<div class="charmview"/>'),538 container: Y.Node.create('<div class="charmview"/>'),
539 deploy: this.get('deploy'),539 deployBundle: this.get('deployBundle'),
540 deployBundle: this.get('deployBundle')540 deployService: this.get('deployService')
541 };541 };
542542
543
543 // If the only thing that changed was the hash, then don't redraw. It's544 // If the only thing that changed was the hash, then don't redraw. It's
544 // just someone clicking a tab in the UI.545 // just someone clicking a tab in the UI.
545 if (this._details && this._hasStateChanged('hash') &&546 if (this._details && this._hasStateChanged('hash') &&
@@ -696,7 +697,10 @@
696 }697 }
697698
698 if (this._hasStateChanged('viewmode') || forceFullscreen) {699 if (this._hasStateChanged('viewmode') || forceFullscreen) {
699 var extraCfg = {};700 var extraCfg = {
701 deployService: this.get('deployService'),
702 deployBundle: this.get('deployBundle')
703 };
700 if (this._viewState.search || this._viewState.charmID) {704 if (this._viewState.search || this._viewState.charmID) {
701 extraCfg.withHome = true;705 extraCfg.withHome = true;
702 }706 }
@@ -789,7 +793,9 @@
789 if (this._hasStateChanged('viewmode') || forceSidebar) {793 if (this._hasStateChanged('viewmode') || forceSidebar) {
790 this._sidebar = new views.Sidebar(794 this._sidebar = new views.Sidebar(
791 this._getViewCfg({795 this._getViewCfg({
792 container: this.get('container')796 container: this.get('container'),
797 deployService: this.get('deployService'),
798 deployBundle: this.get('deployBundle')
793 }));799 }));
794 this._sidebar.render();800 this._sidebar.render();
795 this._sidebar.addTarget(this);801 this._sidebar.addTarget(this);
@@ -1142,11 +1148,19 @@
1142 The "deploy" function prompts the user for service configuration and1148 The "deploy" function prompts the user for service configuration and
1143 deploys a service.1149 deploys a service.
11441150
1145 @attribute deploy1151 @attribute deployService
1146 @default undefined1152 @default undefined
1147 @type {Function}1153 @type {Function}
1148 */1154 */
1149 deploy: {},1155 deployService: {},
1156
1157 /**
1158 * @attribute deployBundle
1159 * @default undefined
1160 * @type {Function}
1161 *
1162 */
1163 deployBundle: {},
11501164
1151 /**1165 /**
1152 The default viewmode1166 The default viewmode
11531167
=== modified file 'app/subapps/browser/views/charm.js'
--- app/subapps/browser/views/charm.js 2013-10-29 22:51:19 +0000
+++ app/subapps/browser/views/charm.js 2013-10-30 13:23:35 +0000
@@ -93,7 +93,7 @@
93 ghostAttributes = {93 ghostAttributes = {
94 icon: this.get('store').iconpath(charm.get('storeId'))94 icon: this.get('store').iconpath(charm.get('storeId'))
95 };95 };
96 this.get('deploy').call(null, charm, ghostAttributes);96 this.get('deployService').call(null, charm, ghostAttributes);
97 },97 },
9898
99 /**99 /**
100100
=== modified file 'app/subapps/browser/views/entity-base.js'
--- app/subapps/browser/views/entity-base.js 2013-10-29 22:51:19 +0000
+++ app/subapps/browser/views/entity-base.js 2013-10-30 13:23:35 +0000
@@ -526,12 +526,28 @@
526 * The "deploy" function prompts the user for service configuration and526 * The "deploy" function prompts the user for service configuration and
527 * deploys a service.527 * deploys a service.
528 *528 *
529 * @attribute deploy529 * The proper deploy function is provided from the browser subapp.
530 * @default undefined530 *
531 * @type {Function}531 * @attribute deployService
532 *532 * @default undefined
533 */533 * @type {Function}
534 deploy: {}534 *
535 */
536 deployService: {},
537
538 /**
539 * The "deploy" function prompts the user for service configuration and
540 * deploys a service.
541 *
542 * The proper deploy function is provided from the browser subapp.
543 *
544 * @attribute deployBundle
545 * @default undefined
546 * @type {Function}
547 *
548 */
549 deployBundle: {}
550
535 };551 };
536552
537 ns.EntityBaseView = EntityBaseView;553 ns.EntityBaseView = EntityBaseView;
538554
=== modified file 'app/subapps/browser/views/view.js'
--- app/subapps/browser/views/view.js 2013-09-26 21:14:50 +0000
+++ app/subapps/browser/views/view.js 2013-10-30 13:23:35 +0000
@@ -91,14 +91,17 @@
91 this.search.on(91 this.search.on(
92 this.search.EVT_SEARCH_CHANGED, this._searchChanged, this)92 this.search.EVT_SEARCH_CHANGED, this._searchChanged, this)
93 );93 );
94 }
9594
96 if (this.search) {
97 this.addEvent(95 this.addEvent(
98 this.search.on(96 this.search.on(
99 this.search.EVT_SEARCH_GOHOME, this._goHome, this)97 this.search.EVT_SEARCH_GOHOME, this._goHome, this)
100 );98 );
10199
100 this.addEvent(
101 this.search.on(
102 this.search.EVT_DEPLOY, this._deployEntity, this)
103 );
104
102 // If the showHome attribute is changed, update our html by adding the105 // If the showHome attribute is changed, update our html by adding the
103 // with-home class to the widget.106 // with-home class to the widget.
104 this.after('withHomeChange', function(ev) {107 this.after('withHomeChange', function(ev) {
@@ -123,10 +126,39 @@
123 }126 }
124 }, this);127 }, this);
125 }128 }
129
126 this._bindViewmodeControls(this.controls);130 this._bindViewmodeControls(this.controls);
127 },131 },
128132
129 /**133 /**
134 * Deploy either a bundle or charm given by the quicksearch widget.
135 *
136 * @method _deployEntity
137 * @param {Y.Event} ev the event object from the widget.
138 *
139 */
140 _deployEntity: function(ev) {
141 var entityType = ev.entityType,
142 entity = ev.data,
143 entityId = ev.id,
144 deployer;
145
146 if (entityType === 'bundle') {
147 deployer = this.get('deployBundle');
148 var bundle = new models.Bundle(entity);
149 deployer(bundle.get('data'));
150 } else {
151 deployer = this.get('deployService');
152 var charm = new models.Charm(entity);
153 var ghostAttributes;
154 ghostAttributes = {
155 icon: this.get('store').iconpath(charm.get('storeId'))
156 };
157 deployer.call(null, charm, ghostAttributes);
158 }
159 },
160
161 /**
130 * Force a navigate event when the search widget says "Home" was clicked.162 * Force a navigate event when the search widget says "Home" was clicked.
131 *163 *
132 * @method _goHome164 * @method _goHome
@@ -278,6 +310,22 @@
278 charmID: {},310 charmID: {},
279311
280 /**312 /**
313 * @attribute deployService
314 * @default undefined
315 * @type {Function}
316 *
317 */
318 deployService: {},
319
320 /**
321 * @attribute deployBundle
322 * @default undefined
323 * @type {Function}
324 *
325 */
326 deployBundle: {},
327
328 /**
281 The list of filters to be used in the rendering of the view.329 The list of filters to be used in the rendering of the view.
282330
283 This is always handed down from the subapp, but default to something331 This is always handed down from the subapp, but default to something
@@ -339,6 +387,7 @@
339 'juju-charm-store',387 'juju-charm-store',
340 'juju-browser-models',388 'juju-browser-models',
341 'juju-models',389 'juju-models',
390 'juju-bundle-models',
342 'querystring-stringify',391 'querystring-stringify',
343 'view',392 'view',
344 'viewmode-controls'393 'viewmode-controls'
345394
=== modified file 'app/templates/bundle-token.handlebars'
--- app/templates/bundle-token.handlebars 2013-10-30 03:33:36 +0000
+++ app/templates/bundle-token.handlebars 2013-10-30 13:23:35 +0000
@@ -1,6 +1,13 @@
1{{! The token class is used by the deploy method in1{{! The token class is used by the deploy method in
2 test/test_charm_running.py. If you change this name, change the other2 test/test_charm_running.py. If you change this name, change the other
3 one too! }}3 one too! }}
4{{#if deployButton}}
5 {{#ifFlag 'searchDeploy'}}
6 <a class="deployButton bundle" href="">
7 <i class="sprite search_add_to_canvas" data-bundleid="{{id}}" title="Deploy bundle"></i>
8 </a>
9 {{/ifFlag}}
10{{/if}}
4<a href="/bundle/{{id}}"11<a href="/bundle/{{id}}"
5 class="token {{size}}"12 class="token {{size}}"
6 data-charmid="/bundle/{{id}}">13 data-charmid="/bundle/{{id}}">
714
=== modified file 'app/templates/charm-token.handlebars'
--- app/templates/charm-token.handlebars 2013-10-30 03:33:36 +0000
+++ app/templates/charm-token.handlebars 2013-10-30 13:23:35 +0000
@@ -1,6 +1,14 @@
1{{! The token class is used by the deploy method in1{{! The token class is used by the deploy method in
2 test/test_charm_running.py. If you change this name, change the other2 test/test_charm_running.py. If you change this name, change the other
3 one too! }}3 one too! }}
4
5{{#if deployButton}}
6 {{#ifFlag 'searchDeploy'}}
7 <a class="deployButton" href="">
8 <i class="sprite search_add_to_canvas" data-charmid="{{storeId}}" title="Deploy service"></i>
9 </a>
10 {{/ifFlag}}
11{{/if}}
4<a href="/{{storeId}}"12<a href="/{{storeId}}"
5 class="token {{size}}"13 class="token {{size}}"
6 data-charmid="{{storeId}}">14 data-charmid="{{storeId}}">
@@ -21,6 +29,8 @@
21 <span class="title">29 <span class="title">
22 {{ name }}30 {{ name }}
23 </span>31 </span>
32
33
24 {{#unless_eq size 'tiny'}}34 {{#unless_eq size 'tiny'}}
25 <span class="metadata">35 <span class="metadata">
26 <div><strong>Deployed {{downloads}} {{pluralize 'time' downloads}}</strong></div>36 <div><strong>Deployed {{downloads}} {{pluralize 'time' downloads}}</strong></div>
2737
=== modified file 'app/views/utils.js'
--- app/views/utils.js 2013-10-25 15:33:34 +0000
+++ app/views/utils.js 2013-10-30 13:23:35 +0000
@@ -1564,7 +1564,7 @@
1564 /*1564 /*
1565 * Check if a flag is set.1565 * Check if a flag is set.
1566 *1566 *
1567 * {{ifFlag 'flag_name'}}1567 * {{#ifFlag 'flag_name'}} {{/ifFlag}}
1568 *1568 *
1569 */1569 */
1570 Y.Handlebars.registerHelper('ifFlag', function(flag, options) {1570 Y.Handlebars.registerHelper('ifFlag', function(flag, options) {
15711571
=== modified file 'app/widgets/charm-search.js'
--- app/widgets/charm-search.js 2013-09-26 21:14:50 +0000
+++ app/widgets/charm-search.js 2013-10-30 13:23:35 +0000
@@ -50,6 +50,7 @@
50 EVT_CLEAR_SEARCH: 'clear_search',50 EVT_CLEAR_SEARCH: 'clear_search',
51 EVT_SEARCH_CHANGED: 'search_changed',51 EVT_SEARCH_CHANGED: 'search_changed',
52 EVT_SEARCH_GOHOME: 'go_home',52 EVT_SEARCH_GOHOME: 'go_home',
53 EVT_DEPLOY: 'charm_deploy',
5354
54 TEMPLATE: templates['browser-search'],55 TEMPLATE: templates['browser-search'],
5556
@@ -207,7 +208,8 @@
207 // be false.208 // be false.
208 var tokenAttrs = Y.merge(charm.getAttrs(), {209 var tokenAttrs = Y.merge(charm.getAttrs(), {
209 size: 'tiny',210 size: 'tiny',
210 is_approved: false211 is_approved: false,
212 deployButton: isCategory(charm.get('id')) ? false : true
211 });213 });
212 var token = new ns.Token(tokenAttrs);214 var token = new ns.Token(tokenAttrs);
213 var html = Y.Node.create(token.TEMPLATE(token.getAttrs()));215 var html = Y.Node.create(token.TEMPLATE(token.getAttrs()));
@@ -245,6 +247,7 @@
245 *247 *
246 */248 */
247 _setupAutocomplete: function() {249 _setupAutocomplete: function() {
250 var self = this;
248 // Bind out helpers to the current objects context, not the auto251 // Bind out helpers to the current objects context, not the auto
249 // complete widget context..252 // complete widget context..
250 var fetchSuggestions = Y.bind(this._fetchSuggestions, this);253 var fetchSuggestions = Y.bind(this._fetchSuggestions, this);
@@ -266,6 +269,77 @@
266 },269 },
267 source: fetchSuggestions270 source: fetchSuggestions
268 });271 });
272
273 // Holder for the deploy logic the AC uses when clicking on a deploy
274 // icon from a result item.
275 this.ac._onDeploy = function(ev) {
276 var isBundle = false,
277 data,
278 found,
279 id;
280
281 // Fire an event up to the View with the charm information so that
282 // it can proceed to build/send the deploy information out to
283 // the environment.
284 id = ev.target.getData('charmId');
285
286 if (!id) {
287 // try to see if this is a bundle clicked on.
288 id = ev.target.getData('bundleId');
289 isBundle = true;
290 }
291 // Find the charm data for the selected item from the set of
292 // results.
293 found = this.get('results').filter(function(result) {
294 if (isBundle) {
295 if (result.raw.bundle.id === id) {
296 return result;
297 }
298 } else {
299 if (result.raw.charm.id === id) {
300 return result;
301 }
302 }
303 });
304
305 // Make sure that we've found a result before returning.
306 if (found.length === 0) {
307 console.error(
308 'Clicked deploy on an item we could not find in results.');
309 } else {
310 if (isBundle) {
311 data = found[0].raw.bundle;
312 } else {
313 data = found[0].raw.charm;
314 }
315
316 self.fire(self.EVT_DEPLOY, {
317 id: id,
318 data: data,
319 entityType: isBundle ? 'bundle' : 'charm'
320 });
321
322 }
323
324 };
325
326 this.ac._onItemClick = function(ev) {
327 // If the selection is coming from the deployButton then we kind of
328 // ignore the way autocomplete works. It's more of a 'quick search'
329 // with a deploy option. No search is really performed after the
330 // deploy button is selected.
331 if (ev.target.hasClass('search_add_to_canvas')) {
332 // Hide the autocomplete widget. You've selected something
333 // that's not really a suggestion, but it should still go away.
334 this.hide();
335 this._onDeploy(ev);
336 } else {
337 var itemNode = ev.currentTarget;
338 this.set('active_item', itemNode);
339 this.selectItem(itemNode, ev);
340 }
341 };
342
269 this.ac.render();343 this.ac.render();
270344
271 this.ac.get('inputNode').on('focus', function(ev) {345 this.ac.get('inputNode').on('focus', function(ev) {
@@ -281,6 +355,7 @@
281 this.get('boundingBox').delegate('click', function(ev) {355 this.get('boundingBox').delegate('click', function(ev) {
282 ev.halt();356 ev.halt();
283 }, 'a', this);357 }, 'a', this);
358
284 },359 },
285360
286 /**361 /**
@@ -293,14 +368,25 @@
293 */368 */
294 _suggestionSelected: function(ev) {369 _suggestionSelected: function(ev) {
295 ev.halt();370 ev.halt();
371
296 var change,372 var change,
297 newVal,373 form = this.get('boundingBox').one('form'),
298 charmid = ev.result.raw.charm.id,374 id,
299 form = this.get('boundingBox').one('form');375 isBundle = false,
300376 newVal;
301 if (charmid.substr(0, 4) === 'cat:') {377
378 if (ev.result.raw.charm) {
379 id = ev.result.raw.charm.id;
380 } else {
381 // Currently we have to pretend to be a charm.
382 // XXX: We should support the idea of a bundle separate from charm? Go
383 // with entity? Something to clean up.
384 id = '/bundle/' + ev.result.raw.bundle.id;
385 }
386
387 if (id.substr(0, 4) === 'cat:') {
302 form.one('input').set('value', '');388 form.one('input').set('value', '');
303 var category = charmid.match(/([^\/]+)-\d\/?/);389 var category = id.match(/([^\/]+)-\d\/?/);
304 change = {390 change = {
305 charmID: null,391 charmID: null,
306 search: true,392 search: true,
@@ -318,7 +404,7 @@
318 form.one('input').set('value', ev.result.text);404 form.one('input').set('value', ev.result.text);
319 newVal = ev.result.text;405 newVal = ev.result.text;
320 change = {406 change = {
321 charmID: charmid,407 charmID: id,
322 filter: {408 filter: {
323 categories: [],409 categories: [],
324 text: newVal,410 text: newVal,
325411
=== modified file 'app/widgets/token.js'
--- app/widgets/token.js 2013-10-09 22:20:53 +0000
+++ app/widgets/token.js 2013-10-30 13:23:35 +0000
@@ -216,6 +216,16 @@
216 charmIcons: {216 charmIcons: {
217 setter: '_charmIconsSetter'217 setter: '_charmIconsSetter'
218 },218 },
219
220 /**
221 * @attribute deployButton
222 * @default false
223 * @type {Boolean}
224 */
225 deployButton: {
226 value: false
227 },
228
219 /**229 /**
220 * @attribute downloads230 * @attribute downloads
221 * @default undefined231 * @default undefined
222232
=== modified file 'lib/views/browser/bws-searchbox.less'
--- lib/views/browser/bws-searchbox.less 2013-10-29 03:07:12 +0000
+++ lib/views/browser/bws-searchbox.less 2013-10-30 13:23:35 +0000
@@ -66,11 +66,12 @@
66 }66 }
67 }67 }
6868
69 .token {69 .token.tiny {
70 padding: 8px 0;70 padding: 8px 0;
7171
72 .title {72 .title {
73 .type15;73 .type15;
74 margin-top: 2px;
74 }75 }
7576
76 .icon,77 .icon,
7778
=== modified file 'lib/views/browser/token.less'
--- lib/views/browser/token.less 2013-10-30 03:33:36 +0000
+++ lib/views/browser/token.less 2013-10-30 13:23:35 +0000
@@ -2,6 +2,21 @@
2 .border-box;2 .border-box;
3 position: relative;3 position: relative;
44
5 .deployButton {
6 display: block;
7 float: right;
8 margin-top: 19px;
9
10 /* Bundle tokens are taller due to the charm icons inside. */
11 &.bundle {
12 margin-top: 34px;
13 }
14
15 i.sprite {
16 display: block;
17 }
18 }
19
5 .token {20 .token {
6 display: block;21 display: block;
7 // Clear floats:22 // Clear floats:
823
=== modified file 'test/test_browser_app.js'
--- test/test_browser_app.js 2013-10-29 19:38:27 +0000
+++ test/test_browser_app.js 2013-10-30 13:23:35 +0000
@@ -34,7 +34,7 @@
3434
35 (function() {35 (function() {
36 describe('browser fullscreen view', function() {36 describe('browser fullscreen view', function() {
37 var container, FullScreen, view, views, Y;37 var container, FullScreen, utils, view, views, Y;
3838
39 before(function(done) {39 before(function(done) {
40 Y = YUI(GlobalConfig).use(40 Y = YUI(GlobalConfig).use(
@@ -43,6 +43,7 @@
43 'juju-tests-utils',43 'juju-tests-utils',
44 'subapp-browser-fullscreen', function(Y) {44 'subapp-browser-fullscreen', function(Y) {
45 views = Y.namespace('juju.browser.views');45 views = Y.namespace('juju.browser.views');
46 utils = Y.namespace('juju-tests.utils');
46 FullScreen = views.FullScreen;47 FullScreen = views.FullScreen;
47 done();48 done();
48 });49 });
@@ -176,6 +177,23 @@
176 });177 });
177 });178 });
178179
180 it('picks up the search widget deploy event', function(done) {
181 var container = utils.makeContainer('subapp-browser'),
182 fakeStore = new Y.juju.charmworld.APIv2({});
183 view = new FullScreen({
184 charmID: 'precise/jenkins-13',
185 store: fakeStore
186 });
187
188 view._deployEntity = function() {
189 container.remove(true);
190 done();
191 };
192
193 view.render(container);
194 view.search.fire(view.search.EVT_DEPLOY);
195 });
196
179 });197 });
180 })();198 })();
181199
@@ -228,7 +246,7 @@
228246
229 (function() {247 (function() {
230 describe('browser sidebar view', function() {248 describe('browser sidebar view', function() {
231 var Y, container, view, views, Sidebar;249 var Y, container, utils, view, views, Sidebar;
232250
233 before(function(done) {251 before(function(done) {
234 Y = YUI(GlobalConfig).use(252 Y = YUI(GlobalConfig).use(
@@ -240,6 +258,7 @@
240 'subapp-browser-sidebar',258 'subapp-browser-sidebar',
241 function(Y) {259 function(Y) {
242 views = Y.namespace('juju.browser.views');260 views = Y.namespace('juju.browser.views');
261 utils = Y.namespace('juju-tests.utils');
243 Sidebar = views.Sidebar;262 Sidebar = views.Sidebar;
244 done();263 done();
245 });264 });
@@ -371,6 +390,23 @@
371 });390 });
372 });391 });
373392
393 it('picks up the search widget deploy event', function(done) {
394 var container = utils.makeContainer('subapp-browser'),
395 fakeStore = new Y.juju.charmworld.APIv2({});
396 view = new Sidebar({
397 charmID: 'precise/jenkins-13',
398 store: fakeStore
399 });
400
401 view._deployEntity = function() {
402 container.remove(true);
403 done();
404 };
405
406 view.render(container);
407 view.search.fire(view.search.EVT_DEPLOY);
408 });
409
374 });410 });
375 })();411 })();
376412
377413
=== modified file 'test/test_browser_charm_details.js'
--- test/test_browser_charm_details.js 2013-10-29 22:51:19 +0000
+++ test/test_browser_charm_details.js 2013-10-30 13:23:35 +0000
@@ -357,7 +357,7 @@
357 container: utils.makeContainer(),357 container: utils.makeContainer(),
358 store: fakeStore358 store: fakeStore
359 });359 });
360 view.set('deploy', function(charm, serviceAttrs) {360 view.set('deployService', function(charm, serviceAttrs) {
361 var serviceCharm = view.get('entity');361 var serviceCharm = view.get('entity');
362 assert.deepEqual(charm, serviceCharm);362 assert.deepEqual(charm, serviceCharm);
363 assert.equal(charm.get('id'), 'cs:precise/ceph-9');363 assert.equal(charm.get('id'), 'cs:precise/ceph-9');
364364
=== modified file 'test/test_browser_search_widget.js'
--- test/test_browser_search_widget.js 2013-09-26 21:14:50 +0000
+++ test/test_browser_search_widget.js 2013-10-30 13:23:35 +0000
@@ -155,6 +155,24 @@
155 });155 });
156 });156 });
157157
158 it('generates a bundle change event when bundle selected', function(done) {
159 search.on(search.EVT_SEARCH_CHANGED, function(ev) {
160 assert.equal(ev.newVal, 'TestBundle');
161 // The bundle id gets prefixed with the /bundle to help routing.
162 assert.equal(ev.change.charmID, '/bundle/~hatch/wiki/7/TestBundle');
163 assert.equal(ev.change.filter.categories.length, 0);
164 done();
165 });
166
167 search._suggestionSelected({
168 halt: function() {},
169 result: {
170 raw: {bundle: {id: '~hatch/wiki/7/TestBundle'}},
171 text: 'TestBundle'
172 }
173 });
174 });
175
158 it('supports an onHome event', function(done) {176 it('supports an onHome event', function(done) {
159 search.on(search.EVT_SEARCH_GOHOME, function() {177 search.on(search.EVT_SEARCH_GOHOME, function() {
160 done();178 done();
@@ -188,6 +206,8 @@
188 utils = Y.namespace('juju-tests.utils');206 utils = Y.namespace('juju-tests.utils');
189 data = utils.loadFixture('data/autocomplete.json');207 data = utils.loadFixture('data/autocomplete.json');
190208
209 cleanIconHelper = utils.stubCharmIconPath();
210
191 // Need the handlebars helper for the token to render.211 // Need the handlebars helper for the token to render.
192 Y.Handlebars.registerHelper(212 Y.Handlebars.registerHelper(
193 'charmFilePath',213 'charmFilePath',
@@ -307,4 +327,32 @@
307 });327 });
308 });328 });
309329
330 it('fires deploy event when the deploy button is selected', function(done) {
331 window.flags.searchDeploy = true;
332 // This is heading into the private, non-publicized events of the AC
333 // widget in an effort to hit the html on render after results come
334 // back.
335 search.ac.after('resultsChange', function(ev) {
336 //Find one of the deployable results and trigger click event.
337 var chosenOne = container.one('.search_add_to_canvas');
338 chosenOne.simulate('click');
339 });
340
341 search.on(search.EVT_DEPLOY, function(ev) {
342 // Verify that the event was called with the correct payload to deploy.
343 assert.equal(ev.entityType, 'charm');
344 assert.equal(ev.id, 'precise/apache2-passenger-3');
345 assert.equal(ev.data.url, 'cs:precise/apache2-passenger-3');
346 window.flags = {};
347 done();
348 });
349
350 // hack into the ac widget to simulate the valueChange event
351 search.ac._afterValueChange({
352 newVal: 'a',
353 src: 'ui'
354 });
355
356 });
357
310});358});
311359
=== modified file 'test/test_token.js'
--- test/test_token.js 2013-10-10 04:28:39 +0000
+++ test/test_token.js 2013-10-30 13:23:35 +0000
@@ -246,4 +246,27 @@
246 assert.equal(token_container.one('span.charms').all('img').size(), 4);246 assert.equal(token_container.one('span.charms').all('img').size(), 4);
247 });247 });
248248
249 it('renders the deployer button when asked to', function() {
250 window.flags.searchDeploy = true;
251 var token = new Token({
252 size: 'tiny',
253 storeId: 'test',
254 deployButton: true,
255 description: 'some description',
256 mainCategory: 'app-servers',
257 iconUrl: 'http://localhost.svg'
258 });
259
260 token.render(token_container);
261 assert.notEqual(
262 token_container.get('innerHTML').indexOf('deployButton'),
263 -1
264 );
265
266 var button = token_container.one('.deployButton');
267 var sprite = button.one('i');
268 assert.equal(sprite.getAttribute('data-charmid'), 'test');
269 window.flags = {};
270 });
271
249});272});

Subscribers

People subscribed via source and target branches