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 isFullscreen as a param and then overwrite it regardless.

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

https://codereview.appspot.com/8679045/diff/3001/app/subapps/browser/views/editorial.js#newcode62
app/subapps/browser/views/editorial.js:62: 'success': function(data) {
these callbacks are pretty large I'd almost like them moved out to their
own methods.

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

https://codereview.appspot.com/8679045/diff/3001/app/subapps/browser/views/fullscreen.js#newcode34
app/subapps/browser/views/fullscreen.js:34: if (typeof container !==
'object') {
I've seen this a couple times here - I am assuming this has something to
do with the multiple render bit..maybe add some documentation as to
what's going on here.

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

https://codereview.appspot.com/8679045/diff/3001/app/templates/charm-token.handlebars#newcode1
app/templates/charm-token.handlebars:1: <a href="/bws/sidebar/{{id}}">
this route will need to be generated then passed in.

https://codereview.appspot.com/8679045/diff/3001/test/test_browser_app.js
File test/test_browser_app.js (left):

https://codereview.appspot.com/8679045/diff/3001/test/test_browser_app.js#oldcode133
test/test_browser_app.js:133: it('caches models fetched from the api for
later use', function() {
Were these the tests you were still working on?

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