Check for tab id and load tab in charm details.

Check for tab id and load tab in charm details.

- Make the hash part of the subapp state
- Do not reload or redraw the charm details because a different tab was
selected
- The anchor should update itself in the url as each tab is selected.
- Hitting a direct link to a specific charm with the tab id anchor will load
the charm details and auto load the tab specified.

https://code.launchpad.net/~rharding/juju-gui/direct-2-tab/+merge/168570

(do not edit description out of merge proposal)

[rharding, 2013-06-11 01:05:11]

Please take a look.

[j.c.sackett, 2013-06-11 13:31:53]

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

https://codereview.appspot.com/10140047/diff/1/app/subapps/browser/browser.js...
app/subapps/browser/browser.js:424: }
You assign the function var hasChanged (I'm assuming for wrapping, b/c otherwise
this seem unnecessary) but then reference this._hasChanged. Is this meant to be
hasChanged, or was var hasChanged supposed to be an assign to this._hasChanged?

https://codereview.appspot.com/10140047/diff/1/app/subapps/browser/views/char...
File app/subapps/browser/views/charm.js (left):

https://codereview.appspot.com/10140047/diff/1/app/subapps/browser/views/char...
app/subapps/browser/views/charm.js:518: this.tabview.render();
That's a neat trick.

[rharding, 2013-06-11 13:47:57]

Replied

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

https://codereview.appspot.com/10140047/diff/1/app/subapps/browser/browser.js...
app/subapps/browser/browser.js:424: }
On 2013/06/11 13:31:53, j.c.sackett wrote:
> You assign the function var hasChanged (I'm assuming for wrapping, b/c
otherwise
> this seem unnecessary) but then reference this._hasChanged. Is this meant to
be
> hasChanged, or was var hasChanged supposed to be an assign to
this._hasChanged?

Yea, this was all a mistake. I hated how long it got, but as you pointed out, I
messed it up. I'm just moving it back to _hasStageChanged and deal with the long
wrapping lines.

[rharding, 2013-06-11 13:51:06]

Please take a look.

[jeff.pihach, 2013-06-11 14:21:26]

LGTM with an additional test request.

https://codereview.appspot.com/10140047/diff/6001/test/test_browser_charm_det...
File test/test_browser_charm_details.js (right):

https://codereview.appspot.com/10140047/diff/6001/test/test_browser_charm_det...
test/test_browser_charm_details.js:767: 
I am wondering if we shouldn't have a test here which tests the url for the
proper hash after changing the tab.

[rharding, 2013-06-11 14:24:21]

On 2013/06/11 14:21:26, jeff.pihach wrote:
> LGTM with an additional test request.
> 
>
https://codereview.appspot.com/10140047/diff/6001/test/test_browser_charm_det...
> File test/test_browser_charm_details.js (right):
> 
>
https://codereview.appspot.com/10140047/diff/6001/test/test_browser_charm_det...
> test/test_browser_charm_details.js:767: 
> I am wondering if we shouldn't have a test here which tests the url for the
> proper hash after changing the tab.

I didn't add a test for this because it's the tabview widget doing that work.
I'd be testing a widget which already has tests.

[j.c.sackett, 2013-06-11 14:26:16]

LGTM.

Thanks for the changes.

[rharding, 2013-06-11 14:50:35]

*** Submitted:

Check for tab id and load tab in charm details.

- Make the hash part of the subapp state
- Do not reload or redraw the charm details because a different tab was
selected
- The anchor should update itself in the url as each tab is selected.
- Hitting a direct link to a specific charm with the tab id anchor will load
the charm details and auto load the tab specified.

R=j.c.sackett, jeff.pihach
CC=
https://codereview.appspot.com/10140047