Make global options a simple property #2461
Conversation
|
Obsoletes #2418 |
|
Tests failed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: BUSTED Commit: f377fb042fb532f1d3aedd042e3c5ebf29c19fdc (Please note that this is a fully automated comment.) |
| @@ -1,15 +1,12 @@ | |||
| /** | |||
| * @file global-options.js | |||
| * @file default-options.js | |||
There was a problem hiding this comment.
This file becomes the record of the initial video.js defaults. The global defaults currently active are always accessible at videojs.options.
|
Tests failed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: BUSTED Commit: 30849709098d2a2fc73874b68800ac3d571bf002 (Please note that this is a fully automated comment.) |
|
Tests failed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: BUSTED Commit: 52ac8c6f0ed63773ef97d3c62eb1469a0257f0ba (Please note that this is a fully automated comment.) |
| @@ -17,7 +17,8 @@ import { createTimeRange } from './utils/time-ranges.js'; | |||
| import { bufferedPercent } from './utils/buffer.js'; | |||
| import FullscreenApi from './fullscreen-api.js'; | |||
| import MediaError from './media-error.js'; | |||
| import globalOptions from './global-options.js'; | |||
| import videojs from './video.js'; | |||
There was a problem hiding this comment.
This creates a circular dependency between player.js and video.js.
There was a problem hiding this comment.
It does but videojs isn't referenced until a player is created, which is safe by then.
|
To summarize the code code comments, I'm good with going back to an object and I think the object should just live at This PR currently spreads a lot of |
|
|
||
| const players_ = {}; |
There was a problem hiding this comment.
Players still get added to Player.players when they're initialized, so right now this would always be empty.
dmlap
force-pushed
the
make-globals-a-global
branch
from
52ac8c6 to
0ff1fce
Compare
|
Tests failed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: BUSTED Commit: 0ff1fced942724c4c507605025c7ce6501cf8129 (Please note that this is a fully automated comment.) |
|
Tests failed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: BUSTED Commit: 0f7367fa9c3612960a70e4c27b94765c33ff3b0f (Please note that this is a fully automated comment.) |
| flash: {}, | ||
|
|
||
| // defaultVolume: 0.85, | ||
| defaultVolume: 0.00, // The freakin seaguls are driving me crazy! |
There was a problem hiding this comment.
Is the default volume intended to be 0?
There was a problem hiding this comment.
Default volume was removed at some point but this got left behind. It can be removed.
Remove getters and setters for the global options because they imply a contract that we were violating all over the place. Convert mergeOptions() so that it does not mutate its first option to follow 4.x behavior.
Make sure the default language is used if no language was specified in the player construction options.
Add a note about the organization of player.test.js. Use Player.players instead of videojs.getPlayers() in the player tests. Don't create an unnecessary object during merges.
dmlap
force-pushed
the
make-globals-a-global
branch
from
0f7367f to
729152e
Compare
|
Tests failed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: BUSTED Commit: 729152e (Please note that this is a fully automated comment.) |
Remove getters and setters for the global options because they imply a contract that we were violating all over the place. Remove Player.players because it was just an alias for videojs.getPlayers(). Convert mergeOptions() so that it does not mutate its first option to follow 4.x behavior.