← Back to team overview

yellow team mailing list archive

Re: application hotkeys (issue 6849078)

 

With the navigate change this looks fine. I might remove the //XXX:
comment at the top of activate keys as even though we were not able to
use the YUI framework for this it is working code and working code
doesn't need to cry out for developer attention.

I'd also add a card indicating that we need design on a page bound to
'?'|'h' or similar that can list the hotkeys (and prehaps a link to help
in the base application template). These are useful but not
discoverable.

With that card in place, +1


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#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?

This is the 'next' route in the App.Router chain, the persistent views
need this to continue processing, but they'd either all need an 'if
(next) next();' guard or not.

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