Merge lp:~rharding/juju-gui/update-urls into lp:juju-gui/experimental

Proposed by Richard Harding
Status: Merged
Merged at revision: 556
Proposed branch: lp:~rharding/juju-gui/update-urls
Merge into: lp:juju-gui/experimental
Diff against target: 1368 lines (+529/-416)
13 files modified
app/modules-debug.js (+9/-0)
app/subapps/browser/browser.js (+184/-68)
app/subapps/browser/templates/browser_charm.handlebars (+3/-7)
app/subapps/browser/templates/editorial.handlebars (+9/-0)
app/subapps/browser/templates/sidebar.handlebars (+2/-4)
app/subapps/browser/views/charm.js (+90/-16)
app/subapps/browser/views/editorial.js (+166/-0)
app/subapps/browser/views/fullscreen.js (+7/-24)
app/subapps/browser/views/sidebar.js (+10/-154)
app/subapps/browser/views/view.js (+0/-59)
app/templates/charm-token.handlebars (+20/-18)
test/test_browser_app.js (+0/-47)
test/test_browser_charm_details.js (+29/-19)
To merge this branch: bzr merge lp:~rharding/juju-gui/update-urls
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+158713@code.launchpad.net

Description of the change

Rearchitect the browser to use routes more

- Use nested routes and route handling to build the browser UI
- Break up the Views in the browser subapp to be more singular per UX
component.
- Adjust tests to fit with this.

https://codereview.appspot.com/8679045/

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

Reviewers: mp+158713_code.launchpad.net,

Message:
Please take a look.

Description:

TBD

https://code.launchpad.net/~rharding/juju-gui/update-urls/+merge/158713

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M app/modules-debug.js
   M app/subapps/browser/browser.js
   M app/subapps/browser/templates/browser_charm.handlebars
   A app/subapps/browser/templates/editorial.handlebars
   M app/subapps/browser/templates/sidebar.handlebars
   M app/subapps/browser/views/charm.js
   A app/subapps/browser/views/editorial.js
   M app/subapps/browser/views/fullscreen.js
   M app/subapps/browser/views/sidebar.js
   M app/subapps/browser/views/view.js
   M app/templates/charm-token.handlebars
   M test/test_browser_app.js
   M test/test_browser_charm_details.js

lp:~rharding/juju-gui/update-urls updated
555. By Richard Harding

Garden

556. By Richard Harding

Garden

Revision history for this message
Richard Harding (rharding) wrote :
Revision history for this message
Richard Harding (rharding) wrote :
Revision history for this message
Jeff Pihach (hatch) wrote :
Download full text (4.8 KiB)

Thanks for the refactor - I have a few comments below that I'd like some
discussion on.

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

https://codereview.appspot.com/8679045/diff/3001/app/subapps/browser/browser.js#newcode110
app/subapps/browser/browser.js:110: var isFullscreen = true;
unused

https://codereview.appspot.com/8679045/diff/3001/app/subapps/browser/browser.js#newcode137
app/subapps/browser/browser.js:137: var containerID = '#subapp-browser',
should this not be
var containerID = this.get('container') ?

https://codereview.appspot.com/8679045/diff/3001/app/subapps/browser/browser.js#newcode146
app/subapps/browser/browser.js:146: if (this._getSubPath(req.path) !==
'fullscreen') {
see comment in routes.

https://codereview.appspot.com/8679045/diff/3001/app/subapps/browser/browser.js#newcode150
app/subapps/browser/browser.js:150: containerID += ' .bws-content';
maybe add a comment here as to what's happening here.

https://codereview.appspot.com/8679045/diff/3001/app/subapps/browser/browser.js#newcode178
app/subapps/browser/browser.js:178: if (!this._sidebar) {
maybe a comment here as to why this is necessary

https://codereview.appspot.com/8679045/diff/3001/app/subapps/browser/browser.js#newcode206
app/subapps/browser/browser.js:206: if (req.path.indexOf('fullscreen')
!== -1) {
see comment in routes

https://codereview.appspot.com/8679045/diff/3001/app/subapps/browser/browser.js#newcode245
app/subapps/browser/browser.js:245: valueFn: function() {
maybe some documentation as to what's going on here.

https://codereview.appspot.com/8679045/diff/3001/app/subapps/browser/browser.js#newcode267
app/subapps/browser/browser.js:267: { path: '/bws/fullscreen/*/',
callbacks: 'fullscreen' },
It looks like these routes are identical and then you are using a string
search to determine the view to render. I would probably investigate
using a route like:
/bws/:viewSize/

and then viewSize will be available as a param in your callbacks so you
won't have to stringsearch for it. IMHO it should also simplify your
routes and callbacks.

https://codereview.appspot.com/8679045/diff/3001/app/subapps/browser/templates/browser_charm.handlebars
File app/subapps/browser/templates/browser_charm.handlebars (right):

https://codereview.appspot.com/8679045/diff/3001/app/subapps/browser/templates/browser_charm.handlebars#newcode4
app/subapps/browser/templates/browser_charm.handlebars:4: <a
href="/bws/sidebar/" class="back">{{#if
isFullscreen}}Back{{else}}Close{{/if}}</a>
this url will need to be generated and then passed in with the hbs
params to work properly with both namespaces.

https://codereview.appspot.com/8679045/diff/3001/app/subapps/browser/templates/browser_charm.handlebars#newcode52
app/subapps/browser/templates/browser_charm.handlebars:52: Change log
isn't changelog one word?

https://codereview.appspot.com/8679045/diff/3001/app/subapps/browser/views/charm.js
File app/subapps/browser/views/charm.js (right):

https://codereview.appspot.com/8679045/diff/3001/app/subapps/browser/views/charm.js#newcode372
app/subapps/browser/views/charm.js:372: isFullscreen = false;
you accept ...

Read more...

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

We had a chat about most of it. I've added a bunch of comment blocks to
help clear up some of the bits.

Some of this is missing for the follow up such as url generation and app
state tracking.

Thanks for catching a few hard coded bits for in progress I missed
cleaning up!

https://codereview.appspot.com/8679045/diff/3001/app/subapps/browser/templates/browser_charm.handlebars
File app/subapps/browser/templates/browser_charm.handlebars (right):

https://codereview.appspot.com/8679045/diff/3001/app/subapps/browser/templates/browser_charm.handlebars#newcode4
app/subapps/browser/templates/browser_charm.handlebars:4: <a
href="/bws/sidebar/" class="back">{{#if
isFullscreen}}Back{{else}}Close{{/if}}</a>
On 2013/04/15 18:21:21, jeff.pihach wrote:
> this url will need to be generated and then passed in with the hbs
params to
> work properly with both namespaces.

Yes, url generation will come in a follow up branch.

https://codereview.appspot.com/8679045/diff/3001/app/subapps/browser/templates/browser_charm.handlebars#newcode52
app/subapps/browser/templates/browser_charm.handlebars:52: Change log
On 2013/04/15 18:21:21, jeff.pihach wrote:
> isn't changelog one word?

I would say so, but we're going off the UX docs given to us which lists
it as two. We can bring up the point to UX.

https://codereview.appspot.com/8679045/diff/3001/app/subapps/browser/views/charm.js
File app/subapps/browser/views/charm.js (right):

https://codereview.appspot.com/8679045/diff/3001/app/subapps/browser/views/charm.js#newcode372
app/subapps/browser/views/charm.js:372: isFullscreen = false;
On 2013/04/15 18:21:21, jeff.pihach wrote:
> you accept isFullscreen as a param and then overwrite it regardless.

Yep, hard coded for early dev and didn't back out.

https://codereview.appspot.com/8679045/

lp:~rharding/juju-gui/update-urls updated
557. By Richard Harding

Update per review

Revision history for this message
Richard Harding (rharding) wrote :
Revision history for this message
j.c.sackett (jcsackett) wrote :

Thanks for this refactor; I have some comments and questions below.

https://codereview.appspot.com/8679045/diff/10002/app/subapps/browser/browser.js
File app/subapps/browser/browser.js (left):

https://codereview.appspot.com/8679045/diff/10002/app/subapps/browser/browser.js#oldcode191
app/subapps/browser/browser.js:191: ]
Don't we need a route like "/bws/sidebar/id*/* to match things like the
readme and interface routes above?

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

https://codereview.appspot.com/8679045/diff/10002/app/subapps/browser/browser.js#newcode139
app/subapps/browser/browser.js:139: // The fullscreen view requires that
there be no editorial content if
The sequence of if statements here seem odd; can't we just check the
subpath thing, and set our containerID &c based on that, as that will
also be false if fullscreen isn't even in the URL?

It's possible I'm just not really understanding some of this branch, as
I'm having trouble figuring out the situation in which we call
renderEditorial but don't want to renderEditorial.

https://codereview.appspot.com/8679045/diff/10002/app/subapps/browser/templates/browser_charm.handlebars
File app/subapps/browser/templates/browser_charm.handlebars (left):

https://codereview.appspot.com/8679045/diff/10002/app/subapps/browser/templates/browser_charm.handlebars#oldcode54
app/subapps/browser/templates/browser_charm.handlebars:54: {{/if}}
Does this (and the events change in views/charm.js) have to do with the
routes &c change, or just a driveby?

https://codereview.appspot.com/8679045/diff/10002/app/subapps/browser/views/charm.js
File app/subapps/browser/views/charm.js (right):

https://codereview.appspot.com/8679045/diff/10002/app/subapps/browser/views/charm.js#newcode366
app/subapps/browser/views/charm.js:366: * @param {Node} container the
node to insert our rendered content into.
These params don't match; _renderCharmView has "charm" and isFullScreen,
not container. It's getting container as an attr.

https://codereview.appspot.com/8679045/diff/10002/app/subapps/browser/views/editorial.js
File app/subapps/browser/views/editorial.js (right):

https://codereview.appspot.com/8679045/diff/10002/app/subapps/browser/views/editorial.js#newcode126
app/subapps/browser/views/editorial.js:126: );
Given views/charm.js has this same failure as apiFailure, and we're
using it here as a callback, we should probably define it in one place
and have both of those places use it.

https://codereview.appspot.com/8679045/diff/10002/app/templates/charm-token.handlebars
File app/templates/charm-token.handlebars (right):

https://codereview.appspot.com/8679045/diff/10002/app/templates/charm-token.handlebars#newcode1
app/templates/charm-token.handlebars:1: <a href="/bws/sidebar/{{id}}">
Can you update the token's less file to make sure pointer type remains
normal now that the entire thing is a link?

https://codereview.appspot.com/8679045/

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

Comments below. Will have an updated branch in a bit.

https://codereview.appspot.com/8679045/diff/10002/app/subapps/browser/browser.js
File app/subapps/browser/browser.js (left):

https://codereview.appspot.com/8679045/diff/10002/app/subapps/browser/browser.js#oldcode191
app/subapps/browser/browser.js:191: ]
On 2013/04/15 21:06:34, j.c.sackett wrote:
> Don't we need a route like "/bws/sidebar/id*/* to match things like
the readme
> and interface routes above?

Short answer: yes
Longer: this functionality never worked and needs to be tied into the
rendering state of the charm details so it'll be implemented later.

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

https://codereview.appspot.com/8679045/diff/10002/app/subapps/browser/browser.js#newcode139
app/subapps/browser/browser.js:139: // The fullscreen view requires that
there be no editorial content if
On 2013/04/15 21:06:34, j.c.sackett wrote:
> The sequence of if statements here seem odd; can't we just check the
subpath
> thing, and set our containerID &c based on that, as that will also be
false if
> fullscreen isn't even in the URL?

> It's possible I'm just not really understanding some of this branch,
as I'm
> having trouble figuring out the situation in which we call
renderEditorial but
> don't want to renderEditorial.

Editorial is kind of a 'default' content. It's also going to be rendered
differently if it's for a fullscreen or sidebar view. Because of that we
track it's state here.

Render editorial is called manually from both sidebar() and
fullscreen(). It determines if it should go through with it based on the
current state.

/bws/sidebar/precise/apach2-2 will call renderEditorial as it's needed
to fill the sidebar while the charm details loads next to it.

/bws/fullscreen/precise/apache2-2 will also enter this path, but we
don't need to render the editorial data so we bypass it.

https://codereview.appspot.com/8679045/diff/10002/app/subapps/browser/templates/browser_charm.handlebars
File app/subapps/browser/templates/browser_charm.handlebars (left):

https://codereview.appspot.com/8679045/diff/10002/app/subapps/browser/templates/browser_charm.handlebars#oldcode54
app/subapps/browser/templates/browser_charm.handlebars:54: {{/if}}
On 2013/04/15 21:06:34, j.c.sackett wrote:
> Does this (and the events change in views/charm.js) have to do with
the routes
> &c change, or just a driveby?

sorry, this was part of some debugging work where I had issues with the
double render causing events to bind twice and thus clicking this would
open and close the comments at once.

In debugging I moved the event to the h3 as a whole vs just the <a> and
missed reverting it, but it works so it didn't come out.

https://codereview.appspot.com/8679045/diff/10002/app/subapps/browser/views/charm.js
File app/subapps/browser/views/charm.js (right):

https://codereview.appspot.com/8679045/diff/10002/app/subapps/browser/views/charm.js#newcode366
app/subapps/browser/views/charm.js:366: * @param {Node} container the
node to insert our rendered content into.
On 2013/04/15 21:06:34, j.c.sackett wrote:
> These params don't match; ...

Read more...

Revision history for this message
j.c.sackett (jcsackett) wrote :

On 2013/04/15 22:15:24, rharding wrote:
> Short answer: yes
> Longer: this functionality never worked and needs to be tied into the
rendering
> state of the charm details so it'll be implemented later.

Ok. Can you XXX or something to that effect?

> Editorial is kind of a 'default' content. It's also going to be
rendered
> differently if it's for a fullscreen or sidebar view. Because of that
we track
> it's state here.

> Render editorial is called manually from both sidebar() and
fullscreen(). It
> determines if it should go through with it based on the current state.

> /bws/sidebar/precise/apach2-2 will call renderEditorial as it's needed
to fill
> the sidebar while the charm details loads next to it.

> /bws/fullscreen/precise/apache2-2 will also enter this path, but we
don't need
> to render the editorial data so we bypass it.

Ok. This still feels weird to me, but I understand it. Thanks.

> sorry, this was part of some debugging work where I had issues with
the double
> render causing events to bind twice and thus clicking this would open
and close
> the comments at once.

> In debugging I moved the event to the h3 as a whole vs just the <a>
and missed
> reverting it, but it works so it didn't come out.

I think it's fine as is, I was just double checking my understanding.

> Sorry, you're correct. This was tweaked and I didn't update the
docstring.
> Thanks!

Thanks for fixing!

> Yes, my goal is to move this to a view/utils but was trying to keep
from doing
> too much in the branch. I'll update this.

You can just XXX it for now and get it later, if you like. I just want
to make sure it gets handled.

> This is very temp. Actually you have to carefully click in a blank
area to
> trigger the link right now. I want to get UX/Huw follow up to fix it
up.

Ok, thanks.

https://codereview.appspot.com/8679045/

Revision history for this message
j.c.sackett (jcsackett) wrote :

LGTM from discussion and with promised fixes. Thanks!

https://codereview.appspot.com/8679045/

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

LGTM Thanks for all the work!

https://codereview.appspot.com/8679045/

lp:~rharding/juju-gui/update-urls updated
558. By Richard Harding

Updates per review

559. By Richard Harding

Updates per review

560. By Richard Harding

lint

561. By Richard Harding

Resolve conflicts from trunk

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

*** Submitted:

Rearchitect the browser to use routes more

- Use nested routes and route handling to build the browser UI
- Break up the Views in the browser subapp to be more singular per UX
component.
- Adjust tests to fit with this.

R=jeff.pihach, j.c.sackett
CC=
https://codereview.appspot.com/8679045

https://codereview.appspot.com/8679045/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'app/modules-debug.js'
2--- app/modules-debug.js 2013-04-09 18:10:36 +0000
3+++ app/modules-debug.js 2013-04-16 00:07:30 +0000
4@@ -290,6 +290,15 @@
5
6 'subapp-browser-sidebar': {
7 fullpath: '/juju-ui/subapps/browser/views/sidebar.js',
8+ requires: [
9+ 'subapp-browser-editorial',
10+ 'subapp-browser-mainview',
11+ 'subapp-browser-charmview'
12+ ]
13+ },
14+
15+ 'subapp-browser-editorial': {
16+ fullpath: '/juju-ui/subapps/browser/views/editorial.js',
17 requires: ['subapp-browser-sidebar']
18 },
19
20
21=== modified file 'app/subapps/browser/browser.js'
22--- app/subapps/browser/browser.js 2013-04-04 19:16:03 +0000
23+++ app/subapps/browser/browser.js 2013-04-16 00:07:30 +0000
24@@ -9,7 +9,8 @@
25 *
26 */
27 YUI.add('subapp-browser', function(Y) {
28- var ns = Y.namespace('juju.subapps');
29+ var ns = Y.namespace('juju.subapps'),
30+ models = Y.namespace('juju.models');
31
32 /**
33 * Browser Sub App for the Juju Gui.
34@@ -19,7 +20,6 @@
35 *
36 */
37 ns.Browser = Y.Base.create('subapp-browser', Y.juju.SubApp, [], {
38-
39 /**
40 * Some routes might have sub parts that hint to where a user wants focus.
41 * In particular we've got the tabs that might have focus. They are the
42@@ -49,7 +49,8 @@
43 */
44 _getViewCfg: function(cfg) {
45 return Y.merge(cfg, {
46- db: this.get('db')
47+ db: this.get('db'),
48+ store: this.get('store')
49 });
50 },
51
52@@ -59,32 +60,42 @@
53 *
54 */
55 views: {
56+ charmDetails: {
57+ type: 'juju.browser.views.BrowserCharmView',
58+ preserve: false
59+ },
60 fullscreen: {
61 type: 'juju.browser.views.FullScreen',
62- preserve: true
63- },
64- fullscreenCharm: {
65- type: 'juju.browser.views.FullScreen',
66 preserve: false
67 },
68 sidebar: {
69 type: 'juju.browser.views.Sidebar',
70- preserve: true
71- },
72- sidebarCharm: {
73- type: 'juju.browser.views.Sidebar',
74 preserve: false
75 }
76 },
77
78 /**
79+ Cleanup after ourselves on destroy.
80+
81+ @method destructor
82+
83+ */
84+ destructor: function() {
85+ this._cacheCharms.destroy();
86+ },
87+
88+ /**
89 * General app initializer
90 *
91 * @method initializer
92 * @param {Object} cfg general init config object.
93 *
94 */
95- initializer: function(cfg) {},
96+ initializer: function(cfg) {
97+ // Hold onto charm data so we can pass model instances to other views when
98+ // charms are selected.
99+ this._cacheCharms = new models.BrowserCharmList();
100+ },
101
102 /**
103 * Render the fullscreen view to the client.
104@@ -96,35 +107,63 @@
105 *
106 */
107 fullscreen: function(req, res, next) {
108- this.showView('fullscreen', this._getViewCfg());
109- next();
110- },
111-
112- /**
113- * Render the fullscreen view of a specific charm to the client.
114- *
115- * @method fullscreenCharm
116- * @param {Request} req current request object.
117- * @param {Response} res current response object.
118- * @param {function} next callable for the next route in the chain.
119- *
120- */
121- fullscreenCharm: function(req, res, next) {
122- var subpath = this._getSubPath(req.path);
123- this.showView('fullscreenCharm', this._getViewCfg({
124- charmID: req.params.id,
125- subpath: subpath
126- }));
127- next();
128- },
129-
130- /**
131- * Destroy the subapp instance.
132- *
133- * @method destructor
134- *
135- */
136- destructor: function() {},
137+ if (!this._fullscreen) {
138+ this._fullscreen = this.showView('fullscreen', this._getViewCfg(), {
139+ 'callback': function(view) {
140+ // if the fullscreen isn't the last part of the path, then ignore
141+ // the editorial content.
142+ if (this._getSubPath(req.path) === 'fullscreen') {
143+ this.renderEditorial(req, res, next);
144+ }
145+ next();
146+ }
147+ });
148+ } else {
149+ next();
150+ }
151+ },
152+
153+ /**
154+ Render editorial content into the parent view when required.
155+
156+ The parent view is either fullscreen/sidebar which determines how the
157+ editorial content is to be rendered.
158+
159+ @method renderEditorial
160+ @param {Request} req current request object.
161+ @param {Response} res current response object.
162+ @param {function} next callable for the next route in the chain.
163+
164+ */
165+ renderEditorial: function(req, res, next) {
166+ var container = this.get('container'),
167+ editorialContainer,
168+ extraCfg = {};
169+
170+ if (req.path.indexOf('fullscreen') !== -1) {
171+ // The fullscreen view requires that there be no editorial content if
172+ // we're looking at a specific charm. The div we dump our content into
173+ // is shared. So if the url is /fullscreen show editorial content, but
174+ // if it's not, there's something else handling displaying the
175+ // view-data.
176+ editorialContainer = container.one(' .bws-view-data');
177+ extraCfg.isFullscreen = true;
178+ } else {
179+ // If this is the sidebar view, then the editorial content goes into a
180+ // different div since we can view both editorial content and
181+ // view-data (such as a charm details) side by side.
182+ editorialContainer = container.one('.bws-content');
183+ }
184+
185+ if (!this._editorial) {
186+ this._editorial = new Y.juju.browser.views.EditorialView(
187+ this._getViewCfg(extraCfg));
188+
189+ this._editorial.render(editorialContainer);
190+ // Add any sidebar charms to the running cache.
191+ this._cacheCharms.add(this._editorial._cacheCharms);
192+ }
193+ },
194
195 /**
196 * Handle the route for the sidebar view.
197@@ -136,8 +175,26 @@
198 *
199 */
200 sidebar: function(req, res, next) {
201- this.showView('sidebar', this._getViewCfg());
202- next();
203+ // Clean up any details we've got.
204+ if (this._details) {
205+ this._details.destroy({remove: true});
206+ }
207+
208+ if (!this._sidebar) {
209+ // Whenever the sidebar view is rendered it needs some editorial
210+ // content to display to the user. We only need once instance though,
211+ // so only render it on the first view. As users click on charm to
212+ // charm and we generate urls /sidebar/precise/xxx we don't want to
213+ // re-render the sidebar content.
214+ this._sidebar = this.showView('sidebar', this._getViewCfg(), {
215+ 'callback': function(view) {
216+ this.renderEditorial(req, res, next);
217+ next();
218+ }
219+ });
220+ } else {
221+ next();
222+ }
223 },
224
225 /**
226@@ -149,53 +206,112 @@
227 * @param {function} next callable for the next route in the chain.
228 *
229 */
230- sidebarCharm: function(req, res, next) {
231- var subpath = this._getSubPath(req.path);
232- this.showView('sidebarCharm', this._getViewCfg({
233- charmID: req.params.id,
234- subpath: subpath
235- }));
236+ charmDetails: function(req, res, next) {
237+ var charmID = req.params.id;
238+ var extraCfg = {
239+ charmID: charmID,
240+ container: Y.Node.create('<div class="charmview"/>')
241+ };
242+
243+ // The details view needs to know if we're using a fullscreen template
244+ // or the sidebar version.
245+ if (req.path.indexOf('fullscreen') !== -1) {
246+ extraCfg.isFullscreen = true;
247+ }
248+
249+ // Gotten from the sidebar creating the cache.
250+ var model = this._cacheCharms.getById(charmID);
251+
252+ if (model) {
253+ extraCfg.charm = model;
254+ }
255+
256+ this._details = new Y.juju.browser.views.BrowserCharmView(
257+ this._getViewCfg(extraCfg));
258+ this._details.render();
259 next();
260 }
261
262 }, {
263 ATTRS: {
264+ /**
265+ @attribute container
266+ @default '#subapp-browser'
267+ @type {String}
268+
269+ */
270 container: {
271 value: '#subapp-browser'
272 },
273- urlNamespace: {
274- value: 'charmstore'
275+
276+ /**
277+ @attribute store
278+ @default Charmworld0
279+ @type {Charmworld0}
280+
281+ */
282+ store: {
283+ /**
284+ We keep one instance of the store and will work on caching results
285+ at the app level so that routes can share api calls. However, in
286+ tests there's no config for talking to the api so we have to watch
287+ out in test runs and allow the store to be broken.
288+
289+ method store.valueFn
290+
291+ */
292+ valueFn: function() {
293+ var url = '';
294+ if (!window.juju_config || ! window.juju_config.charmworldURL) {
295+ console.error('No juju config to fetch charmworld store url');
296+ } else {
297+ url = window.juju_config.charmworldURL;
298+ }
299+ return new Y.juju.Charmworld0({
300+ 'apiHost': url
301+ });
302+ }
303 },
304+
305+ /**
306+ @attribute routes
307+ @default Array of subapp routes.
308+ @type {Array}
309+
310+ */
311 routes: {
312 value: [
313+ // Double routes are needed to catch /fullscreen and /fullscreen/
314 { path: '/bws/fullscreen/', callbacks: 'fullscreen' },
315- { path: '/bws/fullscreen/*id/configuration/',
316- callbacks: 'fullscreenCharm' },
317- { path: '/bws/fullscreen/*id/hooks/', callbacks: 'fullscreenCharm' },
318- { path: '/bws/fullscreen/*id/interfaces/',
319- callbacks: 'fullscreenCharm' },
320- { path: '/bws/fullscreen/*id/qa/', callbacks: 'fullscreenCharm' },
321- { path: '/bws/fullscreen/*id/readme/', callbacks: 'fullscreenCharm' },
322- { path: '/bws/fullscreen/*id/', callbacks: 'fullscreenCharm' },
323+ { path: '/bws/fullscreen/*/', callbacks: 'fullscreen' },
324+ { path: '/bws/fullscreen/*id/', callbacks: 'charmDetails' },
325
326 { path: '/bws/sidebar/', callbacks: 'sidebar' },
327-
328- { path: '/bws/sidebar/*id/configuration/',
329- callbacks: 'sidebarCharm' },
330- { path: '/bws/sidebar/*id/hooks/', callbacks: 'sidebarCharm' },
331- { path: '/bws/sidebar/*id/interfaces/',
332- callbacks: 'sidebarCharm' },
333- { path: '/bws/sidebar/*id/qa/', callbacks: 'sidebarCharm' },
334- { path: '/bws/sidebar/*id/readme/', callbacks: 'sidebarCharm' },
335- { path: '/bws/sidebar/*id/', callbacks: 'sidebarCharm' }
336+ { path: '/bws/sidebar/*/', callbacks: 'sidebar' },
337+ { path: '/bws/sidebar/*id/', callbacks: 'charmDetails' }
338 ]
339+ },
340+
341+ /**
342+ @attribute urlNamespace
343+ @default 'charmstore'
344+ @type {String}
345+
346+ */
347+ urlNamespace: {
348+ value: 'charmstore'
349 }
350+
351 }
352 });
353
354 }, '0.1.0', {
355 requires: [
356+ 'juju-charm-store',
357+ 'juju-models',
358 'sub-app',
359+ 'subapp-browser-charmview',
360+ 'subapp-browser-editorial',
361 'subapp-browser-fullscreen',
362 'subapp-browser-sidebar'
363 ]
364
365=== modified file 'app/subapps/browser/templates/browser_charm.handlebars'
366--- app/subapps/browser/templates/browser_charm.handlebars 2013-04-15 14:16:16 +0000
367+++ app/subapps/browser/templates/browser_charm.handlebars 2013-04-16 00:07:30 +0000
368@@ -1,7 +1,7 @@
369 <div class="charm yui3-g">
370 <div class="content yui3-u-{{#if isFullscreen}}5-6{{else}}1{{/if}}">
371 <div class="nav">
372- <a href="" class="back">{{#if isFullscreen}}Back{{else}}Close{{/if}}</a>
373+ <a href="/bws/sidebar/" class="back">{{#if isFullscreen}}Back{{else}}Close{{/if}}</a>
374 <a href="" class="add">Add</a>
375 <a href="" class="share">
376 <img src="/juju-ui/assets/images/share_icon.jpg" alt="Share" />
377@@ -36,11 +36,8 @@
378 </div>
379
380 <div class="changelog">
381- <h3 class="section-title">
382- {{#if recent_commits}}
383- <a href="" class="expandToggle" data-state="closed">
384- {{/if}}
385- Change log
386+ <h3 class="section-title" data-state="closed">
387+ Change log
388 {{#if recent_commits}}
389 <span class="expand">
390 <span class="more">
391@@ -52,7 +49,6 @@
392 alt="Contract" />
393 </span>
394 </span>
395- </a>
396 {{/if}}
397 </h3>
398 {{#if recent_commits}}
399
400=== added file 'app/subapps/browser/templates/editorial.handlebars'
401--- app/subapps/browser/templates/editorial.handlebars 1970-01-01 00:00:00 +0000
402+++ app/subapps/browser/templates/editorial.handlebars 2013-04-16 00:07:30 +0000
403@@ -0,0 +1,9 @@
404+<div id="bws_editorial">
405+ {{#if isFullscreen}}
406+ <p>Some static content</p>
407+ {{/if}}
408+
409+ <div class="featured"></div>
410+ <div class="popular"></div>
411+ <div class="new"></div>
412+</div>
413
414=== modified file 'app/subapps/browser/templates/sidebar.handlebars'
415--- app/subapps/browser/templates/sidebar.handlebars 2013-04-15 07:26:46 +0000
416+++ app/subapps/browser/templates/sidebar.handlebars 2013-04-16 00:07:30 +0000
417@@ -2,12 +2,10 @@
418 <div id="bws-sidebar" class="yui3-u-1-4">
419 <div class="bws-header">
420 </div>
421- <div class="bws-left">
422- <div class="featured"></div>
423- <div class="popular"></div>
424- <div class="new"></div>
425+ <div class="bws-content">
426 </div>
427 </div>
428 <div class="bws-view-data yui3-u-3-4">
429+ <div></div>
430 </div>
431 </div>
432
433=== modified file 'app/subapps/browser/views/charm.js'
434--- app/subapps/browser/views/charm.js 2013-04-12 07:01:02 +0000
435+++ app/subapps/browser/views/charm.js 2013-04-16 00:07:30 +0000
436@@ -3,6 +3,7 @@
437
438 YUI.add('subapp-browser-charmview', function(Y) {
439 var ns = Y.namespace('juju.browser.views'),
440+ models = Y.namespace('juju.models'),
441 views = Y.namespace('juju.views'),
442 widgets = Y.namespace('juju.widgets'),
443 DATE_FORMAT = '%H:%M %d/%b/%y';
444@@ -29,7 +30,7 @@
445 *
446 */
447 events: {
448- '.changelog .expandToggle': {
449+ '.changelog h3.section-title': {
450 click: '_toggleLog'
451 },
452 '.charm .add': {
453@@ -55,6 +56,31 @@
454 },
455
456 /**
457+ * Shared method to generate a message to the user based on a bad api
458+ * call.
459+ *
460+ * @method apiFailure
461+ * @param {Object} data the json decoded response text.
462+ * @param {Object} request the original io_request object for debugging.
463+ *
464+ */
465+ apiFailure: function(data, request) {
466+ var message;
467+ if (data && data.type) {
468+ message = 'Charm API error of type: ' + data.type;
469+ } else {
470+ message = 'Charm API server did not respond';
471+ }
472+ this.get('db').notifications.add(
473+ new models.Notification({
474+ title: 'Failed to load sidebar content.',
475+ message: message,
476+ level: 'error'
477+ })
478+ );
479+ },
480+
481+ /**
482 * The API retuns the questions and the scores. Combine the data into a
483 * single source to make looping in the handlebars templates nicer.
484 *
485@@ -263,7 +289,6 @@
486
487 }
488 }, this);
489-
490 },
491
492 /**
493@@ -334,29 +359,30 @@
494 },
495
496 /**
497- * Render out the view to the DOM.
498+ * Render the view of a single charm details page.
499 *
500- * @method render
501- * @param {Node} container optional specific container to render out to.
502+ * @method _renderCharmView
503+ * @param {BrowserCharm} charm the charm model instance to view.
504+ * @param {Boolean} isFullscreen is this display for the fullscreen
505+ * experiecne?
506 *
507 */
508- render: function(container, isFullscreen) {
509- var charm = this.get('charm');
510- var tplData = charm.getAttrs();
511+ _renderCharmView: function(charm, isFullscreen) {
512+ this.set('charm', charm);
513+
514+ var tplData = charm.getAttrs(),
515+ container = this.get('container');
516+
517 tplData.isFullscreen = isFullscreen;
518 tplData.prettyCommits = this._formatCommitsForHtml(
519 tplData.recent_commits);
520
521 var tpl = this.template(tplData);
522- var tplNode = Y.Node.create(tpl);
523-
524- container.setHTML(tplNode);
525-
526- // Allow for specifying the container to use. This should reset the
527+ var tplNode = container.setHTML(tpl);
528+
529+ // Set the content then update the container so that it reload
530 // events.
531- if (container) {
532- this.set('container', container);
533- }
534+ Y.one('.bws-view-data').setHTML(tplNode);
535
536 this.tabview = new widgets.browser.TabView({
537 srcNode: tplNode.one('.tabs')
538@@ -374,10 +400,45 @@
539 } else {
540 this._noReadme(tplNode.one('#bws-readme'));
541 }
542+ },
543+
544+ /**
545+ Render out the view to the DOM.
546+
547+ The View might be given either a charmID, which means go fetch the
548+ charm data, or a charm model instance, in which case the view has the
549+ data it needs to render.
550+
551+ @method render
552+
553+ */
554+ render: function() {
555+ var isFullscreen = this.get('isFullscreen');
556+
557+ if (this.get('charm')) {
558+ this._renderCharmView(this.get('charm'), isFullscreen);
559+ } else {
560+ this.get('store').charm(this.get('charmID'), {
561+ 'success': function(data) {
562+ var charm = new models.BrowserCharm(data);
563+ this.set('charm', charm);
564+ this._renderCharmView(this.get('charm'), isFullscreen);
565+ },
566+ 'failure': this.apiFailure
567+ }, this);
568+ }
569 }
570 }, {
571 ATTRS: {
572 /**
573+ @attribute charmID
574+ @default undefined
575+ @type {Int}
576+
577+ */
578+ charmID: {},
579+
580+ /**
581 * The charm we're viewing the details of.
582 *
583 * @attribute charm
584@@ -388,6 +449,16 @@
585 charm: {},
586
587 /**
588+ @attribute isFullscreen
589+ @default false
590+ @type {Boolean}
591+
592+ */
593+ ifFullscreen: {
594+ value: false
595+ },
596+
597+ /**
598 * The store is the api endpoint for fetching data.
599 *
600 * @attribute store
601@@ -408,9 +479,12 @@
602 'datatype-date-format',
603 'event-tracker',
604 'gallery-markdown',
605+ 'juju-charm-store',
606+ 'juju-models',
607 'juju-templates',
608 'juju-views',
609 'juju-view-utils',
610+ 'node',
611 'prettify',
612 'view'
613 ]
614
615=== added file 'app/subapps/browser/views/editorial.js'
616--- app/subapps/browser/views/editorial.js 1970-01-01 00:00:00 +0000
617+++ app/subapps/browser/views/editorial.js 2013-04-16 00:07:30 +0000
618@@ -0,0 +1,166 @@
619+'use strict';
620+
621+
622+/**
623+ * Browser Editorial View.
624+ *
625+ * @module juju.browser
626+ * @submodule views
627+ *
628+ */
629+YUI.add('subapp-browser-editorial', function(Y) {
630+ var ns = Y.namespace('juju.browser.views'),
631+ models = Y.namespace('juju.models'),
632+ views = Y.namespace('juju.views'),
633+ widgets = Y.namespace('juju.widgets');
634+
635+
636+ /**
637+ * Editorial view for landing pages.
638+ *
639+ * @class Editorial
640+ * @extends {juju.browser.views.Editorial}
641+ *
642+ */
643+ ns.EditorialView = Y.Base.create('browser-view-sidebar', Y.View, [], {
644+ template: views.Templates.editorial,
645+
646+ /**
647+ * General YUI initializer.
648+ *
649+ * @method initializer
650+ * @param {Object} cfg configuration object.
651+ *
652+ */
653+ initializer: function(cfg) {
654+ // Hold onto charm data so we can pass model instances to other views when
655+ // charms are selected.
656+ this._cacheCharms = new models.BrowserCharmList();
657+ },
658+
659+ /**
660+ * Load the editorial content into the container specified.
661+ *
662+ * @method render
663+ * @param {Node} container An optional node to override where it's going.
664+ *
665+ */
666+ render: function(container) {
667+ var tpl = this.template(this.getAttrs()),
668+ tplNode = Y.Node.create(tpl),
669+ store = this.get('store');
670+
671+ if (typeof container !== 'object') {
672+ container = this.get('container');
673+ } else {
674+ this.set('container', container);
675+ }
676+
677+ // By default we grab the editorial content from the api to use for
678+ // display.
679+ this.get('store').sidebarEditorial({
680+ 'success': function(data) {
681+ // Add featured charms
682+ var featuredCharms = this.get('store').resultsToCharmlist(
683+ data.result.featured);
684+ var featuredContainer = tplNode.one('.featured');
685+ var featuredCharmContainer = new widgets.browser.CharmContainer({
686+ name: 'Featured Charms',
687+ cutoff: 1,
688+ children: featuredCharms.map(function(charm) {
689+ return charm.getAttrs(); })
690+ });
691+ featuredCharmContainer.render(featuredContainer);
692+
693+ // Add popular charms
694+ var popularCharms = this.get('store').resultsToCharmlist(
695+ data.result.popular);
696+ var popularContainer = tplNode.one('.popular');
697+ var popularCharmContainer = new widgets.browser.CharmContainer({
698+ name: 'Popular Charms',
699+ cutoff: 2,
700+ children: popularCharms.map(function(charm) {
701+ return charm.getAttrs(); })
702+ });
703+ popularCharmContainer.render(popularContainer);
704+
705+ // Add in the charm tokens for the new as well.
706+ var newContainer = tplNode.one('.new');
707+ var newCharms = this.get('store').resultsToCharmlist(
708+ data.result['new']);
709+ var newCharmContainer = new widgets.browser.CharmContainer({
710+ name: 'New Charms',
711+ cutoff: 2,
712+ children: newCharms.map(function(charm) {
713+ return charm.getAttrs(); })
714+ });
715+ newCharmContainer.render(newContainer);
716+
717+ container.setHTML(tplNode);
718+
719+ // Add the charms to the cache for use in other views.
720+ // Start with a reset to empty any current cached models.
721+ this._cacheCharms.reset(newCharms);
722+ this._cacheCharms.add(popularCharms);
723+ this._cacheCharms.add(featuredCharms);
724+ this.charmContainers = [
725+ featuredCharmContainer,
726+ newCharmContainer,
727+ popularCharmContainer
728+ ];
729+ },
730+
731+ 'failure': function(data, request) {
732+ var message;
733+ if (data && data.type) {
734+ message = 'Charm API error of type: ' + data.type;
735+ } else {
736+ message = 'Charm API server did not respond';
737+ }
738+ this.get('db').notifications.add(
739+ new models.Notification({
740+ title: 'Failed to load sidebar content.',
741+ message: message,
742+ level: 'error'
743+ })
744+ );
745+ }
746+ }, this);
747+ },
748+
749+ /**
750+ * Destroy this view and clear from the dom world.
751+ *
752+ * @method destructor
753+ *
754+ */
755+ destructor: function() {
756+ if (this.charmContainers) {
757+ Y.Array.each(this.charmContainers, function(container) {
758+ container.destroy();
759+ });
760+ }
761+ this._cacheCharms.destroy();
762+ }
763+ }, {
764+ ATTRS: {
765+ isFullscreen: {
766+ value: false
767+ },
768+ store: {
769+
770+ }
771+ }
772+ });
773+
774+}, '0.1.0', {
775+ requires: [
776+ 'browser-charm-container',
777+ 'browser-charm-token',
778+ 'browser-search-widget',
779+ 'juju-charm-store',
780+ 'juju-models',
781+ 'juju-templates',
782+ 'view'
783+ ]
784+});
785
786=== modified file 'app/subapps/browser/views/fullscreen.js'
787--- app/subapps/browser/views/fullscreen.js 2013-04-03 15:03:56 +0000
788+++ app/subapps/browser/views/fullscreen.js 2013-04-16 00:07:30 +0000
789@@ -16,44 +16,27 @@
790 *
791 */
792 ns.FullScreen = Y.Base.create('browser-view-fullscreen', ns.MainView, [], {
793- _fullscreenTarget: '/bws/sidebar',
794-
795 template: views.Templates.fullscreen,
796
797-
798 /**
799- * The default view is the editorial rendering. Render this view out.
800+ * Render out the view to the DOM.
801 *
802- * @method _renderEditorialView
803- * @param {Node} container node to render out to.
804+ * @method render
805+ * @param {Node} container optional specific container to render out to.
806 *
807 */
808- _renderEditorialView: function(container) {
809+ render: function(container) {
810 var tpl = this.template(),
811 tplNode = Y.Node.create(tpl);
812
813 this._renderSearchWidget(tplNode);
814
815- if (!Y.Lang.isValue(container)) {
816+ if (typeof container !== 'object') {
817 container = this.get('container');
818+ } else {
819+ this.set('container', container);
820 }
821 container.setHTML(tplNode);
822- },
823-
824- /**
825- * Render out the view to the DOM.
826- *
827- * @method render
828- * @param {Node} container optional specific container to render out to.
829- *
830- */
831- render: function(container) {
832- if (this.get('charmID')) {
833- this._renderCharmView(container);
834- } else {
835- this._renderEditorialView(container);
836- }
837-
838 // Bind our view to the events from the search widget used for controls.
839 this._bindSearchWidgetEvents();
840 }
841
842=== modified file 'app/subapps/browser/views/sidebar.js'
843--- app/subapps/browser/views/sidebar.js 2013-04-11 14:48:10 +0000
844+++ app/subapps/browser/views/sidebar.js 2013-04-16 00:07:30 +0000
845@@ -10,9 +10,7 @@
846 */
847 YUI.add('subapp-browser-sidebar', function(Y) {
848 var ns = Y.namespace('juju.browser.views'),
849- models = Y.namespace('juju.models'),
850- views = Y.namespace('juju.views'),
851- widgets = Y.namespace('juju.widgets');
852+ views = Y.namespace('juju.views');
853
854
855 /**
856@@ -23,56 +21,17 @@
857 *
858 */
859 ns.Sidebar = Y.Base.create('browser-view-sidebar', ns.MainView, [], {
860- _fullscreenTarget: '/bws/fullscreen',
861-
862 template: views.Templates.sidebar,
863- visible: true,
864-
865- events: {
866- '.charm-token': {
867- 'click': '_handleTokenSelect'
868- }
869- },
870-
871- /**
872- * Event handler for selecting a charm from a list on the page. Forces a
873- * render of the charm details view for the user.
874- *
875- * @method _handleTokenSelect
876- * @param {Event} ev the click event from the charm token.
877- *
878- */
879- _handleTokenSelect: function(ev) {
880- var id = ev.currentTarget.getData('charmid');
881- var model = this._cacheCharms.getById(id);
882- var container = this.get('container');
883-
884- // Deselect the currently selected charm and highlight the new one.
885- var selected_charm = container.one('.yui3-charmtoken.active');
886- if (selected_charm) {
887- selected_charm.removeClass('active');
888- }
889- ev.currentTarget.ancestor('.yui3-charmtoken').addClass('active');
890-
891- // Show the details view for this model.
892- this._renderCharmDetails(
893- model,
894- container
895- );
896- },
897-
898- /**
899- * Initially we load editorial content to populate the sidebar. Build this
900- * content.
901- *
902- * @method _renderEditorialView
903- * @param {Node} container A node to stick the rendered output into.
904- *
905- */
906- _renderEditorialView: function(container) {
907+
908+ /**
909+ * Render out the view to the DOM.
910+ *
911+ * @method render
912+ *
913+ */
914+ render: function(container) {
915 var tpl = this.template(),
916- tplNode = Y.Node.create(tpl),
917- store = this.get('store');
918+ tplNode = Y.Node.create(tpl);
919
920 this._renderSearchWidget(tplNode);
921
922@@ -83,104 +42,6 @@
923 }
924
925 container.setHTML(tplNode);
926-
927- // By default we grab the editorial content from the api to use for
928- // display.
929- this.get('store').sidebarEditorial({
930- 'success': function(data) {
931-
932- // Add featured charms
933- var featuredCharms = this.get('store').resultsToCharmlist(
934- data.result.featured);
935- var featuredContainer = container.one('.bws-left .featured');
936- var featuredCharmContainer = new widgets.browser.CharmContainer({
937- name: 'Featured Charms',
938- cutoff: 1,
939- children: featuredCharms.map(function(charm) {
940- return charm.getAttrs(); })
941- });
942- featuredCharmContainer.render(featuredContainer);
943-
944- // Add popular charms
945- var popularCharms = this.get('store').resultsToCharmlist(
946- data.result.popular);
947- var popularContainer = container.one('.bws-left .popular');
948- var popularCharmContainer = new widgets.browser.CharmContainer({
949- name: 'Popular Charms',
950- cutoff: 2,
951- children: popularCharms.map(function(charm) {
952- return charm.getAttrs(); })
953- });
954- popularCharmContainer.render(popularContainer);
955-
956- // Add in the charm tokens for the new as well.
957- var newContainer = container.one('.bws-left .new');
958- var newCharms = this.get('store').resultsToCharmlist(
959- data.result['new']);
960- var newCharmContainer = new widgets.browser.CharmContainer({
961- name: 'New Charms',
962- cutoff: 2,
963- children: newCharms.map(function(charm) {
964- return charm.getAttrs(); })
965- });
966- newCharmContainer.render(newContainer);
967-
968- // Add the charms to the cache for use in other views.
969- // Start with a reset to empty any current cached models.
970- this._cacheCharms.reset(newCharms);
971- this._cacheCharms.add(popularCharms);
972- this._cacheCharms.add(featuredCharms);
973- this.charmContainers = [
974- featuredCharmContainer,
975- newCharmContainer,
976- popularCharmContainer
977- ];
978- },
979-
980- 'failure': function(data, request) {
981- var message;
982- if (data && data.type) {
983- message = 'Charm API error of type: ' + data.type;
984- } else {
985- message = 'Charm API server did not respond';
986- }
987- this.get('db').notifications.add(
988- new models.Notification({
989- title: 'Failed to load sidebar content.',
990- message: message,
991- level: 'error'
992- })
993- );
994- }
995- }, this);
996- },
997-
998- /**
999- * Destroy this view and clear from the dom world.
1000- *
1001- * @method destructor
1002- *
1003- */
1004- destructor: function() {
1005- if (this.charmContainers) {
1006- Y.Array.each(this.charmContainers, function(container) {
1007- container.destroy();
1008- });
1009- }
1010- },
1011-
1012- /**
1013- * Render out the view to the DOM.
1014- *
1015- * @method render
1016- *
1017- */
1018- render: function(container) {
1019- if (this.get('charmID')) {
1020- this._renderCharmView(container);
1021- } else {
1022- this._renderEditorialView(container);
1023- }
1024 // Bind our view to the events from the search widget used for controls.
1025 this._bindSearchWidgetEvents();
1026 }
1027@@ -191,13 +52,8 @@
1028
1029 }, '0.1.0', {
1030 requires: [
1031- 'browser-charm-container',
1032- 'browser-charm-token',
1033 'browser-search-widget',
1034- 'juju-charm-store',
1035- 'juju-models',
1036 'juju-templates',
1037- 'subapp-browser-charmview',
1038 'subapp-browser-mainview',
1039 'view'
1040 ]
1041
1042=== modified file 'app/subapps/browser/views/view.js'
1043--- app/subapps/browser/views/view.js 2013-04-09 18:10:36 +0000
1044+++ app/subapps/browser/views/view.js 2013-04-16 00:07:30 +0000
1045@@ -78,63 +78,6 @@
1046 );
1047 },
1048
1049- /**
1050- * Helper to just render the charm details pane. This is shared in the
1051- * sidebar/fullscreen views.
1052- *
1053- * @method _renderCharmDetails
1054- * @param {BrowserCharm} charm model instance to render from.
1055- * @param {Node} container node to look for a details div in to render to.
1056- *
1057- */
1058- _renderCharmDetails: function(charm, container) {
1059- var detailsNode = container.one('.bws-view-data');
1060- // Destroy any current details.
1061- if (this.details) {
1062- this.details.destroy(true);
1063- }
1064- this.details = new ns.BrowserCharmView({
1065- charm: charm,
1066- store: this.get('store')
1067- });
1068- this.details.render(detailsNode);
1069- },
1070-
1071- /**
1072- * Render the view of a single charm details page.
1073- *
1074- * @method _renderCharmView
1075- * @param {Node} container the node to insert our rendered content into.
1076- *
1077- */
1078- _renderCharmView: function(container) {
1079- var tpl = this.template(),
1080- tplNode = Y.Node.create(tpl);
1081-
1082- // Create/bind the search before we wait for the charm data to load so
1083- // that we're prepared for search events in case that request takes a
1084- // while or even fails.
1085- this._renderSearchWidget(tplNode);
1086-
1087- // We need to have the template in the DOM for sub views to be able to
1088- // expect proper structure.
1089- if (!Y.Lang.isValue(container)) {
1090- container = this.get('container');
1091- }
1092- container.setHTML(tplNode);
1093-
1094- this.get('store').charm(this.get('charmID'), {
1095- 'success': function(data) {
1096- var charmView = new ns.BrowserCharmView({
1097- charm: new models.BrowserCharm(data),
1098- store: this.get('store')
1099- });
1100- charmView.render(tplNode.one('.bws-view-data'), this.isFullscreen());
1101- container.setHTML(tplNode);
1102- },
1103- 'failure': this.apiFailure
1104- }, this);
1105- },
1106
1107 /**
1108 * Render out the main search widget and controls shared across various
1109@@ -271,7 +214,6 @@
1110 */
1111 charmID: {},
1112
1113-
1114 /**
1115 * An instance of the Charmworld API object to hit for any data that
1116 * needs fetching.
1117@@ -295,7 +237,6 @@
1118 *
1119 */
1120 subpath: {}
1121-
1122 }
1123 });
1124
1125
1126=== modified file 'app/templates/charm-token.handlebars'
1127--- app/templates/charm-token.handlebars 2013-04-15 07:26:46 +0000
1128+++ app/templates/charm-token.handlebars 2013-04-16 00:07:30 +0000
1129@@ -1,19 +1,21 @@
1130-<div class="charm-token yui3-g" data-charmid="{{id}}">
1131- <div class="yui3-u-1-4">
1132- <img url="{{ iconfile }}" alt="Icon" />
1133- </div>
1134- <div class="yui3-u-3-4">
1135- <a href="" class="add hidden">
1136- <img src="/juju-ui/assets/images/sidebar_add_icon.jpg" alt="Add" />
1137- </a>
1138- <h3 class="title">
1139- {{ name }}
1140- </h3>
1141- <div class="metadata">
1142- {{ recent_commit_count }} {{pluralize 'commit' recent_commit_count}},
1143- {{ recent_download_count }}
1144- {{pluralize 'download' recent_download_count}}
1145+<a href="/bws/sidebar/{{id}}">
1146+ <div class="charm-token yui3-g" data-charmid="{{id}}">
1147+ <div class="yui3-u-1-4">
1148+ <img url="{{ iconfile }}" alt="Icon" />
1149+ </div>
1150+ <div class="yui3-u-3-4">
1151+ <a href="" class="add hidden">
1152+ <img src="/juju-ui/assets/images/sidebar_add_icon.jpg" alt="Add" />
1153+ </a>
1154+ <h3 class="title">
1155+ {{ name }}
1156+ </h3>
1157+ <div class="metadata">
1158+ {{ recent_commit_count }} {{pluralize 'commit' recent_commit_count}},
1159+ {{ recent_download_count }}
1160+ {{pluralize 'download' recent_download_count}}
1161+ </div>
1162+ <p class="description">{{truncate description 110 }}</p>
1163+ </div>
1164 </div>
1165- <p class="description">{{truncate description 110 }}</p>
1166- </div>
1167-</div>
1168+</a>
1169
1170=== modified file 'test/test_browser_app.js'
1171--- test/test_browser_app.js 2013-04-11 14:48:10 +0000
1172+++ test/test_browser_app.js 2013-04-16 00:07:30 +0000
1173@@ -130,53 +130,6 @@
1174 assert.isTrue(Y.Lang.isObject(container.one('input')));
1175 });
1176
1177- it('caches models fetched from the api for later use', function() {
1178- var container = Y.one('#subapp-browser');
1179- view = new Sidebar();
1180- view._cacheCharms.size().should.eql(0);
1181-
1182- // mock out the request data for the editorial view. We want to make
1183- // sure we're caching the results.
1184- view.get('store').set(
1185- 'datasource',
1186- new Y.DataSource.Local({source: sampleData}));
1187- view.render(container);
1188-
1189- view._cacheCharms.size().should.eql(5);
1190- });
1191-
1192- it('handles details event when clicking on a charm token', function(done) {
1193- var container = Y.one('#subapp-browser');
1194- view = new Sidebar();
1195-
1196- // Test is successful when it completes by hitting this callback we've
1197- // over written.
1198- view._handleTokenSelect = function(ev) {
1199- done();
1200- };
1201-
1202- view.get('store').set(
1203- 'datasource',
1204- new Y.DataSource.Local({source: sampleData}));
1205- view.render(container);
1206- container.one('.charm-token').simulate('click');
1207- });
1208-
1209- it('renders details when clicking on a charm token', function() {
1210- var container = Y.one('#subapp-browser');
1211- view = new Sidebar();
1212-
1213- view.get('store').set(
1214- 'datasource',
1215- new Y.DataSource.Local({source: sampleData}));
1216- view.render(container);
1217- container.one('.charm-token').simulate('click');
1218-
1219- var details_node = container.one('.bws-view-data');
1220- details_node.one('h1').get('text').should.eql('byobu-classroom');
1221- details_node.all('.tabs').size().should.eql(1);
1222- });
1223-
1224 });
1225 })();
1226
1227
1228=== modified file 'test/test_browser_charm_details.js'
1229--- test/test_browser_charm_details.js 2013-04-15 07:26:46 +0000
1230+++ test/test_browser_charm_details.js 2013-04-16 00:07:30 +0000
1231@@ -23,9 +23,13 @@
1232 });
1233
1234 beforeEach(function() {
1235- var docBody = Y.one(document.body);
1236- Y.Node.create('<div id="testcontent">' +
1237- '</div>').appendTo(docBody);
1238+ var docBody = Y.one(document.body),
1239+ testcontent = [
1240+ '<div id=testcontent><div class="bws-view-data">',
1241+ '</div></div>'
1242+ ].join();
1243+
1244+ Y.Node.create(testcontent).appendTo(docBody);
1245
1246 // Mock out a dummy location for the Store used in view instances.
1247 window.juju_config = {
1248@@ -87,10 +91,11 @@
1249 ],
1250 id: 'precise/ceph-9'
1251 }),
1252+ container: Y.Node.create('<div class="charmview"/>'),
1253 store: fakeStore
1254 });
1255
1256- view.render(node);
1257+ view.render();
1258 Y.one('#bws-readme').get('text').should.eql('README content.');
1259 });
1260
1261@@ -102,7 +107,8 @@
1262 'hooks/install'
1263 ],
1264 id: 'precise/ceph-9'
1265- })
1266+ }),
1267+ container: Y.Node.create('<div class="charmview"/>')
1268 });
1269
1270 // Hook up to the callback for the click event.
1271@@ -112,8 +118,7 @@
1272 done();
1273 };
1274
1275- view.render(node);
1276- view.get('container').should.eql(node);
1277+ view.render();
1278 node.one('.charm .add').simulate('click');
1279 });
1280
1281@@ -140,10 +145,11 @@
1282 ],
1283 id: 'precise/ceph-9'
1284 }),
1285+ container: Y.Node.create('<div class="charmview"/>'),
1286 store: fakeStore
1287 });
1288
1289- view.render(node);
1290+ view.render();
1291 Y.one('#bws-hooks').all('select option').size().should.equal(3);
1292
1293 // Select the hooks install and the content should update.
1294@@ -180,10 +186,11 @@
1295 ],
1296 id: 'precise/ceph-9'
1297 }),
1298+ container: Y.Node.create('<div class="charmview"/>'),
1299 store: fakeStore
1300 });
1301
1302- view.render(node);
1303+ view.render();
1304 Y.one('#bws-readme').get('innerHTML').should.eql(
1305 '<h1>README Header</h1>');
1306 });
1307@@ -200,9 +207,10 @@
1308 'type': 'int'
1309 }
1310 }
1311- })
1312+ }),
1313+ container: Y.Node.create('<div class="charmview"/>')
1314 });
1315- view.render(node);
1316+ view.render();
1317
1318 Y.one('#bws-configuration dd div').get('text').should.eql(
1319 'Default: 9160');
1320@@ -235,9 +243,10 @@
1321 charm: new models.BrowserCharm({
1322 files: [],
1323 id: 'precise/ceph-9'
1324- })
1325+ }),
1326+ container: Y.Node.create('<div class="charmview"/>')
1327 });
1328- view.render(node);
1329+ view.render();
1330
1331 view._loadQAContent = function() {
1332 // This test is just verifying that we don't timeout. The event fired,
1333@@ -274,7 +283,8 @@
1334 // We don't want any files so we don't have to mock/load them.
1335 data.files = [];
1336 var view = new CharmView({
1337- charm: new models.BrowserCharm(data)
1338+ charm: new models.BrowserCharm(data),
1339+ container: Y.Node.create('<div class="charmview"/>')
1340 });
1341
1342 // Hook up to the callback for the click event.
1343@@ -283,9 +293,8 @@
1344 done();
1345 };
1346
1347- view.render(node);
1348- view.get('container').should.eql(node);
1349- node.one('.changelog .expandToggle').simulate('click');
1350+ view.render();
1351+ node.one('.changelog .expand').simulate('click');
1352 });
1353
1354 it('changelog is reformatted and displayed', function() {
1355@@ -295,10 +304,11 @@
1356 // We don't want any files so we don't have to mock/load them.
1357 data.files = [];
1358 view = new CharmView({
1359- charm: new models.BrowserCharm(data)
1360+ charm: new models.BrowserCharm(data),
1361+ container: Y.Node.create('<div class="charmview"/>')
1362 });
1363
1364- view.render(node);
1365+ view.render();
1366 // Basics that we have the right number of nodes.
1367 node.all('.remaining li').size().should.eql(9);
1368 node.all('.first p').size().should.eql(1);

Subscribers

People subscribed via source and target branches