From 2878c1d0d48e02204c69d37afb7eb608d22a7039 Mon Sep 17 00:00:00 2001 From: Brandon Casey <2381475+brandonocasey@users.noreply.github.com> Date: Mon, 17 Jun 2019 14:04:25 -0400 Subject: [PATCH] feat(events): add any function (#5977) This new events function allows you to listen to a list of events and know that only one handler will ever be called for the group. With just one event, it'll function similarly to `.one`. Examples: Single event ``` const player = videojs('some-player-id'); player.any('a', (e) => console.log(e.type + ' triggered'); player.trigger('a'); // logs 'a triggered' player.trigger('a'); // logs nothing as the listener has been removed. ``` Multiple Events ``` const player = videojs('some-player-id'); player.any(['a', 'b', 'c', 'd'], (e) => console.log(e.type + ' triggered'); player.trigger('d'); // logs 'd triggered' player.trigger('a'); player.trigger('b'); player.trigger('c'); player.trigger('d'); // all triggers above log nothing as the listener is removed after the first 'd' trigger. ``` --- src/js/event-target.js | 12 +++- src/js/mixins/evented.js | 51 ++++++++++++++- src/js/utils/events.js | 26 ++++++++ test/unit/events.test.js | 36 +++++++++++ test/unit/mixins/evented.test.js | 108 +++++++++++++++++++++++++++++-- 5 files changed, 227 insertions(+), 6 deletions(-) diff --git a/src/js/event-target.js b/src/js/event-target.js index 6f53a722a..84efd793b 100644 --- a/src/js/event-target.js +++ b/src/js/event-target.js @@ -110,7 +110,7 @@ EventTarget.prototype.removeEventListener = EventTarget.prototype.off; * The function to be called once for each event name. */ EventTarget.prototype.one = function(type, fn) { - // Remove the addEventListener alialing Events.on + // Remove the addEventListener aliasing Events.on // so we don't get into an infinite type loop const ael = this.addEventListener; @@ -119,6 +119,16 @@ EventTarget.prototype.one = function(type, fn) { this.addEventListener = ael; }; +EventTarget.prototype.any = function(type, fn) { + // Remove the addEventListener aliasing Events.on + // so we don't get into an infinite type loop + const ael = this.addEventListener; + + this.addEventListener = () => {}; + Events.any(this, type, fn); + this.addEventListener = ael; +}; + /** * This function causes an event to happen. This will then cause any `event listeners` * that are waiting for that event, to get called. If there are no `event listeners` diff --git a/src/js/mixins/evented.js b/src/js/mixins/evented.js index 95a91d106..92b156860 100644 --- a/src/js/mixins/evented.js +++ b/src/js/mixins/evented.js @@ -242,7 +242,7 @@ const EventedMixin = { /** * Add a listener to an event (or events) on this object or another evented - * object. The listener will only be called once and then removed. + * object. The listener will be called once per event and then removed. * * @param {string|Array|Element|Object} targetOrType * If this is a string or array, it represents the event type(s) @@ -272,6 +272,10 @@ const EventedMixin = { // Targeting another evented object. } else { + // TODO: This wrapper is incorrect! It should only + // remove the wrapper for the event type that called it. + // Instead all listners are removed on the first trigger! + // see https://github.com/videojs/video.js/issues/5962 const wrapper = (...largs) => { this.off(target, type, wrapper); listener.apply(null, largs); @@ -284,6 +288,51 @@ const EventedMixin = { } }, + /** + * Add a listener to an event (or events) on this object or another evented + * object. The listener will only be called once for the first event that is triggered + * then removed. + * + * @param {string|Array|Element|Object} targetOrType + * If this is a string or array, it represents the event type(s) + * that will trigger the listener. + * + * Another evented object can be passed here instead, which will + * cause the listener to listen for events on _that_ object. + * + * In either case, the listener's `this` value will be bound to + * this object. + * + * @param {string|Array|Function} typeOrListener + * If the first argument was a string or array, this should be the + * listener function. Otherwise, this is a string or array of event + * type(s). + * + * @param {Function} [listener] + * If the first argument was another evented object, this will be + * the listener function. + */ + any(...args) { + const {isTargetingSelf, target, type, listener} = normalizeListenArgs(this, args); + + // Targeting this evented object. + if (isTargetingSelf) { + listen(target, 'any', type, listener); + + // Targeting another evented object. + } else { + const wrapper = (...largs) => { + this.off(target, type, wrapper); + listener.apply(null, largs); + }; + + // Use the same function ID as the listener so we can remove it later + // it using the ID of the original listener. + wrapper.guid = listener.guid; + listen(target, 'any', type, wrapper); + } + }, + /** * Removes listener(s) from event(s) on an evented object. * diff --git a/src/js/utils/events.js b/src/js/utils/events.js index 16b80b8de..f5ff79e80 100644 --- a/src/js/utils/events.js +++ b/src/js/utils/events.js @@ -476,3 +476,29 @@ export function one(elem, type, fn) { func.guid = fn.guid = fn.guid || Guid.newGUID(); on(elem, type, func); } + +/** + * Trigger a listener only once and then turn if off for all + * configured events + * + * @param {Element|Object} elem + * Element or object to bind to. + * + * @param {string|string[]} type + * Name/type of event + * + * @param {Event~EventListener} fn + * Event listener function + */ +export function any(elem, type, fn) { + const func = function() { + off(elem, type, func); + fn.apply(this, arguments); + }; + + // copy the guid to the new function so it can removed using the original function's ID + func.guid = fn.guid = fn.guid || Guid.newGUID(); + + // multiple ons, but one off for everything + on(elem, type, func); +} diff --git a/test/unit/events.test.js b/test/unit/events.test.js index ee90cc11a..bc0cde5ef 100644 --- a/test/unit/events.test.js +++ b/test/unit/events.test.js @@ -347,3 +347,39 @@ QUnit.test('retrigger with an object should use the old element as target', func Events.off(el1, 'click'); Events.off(el2, 'click'); }); + +QUnit.test('should listen only once for any', function(assert) { + const el = document.createElement('div'); + let triggered = 0; + const listener = () => triggered++; + + Events.any(el, 'click', listener); + assert.equal(triggered, 0, 'listener was not yet triggered'); + // 1 click + Events.trigger(el, 'click'); + + assert.equal(triggered, 1, 'listener was triggered'); + // No click should happen. + Events.trigger(el, 'click'); + assert.equal(triggered, 1, 'listener was not triggered again'); +}); + +QUnit.test('only the first event should call listener via any', function(assert) { + const el = document.createElement('div'); + let triggered = 0; + const listener = () => triggered++; + + Events.any(el, ['click', 'event1', 'event2'], listener); + assert.equal(triggered, 0, 'listener was not yet triggered'); + + // 1 click + Events.trigger(el, 'click'); + assert.equal(triggered, 1, 'listener was triggered'); + // nothing below here should trigger the Callback + Events.trigger(el, 'click'); + Events.trigger(el, 'event1'); + Events.trigger(el, 'event1'); + Events.trigger(el, 'event2'); + Events.trigger(el, 'event2'); + assert.equal(triggered, 1, 'listener was not triggered again'); +}); diff --git a/test/unit/mixins/evented.test.js b/test/unit/mixins/evented.test.js index 4e8e4a66e..4bd98923c 100644 --- a/test/unit/mixins/evented.test.js +++ b/test/unit/mixins/evented.test.js @@ -61,11 +61,11 @@ QUnit.test('evented() with custom element', function(assert) { ); }); -QUnit.test('on() and one() errors', function(assert) { +QUnit.test('on(), one(), and any() errors', function(assert) { const targeta = this.targets.a = evented({}); const targetb = this.targets.b = evented({}); - ['on', 'one'].forEach(method => { + ['on', 'one', 'any'].forEach(method => { assert.throws(() => targeta[method](), errors.type, 'the expected error is thrown'); assert.throws(() => targeta[method](' '), errors.type, 'the expected error is thrown'); assert.throws(() => targeta[method]([]), errors.type, 'the expected error is thrown'); @@ -165,6 +165,63 @@ QUnit.test('one() can add a listener to an array of event types on this object', }); }); +QUnit.test('one() can add a listener to an array of event types on this object', function(assert) { + const a = this.targets.a = evented({}); + const spy = sinon.spy(); + + a.one(['x', 'y'], spy); + a.trigger('x'); + a.trigger('y'); + a.trigger('x'); + a.trigger('y'); + + assert.strictEqual(spy.callCount, 2, 'the listener was called the expected number of times'); + + validateListenerCall(spy.getCall(0), a, { + type: 'x', + target: a.eventBusEl_ + }); + + validateListenerCall(spy.getCall(1), a, { + type: 'y', + target: a.eventBusEl_ + }); +}); + +QUnit.test('any() can add a listener to one event type on this object', function(assert) { + const a = this.targets.a = evented({}); + const spy = sinon.spy(); + + a.any('x', spy); + a.trigger('x'); + a.trigger('x'); + + assert.strictEqual(spy.callCount, 1, 'the listener was called the expected number of times'); + + validateListenerCall(spy.getCall(0), a, { + type: 'x', + target: a.eventBusEl_ + }); +}); + +QUnit.test('any() can add a listener to an array of event types on this object', function(assert) { + const a = this.targets.a = evented({}); + const spy = sinon.spy(); + + a.any(['x', 'y'], spy); + a.trigger('x'); + a.trigger('y'); + a.trigger('x'); + a.trigger('y'); + + assert.strictEqual(spy.callCount, 1, 'the listener was called the expected number of times'); + + validateListenerCall(spy.getCall(0), a, { + type: 'x', + target: a.eventBusEl_ + }); +}); + QUnit.test('on() can add a listener to one event type on a different target object', function(assert) { const a = this.targets.a = evented({}); const b = this.targets.b = evented({}); @@ -229,8 +286,9 @@ QUnit.test('one() can add a listener to one event type on a different target obj }); }); -// The behavior here unfortunately differs from the identical case where "a" -// listens to itself. This is something that should be resolved... +// TODO: This test is incorrect! this listener should be called twice, +// but instead all listners are removed on the first trigger! +// see https://github.com/videojs/video.js/issues/5962 QUnit.test('one() can add a listener to an array of event types on a different target object', function(assert) { const a = this.targets.a = evented({}); const b = this.targets.b = evented({}); @@ -254,6 +312,48 @@ QUnit.test('one() can add a listener to an array of event types on a different t }); }); +QUnit.test('any() can add a listener to one event type on a different target object', function(assert) { + const a = this.targets.a = evented({}); + const b = this.targets.b = evented({}); + const spy = sinon.spy(); + + a.any(b, 'x', spy); + b.trigger('x'); + + // Make sure we aren't magically binding a listener to "a". + a.trigger('x'); + + assert.strictEqual(spy.callCount, 1, 'the listener was called the expected number of times'); + + validateListenerCall(spy.getCall(0), a, { + type: 'x', + target: b.eventBusEl_ + }); +}); + +QUnit.test('any() can add a listener to an array of event types on a different target object', function(assert) { + const a = this.targets.a = evented({}); + const b = this.targets.b = evented({}); + const spy = sinon.spy(); + + a.any(b, ['x', 'y'], spy); + b.trigger('x'); + b.trigger('y'); + b.trigger('x'); + b.trigger('y'); + + // Make sure we aren't magically binding a listener to "a". + a.trigger('x'); + a.trigger('y'); + + assert.strictEqual(spy.callCount, 1, 'the listener was called the expected number of times'); + + validateListenerCall(spy.getCall(0), a, { + type: 'x', + target: b.eventBusEl_ + }); +}); + QUnit.test('off() with no arguments will remove all listeners from all events on this object', function(assert) { const a = this.targets.a = evented({}); const spyX = sinon.spy();