Merge lp:~rharding/juju-gui/update-urls into lp:juju-gui/experimental
- update-urls
- Merge into trunk
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 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju GUI Hackers | Pending | ||
Review via email: mp+158713@code.launchpad.net |
Commit message
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.
Richard Harding (rharding) wrote : | # |
- 555. By Richard Harding
-
Garden
- 556. By Richard Harding
-
Garden
Richard Harding (rharding) wrote : | # |
Please take a look.
Richard Harding (rharding) wrote : | # |
Please take a look.
Jeff Pihach (hatch) wrote : | # |
Thanks for the refactor - I have a few comments below that I'd like some
discussion on.
https:/
File app/subapps/
https:/
app/subapps/
unused
https:/
app/subapps/
should this not be
var containerID = this.get(
https:/
app/subapps/
'fullscreen') {
see comment in routes.
https:/
app/subapps/
maybe add a comment here as to what's happening here.
https:/
app/subapps/
maybe a comment here as to why this is necessary
https:/
app/subapps/
!== -1) {
see comment in routes
https:/
app/subapps/
maybe some documentation as to what's going on here.
https:/
app/subapps/
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:/
File app/subapps/
https:/
app/subapps/
href="/
isFullscreen}
this url will need to be generated and then passed in with the hbs
params to work properly with both namespaces.
https:/
app/subapps/
isn't changelog one word?
https:/
File app/subapps/
https:/
app/subapps/
you accept ...
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:/
File app/subapps/
https:/
app/subapps/
href="/
isFullscreen}
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:/
app/subapps/
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:/
File app/subapps/
https:/
app/subapps/
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.
- 557. By Richard Harding
-
Update per review
Richard Harding (rharding) wrote : | # |
Please take a look.
j.c.sackett (jcsackett) wrote : | # |
Thanks for this refactor; I have some comments and questions below.
https:/
File app/subapps/
https:/
app/subapps/
Don't we need a route like "/bws/sidebar/id*/* to match things like the
readme and interface routes above?
https:/
File app/subapps/
https:/
app/subapps/
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:/
File app/subapps/
https:/
app/subapps/
Does this (and the events change in views/charm.js) have to do with the
routes &c change, or just a driveby?
https:/
File app/subapps/
https:/
app/subapps/
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:/
File app/subapps/
https:/
app/subapps/
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:/
File app/templates/
https:/
app/templates/
Can you update the token's less file to make sure pointer type remains
normal now that the entire thing is a link?
Richard Harding (rharding) wrote : | # |
Comments below. Will have an updated branch in a bit.
https:/
File app/subapps/
https:/
app/subapps/
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:/
File app/subapps/
https:/
app/subapps/
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/
to fill the sidebar while the charm details loads next to it.
/bws/fullscreen
don't need to render the editorial data so we bypass it.
https:/
File app/subapps/
https:/
app/subapps/
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:/
File app/subapps/
https:/
app/subapps/
node to insert our rendered content into.
On 2013/04/15 21:06:34, j.c.sackett wrote:
> These params don't match; ...
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/
to fill
> the sidebar while the charm details loads next to it.
> /bws/fullscreen
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.
j.c.sackett (jcsackett) wrote : | # |
LGTM from discussion and with promised fixes. Thanks!
Jeff Pihach (hatch) wrote : | # |
LGTM Thanks for all the work!
- 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
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:/
Preview Diff
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); |
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: debug.js browser/ browser. js browser/ templates/ browser_ charm.handlebar s browser/ templates/ editorial. handlebars browser/ templates/ sidebar. handlebars browser/ views/charm. js browser/ views/editorial .js browser/ views/fullscree n.js browser/ views/sidebar. js browser/ views/view. js charm-token. handlebars browser_ app.js browser_ charm_details. js
A [revision details]
M app/modules-
M app/subapps/
M app/subapps/
A app/subapps/
M app/subapps/
M app/subapps/
A app/subapps/
M app/subapps/
M app/subapps/
M app/subapps/
M app/templates/
M test/test_
M test/test_