Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add 'beforepluginsetup' event and named plugin setup events (e.g. 'pluginsetup:foo') #4255

Merged
merged 3 commits into from
May 11, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions docs/guides/plugins.md
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,14 @@ Like components, advanced plugins offer an implementation of events. This includ

By offering a built-in events system, advanced plugins offer a wider range of options for code structure with a pattern familiar to most web developers.

##### Extra Event Data

All events triggered by plugins include an additional data object as a second argument. This object has three properties:

- `name`: The name of the plugin (e.g. `"examplePlugin"`) as a string.
- `plugin`: The plugin constructor (e.g. `ExamplePlugin`).
- `instance`: The plugin constructor instance.

#### Statefulness

A new concept introduced for advanced plugins is _statefulness_. This is similar to React components' `state` property and `setState` method.
Expand Down Expand Up @@ -307,6 +315,19 @@ player.examplePlugin({customClass: 'example-class'});

These two methods are functionally identical - use whichever you prefer!

### Plugin Setup Events

Occasionally, a use-case arises where some code needs to wait for a plugin to be initialized. As of Video.js 6, this can be achieved by listening for `pluginsetup` events on the player.

For any given plugin initialization, there are four events to be aware of:

- `beforepluginsetup`: Triggered immediately before any plugin is initialized.
- `beforepluginsetup:examplePlugin` Triggered immediately before the `examplePlugin` is initialized.
- `pluginsetup`: Triggered after any plugin is initialized.
- `pluginsetup:examplePlugin`: Triggered after he `examplePlugin` is initialized.

These events work for both basic and advanced plugins. They are triggered on the player and each includes an object of [extra event data](#extra-event-data) as a second argument to its listeners.

## References

* [Player API][api-player]
Expand Down
71 changes: 63 additions & 8 deletions src/js/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,27 @@ const markPluginAsActive = (player, name) => {
player[PLUGIN_CACHE_KEY][name] = true;
};

/**
* Triggers a pair of plugin setup events.
*
* @private
* @param {Player} player
* A Video.js player instance.
*
* @param {Plugin~PluginEventHash} hash
* A plugin event hash.
*
* @param {Boolean} [before]
* If true, prefixes the event name with "before". In other words,
* use this to trigger "beforepluginsetup" instead of "pluginsetup".
*/
const triggerSetupEvent = (player, hash, before) => {
const eventName = (before ? 'before' : '') + 'pluginsetup';

player.trigger(eventName, hash);
player.trigger(eventName + ':' + hash.name, hash);
};

/**
* Takes a basic plugin function and returns a wrapper function which marks
* on the player that the plugin has been activated.
Expand All @@ -91,15 +112,20 @@ const markPluginAsActive = (player, name) => {
*/
const createBasicPlugin = function(name, plugin) {
const basicPluginWrapper = function() {

// We trigger the "beforepluginsetup" and "pluginsetup" events on the player
// regardless, but we want the hash to be consistent with the hash provided
// for advanced plugins.
//
// The only potentially counter-intuitive thing here is the `instance` in
// the "pluginsetup" event is the value returned by the `plugin` function.
triggerSetupEvent(this, {name, plugin, instance: null}, true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm up in the air about wether we should pass data along with the event. I understand why we would need to but, it still goes against most of the practices that we have for other events. Anyone else want to chime in here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it's already the case for pluginsetup.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true, I guess this is just what we have to do

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I do know some plugins do return something from the method, so, providing it is nice.


const instance = plugin.apply(this, arguments);

markPluginAsActive(this, name);
triggerSetupEvent(this, {name, plugin, instance});

// We trigger the "pluginsetup" event on the player regardless, but we want
// the hash to be consistent with the hash provided for advanced plugins.
// The only potentially counter-intuitive thing here is the `instance` is the
// value returned by the `plugin` function.
this.trigger('pluginsetup', {name, plugin, instance});
return instance;
};

Expand Down Expand Up @@ -133,11 +159,15 @@ const createPluginFactory = (name, PluginSubClass) => {
PluginSubClass.prototype.name = name;

return function(...args) {
triggerSetupEvent(this, {name, plugin: PluginSubClass, instance: null}, true);

const instance = new PluginSubClass(...[this, ...args]);

// The plugin is replaced by a function that returns the current instance.
this[name] = () => instance;

triggerSetupEvent(this, instance.getEventHash());

return instance;
};
};
Expand All @@ -147,7 +177,10 @@ const createPluginFactory = (name, PluginSubClass) => {
*
* @mixes module:evented~EventedMixin
* @mixes module:stateful~StatefulMixin
* @fires Player#beforepluginsetup
* @fires Player#beforepluginsetup:$name
* @fires Player#pluginsetup
* @fires Player#pluginsetup:$name
* @listens Player#dispose
* @throws {Error}
* If attempting to instantiate the base {@link Plugin} class
Expand All @@ -164,12 +197,12 @@ class Plugin {
* A Video.js player instance.
*/
constructor(player) {
this.player = player;

if (this.constructor === Plugin) {
throw new Error('Plugin must be sub-classed; not directly instantiated.');
}

this.player = player;

// Make this object evented, but remove the added `trigger` method so we
// use the prototype version instead.
evented(this);
Expand All @@ -184,7 +217,6 @@ class Plugin {

// If the player is disposed, dispose the plugin.
player.on('dispose', this.dispose);
player.trigger('pluginsetup', this.getEventHash());
}

/**
Expand Down Expand Up @@ -432,13 +464,36 @@ Player.prototype.hasPlugin = function(name) {

export default Plugin;

/**
* Signals that a plugin is about to be set up on a player.
*
* @event Player#beforepluginsetup
* @type {Plugin~PluginEventHash}
*/

/**
* Signals that a plugin is about to be set up on a player - by name. The name
* is the name of the plugin.
*
* @event Player#beforepluginsetup:$name
* @type {Plugin~PluginEventHash}
*/

/**
* Signals that a plugin has just been set up on a player.
*
* @event Player#pluginsetup
* @type {Plugin~PluginEventHash}
*/

/**
* Signals that a plugin has just been set up on a player - by name. The name
* is the name of the plugin.
*
* @event Player#pluginsetup:$name
* @type {Plugin~PluginEventHash}
*/

/**
* @typedef {Object} Plugin~PluginEventHash
*
Expand Down
35 changes: 23 additions & 12 deletions test/unit/plugin-advanced.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,24 +67,35 @@ QUnit.test('setup', function(assert) {
);
});

QUnit.test('"pluginsetup" event', function(assert) {
QUnit.test('all "pluginsetup" events', function(assert) {
const setupSpy = sinon.spy();
const events = [
'beforepluginsetup',
'beforepluginsetup:mock',
'pluginsetup',
'pluginsetup:mock'
];

this.player.on('pluginsetup', setupSpy);
this.player.on(events, setupSpy);

const instance = this.player.mock();
const event = setupSpy.firstCall.args[0];
const hash = setupSpy.firstCall.args[1];

assert.strictEqual(setupSpy.callCount, 1, 'the "pluginsetup" event was triggered');
assert.strictEqual(event.type, 'pluginsetup', 'the event has the correct type');
assert.strictEqual(event.target, this.player.el_, 'the event has the correct target');
events.forEach((type, i) => {
const event = setupSpy.getCall(i).args[0];
const hash = setupSpy.getCall(i).args[1];

assert.deepEqual(hash, {
name: 'mock',
instance,
plugin: this.MockPlugin
}, 'the event hash object is correct');
assert.strictEqual(event.type, type, `the "${type}" event was triggered`);
assert.strictEqual(event.target, this.player.el_, 'the event has the correct target');

assert.deepEqual(hash, {
name: 'mock',

// The "before" events have a `null` instance and the others have the
// return value of the plugin factory.
instance: i < 2 ? null : instance,
plugin: this.MockPlugin
}, 'the event hash object is correct');
});
});

QUnit.test('defaultState static property is used to populate state', function(assert) {
Expand Down
34 changes: 23 additions & 11 deletions test/unit/plugin-basic.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,23 +47,35 @@ QUnit.test('setup', function(assert) {
assert.ok(this.player.hasPlugin('basic'), 'player has the plugin available');
});

QUnit.test('"pluginsetup" event', function(assert) {
QUnit.test('all "pluginsetup" events', function(assert) {
const setupSpy = sinon.spy();
const events = [
'beforepluginsetup',
'beforepluginsetup:basic',
'pluginsetup',
'pluginsetup:basic'
];

this.player.on('pluginsetup', setupSpy);
this.player.on(events, setupSpy);

const instance = this.player.basic();
const event = setupSpy.firstCall.args[0];
const hash = setupSpy.firstCall.args[1];

assert.strictEqual(setupSpy.callCount, 1, 'the "pluginsetup" event was triggered');
assert.strictEqual(event.type, 'pluginsetup', 'the event has the correct type');
events.forEach((type, i) => {
const event = setupSpy.getCall(i).args[0];
const hash = setupSpy.getCall(i).args[1];

assert.deepEqual(hash, {
name: 'basic',
instance,
plugin: this.basic
}, 'the event hash object is correct');
assert.strictEqual(event.type, type, `the "${type}" event was triggered`);
assert.strictEqual(event.target, this.player.el_, 'the event has the correct target');

assert.deepEqual(hash, {
name: 'basic',

// The "before" events have a `null` instance and the others have the
// return value of the plugin factory.
instance: i < 2 ? null : instance,
plugin: this.basic
}, 'the event hash object is correct');
});
});

QUnit.test('properties are copied', function(assert) {
Expand Down