#3498 closed enhancement (fixed)
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)
Change History (18)
Changed 14 years ago by
Attachment: | mouseenter.diff added |
---|
comment:1 Changed 14 years ago by
need: | Review → Commit |
---|---|
Status: | new → assigned |
comment:2 Changed 14 years ago by
Component: | unfilled → event |
---|
comment:3 Changed 14 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
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);
comment:4 follow-up: 8 Changed 14 years ago by
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
comment:5 Changed 14 years ago by
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.
comment:6 Changed 14 years ago by
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.
comment:7 Changed 14 years ago by
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.
comment:8 Changed 14 years ago by
Replying to 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.
comment:9 Changed 14 years ago by
Cc: | john added |
---|
comment:10 Changed 14 years ago by
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
comment:11 Changed 14 years ago by
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.
comment:12 follow-up: 13 Changed 14 years ago by
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 ;)
comment:13 Changed 14 years ago by
Replying to 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.
comment:14 Changed 14 years ago by
ahh claro - I agree then. but it still should be:
hover: function(over, out){ this.onmouseenter(over).onmouseleave(out) },
in the interest of size
comment:17 Changed 14 years ago by
Cc: | flesler added; john removed |
---|---|
Summary: | enhancement: Allow for optional second function on hover + add .mouseenter/leave func → Add mouseenter and mouseleave shortcuts |
shorten hover, add optional second param, add mouseenter/leave methods