Code review comment for lp:~rharding/juju-gui/update-urls

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/