Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

fix(jqLite): change implementation of mouseenter/mouseleave event #2383

Closed
wants to merge 2 commits into from

Conversation

gockxml
Copy link
Contributor

@gockxml gockxml commented Apr 12, 2013

Implement mouseenter/mouseleave event refering to http://www.quirksmode.org/js/events_mouse.html#link8 and jQuery source code (not dependent on jQuery).
The old implementation is wrong. When moving mouse from a parent element into a child element, it would trigger mouseleave event, which should not.
And the old test about mouseenter/mouseleave is wrong too. It just triggers mouseover and mouseout events, cannot describe the process of mouse moving from one element to nother element, which is important for mouseenter/mouseleave.

Closes #2131, #1811

Implement mouseenter/mouseleave event refering to
http://www.quirksmode.org/js/events_mouse.html#link8 and jQuery source
code(not
dependent on jQuery).
The old implementation is wrong. When moving mouse from a parent element
into a
child element, it would trigger mouseleave event, which should not.
And the old test about mouseenter/mouseleave is wrong too. It just
triggers
mouseover and mouseout events, cannot describe the process of mouse
moving from
one element to another element, which is important for
mouseenter/mouseleave.

Closes angular#2131, angular#1811
@petebacondarwin
Copy link
Contributor

@gockxml - shouldn't we be using the native mouseenter and mouseleave events if they are supported by the browser rather than always polyfilling? It is interesting to notice that ng-mouseenter and ng-mouseleave work as expected on browsers that support these events natively. Not sure what would happen in a browser that doesn't.

Other than that, this looks good to me.

@petebacondarwin
Copy link
Contributor

  • Contributor signed CLA now or in the past (if you just signed, leave a comment here with your real name)
  • PR doesn't introduce new api
  • PR doesn't contain a breaking change
  • PR contains unit tests
  • PR contains e2e tests (if suitable)
  • PR contains documentation update (if suitable)
  • PR passes all tests on Travis (sanity)
  • PR passes all tests on ci.angularjs.org (cross-browser compatibility)
  • PR is rebased against recent master
  • PR is squashed into one commit per logical change
  • PR's commit messages are descriptive and allows us to autogenerate release notes (required commit message format)
  • All changes requested in review have been implemented

@petebacondarwin
Copy link
Contributor

@gockxml
Copy link
Contributor Author

gockxml commented Apr 17, 2013

@petebacondarwin Sorry, I didn't know that firefox and opera support native mouseleave. I just implemented the way jQuery has done. However, DOM event support detection is not quite easy and neat. On second thought, maybe always polyfilling is not that bad. :)
The old version of ng-mouseleave didn't use the native event. It was simulation as well. Anway, I just changed the implementation of how to simulate mouseleave using mouseout. It is a really small scope of code.

@gockxml
Copy link
Contributor Author

gockxml commented Apr 17, 2013

@petebacondarwin I am a really newbie on github and all the open source things. And I am not a English speaker. So please forgive me to ask silly questions. I guess I need to pass the IE8 test before you can merge it?

I have signed signed CLA and my real name is Zhiliang Ge.

@petebacondarwin
Copy link
Contributor

I'm afraid that we do need to pass on IE.

@florianorben
Copy link

would also be nice to add contains() to angular.element, if we're adding it in here anyway

@petebacondarwin
Copy link
Contributor

@gockxml - Did you get anywhere trying to get tests to pass on IE?

Implement mouseenter/mouseleave event refering to
http://www.quirksmode.org/js/events_mouse.html#link8 and jQuery source
code(not
dependent on jQuery).
The old implementation is wrong. When moving mouse from a parent element
into a
child element, it would trigger mouseleave event, which should not.
And the old test about mouseenter/mouseleave is wrong too. It just
triggers
mouseover and mouseout events, cannot describe the process of mouse
moving from
one element to another element, which is important for
mouseenter/mouseleave.
@gockxml
Copy link
Contributor Author

gockxml commented Apr 26, 2013

@petebacondarwin Sorry, I spent much time on attempting to test IE in vm similar to ci.angular.org, but failed. :( Finally I decided to test and debug it in IE8 manually. Now it should pass the IE test.

@petebacondarwin
Copy link
Contributor

Landed as 06f2b2a. Thanks!

@petebacondarwin
Copy link
Contributor

@florianorben - I hear what you are saying. Do you want to create a PR for this?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong behavior of 'mouseenter' and 'mouseleave' inside nested elements.
3 participants