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

Ember's use of touch event listeners can be bad for scroll performance #12783

Closed
RByers opened this issue Jan 8, 2016 · 10 comments
Closed

Ember's use of touch event listeners can be bad for scroll performance #12783

RByers opened this issue Jan 8, 2016 · 10 comments

Comments

@RByers
Copy link

RByers commented Jan 8, 2016

Ember indiscriminately adds touch event listeners to the root element (eg. body). This means that scrolling any page that uses ember will need to be blocked on whatever JS is running. For some sites this is fine (never run anything longer than ~50ms) but for many others it results in a terrible user experience on browsers that dispatch touch events.

I came across this when doing a performance audit of this Nasa page. The page has all sorts of performance problems on mobile, but if you remove the Ember touch event listeners (or force them to be 'passive') it scrolls beautifully.

I'm working on adding a feature to the web called passive event listeners which may make it at lot easier for you to solve this problem. I'd love any feedback you have (feel free to file issues here).

I don't know Ember at all, so it's not at all clear to me what would need to change in Ember to fully adopt such a model. If the basic idea seems sound to you, perhaps we can use this issue to discuss how Ember may be able to adopt it?

@stefanpenner
Copy link
Member

Inverting these dispatcher introduced event handlers to instead be on-demand i something we have wanted to do for ages, its just a matter of finding the time, and triaging the larger set of work.

If someone is motivated, we would support the effort.

Typically proposals or feature requests are to be submitted as issues or if more fleshed out RFCS to the rfc repo. This issue tracker is intended for bugs etc, and this falls into the would be nice, but totally a new thing category. This should be outlined in our CONTRIBUTING.md.

Im going to close this, and instead suggest it be opened as an issue on the rfc repo.

@rwjblue
Copy link
Member

rwjblue commented Jan 8, 2016

In general from an application perspective, it is possible to disable any listeners that you do not need/want via customEvents property of your application. This was added/fixed in #12059 and has been supported since 2.1.0-beta.1.

Obviously, this does not solve the generic case of only adding listeners when needed, but it is a useful tool for applications with specific performance needs...

@yankeeinlondon
Copy link

@rwjblue so I was intrigued since I am now getting a [Violation] Added non-passive event listener to a scroll-blocking 'mousewheel' event. and eventually ended up here ...

I tried changing the app.js for the dummy app of my addon to:

App = Ember.Application.extend({
  modulePrefix: config.modulePrefix,
  podModulePrefix: config.podModulePrefix,
  Resolver,
  customEvents: {
    mousewheel: null
  }
});

But I'm still being labelled a "violator". :)

Am I doing something wrong?

@krisselden
Copy link
Contributor

@ksnyde you need to disable all events that can preventDefault on scroll, so touchstart, touchmove, mousedown, mousemove, etc

@krisselden
Copy link
Contributor

also need to check that you don't have something besides Ember adding these.

@yankeeinlondon
Copy link

Thanks @krisselden. the message brings up the mousewheel event specifically , is that a red herring?

@yankeeinlondon
Copy link

Also I guess the bigger question (for me at least) is whether one can disable events on a component by component basis rather than at the App level. I think, will be checking soon, that if I set tagName: '' then that would allow my component to manage it's own events?

FYI this is important for interactive d3 visualizations whose events are blocked on a normal component (aka, tagName is svg or canvas)

@krisselden
Copy link
Contributor

@ksnyde you have to disable them at the app level because we are using event delegation and it would be breaking to change these events to passive. It is only a subset of events that can prevent scroll with preventDefault, and it maybe some event handling in some d3 code or plugin that is adding the listener. mousewheel and touchstart, etc are common in drag/zoom code.

@filipe-torres
Copy link

filipe-torres commented Oct 9, 2019

My approach to solve this problem was add and remove event listeners on didInsertElement and willDestroyElement lifecycle functions, setting {passive: true} as last parameter when adding the event listener:

export default Component({

  didInsertElement() {
    this._super(...arguments);
    this.functionName = () => { 
        // function body
    };
    this.element.addEventListener('wheel', this.functionName, {passive: true});
  },

  willDestroyElement() {
    this._super(...arguments);
    this.element.removeEventListener('wheel', this.functionName);
  }
});

Copy link
Member

rwjblue commented Oct 10, 2019

FWIW, in the Octane model using {{on allows exactly that (it calls element.addEventListener / element.removeEventListener and you can set passive). In the long run, when {{action is unused and no Ember.Component's use these events, we will be able to remove the global event dispatcher system that has existed for quite some time. In the meantime, you can disable events that you don't want to have setup by the global event dispatcher yourself (some documentation here).

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

6 participants