Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhanced Pseudos:keys and new Pseudos:stop #245

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Enhanced Pseudos:keys and new Pseudos:stop #245

wants to merge 3 commits into from

Conversation

w00fz
Copy link
Member

@w00fz 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);

…p , prevents propagation of events directly from the pseudo - updated docs - added more specs for both Pseudos:keys and new Pseudos:stop
@cpojer
Copy link
Member

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.

@arian
Copy link
Member

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/

@cpojer
Copy link
Member

cpojer commented May 23, 2011

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

@seanmonstar
Copy link

#234 anyone?

w00fz added 2 commits May 26, 2011 12:30
…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.
…d :preventDefault

 - added Specs for all 3 pseudos
 - updated docs
@w00fz
Copy link
Member Author

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 .

@arian
Copy link
Member

arian commented May 26, 2011

I approve

@cpojer
Copy link
Member

cpojer commented May 27, 2011

@arian then merge it

@arian
Copy link
Member

arian commented May 27, 2011

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

On Fri, May 27, 2011 at 12:48 PM, cpojer
[email protected]
wrote:

@arian then merge it

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

@anutron
Copy link
Member

anutron commented Jun 5, 2011

blerg to merges.

@arian
Copy link
Member

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.

@timwienk
Copy link
Member

timwienk commented Jun 6, 2011

Specifically this branch.

@timwienk
Copy link
Member

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.

@w00fz
Copy link
Member Author

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 [email protected] 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)

@arian
Copy link
Member

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants