Code review comment for lp:~jcsackett/juju-gui/abstract-default-viewmode

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

LGTM with the change to the var name to defaultViewmode.

I'd prefer to see this along with the introduction of the config param
so that qa'ing it could be done with a change to the config file vs the
actual View's default ATTR value.

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

https://codereview.appspot.com/10870043/diff/1/app/subapps/browser/browser.js#newcode964
app/subapps/browser/browser.js:964: default_viewmode: {
camelCase please

https://codereview.appspot.com/10870043/diff/1/test/test_browser_app.js
File test/test_browser_app.js (right):

https://codereview.appspot.com/10870043/diff/1/test/test_browser_app.js#newcode266
test/test_browser_app.js:266: app = new
browser.Browser({default_viewmode: 'sidebar'});
you don't need to add this here as it's picking up the default value.
Removing it verifies the default value is correct.

https://codereview.appspot.com/10870043/

« Back to merge proposal