Skip to content

Enhanced Pseudos:keys and new Pseudos:stop - #245

Open
w00fz wants to merge 3 commits into
mootools:masterfrom
w00fz:master
Open

Enhanced Pseudos:keys and new Pseudos:stop#245
w00fz wants to merge 3 commits into
mootools:masterfrom
w00fz:master

Conversation

@w00fz

@w00fz w00fz commented May 23, 2011

Copy link
Copy Markdown
Member

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

cpojer commented May 23, 2011

Copy link
Copy Markdown
Member

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

arian commented May 23, 2011

Copy link
Copy Markdown
Member

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

cpojer commented May 23, 2011

Copy link
Copy Markdown
Member

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

@seanmonstar

Copy link
Copy Markdown

#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

w00fz commented May 26, 2011

Copy link
Copy Markdown
Member Author

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

arian commented May 26, 2011

Copy link
Copy Markdown
Member

I approve

@cpojer

cpojer commented May 27, 2011

Copy link
Copy Markdown
Member

@arian then merge it

@arian

arian commented May 27, 2011

Copy link
Copy Markdown
Member

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

@anutron

anutron commented Jun 5, 2011

Copy link
Copy Markdown
Member

blerg to merges.

@arian

arian commented Jun 5, 2011

Copy link
Copy Markdown
Member

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

timwienk commented Jun 6, 2011

Copy link
Copy Markdown
Member

Specifically this branch.

@timwienk

timwienk commented Jun 6, 2011

Copy link
Copy Markdown
Member

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

w00fz commented Jun 6, 2011

Copy link
Copy Markdown
Member Author

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)

@arian

arian commented Jul 31, 2011

Copy link
Copy Markdown
Member

@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