1
0
mirror of https://github.com/videojs/video.js.git synced 2025-07-17 01:42:41 +02:00

refactor: Remove method Chaining from videojs (#3860)

Remove method chaining from videojs. This helps keep the methods consistent especially since play() now returns a Promise in some cases. Also, it can add some performance benefits.

BREAKING CHANGE: player methods no longer return a player instance when called. Fixes #3704.
This commit is contained in:
Brandon Casey
2017-01-18 01:50:22 -05:00
committed by Gary Katsevman
parent 0bba3196d8
commit 8f07f5d57c
9 changed files with 179 additions and 335 deletions

View File

@ -108,9 +108,8 @@ class ClickableComponent extends Component {
* @param {Element} [el=this.el()]
* Element to set the title on.
*
* @return {string|ClickableComponent}
* @return {string}
* - The control text when getting
* - Returns itself when setting; method can be chained.
*/
controlText(text, el = this.el()) {
if (!text) {
@ -122,8 +121,6 @@ class ClickableComponent extends Component {
this.controlText_ = text;
this.controlTextEl_.innerHTML = localizedText;
el.setAttribute('title', localizedText);
return this;
}
/**
@ -138,9 +135,6 @@ class ClickableComponent extends Component {
/**
* Enable this `Component`s element.
*
* @return {ClickableComponent}
* Returns itself; method can be chained.
*/
enable() {
this.removeClass('vjs-disabled');
@ -152,14 +146,10 @@ class ClickableComponent extends Component {
this.on('click', this.handleClick);
this.on('focus', this.handleFocus);
this.on('blur', this.handleBlur);
return this;
}
/**
* Disable this `Component`s element.
*
* @return {ClickableComponent}
* Returns itself; method can be chained.
*/
disable() {
this.addClass('vjs-disabled');
@ -171,7 +161,6 @@ class ClickableComponent extends Component {
this.off('click', this.handleClick);
this.off('focus', this.handleFocus);
this.off('blur', this.handleBlur);
return this;
}
/**

View File

@ -573,9 +573,6 @@ class Component {
* The event handler if `first` is a `Component` and `second` is an event name
* or an Array of event names.
*
* @return {Component}
* Returns itself; method can be chained.
*
* @listens Component#dispose
*/
on(first, second, third) {
@ -618,8 +615,6 @@ class Component {
target.on('dispose', cleanRemover);
}
}
return this;
}
/**
@ -635,9 +630,6 @@ class Component {
* @param {EventTarget~EventListener} [third]
* The event handler if `first` is a `Component` and `second` is an event name
* or an Array of event names.
*
* @return {Component}
* Returns itself; method can be chained.
*/
off(first, second, third) {
if (!first || typeof first === 'string' || Array.isArray(first)) {
@ -662,8 +654,6 @@ class Component {
target.off('dispose', fn);
}
}
return this;
}
/**
@ -678,9 +668,6 @@ class Component {
* @param {EventTarget~EventListener} [third]
* The event handler if `first` is a `Component` and `second` is an event name
* or an Array of event names.
*
* @return {Component}
* Returns itself; method can be chained.
*/
one(first, second, third) {
if (typeof first === 'string' || Array.isArray(first)) {
@ -700,8 +687,6 @@ class Component {
this.on(target, type, newFunc);
}
return this;
}
/**
@ -713,13 +698,9 @@ class Component {
*
* @param {Object} [hash]
* Data hash to pass along with the event
*
* @return {Component}
* Returns itself; method can be chained.
*/
trigger(event, hash) {
Events.trigger(this.el_, event, hash);
return this;
}
/**
@ -731,9 +712,6 @@ class Component {
*
* @param {boolean} [sync=false]
* Execute the listener synchronously if `Component` is ready.
*
* @return {Component}
* Returns itself; method can be chained.
*/
ready(fn, sync = false) {
if (fn) {
@ -749,7 +727,6 @@ class Component {
this.readyQueue_.push(fn);
}
}
return this;
}
/**
@ -847,13 +824,9 @@ class Component {
*
* @param {string} classToAdd
* CSS class name to add
*
* @return {Component}
* Returns itself; method can be chained.
*/
addClass(classToAdd) {
Dom.addElClass(this.el_, classToAdd);
return this;
}
/**
@ -861,13 +834,9 @@ class Component {
*
* @param {string} classToRemove
* CSS class name to remove
*
* @return {Component}
* Returns itself; method can be chained.
*/
removeClass(classToRemove) {
Dom.removeElClass(this.el_, classToRemove);
return this;
}
/**
@ -880,65 +849,45 @@ class Component {
*
* @param {boolean|Dom~predicate} [predicate]
* An {@link Dom~predicate} function or a boolean
*
* @return {Component}
* Returns itself; method can be chained.
*/
toggleClass(classToToggle, predicate) {
Dom.toggleElClass(this.el_, classToToggle, predicate);
return this;
}
/**
* Show the `Component`s element if it is hidden by removing the
* 'vjs-hidden' class name from it.
*
* @return {Component}
* Returns itself; method can be chained.
*/
show() {
this.removeClass('vjs-hidden');
return this;
}
/**
* Hide the `Component`s element if it is currently showing by adding the
* 'vjs-hidden` class name to it.
*
* @return {Component}
* Returns itself; method can be chained.
*/
hide() {
this.addClass('vjs-hidden');
return this;
}
/**
* Lock a `Component`s element in its visible state by adding the 'vjs-lock-showing'
* class name to it. Used during fadeIn/fadeOut.
*
* @return {Component}
* Returns itself; method can be chained.
*
* @private
*/
lockShowing() {
this.addClass('vjs-lock-showing');
return this;
}
/**
* Unlock a `Component`s element from its visible state by removing the 'vjs-lock-showing'
* class name from it. Used during fadeIn/fadeOut.
*
* @return {Component}
* Returns itself; method can be chained.
*
* @private
*/
unlockShowing() {
this.removeClass('vjs-lock-showing');
return this;
}
/**
@ -969,14 +918,10 @@ class Component {
* @param {string} value
* Value to set the attribute to.
*
* @return {Component}
* Returns itself; method can be chained.
*
* @see [DOM API]{@link https://developer.mozilla.org/en-US/docs/Web/API/Element/setAttribute}
*/
setAttribute(attribute, value) {
Dom.setAttribute(this.el_, attribute, value);
return this;
}
/**
@ -985,14 +930,10 @@ class Component {
* @param {string} attribute
* Name of the attribute to remove.
*
* @return {Component}
* Returns itself; method can be chained.
*
* @see [DOM API]{@link https://developer.mozilla.org/en-US/docs/Web/API/Element/removeAttribute}
*/
removeAttribute(attribute) {
Dom.removeAttribute(this.el_, attribute);
return this;
}
/**
@ -1005,10 +946,9 @@ class Component {
* @param {boolean} [skipListeners]
* Skip the resize event trigger
*
* @return {Component|number|string}
* - The width when getting, zero if there is no width. Can be a string
* @return {number|string}
* The width when getting, zero if there is no width. Can be a string
* postpixed with '%' or 'px'.
* - Returns itself when setting; method can be chained.
*/
width(num, skipListeners) {
return this.dimension('width', num, skipListeners);
@ -1024,10 +964,9 @@ class Component {
* @param {boolean} [skipListeners]
* Skip the resize event trigger
*
* @return {Component|number|string}
* - The width when getting, zero if there is no width. Can be a string
* @return {number|string}
* The width when getting, zero if there is no width. Can be a string
* postpixed with '%' or 'px'.
* - Returns itself when setting; method can be chained.
*/
height(num, skipListeners) {
return this.dimension('height', num, skipListeners);
@ -1041,13 +980,11 @@ class Component {
*
* @param {number|string} height
* Height to set the `Component`s element to.
*
* @return {Component}
* Returns itself; method can be chained.
*/
dimensions(width, height) {
// Skip resize listeners on width for optimization
return this.width(width, true).height(height);
this.width(width, true);
this.height(height);
}
/**
@ -1075,9 +1012,8 @@ class Component {
* @param {boolean} [skipListeners]
* Skip resize event trigger
*
* @return {Component}
* - the dimension when getting or 0 if unset
* - Returns itself when setting; method can be chained.
* @return {number}
* The dimension when getting or 0 if unset
*/
dimension(widthOrHeight, num, skipListeners) {
if (num !== undefined) {
@ -1106,8 +1042,7 @@ class Component {
this.trigger('resize');
}
// Return component
return this;
return;
}
// Not setting a value, so getting it

View File

@ -245,9 +245,6 @@ class MenuButton extends ClickableComponent {
/**
* Disable the `MenuButton`. Don't allow it to be clicked.
*
* @return {MenuButton}
* Returns itself; method can be chained.
*/
disable() {
// Unpress, but don't force focus on this button
@ -257,19 +254,15 @@ class MenuButton extends ClickableComponent {
this.enabled_ = false;
return super.disable();
super.disable();
}
/**
* Enable the `MenuButton`. Allow it to be clicked.
*
* @return {MenuButton}
* Returns itself; method can be chained.
*/
enable() {
this.enabled_ = true;
return super.enable();
super.enable();
}
}

View File

@ -153,9 +153,6 @@ class ModalDialog extends Component {
*
* @fires ModalDialog#beforemodalopen
* @fires ModalDialog#modalopen
*
* @return {ModalDialog}
* Returns itself; method can be chained.
*/
open() {
if (!this.opened_) {
@ -201,7 +198,6 @@ class ModalDialog extends Component {
this.trigger('modalopen');
this.hasBeenOpened_ = true;
}
return this;
}
/**
@ -226,12 +222,11 @@ class ModalDialog extends Component {
*
* @fires ModalDialog#beforemodalclose
* @fires ModalDialog#modalclose
*
* @return {ModalDialog}
* Returns itself; method can be chained.
*/
close() {
if (this.opened_) {
if (!this.opened_) {
return;
}
const player = this.player();
/**
@ -267,8 +262,6 @@ class ModalDialog extends Component {
this.dispose();
}
}
return this;
}
/**
* Check to see if the `ModalDialog` is closeable via the UI.
@ -310,12 +303,9 @@ class ModalDialog extends Component {
/**
* Fill the modal's content element with the modal's "content" option.
* The content element will be emptied before this change takes place.
*
* @return {ModalDialog}
* Returns itself; method can be chained.
*/
fill() {
return this.fillWith(this.content());
this.fillWith(this.content());
}
/**
@ -327,9 +317,6 @@ class ModalDialog extends Component {
*
* @param {Mixed} [content]
* The same rules apply to this as apply to the `content` option.
*
* @return {ModalDialog}
* Returns itself; method can be chained.
*/
fillWith(content) {
const contentEl = this.contentEl();
@ -364,8 +351,6 @@ class ModalDialog extends Component {
} else {
parentEl.appendChild(contentEl);
}
return this;
}
/**
@ -373,9 +358,6 @@ class ModalDialog extends Component {
*
* @fires ModalDialog#beforemodalempty
* @fires ModalDialog#modalempty
*
* @return {ModalDialog}
* Returns itself; method can be chained.
*/
empty() {
/**
@ -394,7 +376,6 @@ class ModalDialog extends Component {
* @type {EventTarget~Event}
*/
this.trigger('modalempty');
return this;
}
/**

View File

@ -568,7 +568,7 @@ class Player extends Component {
* The value to set the `Player's width to.
*
* @return {number}
* The current width of the `Player`.
* The current width of the `Player` when getting.
*/
width(value) {
return this.dimension('width', value);
@ -581,7 +581,7 @@ class Player extends Component {
* The value to set the `Player's heigth to.
*
* @return {number}
* The current heigth of the `Player`.
* The current height of the `Player` when getting.
*/
height(value) {
return this.dimension('height', value);
@ -598,9 +598,8 @@ class Player extends Component {
* @param {number} [value]
* Value for dimension specified in the first argument.
*
* @return {Player|number}
* - Returns itself when setting; method can be chained.
* - The dimension arguments value when getting (width/height).
* @return {number}
* The dimension arguments value when getting (width/height).
*/
dimension(dimension, value) {
const privDimension = dimension + '_';
@ -617,14 +616,13 @@ class Player extends Component {
if (isNaN(parsedVal)) {
log.error(`Improper value "${value}" supplied for for ${dimension}`);
return this;
return;
}
this[privDimension] = parsedVal;
}
this.updateStyleEl_();
return this;
}
/**
@ -1098,7 +1096,7 @@ class Player extends Component {
this.removeClass('vjs-has-started');
}
}
return this;
return;
}
return !!this.hasStarted_;
}
@ -1594,7 +1592,6 @@ class Player extends Component {
*/
pause() {
this.techCall_('pause');
return this;
}
/**
@ -1617,12 +1614,13 @@ class Player extends Component {
* @param {boolean} [isScrubbing]
* wether the user is or is not scrubbing
*
* @return {boolean|Player}
* A instance of the player that called this function when setting,
* and the value of scrubbing when getting
* @return {boolean}
* The value of scrubbing when getting
*/
scrubbing(isScrubbing) {
if (isScrubbing !== undefined) {
if (typeof isScrubbing === 'undefined') {
return this.scrubbing_;
}
this.scrubbing_ = !!isScrubbing;
if (isScrubbing) {
@ -1630,11 +1628,6 @@ class Player extends Component {
} else {
this.removeClass('vjs-scrubbing');
}
return this;
}
return this.scrubbing_;
}
/**
@ -1643,16 +1636,13 @@ class Player extends Component {
* @param {number|string} [seconds]
* The time to seek to in seconds
*
* @return {Player|number}
* @return {number}
* - the current time in seconds when getting
* - a reference to the current player object when setting
*/
currentTime(seconds) {
if (seconds !== undefined) {
if (typeof seconds !== 'undefined') {
this.techCall_('setCurrentTime', seconds);
return this;
return;
}
// cache last currentTime and return. default to 0 seconds
@ -1678,10 +1668,8 @@ class Player extends Component {
* @param {number} [seconds]
* The duration of the video to set in seconds
*
* @return {number|Player}
* @return {number}
* - The duration of the video in seconds when getting
* - A reference to the player that called this function
* when setting
*/
duration(seconds) {
if (seconds === undefined) {
@ -1710,8 +1698,6 @@ class Player extends Component {
*/
this.trigger('durationchange');
}
return this;
}
/**
@ -1788,9 +1774,8 @@ class Player extends Component {
* - 1.0 is 100%/full
* - 0.5 is half volume or 50%
*
* @return {Player|number}
* a reference to the calling player when setting and the
* current volume as a percent when getting
* @return {number}
* The current volume as a percent when getting
*/
volume(percentAsDecimal) {
let vol;
@ -1801,7 +1786,7 @@ class Player extends Component {
this.cache_.volume = vol;
this.techCall_('setVolume', vol);
return this;
return;
}
// Default to 1 when returning current volume.
@ -1816,15 +1801,14 @@ class Player extends Component {
* - true to mute
* - false to unmute
*
* @return {boolean|Player}
* @return {boolean}
* - true if mute is on and getting
* - false if mute is off and getting
* - A reference to the current player when setting
*/
muted(muted) {
if (muted !== undefined) {
this.techCall_('setMuted', muted);
return this;
return;
}
return this.techGet_('muted') || false;
}
@ -1851,15 +1835,14 @@ class Player extends Component {
* @param {boolean} [isFS]
* Set the players current fullscreen state
*
* @return {boolean|Player}
* @return {boolean}
* - true if fullscreen is on and getting
* - false if fullscreen is off and getting
* - A reference to the current player when setting
*/
isFullscreen(isFS) {
if (isFS !== undefined) {
this.isFullscreen_ = !!isFS;
return this;
return;
}
return !!this.isFullscreen_;
}
@ -1874,8 +1857,6 @@ class Player extends Component {
* Safari.
*
* @fires Player#fullscreenchange
* @return {Player}
* A reference to the current player
*/
requestFullscreen() {
const fsApi = FullscreenApi;
@ -1921,17 +1902,12 @@ class Player extends Component {
*/
this.trigger('fullscreenchange');
}
return this;
}
/**
* Return the video to its normal size after having been in full screen mode
*
* @fires Player#fullscreenchange
*
* @return {Player}
* A reference to the current player
*/
exitFullscreen() {
const fsApi = FullscreenApi;
@ -1951,8 +1927,6 @@ class Player extends Component {
*/
this.trigger('fullscreenchange');
}
return this;
}
/**
@ -2152,9 +2126,8 @@ class Player extends Component {
* @param {Tech~SourceObject|Tech~SourceObject[]} [source]
* One SourceObject or an array of SourceObjects
*
* @return {string|Player}
* - The current video source when getting
* - The player when setting
* @return {string}
* The current video source when getting
*/
src(source) {
if (source === undefined) {
@ -2218,8 +2191,6 @@ class Player extends Component {
}, true);
}
}
return this;
}
/**
@ -2257,26 +2228,18 @@ class Player extends Component {
/**
* Begin loading the src data.
*
* @return {Player}
* A reference to the player
*/
load() {
this.techCall_('load');
return this;
}
/**
* Reset the player. Loads the first tech in the techOrder,
* and calls `reset` on the tech`.
*
* @return {Player}
* A reference to the player
*/
reset() {
this.loadTech_(toTitleCase(this.options_.techOrder[0]), null);
this.techCall_('reset');
return this;
}
/**
@ -2344,15 +2307,14 @@ class Player extends Component {
* - true means that we should preload
* - false maens that we should not preload
*
* @return {string|Player}
* - the preload attribute value when getting
* - the player when setting
* @return {string}
* The preload attribute value when getting
*/
preload(value) {
if (value !== undefined) {
this.techCall_('setPreload', value);
this.options_.preload = value;
return this;
return;
}
return this.techGet_('preload');
}
@ -2362,17 +2324,16 @@ class Player extends Component {
*
* @param {boolean} [value]
* - true means that we should autoplay
* - false maens that we should not autoplay
* - false means that we should not autoplay
*
* @return {string|Player}
* - the current value of autoplay
* - the player when setting
* @return {string}
* The current value of autoplay when getting
*/
autoplay(value) {
if (value !== undefined) {
this.techCall_('setAutoplay', value);
this.options_.autoplay = value;
return this;
return;
}
return this.techGet_('autoplay', value);
}
@ -2384,15 +2345,14 @@ class Player extends Component {
* - true means that we should loop the video
* - false means that we should not loop the video
*
* @return {string|Player}
* - the current value of loop when getting
* - the player when setting
* @return {string}
* The current value of loop when getting
*/
loop(value) {
if (value !== undefined) {
this.techCall_('setLoop', value);
this.options_.loop = value;
return this;
return;
}
return this.techGet_('loop');
}
@ -2405,9 +2365,8 @@ class Player extends Component {
* @param {string} [src]
* Poster image source URL
*
* @return {string|Player}
* - the current value of poster when getting
* - the player when setting
* @return {string}
* The current value of poster when getting
*/
poster(src) {
if (src === undefined) {
@ -2434,8 +2393,6 @@ class Player extends Component {
* @type {EventTarget~Event}
*/
this.trigger('posterchange');
return this;
}
/**
@ -2468,9 +2425,8 @@ class Player extends Component {
* - true to turn controls on
* - false to turn controls off
*
* @return {boolean|Player}
* - the current value of controls when getting
* - the player when setting
* @return {boolean}
* The current value of controls when getting
*/
controls(bool) {
if (bool !== undefined) {
@ -2510,7 +2466,7 @@ class Player extends Component {
}
}
}
return this;
return;
}
return !!this.controls_;
}
@ -2529,9 +2485,8 @@ class Player extends Component {
* - true to turn native controls on
* - false to turn native controls off
*
* @return {boolean|Player}
* - the current value of native controls when getting
* - the player when setting
* @return {boolean}
* The current value of native controls when getting
*/
usingNativeControls(bool) {
if (bool !== undefined) {
@ -2562,7 +2517,7 @@ class Player extends Component {
this.trigger('usingcustomcontrols');
}
}
return this;
return;
}
return !!this.usingNativeControls_;
}
@ -2576,9 +2531,8 @@ class Player extends Component {
* A MediaError or a string/number to be turned
* into a MediaError
*
* @return {MediaError|null|Player}
* - The current MediaError when getting (or null)
* - The player when setting
* @return {MediaError|null}
* The current MediaError when getting (or null)
*/
error(err) {
if (err === undefined) {
@ -2592,7 +2546,7 @@ class Player extends Component {
if (this.errorDisplay) {
this.errorDisplay.close();
}
return this;
return;
}
this.error_ = new MediaError(err);
@ -2610,7 +2564,7 @@ class Player extends Component {
*/
this.trigger('error');
return this;
return;
}
/**
@ -2632,9 +2586,9 @@ class Player extends Component {
* @param {boolean} [bool]
* - true if the user is active
* - false if the user is inactive
* @return {boolean|Player}
* - the current value of userActive when getting
* - the player when setting
*
* @return {boolean}
* The current value of userActive when getting
*/
userActive(bool) {
if (bool !== undefined) {
@ -2681,7 +2635,7 @@ class Player extends Component {
this.trigger('userinactive');
}
}
return this;
return;
}
return this.userActive_;
}
@ -2782,14 +2736,13 @@ class Player extends Component {
* @param {number} [rate]
* New playback rate to set.
*
* @return {number|Player}
* - The current playback rate when getting or 1.0
* - the player when setting
* @return {number}
* The current playback rate when getting or 1.0
*/
playbackRate(rate) {
if (rate !== undefined) {
this.techCall_('setPlaybackRate', rate);
return this;
return;
}
if (this.tech_ && this.tech_.featuresPlaybackRate) {
@ -2805,14 +2758,13 @@ class Player extends Component {
* - true signals that this is an audio player
* - false signals that this is not an audio player
*
* @return {Player|boolean}
* - the current value of isAudio when getting
* - the player if setting
* @return {boolean}
* The current value of isAudio when getting
*/
isAudio(bool) {
if (bool !== undefined) {
this.isAudio_ = !!bool;
return this;
return;
}
return !!this.isAudio_;
@ -3021,9 +2973,8 @@ class Player extends Component {
* @param {string} [code]
* the language code to set the player to
*
* @return {string|Player}
* - The current language code when getting
* - A reference to the player when setting
* @return {string}
* The current language code when getting
*/
language(code) {
if (code === undefined) {
@ -3031,7 +2982,6 @@ class Player extends Component {
}
this.language_ = String(code).toLowerCase();
return this;
}
/**
@ -3098,7 +3048,8 @@ class Player extends Component {
this.removeChild(modal);
});
return modal.open();
modal.open();
return modal;
}
/**

View File

@ -281,10 +281,9 @@ class Slider extends Component {
* - true if slider is vertical,
* - false is horizontal
*
* @return {boolean|Slider}
* @return {boolean}
* - true if slider is vertical, and getting
* - false is horizontal, and getting
* - a reference to this object when setting
* - false if the slider is horizontal, and getting
*/
vertical(bool) {
if (bool === undefined) {
@ -298,8 +297,6 @@ class Slider extends Component {
} else {
this.addClass('vjs-slider-horizontal');
}
return this;
}
}

View File

@ -1118,9 +1118,6 @@ Tech.withSourceHandlers = function(_Tech) {
*
* @param {Tech~SourceObject} source
* A source object with src and type keys
*
* @return {Tech}
* Returns itself; this method is chainable
*/
_Tech.prototype.setSource = function(source) {
let sh = _Tech.selectSourceHandler(source, this.options_);
@ -1145,8 +1142,6 @@ Tech.withSourceHandlers = function(_Tech) {
this.sourceHandler_ = sh.handleSource(source, this, this.options_);
this.on('dispose', this.disposeSourceHandler);
return this;
};
/**

View File

@ -595,13 +595,13 @@ QUnit.test('dimension() should treat NaN and null as zero', function(assert) {
newWidth = comp.dimension('width', null);
assert.notEqual(newWidth, width, 'new width and old width are not the same');
assert.equal(newWidth, comp, 'we set a value, so, return value is component');
assert.equal(newWidth, undefined, 'we set a value, so, return value is undefined');
assert.equal(comp.width(), 0, 'the new width is zero');
const newHeight = comp.dimension('height', NaN);
assert.notEqual(newHeight, height, 'new height and old height are not the same');
assert.equal(newHeight, comp, 'we set a value, so, return value is component');
assert.equal(newHeight, undefined, 'we set a value, so, return value is undefined');
assert.equal(comp.height(), 0, 'the new height is zero');
comp.width(width);

View File

@ -87,15 +87,6 @@ QUnit.test('should create a close button by default', function(assert) {
assert.strictEqual(btn.el().parentNode, this.el, 'close button is a child of el');
});
QUnit.test('returns `this` for expected methods', function(assert) {
const methods = ['close', 'empty', 'fill', 'fillWith', 'open'];
assert.expect(methods.length);
methods.forEach(function(method) {
assert.strictEqual(this[method](), this, '`' + method + '()` returns `this`');
}, this.modal);
});
QUnit.test('open() triggers events', function(assert) {
const modal = this.modal;
const beforeModalOpenSpy = sinon.spy(function() {
@ -108,10 +99,9 @@ QUnit.test('open() triggers events', function(assert) {
assert.expect(4);
modal.
on('beforemodalopen', beforeModalOpenSpy).
on('modalopen', modalOpenSpy).
open();
modal.on('beforemodalopen', beforeModalOpenSpy);
modal.on('modalopen', modalOpenSpy);
modal.open();
assert.strictEqual(beforeModalOpenSpy.callCount, 1, 'beforemodalopen spy was called');
assert.strictEqual(modalOpenSpy.callCount, 1, 'modalopen spy was called');
@ -127,7 +117,9 @@ QUnit.test('open() removes "vjs-hidden" class', function(assert) {
QUnit.test('open() cannot be called on an opened modal', function(assert) {
const spy = sinon.spy();
this.modal.on('modalopen', spy).open().open();
this.modal.on('modalopen', spy);
this.modal.open();
this.modal.open();
assert.expect(1);
assert.strictEqual(spy.callCount, 1, 'modal was only opened once');
@ -145,11 +137,10 @@ QUnit.test('close() triggers events', function(assert) {
assert.expect(4);
modal.
on('beforemodalclose', beforeModalCloseSpy).
on('modalclose', modalCloseSpy).
open().
close();
modal.on('beforemodalclose', beforeModalCloseSpy);
modal.on('modalclose', modalCloseSpy);
modal.open();
modal.close();
assert.strictEqual(beforeModalCloseSpy.callCount, 1, 'beforemodalclose spy was called');
assert.strictEqual(modalCloseSpy.callCount, 1, 'modalclose spy was called');
@ -157,18 +148,21 @@ QUnit.test('close() triggers events', function(assert) {
QUnit.test('close() adds the "vjs-hidden" class', function(assert) {
assert.expect(1);
this.modal.open().close();
this.modal.open();
this.modal.close();
assert.ok(this.modal.hasClass('vjs-hidden'), 'modal is hidden upon close');
});
QUnit.test('pressing ESC triggers close(), but only when the modal is opened', function(assert) {
const spy = sinon.spy();
this.modal.on('modalclose', spy).handleKeyPress({which: ESC});
this.modal.on('modalclose', spy);
this.modal.handleKeyPress({which: ESC});
assert.expect(2);
assert.strictEqual(spy.callCount, 0, 'ESC did not close the closed modal');
this.modal.open().handleKeyPress({which: ESC});
this.modal.open();
this.modal.handleKeyPress({which: ESC});
assert.strictEqual(spy.callCount, 1, 'ESC closed the now-opened modal');
});
@ -176,7 +170,9 @@ QUnit.test('close() cannot be called on a closed modal', function(assert) {
const spy = sinon.spy();
this.modal.on('modalclose', spy);
this.modal.open().close().close();
this.modal.open();
this.modal.close();
this.modal.close();
assert.expect(1);
assert.strictEqual(spy.callCount, 1, 'modal was only closed once');
@ -227,11 +223,10 @@ QUnit.test('opened()', function(assert) {
this.modal.open();
assert.strictEqual(this.modal.opened(), true, 'the modal is open');
this.modal.
close().
on('modalopen', openSpy).
on('modalclose', closeSpy).
opened(true);
this.modal.close();
this.modal.on('modalopen', openSpy);
this.modal.on('modalclose', closeSpy);
this.modal.opened(true);
this.modal.opened(true);
this.modal.opened(false);
@ -260,10 +255,9 @@ QUnit.test('fillWith()', function(assert) {
contentEl.appendChild(el);
});
this.modal.
on('beforemodalfill', beforeFillSpy).
on('modalfill', fillSpy).
fillWith(children);
this.modal.on('beforemodalfill', beforeFillSpy);
this.modal.on('modalfill', fillSpy);
this.modal.fillWith(children);
assert.expect(5 + children.length);
assert.strictEqual(contentEl.children.length, children.length, 'has the right number of children');
@ -282,11 +276,10 @@ QUnit.test('empty()', function(assert) {
const beforeEmptySpy = sinon.spy();
const emptySpy = sinon.spy();
this.modal.
fillWith([Dom.createEl(), Dom.createEl()]).
on('beforemodalempty', beforeEmptySpy).
on('modalempty', emptySpy).
empty();
this.modal.fillWith([Dom.createEl(), Dom.createEl()]);
this.modal.on('beforemodalempty', beforeEmptySpy);
this.modal.on('modalempty', emptySpy);
this.modal.empty();
assert.expect(5);
assert.strictEqual(this.modal.contentEl().children.length, 0, 'removed all `contentEl()` children');
@ -302,7 +295,8 @@ QUnit.test('closeable()', function(assert) {
assert.expect(8);
assert.strictEqual(this.modal.closeable(), true, 'the modal is closed');
this.modal.open().closeable(false);
this.modal.open();
this.modal.closeable(false);
assert.notOk(this.modal.getChild('closeButton'), 'the close button is no longer a child of the modal');
assert.notOk(initialCloseButton.el(), 'the initial close button was disposed');
@ -312,13 +306,15 @@ QUnit.test('closeable()', function(assert) {
this.modal.close();
assert.notOk(this.modal.opened(), 'the modal was closed programmatically');
this.modal.open().closeable(true);
this.modal.open();
this.modal.closeable(true);
assert.ok(this.modal.getChild('closeButton'), 'a new close button was created');
this.modal.getChild('closeButton').trigger('click');
assert.notOk(this.modal.opened(), 'the modal was closed by the new close button');
this.modal.open().handleKeyPress({which: ESC});
this.modal.open();
this.modal.handleKeyPress({which: ESC});
assert.notOk(this.modal.opened(), 'the modal was closed by the ESC key');
});
@ -331,7 +327,9 @@ QUnit.test('"content" option (fills on first open() invocation)', function(asser
const spy = sinon.spy();
modal.on('modalfill', spy);
modal.open().close().open();
modal.open();
modal.close();
modal.open();
assert.expect(3);
assert.strictEqual(modal.content(), modal.options_.content, 'has the expected content');
@ -347,8 +345,10 @@ QUnit.test('"temporary" option', function(assert) {
temp.on('dispose', tempSpy);
perm.on('dispose', permSpy);
temp.open().close();
perm.open().close();
temp.open();
temp.close();
perm.open();
perm.close();
assert.expect(2);
assert.strictEqual(tempSpy.callCount, 1, 'temporary modals are disposed');
@ -365,7 +365,9 @@ QUnit.test('"fillAlways" option', function(assert) {
const spy = sinon.spy();
modal.on('modalfill', spy);
modal.open().close().open();
modal.open();
modal.close();
modal.open();
assert.expect(1);
assert.strictEqual(spy.callCount, 2, 'the modal was filled on each open call');
@ -393,6 +395,7 @@ QUnit.test('"uncloseable" option', function(assert) {
assert.strictEqual(modal.closeable(), false, 'the modal is uncloseable');
assert.notOk(modal.getChild('closeButton'), 'the close button is not present');
modal.open().handleKeyPress({which: ESC});
modal.open();
modal.handleKeyPress({which: ESC});
assert.strictEqual(spy.callCount, 0, 'ESC did not close the modal');
});