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

fix: Allow evented objects, such as components and plugins, to listen to the window object in addition to DOM objects. #5255

Merged
merged 4 commits into from
Jun 20, 2018
Merged
Show file tree
Hide file tree
Changes from 3 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: 17 additions & 4 deletions src/js/mixins/evented.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,19 @@ import * as Fn from '../utils/fn';
import * as Obj from '../utils/obj';
import EventTarget from '../event-target';

/**
* Returns whether or not an object supports native events.
*
* @param {Object} object
* An object to test.
*
* @return {boolean}
* Whether or not an object supports native events.
*/
const supportsNativeEvents = (object) =>
(typeof window.EventTarget === 'function' && object instanceof window.EventTarget) ||
typeof object.addEventListener === 'function';
Copy link
Member

Choose a reason for hiding this comment

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

what about only just doing the addEventListener check?


/**
* Returns whether or not an object has had the evented mixin applied.
*
Expand Down Expand Up @@ -50,7 +63,7 @@ const isValidEventType = (type) =>
* The object to test.
*/
const validateTarget = (target) => {
if (!target.nodeName && !isEvented(target)) {
if (!supportsNativeEvents(target) && !isEvented(target)) {
throw new Error('Invalid target; must be a DOM node or evented object.');
}
};
Expand Down Expand Up @@ -154,7 +167,7 @@ const normalizeListenArgs = (self, args) => {
const listen = (target, method, type, listener) => {
validateTarget(target);

if (target.nodeName) {
if (supportsNativeEvents(target)) {
Events[method](target, type, listener);
} else {
target[method](type, listener);
Expand Down Expand Up @@ -307,7 +320,7 @@ const EventedMixin = {
// the same guid as the event listener in on().
this.off('dispose', listener);

if (target.nodeName) {
if (supportsNativeEvents(target)) {
Events.off(target, type, listener);
Events.off(target, 'dispose', listener);
} else if (isEvented(target)) {
Expand Down Expand Up @@ -346,7 +359,7 @@ const EventedMixin = {
* @param {String} [options.eventBusKey]
* By default, adds a `eventBusEl_` DOM element to the target object,
* which is used as an event bus. If the target object already has a
* DOM element that should be used, pass its key here.
* DOM element which should be used, pass its key here.
*
* @return {Object}
* The target object.
Expand Down
235 changes: 222 additions & 13 deletions test/unit/mixins/evented.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
/* eslint-env qunit */
import document from 'global/document';
import window from 'global/window';
import sinon from 'sinon';
import evented from '../../../src/js/mixins/evented';
import * as Dom from '../../../src/js/utils/dom';
Expand All @@ -11,6 +13,20 @@ const errors = {
target: new Error('Invalid target; must be a DOM node or evented object.')
};

// Cross-browser event creation.
const createEvent = (type) => {
let event;

try {
event = new window.Event(type);
} catch (x) {
event = document.createEvent('Event');
event.initEvent(type, true, true);
}

return event;
};

const validateListenerCall = (call, thisValue, eventExpectation) => {
const eventActual = call.args[0];

Expand All @@ -31,7 +47,13 @@ QUnit.module('mixins: evented', {
},

afterEach() {
Object.keys(this.targets).forEach(k => this.targets[k].trigger('dispose'));
Object.keys(this.targets).forEach(k => {
if (typeof this.targets[k].trigger === 'function') {
this.targets[k].trigger('dispose');
} else {
this.targets[k] = null;
}
});
}
});

Expand Down Expand Up @@ -61,7 +83,7 @@ QUnit.test('evented() with custom element', function(assert) {
);
});

QUnit.test('on() and one() errors', function(assert) {
QUnit.test('on() and one() error states', function(assert) {
const target = this.targets.a = evented({});

['on', 'one'].forEach(method => {
Expand All @@ -74,7 +96,7 @@ QUnit.test('on() and one() errors', function(assert) {
});
});

QUnit.test('off() errors', function(assert) {
QUnit.test('off() error states', function(assert) {
const target = this.targets.a = evented({});

// An invalid event actually causes an invalid target error because it
Expand All @@ -86,7 +108,7 @@ QUnit.test('off() errors', function(assert) {
assert.throws(() => target.off(evented({}), 'x', null), errors.listener, 'the expected error is thrown');
});

QUnit.test('on() can add a listener to one event type on this object', function(assert) {
QUnit.test('on() can add a listener to one event type on itself', function(assert) {
const a = this.targets.a = evented({});
const spy = sinon.spy();

Expand All @@ -101,7 +123,7 @@ QUnit.test('on() can add a listener to one event type on this object', function(
});
});

QUnit.test('on() can add a listener to an array of event types on this object', function(assert) {
QUnit.test('on() can add a listener to an array of event types on itself', function(assert) {
const a = this.targets.a = evented({});
const spy = sinon.spy();

Expand All @@ -122,7 +144,7 @@ QUnit.test('on() can add a listener to an array of event types on this object',
});
});

QUnit.test('one() can add a listener to one event type on this object', function(assert) {
QUnit.test('one() can add a listener to one event type on itself', function(assert) {
const a = this.targets.a = evented({});
const spy = sinon.spy();

Expand All @@ -138,7 +160,7 @@ QUnit.test('one() can add a listener to one event type on this object', function
});
});

QUnit.test('one() can add a listener to an array of event types on this object', function(assert) {
QUnit.test('one() can add a listener to an array of event types on itself', function(assert) {
const a = this.targets.a = evented({});
const spy = sinon.spy();

Expand All @@ -161,7 +183,7 @@ QUnit.test('one() can add a listener to an array of event types on this object',
});
});

QUnit.test('on() can add a listener to one event type on a different target object', function(assert) {
QUnit.test('on() can add a listener to one event type on a different evented object', function(assert) {
const a = this.targets.a = evented({});
const b = this.targets.b = evented({});
const spy = sinon.spy();
Expand All @@ -180,7 +202,47 @@ QUnit.test('on() can add a listener to one event type on a different target obje
});
});

QUnit.test('on() can add a listener to an array of event types on a different target object', function(assert) {
QUnit.test('on() can add a listener to one event type on a node object', function(assert) {
const a = this.targets.a = evented({});
const b = this.targets.b = Dom.createEl('div');
const spy = sinon.spy();
const event = createEvent('x');

a.on(b, 'x', spy);
b.dispatchEvent(event);

// 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
});
});

QUnit.test('on() can add a listener to one event type on the window object', function(assert) {
const a = this.targets.a = evented({});
const b = this.targets.b = window;
const spy = sinon.spy();
const event = createEvent('x');

a.on(b, 'x', spy);
b.dispatchEvent(event);

// 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
});
});

QUnit.test('on() can add a listener to an array of event types on a different evented object', function(assert) {
const a = this.targets.a = evented({});
const b = this.targets.b = evented({});
const spy = sinon.spy();
Expand All @@ -206,13 +268,70 @@ QUnit.test('on() can add a listener to an array of event types on a different ta
});
});

QUnit.test('one() can add a listener to one event type on a different target object', function(assert) {
QUnit.test('on() can add a listener to an array of event types on a node object', function(assert) {
const a = this.targets.a = evented({});
const b = this.targets.b = Dom.createEl('div');
const spy = sinon.spy();
const x = createEvent('x');
const y = createEvent('y');

a.on(b, ['x', 'y'], spy);
b.dispatchEvent(x);
b.dispatchEvent(y);

// Make sure we aren't magically binding a listener to "a".
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: b
});

validateListenerCall(spy.getCall(1), a, {
type: 'y',
target: b
});
});

QUnit.test('on() can add a listener to an array of event types on the window object', function(assert) {
const a = this.targets.a = evented({});
const b = this.targets.b = window;
const spy = sinon.spy();
const x = createEvent('x');
const y = createEvent('y');

a.on(b, ['x', 'y'], spy);
b.dispatchEvent(x);
b.dispatchEvent(y);

// Make sure we aren't magically binding a listener to "a".
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: b
});

validateListenerCall(spy.getCall(1), a, {
type: 'y',
target: b
});
});

QUnit.test('one() can add a listener to one event type on a different evented object', function(assert) {
const a = this.targets.a = evented({});
const b = this.targets.b = evented({});
const spy = sinon.spy();

a.one(b, 'x', spy);
b.trigger('x');
b.trigger('x');

// Make sure we aren't magically binding a listener to "a".
a.trigger('x');
Expand All @@ -225,9 +344,49 @@ 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...
QUnit.test('one() can add a listener to an array of event types on a different target object', function(assert) {
QUnit.test('one() can add a listener to one event type on a node object', function(assert) {
const a = this.targets.a = evented({});
const b = this.targets.b = Dom.createEl('div');
const spy = sinon.spy();
const event = createEvent('x');

a.one(b, 'x', spy);
b.dispatchEvent(event);
b.dispatchEvent(event);

// 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
});
});

QUnit.test('one() can add a listener to one event type on the window object', function(assert) {
const a = this.targets.a = evented({});
const b = this.targets.b = window;
const spy = sinon.spy();
const event = createEvent('x');

a.one(b, 'x', spy);
b.dispatchEvent(event);
b.dispatchEvent(event);

// 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
});
});

QUnit.test('one() can add a listener to an array of event types on a different evented object', function(assert) {
const a = this.targets.a = evented({});
const b = this.targets.b = evented({});
const spy = sinon.spy();
Expand All @@ -250,6 +409,56 @@ QUnit.test('one() can add a listener to an array of event types on a different t
});
});

QUnit.test('one() can add a listener to an array of event types on a node object', function(assert) {
const a = this.targets.a = evented({});
const b = this.targets.b = Dom.createEl('div');
const spy = sinon.spy();
const x = createEvent('x');
const y = createEvent('y');

a.one(b, ['x', 'y'], spy);
b.dispatchEvent(x);
b.dispatchEvent(y);
b.dispatchEvent(x);
b.dispatchEvent(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
});
});

QUnit.test('one() can add a listener to an array of event types on the window object', function(assert) {
const a = this.targets.a = evented({});
const b = this.targets.b = window;
const spy = sinon.spy();
const x = createEvent('x');
const y = createEvent('y');

a.one(b, ['x', 'y'], spy);
b.dispatchEvent(x);
b.dispatchEvent(y);
b.dispatchEvent(x);
b.dispatchEvent(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
});
});

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();
Expand Down