Skip to main content

Bug Tracker

Side navigation

#3498 closed enhancement (fixed)

Opened October 20, 2008 12:13PM UTC

Closed October 21, 2008 01:51AM UTC

Last modified May 06, 2009 02:19AM UTC

Add mouseenter and mouseleave shortcuts

Reported by: phiggins Owned by: flesler
Priority: minor Milestone: 1.3
Component: event Version: 1.2.6
Keywords: Cc: flesler
Blocked by: Blocking:
Description

.hover requires two parameters. It might be work considering delegating one function between the two events by only registering a single function.

also, .mouseenter and .mouseleave are omitted from the list of convenience methods attached to .bind().

attached patch resolves both.

Attachments (1)
  • mouseenter.diff (0.8 KB) - added by phiggins October 20, 2008 12:14PM UTC.

    shorten hover, add optional second param, add mouseenter/leave methods

Change History (17)

Changed October 21, 2008 01:49AM UTC by flesler comment:1

need: ReviewCommit
status: newassigned

Changed October 21, 2008 01:49AM UTC by flesler comment:2

component: unfilledevent

Changed October 21, 2008 01:51AM UTC by flesler comment:3

resolution: → fixed
status: assignedclosed

I agree about $.fn.mouseenter/mouseleave, but definitely not about 1 function for both events.

Applied first part at [5895].

That's not generic enough and can be unexpected for some users.

You can simply do that by: $(..).bind('mouseenter mouseleave',someFn);

Changed October 21, 2008 02:31AM UTC by phiggins comment:4

great - the delegation seems like it could be something taught as a best practice, though admittedly the implementation is silly ... it seems something you can teach inadvertently through .hover, but if you really want to explain the space-separated-bind-events, who am I to argue. Thanks for applying the patch. My stuff is AFL/BSD, if it matters (and it should)

regards-

phiggins

Changed October 21, 2008 02:41AM UTC by phiggins comment:5

to be clear, I wasn't changing the API in place at all - just offering the advanced users an option to reuse a single function -- it seems convenient to "be able to", rather than the terse syntax of registering the function twice manually -- and it only adds 5 bytes above the current patch to allow the option. unless you were reserving a null second param to hover for something more significant ... I like the shorthand myself.

Changed October 21, 2008 02:49AM UTC by phiggins comment:6

also, not to nitpick - but even without the optional second parameter, it would be smaller to use my .mouseenter/.leave patch with no options than .bind('mouseenter')/.bind("mouseleave") even when considering how gzip works.

Changed October 21, 2008 03:08AM UTC by kswedberg comment:7

heh. I've been meaning to add a ticket for .mouseenter() and .mouseleave() for months. glad someone else took the bull by the horns, so to speak. thanks, phiggins.

Changed October 21, 2008 10:59PM UTC by john comment:8

Replying to [comment:4 phiggins]:

My stuff is AFL/BSD, if it matters (and it should)

Wait... you provided a patch, on the jQuery project tracker, with no specified license and you are now retroactively adding an incompatible license to the patch after it had already been applied?

Please tell that there's some miscommunication going on here.

Changed October 21, 2008 10:59PM UTC by john comment:9

cc: → john

Changed October 21, 2008 11:09PM UTC by phiggins comment:10

no I was being cheeky -- I suppose I supplied the patch as mit/gpl, but typically do my stuff in bsd/afl. there is no retroactive or anything shady going on, just open source.

either way, the patch is enough of a no-brainer not to carry any ip imho

regards

Peter Higgins

Changed October 22, 2008 12:06AM UTC by flesler comment:11

First time I hear about licensing for a patch for jQuery :)

Anyway...

>also, not to nitpick - but even without the optional second parameter, it >would be smaller to use my .mouseenter/.leave patch with no options than >.bind('mouseenter')/.bind("mouseleave") even when considering how gzip works.

Right, I forgot to use the shorthands... do I need to worry about licensing ? :D

>it seems convenient to "be able to", rather than the terse syntax of >registering the function twice manually -- and it only adds 5 bytes above >the current patch to allow the option.

I agree that it's a small change. The problem is that many users think they can just send 1 fn to hover() assuming the 2nd is set to an empty function.

On this situation, they could get unexpected side effects.

Changed October 22, 2008 12:27AM UTC by phiggins comment:12

re: shorthands / licensing: no. a) is a no-brainer patch and b) in the full spirit of oss, sorry I even mentioned the word :P

re: second param: It really is a matter of documentation at this point, unless I'm unclear as to what .bind('mouseleave', undefined) would result in. seems like it throws an error, but by your description it would fail silently? Maybe i'm just confused. Are you saying that $(.).hover(function(){..}) only registers a .mouseenter()? or is it documented otherwise and you are trying to keep the errors thrown when passing only one?

Allowing both to be on the first position would confuse someone I agree, but if clearly documented it becomes moot, and a handy feature.

Unless the exception throwing is a way of telling people "you need two functions here". The documents only state you need both, and in the case of anonymous functions putting both on one callback seems something worthwhile, though not something worth arguing over ;)

Changed October 22, 2008 01:34AM UTC by flesler comment:13

Replying to [comment:12 phiggins]:

re: second param: It really is a matter of documentation at this point, unless I'm unclear as to what .bind('mouseleave', undefined) would result in. seems like it throws an error, but by your description it would fail silently? Maybe i'm just confused. Are you saying that $(.).hover(function(){..}) only registers a .mouseenter()? or is it documented otherwise and you are trying to keep the errors thrown when passing only one?

No, I know it fails, but that makes the user realize they made a mistake. If we'd silently bind the function twice, they wouldn't know.

We had indeed documented that hover requires 2 functions. Still, so many users ignore that.

I think silently binding the fn twice would add more side effects than advantages.

Considering that you can use the shortcut I proposed that is a few bytes longer, and also that this doesn't seem to be useful in too many situations.

Changed October 22, 2008 02:10AM UTC by phiggins comment:14

ahh claro - I agree then. but it still should be:

hover: function(over, out){ this.onmouseenter(over).onmouseleave(out) },

in the interest of size

Changed October 22, 2008 10:07AM UTC by flesler comment:15

Added the shortcuts.

Thanks

Changed January 07, 2009 02:25PM UTC by flesler comment:16

cc: johnflesler
summary: enhancement: Allow for optional second function on hover + add .mouseenter/leave funcAdd mouseenter and mouseleave shortcuts

Changed May 06, 2009 02:19AM UTC by brandon comment:17

This was a lively ticket. FYI... the one function to hover enhancement was applied in r6342. Ticket #4234.