Changes browser subapp default to be configurable

Changes browser subapp default to be configurable

Rather than using sidebar, the browser subapp has a default viewmode as an attr.
Where sidebar was used before, this attr is instead used. You can qa it by
setting the default for the attr to fullscreen. For now at least, the default
for the attr remains "sidebar".

https://code.launchpad.net/~jcsackett/juju-gui/abstract-default-viewmode/+merge/172560

(do not edit description out of merge proposal)

[j.c.sackett, 2013-07-02 13:31:47]

Please take a look.

[rharding, 2013-07-02 13:55:48]

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...
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#newco...
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.

[j.c.sackett, 2013-07-02 14:30:27]

On 2013/07/02 13:55:48, 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...
> app/subapps/browser/browser.js:964: default_viewmode: {
> camelCase please

Got it.

> 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#newco...
> 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.

I only passed it in so you can see the correlation between the set value and the
results of the test.

[j.c.sackett, 2013-07-02 15:03:36]

Please take a look.

[jeff.pihach, 2013-07-02 15:13:15]

LGTM'd thanks for this improvement! 

As mentioned in IRC - next step being able to be set via the config.js files.

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

https://codereview.appspot.com/10870043/diff/7001/test/test_browser_app.js#ne...
test/test_browser_app.js:245: it('* route uses the default viewmode', function()
{
it('routes * by using the default viewmode' ... 

PTDD  (pedantic tdd) ;-)

[j.c.sackett, 2013-07-02 15:34:40]

On 2013/07/02 15:13:15, jeff.pihach wrote:
> LGTM'd thanks for this improvement! 
> 
> As mentioned in IRC - next step being able to be set via the config.js files.
> 
> https://codereview.appspot.com/10870043/diff/7001/test/test_browser_app.js
> File test/test_browser_app.js (right):
> 
>
https://codereview.appspot.com/10870043/diff/7001/test/test_browser_app.js#ne...
> test/test_browser_app.js:245: it('* route uses the default viewmode',
function()
> {
> it('routes * by using the default viewmode' ... 
> 
> PTDD  (pedantic tdd) ;-)

I think that's probably pedantic bdd. :-P I've fixed it.

[j.c.sackett, 2013-07-03 17:36:04]

*** Submitted:

Changes browser subapp default to be configurable

Rather than using sidebar, the browser subapp has a default viewmode as an attr.
Where sidebar was used before, this attr is instead used. You can qa it by
setting the default for the attr to fullscreen. For now at least, the default
for the attr remains "sidebar".

R=rharding, jeff.pihach
CC=
https://codereview.appspot.com/10870043