From 250e55b0d98d0855ede51ca6245cecd559b299ed Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 20 Sep 2018 22:43:44 +0200 Subject: [PATCH 1/3] Ensure that all event properties are included, even if no (internal) listeners are registered, when re-dispatching events to the DOM (PR 10019 follow-up) --- web/ui_utils.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/web/ui_utils.js b/web/ui_utils.js index d0e59d58d..c0c11f277 100644 --- a/web/ui_utils.js +++ b/web/ui_utils.js @@ -708,12 +708,13 @@ class EventBus { let eventListeners = this._listeners[eventName]; if (!eventListeners || eventListeners.length === 0) { if (this._dispatchToDOM) { - this._dispatchDOMEvent(eventName); + const args = Array.prototype.slice.call(arguments, 1); + this._dispatchDOMEvent(eventName, args); } return; } // Passing all arguments after the eventName to the listeners. - let args = Array.prototype.slice.call(arguments, 1); + const args = Array.prototype.slice.call(arguments, 1); // Making copy of the listeners array in case if it will be modified // during dispatch. eventListeners.slice(0).forEach(function (listener) { From f317a2cb40813a5dde2942266b55559be53b868d Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 20 Sep 2018 23:08:38 +0200 Subject: [PATCH 2/3] Ensure that the DOM event listeners are removed at the end of the relevant `EventBus` unit-tests, to prevent the tests from interfering with each other --- test/unit/ui_utils_spec.js | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/test/unit/ui_utils_spec.js b/test/unit/ui_utils_spec.js index 4ba2a22e0..eab665fe1 100644 --- a/test/unit/ui_utils_spec.js +++ b/test/unit/ui_utils_spec.js @@ -272,13 +272,17 @@ describe('ui_utils', function() { eventBus.on('test', function() { count++; }); - document.addEventListener('test', function() { - count++; - }); + function domEventListener() { + done.fail('shall not dispatch DOM event.'); + } + document.addEventListener('test', domEventListener); + eventBus.dispatch('test'); Promise.resolve().then(() => { expect(count).toEqual(1); + + document.removeEventListener('test', domEventListener); done(); }); }); @@ -291,13 +295,17 @@ describe('ui_utils', function() { eventBus.on('test', function() { count++; }); - document.addEventListener('test', function() { + function domEventListener() { count++; - }); + } + document.addEventListener('test', domEventListener); + eventBus.dispatch('test'); Promise.resolve().then(() => { expect(count).toEqual(2); + + document.removeEventListener('test', domEventListener); done(); }); }); From 39776168a263b79837058c8a1a08b192bae84788 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 21 Sep 2018 14:12:01 +0200 Subject: [PATCH 3/3] Add `EventBus` unit-tests to ensure that the (optional) argument handling works correctly --- test/unit/ui_utils_spec.js | 48 ++++++++++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/test/unit/ui_utils_spec.js b/test/unit/ui_utils_spec.js index eab665fe1..145b30d6a 100644 --- a/test/unit/ui_utils_spec.js +++ b/test/unit/ui_utils_spec.js @@ -182,12 +182,25 @@ describe('ui_utils', function() { it('dispatch event', function () { var eventBus = new EventBus(); var count = 0; - eventBus.on('test', function () { + eventBus.on('test', function(evt) { + expect(evt).toEqual(undefined); count++; }); eventBus.dispatch('test'); expect(count).toEqual(1); }); + it('dispatch event with arguments', function() { + const eventBus = new EventBus(); + let count = 0; + eventBus.on('test', function(evt) { + expect(evt).toEqual({ abc: 123, }); + count++; + }); + eventBus.dispatch('test', { + abc: 123, + }); + expect(count).toEqual(1); + }); it('dispatch different event', function () { var eventBus = new EventBus(); var count = 0; @@ -269,7 +282,8 @@ describe('ui_utils', function() { } const eventBus = new EventBus(); let count = 0; - eventBus.on('test', function() { + eventBus.on('test', function(evt) { + expect(evt).toEqual(undefined); count++; }); function domEventListener() { @@ -292,10 +306,12 @@ describe('ui_utils', function() { } const eventBus = new EventBus({ dispatchToDOM: true, }); let count = 0; - eventBus.on('test', function() { + eventBus.on('test', function(evt) { + expect(evt).toEqual(undefined); count++; }); - function domEventListener() { + function domEventListener(evt) { + expect(evt.detail).toEqual({}); count++; } document.addEventListener('test', domEventListener); @@ -305,6 +321,30 @@ describe('ui_utils', function() { Promise.resolve().then(() => { expect(count).toEqual(2); + document.removeEventListener('test', domEventListener); + done(); + }); + }); + it('should re-dispatch to DOM, with arguments (without internal listeners)', + function(done) { + if (isNodeJS()) { + pending('Document in not supported in Node.js.'); + } + const eventBus = new EventBus({ dispatchToDOM: true, }); + let count = 0; + function domEventListener(evt) { + expect(evt.detail).toEqual({ abc: 123, }); + count++; + } + document.addEventListener('test', domEventListener); + + eventBus.dispatch('test', { + abc: 123, + }); + + Promise.resolve().then(() => { + expect(count).toEqual(1); + document.removeEventListener('test', domEventListener); done(); });