Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Enhanced Pseudos:keys and new Pseudos:stop #245

Open
wants to merge 3 commits into
from

Conversation

Projects
None yet
6 participants
Member

w00fz commented May 23, 2011

The Pseudos:keys now supports multiple key combinations element.addEvent('keydown:keys('shift+a, ,, delete, esc', myFun); and there's a new Pseudos:stop element.addEvent('click:stop', myFun);

- support for multiple key-combos for Pseudos:keys - new Pseudos:sto…
…p , prevents propagation of events directly from the pseudo - updated docs - added more specs for both Pseudos:keys and new Pseudos:stop
Owner

cpojer commented May 23, 2011

I like the extension to Keyboard, but I dislike the :stop pseudo. It's a good idea but we shouldn't encourage our users to use event.stop() when you really want event.preventDefault(). There are only few cases where you want to stop propagating. I'm in favor of adding click:preventDefault instead of click:stop. I might agree to adding all three, ie. click:stop, click:stopPropagation, click:preventDefault.

Owner

arian commented May 23, 2011

Pretty sweet! Usually I just use event.stop() but that might not be best practice. Adding preventDefault and stopPropagation might solve that: http://jsfiddle.net/v2Wbs/

Owner

cpojer commented May 23, 2011

Usually when you say event.stop() you really mean event.preventDefault()

#234 anyone?

w00fz added some commits May 26, 2011

Revert " - support for multiple key-combos for Pseudos:keys - new Pse…
…udos:stop , prevents propagation of events directly from the pseudo - updated docs - added more specs for both Pseudos:keys and new Pseudos:stop"

This reverts commit 418800c.
- enhancing Pseudos.Stop to include all 3 :stop, :stopPropagation an…
…d :preventDefault

 - added Specs for all 3 pseudos
 - updated docs
Member

w00fz commented May 26, 2011

I have reverted my multiple keys support on the Pseudo.Keys (should pull from @seanmonstar, I haven't seen it was there already, sorry about that) and Pseudos.Stop now has support for all 3 :stopPropagation, :preventDefault, :stop.
In the Docs they are in that order, with :stop as last, just for @cpojer .

Owner

arian commented May 26, 2011

I approve

Owner

cpojer commented May 27, 2011

@arian then merge it

Owner

arian commented May 27, 2011

@cpojer: what do you think i'm doing?

On Fri, May 27, 2011 at 12:48 PM, cpojer
reply@reply.github.com
wrote:

@arian then merge it

Reply to this email directly or view it on GitHub:
#245 (comment)

Owner

anutron commented Jun 5, 2011

blerg to merges.

Owner

arian commented Jun 5, 2011

This should be pushed / merged / rebased / whatever together with #234. A week ago I did that, but @timwienk has some improved branch, so I reverted that again. However it should still be pushed.

Owner

timwienk commented Jun 6, 2011

Specifically this branch.

Owner

timwienk commented Jun 6, 2011

I just pulled w00fz' pseudos on top of my branch. I did another commit, which I think is a good idea.

I think that if we're going to add these pseudos, it's a good idea to make them a bit more flexible and check the first argument. If you guys don't agree, I can obviously push without that commit.

Specs seem to pass in Chrome.

Member

w00fz commented Jun 6, 2011

I'm totally fine with it. Although most of the times it's a true check it gives some more flexibility which is good and does cost nothing.

// Djamil Legato

On Jun 6, 2011, at 12:08 AM, timwienk reply@reply.github.com wrote:

I just pulled w00fz' pseudos on top of my branch. I did another commit, which I think is a good idea.

I think that if we're going to add these pseudos, I think it's a good idea to make them a bit more flexible and check the first argument. If you guys don't agree, I can obviously push without that commit.

Specs seem to pass in Chrome.

Reply to this email directly or view it on GitHub:
#245 (comment)

Owner

arian commented Jul 31, 2011

@timwienk, @w00fz, @seanmonstar: what's the status of this? is it pushable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment