Prevent control bar from attatching event listeners when disabled. Fixes #556 #615
Conversation
| } else if (player.controls() === true) { | ||
| this.setup(); | ||
| // Controls fade when not in use | ||
| } else { |
There was a problem hiding this comment.
Controls should really either be true or false. What does this third option provide for?
There was a problem hiding this comment.
The third option is the way the player works by default. See the demo video on videojs.org: when the video starts the controls are visible, but they fade away after a few seconds. This is in comparison to true or false where the controls are either always visible or always hidden.
630.408.3734
On Thursday, July 18, 2013 at 3:50 PM, Steve Heffernan wrote:
In src/js/control-bar/control-bar.js:
this.disable(); > + // Controls are always on > + } else if (player.controls() === true) { > + this.setup(); > + // Controls fade when not in use > + } else {
Controls should really either be true or false. What does this third option provide for?—
Reply to this email directly or view it on GitHub (https://github.com/videojs/video.js/pull/615/files#r5280396).
|
@aniccolai, I pulled these changes into #672. Would love to get your feedback on the rest of the changes made. |
Issue #556, Reproduce: http://jsbin.com/ikewix/3/edit
This commit refactors how videojs handles the control options, ie: {controls: true} and {controls: false}. The biggest change is that the ControlBar no longer attaches its event listeners if has been disabled through the options.
I think this change makes sense since the ControlBar doesn't need the event listeners if it's disabled. It might affect anyone trying to reenable after init has been called, but this doesn't seem like it's a common use case.