← Back to team overview

yellow team mailing list archive

Re: application hotkeys (issue 6849078)

 

Thanks for the reply!  +1 with the navigateTo change


https://codereview.appspot.com/6849078/diff/1/app/app.js
File app/app.js (right):

https://codereview.appspot.com/6849078/diff/1/app/app.js#newcode145
app/app.js:145: this.show_environment();
On 2012/11/19 19:07:54, thiago wrote:
> On 2012/11/19 18:36:51, matthew.scott wrote:
> > I'm not sure this is the right way to go about this, as it causes a
> regression.
> > If I view a service, then hit alt+E, the environment view is shown,
but not
> > navigated to, leaving the url, in my case,
> http://localhost:8888/service/mysql/.
> >  This breaks back-button behavior.
> >
> > * view one service, mysql
> > * hit alt+E
> > * view another service, wordpress
> > * hit back: get taken to viewing mysql, because that was the last
thing in the
> > URL stack.

> Good catch. Thanks.
> I've solved this issue by firing a 'navidateTo' event. So now instead
of
> this.show_environment(), we call this.fire('navigateTo', { url: '/'
});

Plugged this in, works well.  Thanks!

https://codereview.appspot.com/6849078/diff/1/app/app.js#newcode557
app/app.js:557: if (next) {
On 2012/11/19 19:07:54, thiago wrote:
> On 2012/11/19 18:36:51, matthew.scott wrote:
> > Perhaps Y.Lang.isFunction(next), here?

> Hmmm. Not shure. I just want to test if we have the next object (for
unit tests
> purposes). If the developer passes one thing other than a function, we
should
> just let the exception be thrown. wdyt?

I don't feel that strongly either way :)  Feel free to keep it as is,
unless someone else minds.

https://codereview.appspot.com/6849078/

-- 
https://code.launchpad.net/~tveronezi/juju-gui/hotkeys/+merge/131382
Your team Juju GUI Hackers is subscribed to branch lp:juju-gui.


References