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
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 browser/ browser. js (right):
File app/subapps/
https:/ /codereview. appspot. com/10870043/ diff/1/ app/subapps/ browser/ browser. js#newcode964 browser/ browser. js:964: default_viewmode: {
app/subapps/
camelCase please
https:/ /codereview. appspot. com/10870043/ diff/1/ test/test_ browser_ app.js browser_ app.js (right):
File test/test_
https:/ /codereview. appspot. com/10870043/ diff/1/ test/test_ browser_ app.js# newcode266 browser_ app.js: 266: app = new Browser( {default_ viewmode: 'sidebar'});
test/test_
browser.
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/