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

fix: Allow evented objects, such as components and plugins, to listen to the window object in addition to DOM objects. #5255

Merged
merged 4 commits into from
Jun 20, 2018

Conversation

misteroneill
Copy link
Member

@misteroneill misteroneill commented Jun 15, 2018

I don't think there's an issue for this (I looked), but the bug is that objects using the new-ish evented mixin cannot listen to the window object, preventing things like:

component.on(window, 'scroll', throttledListener);

This fixes that so anything that's a native EventTarget works.

Specific Changes proposed

Allow any native EventTarget to be a listening target.

Requirements Checklist

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
    • Change has been verified in an actual browser (Chome, Firefox, IE)
    • Unit Tests updated or fixed
    • Docs/guides updated
    • Example created (starter template on JSBin)
  • Reviewed by Two Core Contributors

*/
const supportsNativeEvents = (object) =>
(typeof window.EventTarget === 'function' && object instanceof window.EventTarget) ||
typeof object.addEventListener === 'function';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about only just doing the addEventListener check?

@gkatsev
Copy link
Member

gkatsev commented Jun 19, 2018

Confirmed that this works via https://deploy-preview-5255--videojs-docs.netlify.com/test-example/ and

player.on(window, 'click', (e) => console.log(e.type))

@gkatsev gkatsev merged commit 7fd29b4 into master Jun 20, 2018
@gkatsev gkatsev deleted the fix/window-events branch June 20, 2018 21:05
gkatsev added a commit that referenced this pull request Jul 5, 2018
…to listen to the window object in addition to DOM objects. (#5255)"

This reverts commit 7fd29b4.

With this change, playing back HLS content via VHS caused an infinite
event loop to be printed out. See issue #5281
gkatsev added a commit that referenced this pull request Jul 6, 2018
…to listen to the window object in addition to DOM objects. (#5255)" (#5301)

This reverts commit 7fd29b4.

With this change, playing back HLS content via VHS caused an infinite
event loop to be printed out. See issue #5281
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants