← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] lp:~sinzui/launchpad/overlay-tab into lp:launchpad

 

Review: Approve

Thanks Curtis, this is very helpful for keyboard users.

I'm not 100% sure, but I think that using the keyup event is better as there can be some keydown/keypress quirks. 

See: http://www.bloggingdeveloper.com/post/KeyPress-KeyDown-KeyUp-The-Difference-Between-Javascript-Key-Events.aspx

for a little example of the diff in keydown/press but I think keyup is pretty standard. Of course now I can't seem to find anything verifying this. 

[edit: I might thinking along the lines of this http://stackoverflow.com/a/7590268/159689]

The tab event handler looks to be fetching the list of tabNodes on every keydown event. Is this cacheable? I know that the dom can change some so perhaps we can't get rid of this need to requery the list over and over.

#159
s/opton/option

#181
Is there supposed to be anything in this blank addition to the content?

#225
The way I understand things, the target should be node(1) vs node(0) [where 0 is the close item we're skipping]
-- 
https://code.launchpad.net/~sinzui/launchpad/overlay-tab/+merge/112901
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.


References