videojs / video.js

Video.js - open source HTML5 video player
https://videojs.com
Other
37.88k stars 7.43k forks source link

Change slider target from document to player.el_ when it listen to mouse and touch events #1962

Open carpasse opened 9 years ago

carpasse commented 9 years ago

Hello guys

I am currently experiencing a problem with the video player inside a mobile slide gallery.

The video progress can not be modified because the swipe of the progress bar triggers a swipe on the slide gallery.

I took a look at the code and the slide component listens to the mouse and touch events directly from the document.

  this.on(document, 'mousemove', this.onMouseMove);
  this.on(document, 'mouseup', this.onMouseUp);
  this.on(document, 'touchmove', this.onMouseMove);
  this.on(document, 'touchend', this.onMouseUp);

The problem is is that I can not stop propagation of these events since they are bind to the document.

I propose that we change the code to:

  this.on(player.el(), 'mousemove', this.onMouseMove);
  this.on(player.el(), 'mouseup', this.onMouseUp);
  this.on(player.el(), 'touchmove', this.onMouseMove);
  this.on(player.el(), 'touchend', this.onMouseUp);

This will make it necessary to change the code of the progressBar control and probably the volume control but it will give the the possibility to the controls that rely on the slider, to stopPropagation onMouseMove and onMouseUp handlers and therefore, prevents issues like the one I described.

I can implement this change if you guys want.

gkatsev commented 8 years ago

That sounds good but there is an issue with only having it work inside the player element. If you were to press or touch the slider and then move around, by listening to the document, it means that you can move your mouse or finger anywhere on the page and have it still track your movement in the slider. This is particularly useful on mobile where you want to seek but don't want your finger over the video while you're seeking. Not sure which one is better to keep, though, I'd lean towards better touch support than carousel support. Maybe we just need to make it configurable or something.

Also, re: carousels http://shouldiuseacarousel.com/ :P