Revert making volumeMenuButton vertical by default #1592
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Just noting it's a breaking (albeit small) change for the brightcove player. We'll need to make sure we set this option explicitly before updating to a version that contains this PR. |
|
Yep, figured we'd need to account for this on the BC player side if / when this gets pulled in. There's no hurry for this one, so it might be a minute. |
jgubman
added a commit
to jgubman/video.js
that referenced
this pull request
Oct 29, 2014
* 'stable' of https://github.com/videojs/video.js: (22 commits) Added line to changelog for 4.10.1 Release 4.10.1 Removed alert... Release 4.10.0 @heff turned on the custom html controls for touch devices. closes videojs#1617 updated options to include sans-children, and included self-hosted font blurb added CORS information to tracks guide. Fixes videojs#837 doc updates + readme quick start updated options guide to include components @heff Added the ability to set options for child components directly in the parent options. closes videojs#1599 @DevGavin added a Simplified Chinese translation. closes videojs#1593 @mmcc fixed an issue with the VolumeButton assuming it was vertical by default. closes videojs#1592 @heff enhanced the event listener API to allow for auto-cleanup of listeners on other componenets and elements. closes videojs#1588 @mmcc fixed an issue where errors on source tags could get missed. closes videojs#1575 Added doc for remaining time and removed onWaitEnd doc maybe actually check for keyLocation Update languages.md @heff updated the poster to use CSS styles to display; fixed the poster not showing if not originally set. closes videojs#1568 Release 4.9.1 Bumped to videojs-swf v4.5.1 to fix a data sanitization issue. closes videojs#1587 ...
|
The |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There's currently no way to disable the vertical volume bar when using
volumeMenuButton, but it seems like the default should more closely match the existing functionality regardless. IfvolumeMenuButtonis used instead ofvolumeControlwith no other settings, the expected behavior should be the same slider behavior as before, just in a menu. If someone wants to make it vertical, that should be explicitly set in the options.I originally set out with this PR to also fix the CSS if someone did set the vertical option, but I've since thought better of it for multiple reasons (being a pain in the butt not being the least of those). Until #1553 is resolved, I don't think it's reasonable for us (or someone working on a skin) to need to account for both layouts. If someone explicitly sets vertical to true, they would also need to customize their own skin to account for the vertical slider. Someone should not need to customize the CSS for something that's a default. Also, it's a pain in the butt to fix now, so I'm deferring that fix until we come up with a good fix for #1553.