Generic show view event

Generic show view event

Instead of wrapping the 'navigate' method call with multiple 'show' events, we want call the 'navigate' method from a single event.

So, instead of...

this.on('*:showService', this.navigate_to_service);
this.on('*:showUnit', this.navigate_to_unit);
this.on('*:showCharm', this.navigate_to_charm);
this.on('*:showEnvironment', this.navigate_to_environment);

... we will have...

this.on('*:navigateTo', function(e) {
 console.log('navigateTo', e);
 this.navigate(e.url);
}, this);

https://code.launchpad.net/~tveronezi/juju-gui/generic-show-view-event/+merge/133247

(do not edit description out of merge proposal)

[thiago, 2012-11-07 13:53:23]

Please take a look.

[benji, 2012-11-07 18:12:57]

This branch looks good.  The added test assertions were a nice touch.

The only problem I saw was some object literal style deviation.  The style guide
describes our preferred style there.  After that is fixed this branch looks good
to land.

https://codereview.appspot.com/6819104/diff/1/app/views/charm.js
File app/views/charm.js (right):

https://codereview.appspot.com/6819104/diff/1/app/views/charm.js#newcode90
app/views/charm.js:90: this.fire('navigateTo', {
This doesn't match our normal object literal multi-line format.  Perhaps it
should just be a one-liner instead.

https://codereview.appspot.com/6819104/diff/1/app/views/charm.js#newcode135
app/views/charm.js:135: self.fire('navigateTo', {
Insert another object literal style nitpick here.

https://codereview.appspot.com/6819104/diff/1/app/views/service.js
File app/views/service.js (right):

https://codereview.appspot.com/6819104/diff/1/app/views/service.js#newcode219
app/views/service.js:219: this.fire('navigateTo', {
object literal style

https://codereview.appspot.com/6819104/diff/1/app/views/service.js#newcode866
app/views/service.js:866: this.fire('navigateTo', {
object literal style

https://codereview.appspot.com/6819104/diff/1/app/views/unit.js
File app/views/unit.js (right):

https://codereview.appspot.com/6819104/diff/1/app/views/unit.js#newcode211
app/views/unit.js:211: this.fire('navigateTo', {
object literal style

[bac, 2012-11-07 21:46:06]

Since Benji picked all of the nits I think this branch looks good.  Nice
simplification and, as Benji said, the extra bit of testing is good.

Thanks!

[thiago, 2012-11-08 16:22:01]

Thanks guys!

https://codereview.appspot.com/6819104/diff/1/app/views/charm.js
File app/views/charm.js (right):

https://codereview.appspot.com/6819104/diff/1/app/views/charm.js#newcode90
app/views/charm.js:90: this.fire('navigateTo', {
On 2012/11/07 18:12:57, benji wrote:
> This doesn't match our normal object literal multi-line format.  Perhaps it
> should just be a one-liner instead.

Done.

https://codereview.appspot.com/6819104/diff/1/app/views/charm.js#newcode135
app/views/charm.js:135: self.fire('navigateTo', {
On 2012/11/07 18:12:57, benji wrote:
> Insert another object literal style nitpick here.

Done.

https://codereview.appspot.com/6819104/diff/1/app/views/service.js
File app/views/service.js (right):

https://codereview.appspot.com/6819104/diff/1/app/views/service.js#newcode219
app/views/service.js:219: this.fire('navigateTo', {
On 2012/11/07 18:12:57, benji wrote:
> object literal style

Done.

https://codereview.appspot.com/6819104/diff/1/app/views/service.js#newcode866
app/views/service.js:866: this.fire('navigateTo', {
On 2012/11/07 18:12:57, benji wrote:
> object literal style

Done.

https://codereview.appspot.com/6819104/diff/1/app/views/unit.js
File app/views/unit.js (right):

https://codereview.appspot.com/6819104/diff/1/app/views/unit.js#newcode211
app/views/unit.js:211: this.fire('navigateTo', {
On 2012/11/07 18:12:57, benji wrote:
> object literal style

Done.

[thiago, 2012-11-08 16:28:14]

*** Submitted:

Generic show view event

Instead of wrapping the 'navigate' method call with multiple 'show' events, we
want call the 'navigate' method from a single event.

So, instead of...

this.on('*:showService', this.navigate_to_service);
this.on('*:showUnit', this.navigate_to_unit);
this.on('*:showCharm', this.navigate_to_charm);
this.on('*:showEnvironment', this.navigate_to_environment);

... we will have...

this.on('*:navigateTo', function(e) {
 console.log('navigateTo', e);
 this.navigate(e.url);
}, this);

R=benji, bac
CC=
https://codereview.appspot.com/6819104