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

mouseEnter/Leave event delegation broken w/o jQuery #16591

Closed
simonihmig opened this issue Apr 30, 2018 · 5 comments
Closed

mouseEnter/Leave event delegation broken w/o jQuery #16591

simonihmig opened this issue Apr 30, 2018 · 5 comments

Comments

@simonihmig
Copy link
Contributor

For components that have a mouseEnter() (and/or mouseLeave()) event handler, these are not called anymore when running without jQuery.

I think this is because those events don't bubble, so event delegation based on the root element, which is what the EventDispatcher is doing, does not work. Those events get only triggered for the element having the listener (the app's rootElement in this case). See https://developer.mozilla.org/en-US/docs/Web/Events/mouseenter

A possible workaround would be to use mouseover under the hood, to enable event delegation, and to make sure it behaves the same as the mouseEnter (i.e. doesn't trigger again for child elements). jQuery seems to do the same, that's why it works in jQuery mode: https://github.com/jquery/jquery/blob/899c56f6ada26821e8af12d9f35fa039100e838e/src/event.js#L666-L700

I could work on a PR, if we agree this ^ is the way forward!?

@rwjblue
Copy link
Member

rwjblue commented Apr 30, 2018

I'm somewhat hesitant to bake this in without other changes to event dispatching to make it more "pay as you go". I believe that most applications don't use mouseenter / mouseleave / mouseover and always listening to them has non-trivial cost.

A couple "simple" (I know, not always simple) work arounds for folks suffering from this right now:

  • Refactor your component to explicitly add an event listener in didInsertElement:
export default Component.extend({
  didInsertElement() {
    let handler = this._mouseenter = Ember.run.bind(this, this.mouseEnter);
    this.element.addEventListener('mouseenter', handler);
  },
  willDestroyElement() {
    this.element.removeEventListener('mouseenter', this._mouseenter);
  }
});
  • Refactor your template to use closure actions:
<div onmouseenter={{action 'mouseEntered'}}>Hover over me!</div>

@simonihmig
Copy link
Contributor Author

I'm somewhat hesitant to bake this...

Hm, in what way would that get baked in? If we can change the implementation to be more "pay as you go" without changing the public API, we can do this any time later. And having to support mouseEnter out of the box is already "baked in", in the sense that it is public API (by being documented) so we can't just ignore it, can we?

I believe that most applications don't use mouseenter / mouseleave / mouseover and always listening to them has non-trivial cost.

Right. I certainly see the benefit of a more smarter approach to handling these events. But again, for now they are covered under Ember's public API. And the cost is already incurring when running w/ jQuery, so fixing it for native mode would just bring the implementation on par, right?

Refactor your component to explicitly add an event listener in didInsertElement

Yes, we "fixed" it like this for now...

@rwjblue
Copy link
Member

rwjblue commented Apr 30, 2018

Hm, in what way would that get baked in? If we can change the implementation to be more "pay as you go" without changing the public API, we can do this any time later.

I’m sold. 😃 Would love a PR...

simonihmig added a commit to simonihmig/ember.js that referenced this issue May 2, 2018
As these events don't bubble, the `EventDispatcher`'s event delegation approach does not work here, when not using jQuery.
jQuery has special handling of these events, by listening to `mouseover`/`mouseout` instead and dispatching fake
`mouseenter`/`mouseleave` events. This adds similar handling to `EventDispatcher`'s native mode for these events.

Fixes emberjs#16591
simonihmig added a commit to simonihmig/ember.js that referenced this issue May 2, 2018
As these events don't bubble, the `EventDispatcher`'s event delegation approach does not work here, when not using jQuery. jQuery has special handling of these events, by listening to `mouseover`/`mouseout` instead and dispatching fake `mouseenter`/`mouseleave` events. This adds similar handling to `EventDispatcher`'s native mode for these events.

Fixes emberjs#16591
simonihmig added a commit to simonihmig/ember.js that referenced this issue May 3, 2018
As these events don't bubble, the `EventDispatcher`'s event delegation approach does not work here, when not using jQuery. jQuery has special handling of these events, by listening to `mouseover`/`mouseout` instead and dispatching fake `mouseenter`/`mouseleave` events. This adds similar handling to `EventDispatcher`'s native mode for these events.

Fixes emberjs#16591
simonihmig added a commit to simonihmig/ember.js that referenced this issue Jun 7, 2018
As these events don't bubble, the `EventDispatcher`'s event delegation approach does not work here, when not using jQuery. jQuery has special handling of these events, by listening to `mouseover`/`mouseout` instead and dispatching fake `mouseenter`/`mouseleave` events. This adds similar handling to `EventDispatcher`'s native mode for these events.

Fixes emberjs#16591
simonihmig added a commit to simonihmig/ember.js that referenced this issue Jun 7, 2018
As these events don't bubble, the `EventDispatcher`'s event delegation approach does not work here, when not using jQuery. jQuery has special handling of these events, by listening to `mouseover`/`mouseout` instead and dispatching fake `mouseenter`/`mouseleave` events. This adds similar handling to `EventDispatcher`'s native mode for these events.

Fixes emberjs#16591
simonihmig added a commit to simonihmig/ember.js that referenced this issue Jun 7, 2018
As these events don't bubble, the `EventDispatcher`'s event delegation approach does not work here, when not using jQuery. jQuery has special handling of these events, by listening to `mouseover`/`mouseout` instead and dispatching fake `mouseenter`/`mouseleave` events. This adds similar handling to `EventDispatcher`'s native mode for these events.

Fixes emberjs#16591
simonihmig added a commit to simonihmig/ember.js that referenced this issue Jun 7, 2018
As these events don't bubble, the `EventDispatcher`'s event delegation approach does not work here, when not using jQuery. jQuery has special handling of these events, by listening to `mouseover`/`mouseout` instead and dispatching fake `mouseenter`/`mouseleave` events. This adds similar handling to `EventDispatcher`'s native mode for these events.

Fixes emberjs#16591
kategengler pushed a commit that referenced this issue Aug 21, 2018
As these events don't bubble, the `EventDispatcher`'s event delegation approach does not work here, when not using jQuery. jQuery has special handling of these events, by listening to `mouseover`/`mouseout` instead and dispatching fake `mouseenter`/`mouseleave` events. This adds similar handling to `EventDispatcher`'s native mode for these events.

Fixes #16591

(cherry picked from commit 7aa1ae7)
@amk221
Copy link
Contributor

amk221 commented Aug 29, 2018

I'm experiencing some issues with this, here's a demo repo
https://github.com/amk221/-ember-jquery-mouse-events#example

[edit]: Issue: #16922

@rwjblue
Copy link
Member

rwjblue commented Aug 30, 2018 via email

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

No branches or pull requests

3 participants