From 1cb8c4887e949f78c71b24ac286c3b553771a5ec Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Tue, 7 Jun 2022 13:32:51 +0300 Subject: [PATCH 1/3] feat(Observable): return value return disposer for on/once void for fire/off --- src/mixins/observable.mixin.js | 36 +++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/src/mixins/observable.mixin.js b/src/mixins/observable.mixin.js index c642ee682f5..41ef28403bc 100644 --- a/src/mixins/observable.mixin.js +++ b/src/mixins/observable.mixin.js @@ -24,8 +24,7 @@ * @alias on * @param {String|Object} eventName Event name (eg. 'after:render') or object with key/value pairs (eg. {'after:render': handler, 'selection:cleared': handler}) * @param {Function} handler Function that receives a notification when an event of the specified type occurs - * @return {Self} thisArg - * @chainable + * @return {Function} disposer */ function on(eventName, handler) { if (!this.__eventListeners) { @@ -43,7 +42,7 @@ } this.__eventListeners[eventName].push(handler); } - return this; + return off.bind(this, eventName, handler); } function _once(eventName, handler) { @@ -52,19 +51,30 @@ this.off(eventName, _handler); }.bind(this); this.on(eventName, _handler); + return _handler; } + /** + * Observes specified event **once** + * @memberOf fabric.Observable + * @alias once + * @param {String|Object} eventName Event name (eg. 'after:render') or object with key/value pairs (eg. {'after:render': handler, 'selection:cleared': handler}) + * @param {Function} handler Function that receives a notification when an event of the specified type occurs + * @return {Function} disposer + */ function once(eventName, handler) { // one object with key/value pairs was passed if (arguments.length === 1) { + var handlers = {}; for (var prop in eventName) { - _once.call(this, prop, eventName[prop]); + handlers[prop] = _once.call(this, prop, eventName[prop]); } + return off.bind(this, handlers); } else { - _once.call(this, eventName, handler); + var _handler = _once.call(this, eventName, handler); + return off.bind(this, eventName, _handler); } - return this; } /** @@ -74,12 +84,10 @@ * @alias off * @param {String|Object} eventName Event name (eg. 'after:render') or object with key/value pairs (eg. {'after:render': handler, 'selection:cleared': handler}) * @param {Function} handler Function to be deleted from EventListeners - * @return {Self} thisArg - * @chainable */ function off(eventName, handler) { if (!this.__eventListeners) { - return this; + return; } // remove all key/value pairs (event name -> event handler) @@ -89,7 +97,7 @@ } } // one object with key/value pairs was passed - else if (arguments.length === 1 && typeof arguments[0] === 'object') { + else if (typeof eventName === 'object' && typeof handler === 'undefined') { for (var prop in eventName) { _removeEventListener.call(this, prop, eventName[prop]); } @@ -97,7 +105,6 @@ else { _removeEventListener.call(this, eventName, handler); } - return this; } /** @@ -105,17 +112,15 @@ * @memberOf fabric.Observable * @param {String} eventName Event name to fire * @param {Object} [options] Options object - * @return {Self} thisArg - * @chainable */ function fire(eventName, options) { if (!this.__eventListeners) { - return this; + return; } var listenersForEvent = this.__eventListeners[eventName]; if (!listenersForEvent) { - return this; + return; } for (var i = 0, len = listenersForEvent.length; i < len; i++) { @@ -124,7 +129,6 @@ this.__eventListeners[eventName] = listenersForEvent.filter(function(value) { return value !== false; }); - return this; } /** From 8443f0e84363ec00d32001f404f3c3669a823417 Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Tue, 7 Jun 2022 13:36:54 +0300 Subject: [PATCH 2/3] fix(): tests --- test/unit/object.js | 21 +++--------- test/unit/observable.js | 71 +++++++++++++++++++++++++++++------------ 2 files changed, 55 insertions(+), 37 deletions(-) diff --git a/test/unit/object.js b/test/unit/object.js index e0d16384971..7f5fcc66b36 100644 --- a/test/unit/object.js +++ b/test/unit/object.js @@ -579,29 +579,16 @@ }, 1000); }); - QUnit.test('on off fire are chainable', function(assert) { - var object = new fabric.Object({ left: 20, top: 30, width: 40, height: 50, angle: 43 }); - var ret; - ret = object.fire(''); - assert.equal(ret, object, 'fire is chainable when no events are registered at all'); - ret = object.on('hi', function() {}); - assert.equal(ret, object, 'on is chainable'); - ret = object.fire('bye'); - assert.equal(ret, object, 'fire is chainable when firing a non registerd event'); - ret = object.fire('hi'); - assert.equal(ret, object, 'fire is chainable when firing a registerd event'); - ret = object.off('hi'); - assert.equal(ret, object, 'off is chainable'); - }); - QUnit.test('observable', function(assert) { var object = new fabric.Object({ left: 20, top: 30, width: 40, height: 50, angle: 43 }); var fooFired = false, barFired = false; - object.on('foo', function() { fooFired = true; }); - object.on('bar', function() { barFired = true; }); + var fooDisposer = object.on('foo', function() { fooFired = true; }); + var barDisposer = object.on('bar', function () { barFired = true; }); + assert.ok(typeof fooDisposer === 'function', 'should return disposer'); + assert.ok(typeof barDisposer === 'function', 'should return disposer'); object.fire('foo'); assert.ok(fooFired); diff --git a/test/unit/observable.js b/test/unit/observable.js index 14e3bb30b89..6957aa77946 100644 --- a/test/unit/observable.js +++ b/test/unit/observable.js @@ -326,30 +326,61 @@ QUnit.test('adding events', function(assert) { }); -QUnit.test('chaining', function(assert) { +QUnit.test('disposing', function(assert) { var foo = { }; fabric.util.object.extend(foo, fabric.Observable); - var event1Fired = false, event2Fired = false; - foo - .on('event1', function() { - event1Fired = true; - }) - .on('event2', function() { - event2Fired = true; + var fired = new Array(7).fill(false); + var getEventName = function (index) { + return `event${index + 1}`; + } + var createHandler = function (index) { + return function () { + fired[index] = true; + } + } + var attach = function () { + return [ + foo.on(getEventName(0), createHandler(0)), + foo.on(getEventName(1), createHandler(1)), + foo.once(getEventName(2), createHandler(2)), + foo.on({ + [getEventName(3)]: createHandler(3), + [getEventName(4)]: createHandler(4), + }), + foo.once({ + [getEventName(5)]: createHandler(5), + [getEventName(6)]: createHandler(6), + }) + ]; + } + var disposers; + var fireAll = function () { + fired.forEach(function (__, index) { + foo.fire(getEventName(index)); }); + } + var dispose = function () { + disposers.forEach(function (disposer) { + disposer(); + }); + } - foo.fire('event2').fire('event1'); - - assert.equal(event1Fired, true); - assert.equal(event2Fired, true); - - event1Fired = false; - event2Fired = false; - - foo.off('event1').off('event2'); - foo.fire('event2').fire('event1'); + // dispose before firing + disposers = attach(); + disposers.forEach(function (disposer) { + disposer(); + }); + fireAll(); + assert.deepEqual(fired, new Array(fired.length).fill(false)); + + // dispose after firing + disposers = attach(); + fireAll(); + assert.deepEqual(fired, new Array(fired.length).fill(true)); + fired = new Array(fired.length).fill(false); + dispose(); + fireAll(); + assert.deepEqual(fired, new Array(fired.length).fill(false)); - assert.equal(event1Fired, false); - assert.equal(event2Fired, false); }); From 55bc5c70c79e7e834391b8ee1b17dbfb53aace2c Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Tue, 7 Jun 2022 13:44:44 +0300 Subject: [PATCH 3/3] Update observable.js --- test/unit/observable.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/unit/observable.js b/test/unit/observable.js index 6957aa77946..1d5b2ccef4e 100644 --- a/test/unit/observable.js +++ b/test/unit/observable.js @@ -354,7 +354,7 @@ QUnit.test('disposing', function(assert) { }) ]; } - var disposers; + var disposers = []; var fireAll = function () { fired.forEach(function (__, index) { foo.fire(getEventName(index)); @@ -368,9 +368,7 @@ QUnit.test('disposing', function(assert) { // dispose before firing disposers = attach(); - disposers.forEach(function (disposer) { - disposer(); - }); + dispose(); fireAll(); assert.deepEqual(fired, new Array(fired.length).fill(false));