From 54c59238f5fdbf2fef1df73ce33fcfbb01fec23c Mon Sep 17 00:00:00 2001 From: jake downs Date: Tue, 3 Dec 2019 09:47:18 -0800 Subject: [PATCH 01/23] Fixes #4115 - triggers in/out events for sub targets; and calls _setCursorFromEvent for sub targets pass linting --- src/mixins/canvas_events.mixin.js | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/mixins/canvas_events.mixin.js b/src/mixins/canvas_events.mixin.js index c88ad098deb..6dd093dd74b 100644 --- a/src/mixins/canvas_events.mixin.js +++ b/src/mixins/canvas_events.mixin.js @@ -777,7 +777,7 @@ __onMouseMove: function (e) { this._handleEvent(e, 'move:before'); this._cacheTransformEventData(e); - var target, pointer; + var target, pointer, _this = this; if (this.isDrawingMode) { this._onMouseMoveInDrawingMode(e); @@ -803,6 +803,14 @@ target = this.findTarget(e) || null; this._setCursorFromEvent(e, target); this._fireOverOutEvents(target, e); + // handle triggering on SubTargets + this.targets.map(function(subTarget,k){ + _this._fireOverOutEvents(subTarget, e, '_hoveredTarget' + (_this.targets.length - k - 1)); + }); + // hoverCursor should come from top-most subtarget + this.targets.slice(0).reverse().map(function(subTarget){ + _this._setCursorFromEvent(e, subTarget); + }); } else { this._transformObject(e); @@ -815,11 +823,12 @@ * Manage the mouseout, mouseover events for the fabric object on the canvas * @param {Fabric.Object} target the target where the target from the mousemove event * @param {Event} e Event object fired on mousemove + * @param {String} targetName property on the canvas where the target is stored * @private */ - _fireOverOutEvents: function(target, e) { + _fireOverOutEvents: function(target, e, targetName) { this.fireSyntheticInOutEvents(target, e, { - targetName: '_hoveredTarget', + targetName: targetName || '_hoveredTarget', canvasEvtOut: 'mouse:out', evtOut: 'mouseout', canvasEvtIn: 'mouse:over', @@ -866,6 +875,7 @@ if (outFires) { canvasEvtOut && this.fire(canvasEvtOut, outOpt); oldTarget.fire(config.evtOut, outOpt); + delete this[config.targetName]; // de-reference old target to prevent leaks } if (inFires) { canvasEvtIn && this.fire(canvasEvtIn, inOpt); From b6fa464c0fe2cb60bb73f8a64fc29806abe396df Mon Sep 17 00:00:00 2001 From: jake downs Date: Wed, 4 Dec 2019 14:18:57 -0800 Subject: [PATCH 02/23] added a .__guid property to each object to simplify hoveredTargets and draggedoverTargets tracking --- src/canvas.class.js | 21 ++++- src/mixins/canvas_events.mixin.js | 116 ++++++++++++++++++++-------- src/mixins/canvas_grouping.mixin.js | 15 +++- src/shapes/object.class.js | 19 +++++ test/unit/canvas.js | 23 +++++- test/unit/canvas_events.js | 13 +++- 6 files changed, 163 insertions(+), 44 deletions(-) diff --git a/src/canvas.class.js b/src/canvas.class.js index 94268c72cfa..da659929f81 100644 --- a/src/canvas.class.js +++ b/src/canvas.class.js @@ -65,6 +65,10 @@ this._initStatic(el, options); this._initInteractive(); this._createCacheCanvas(); + this._hoveredTargets = {}; + this._hoveredTargetsOrdered = []; + this._draggedoverTargets = {}; + this._draggedoverTargetsOrdered = []; }, /** @@ -1477,8 +1481,21 @@ this.fire('selection:cleared', { target: obj }); obj.fire('deselected'); } - if (this._hoveredTarget === obj) { - this._hoveredTarget = null; + if (this._hoveredTargets[obj.__guid]) { + var hoveredOrderedIndex = this._hoveredTargetsOrdered.indexOf(obj.__guid); + if (hoveredOrderedIndex > -1) { + this._hoveredTargetsOrdered.splice(hoveredOrderedIndex, 1); + } + this._hoveredTargets[obj.__guid] = null; + delete this._hoveredTargets[obj.__guid]; + } + if (this._draggedoverTargets[obj.__guid]) { + var draggedoverOrderedIndex = this._draggedoverTargetsOrdered.indexOf(obj.__guid); + if (draggedoverOrderedIndex > -1) { + this._draggedoverTargetsOrdered.splice(draggedoverOrderedIndex, 1); + } + this._draggedoverTargets[obj.__guid] = null; + delete this._draggedoverTargets[obj.__guid]; } this.callSuper('_onObjectRemoved', obj); }, diff --git a/src/mixins/canvas_events.mixin.js b/src/mixins/canvas_events.mixin.js index 6dd093dd74b..df5cb249999 100644 --- a/src/mixins/canvas_events.mixin.js +++ b/src/mixins/canvas_events.mixin.js @@ -168,7 +168,6 @@ _onMouseOut: function(e) { var target = this._hoveredTarget; this.fire('mouse:out', { target: target, e: e }); - this._hoveredTarget = null; target && target.fire('mouseout', { e: e }); if (this._iTextInstances) { this._iTextInstances.forEach(function(obj) { @@ -177,6 +176,13 @@ } }); } + var hoveredOrderedIndex = this._hoveredTargetsOrdered.indexOf(target ? target.__guid : null); + if (hoveredOrderedIndex > -1) { + // de-ref to prevent memory leaks + this._hoveredTargetsOrdered.splice(hoveredOrderedIndex,1); + this._hoveredTargets[target ? target.__guid : null] = null; + delete this._hoveredTargets[target ? target.__guid : null]; + } }, /** @@ -192,7 +198,8 @@ // side effects we added to it. if (!this.currentTransform && !this.findTarget(e)) { this.fire('mouse:over', { target: null, e: e }); - this._hoveredTarget = null; + this._hoveredTargets = {}; + this._hoveredTargetsOrdered = []; } }, @@ -231,7 +238,7 @@ _onDragOver: function(e) { e.preventDefault(); var target = this._simpleEventHandler('dragover', e); - this._fireEnterLeaveEvents(target, e); + this._fireEnterLeaveEvents([target].concat(this.targets), e); }, /** @@ -801,12 +808,12 @@ } else if (!this._currentTransform) { target = this.findTarget(e) || null; + + // console.log(target, this.targets); + this._setCursorFromEvent(e, target); - this._fireOverOutEvents(target, e); - // handle triggering on SubTargets - this.targets.map(function(subTarget,k){ - _this._fireOverOutEvents(subTarget, e, '_hoveredTarget' + (_this.targets.length - k - 1)); - }); + this._fireOverOutEvents([target].concat(this.targets), e); + // hoverCursor should come from top-most subtarget this.targets.slice(0).reverse().map(function(subTarget){ _this._setCursorFromEvent(e, subTarget); @@ -821,14 +828,14 @@ /** * Manage the mouseout, mouseover events for the fabric object on the canvas - * @param {Fabric.Object} target the target where the target from the mousemove event + * @param {Array} [fabric.Object] target Array of targets from the mousemove event * @param {Event} e Event object fired on mousemove * @param {String} targetName property on the canvas where the target is stored * @private */ - _fireOverOutEvents: function(target, e, targetName) { - this.fireSyntheticInOutEvents(target, e, { - targetName: targetName || '_hoveredTarget', + _fireOverOutEvents: function(targets, e) { + this.fireSyntheticInOutEvents(targets, e, { + targetName: '_hoveredTargets', canvasEvtOut: 'mouse:out', evtOut: 'mouseout', canvasEvtIn: 'mouse:over', @@ -842,9 +849,9 @@ * @param {Event} e Event object fired on ondrag * @private */ - _fireEnterLeaveEvents: function(target, e) { - this.fireSyntheticInOutEvents(target, e, { - targetName: '_draggedoverTarget', + _fireEnterLeaveEvents: function(targets, e) { + this.fireSyntheticInOutEvents(targets, e, { + targetName: '_draggedoverTargets', evtOut: 'dragleave', evtIn: 'dragenter', }); @@ -852,7 +859,7 @@ /** * Manage the synthetic in/out events for the fabric objects on the canvas - * @param {Fabric.Object} target the target where the target from the supported events + * @param {Array} targets [fabric.Object] targets from the supported events * @param {Event} e Event object fired * @param {Object} config configuration for the function to work * @param {String} config.targetName property on the canvas where the old target is stored @@ -862,24 +869,71 @@ * @param {String} config.evtIn name of the event to fire for in * @private */ - fireSyntheticInOutEvents: function(target, e, config) { - var inOpt, outOpt, oldTarget = this[config.targetName], outFires, inFires, - targetChanged = oldTarget !== target, canvasEvtIn = config.canvasEvtIn, canvasEvtOut = config.canvasEvtOut; - if (targetChanged) { - inOpt = { e: e, target: target, previousTarget: oldTarget }; - outOpt = { e: e, target: oldTarget, nextTarget: target }; - this[config.targetName] = target; - } - inFires = target && targetChanged; - outFires = oldTarget && targetChanged; + fireSyntheticInOutEvents: function(targets, e, config) { + targets = targets || []; + var _this = this, + targetsKeyed = {}, + targetGuidsOrdered = [], + oldTargets = this[config.targetName], + oldTargetsOrdered = this[config.targetName + 'Ordered'], + outFires, + inFires, + targetsChanged, + canvasEvtIn = config.canvasEvtIn, + canvasEvtOut = config.canvasEvtOut; + + // array => object conversion for comparison + targets && targets.map(function(target){ + if (target) { + targetsKeyed[target.__guid] = target; + } + if (target) { + targetGuidsOrdered.push(target.__guid); + }; + }); + + var string1 = JSON.stringify(Object.keys(oldTargets)); + var string2 = JSON.stringify(Object.keys(targetsKeyed)); + targetsChanged = string1 !== string2; + + if (targetsChanged) { + this[config.targetName] = targetsKeyed; + this[config.targetName + 'Ordered'] = targetGuidsOrdered; + } + + inFires = targets.length && targetsChanged; + outFires = Object.keys(oldTargets).length && targetsChanged; + if (outFires) { - canvasEvtOut && this.fire(canvasEvtOut, outOpt); - oldTarget.fire(config.evtOut, outOpt); - delete this[config.targetName]; // de-reference old target to prevent leaks + oldTargetsOrdered.map(function(uuid){ + var oldTarget = oldTargets[uuid]; + if (!oldTarget){ + return; + } + var outOpt = { + e: e, + target: oldTarget, + previousTargets: oldTargets, + previousTargetsOrdered: oldTargetsOrdered + }; + canvasEvtOut && _this.fire(canvasEvtOut, outOpt); + oldTarget.fire(config.evtOut, outOpt); + }); } if (inFires) { - canvasEvtIn && this.fire(canvasEvtIn, inOpt); - target.fire(config.evtIn, inOpt); + targets.map(function(target){ + if (!target){ + return; + } + var inOpt = { + e: e, + target: target, + previousTargets: oldTargets, + previousTargetsOrdered: oldTargetsOrdered + }; + canvasEvtIn && _this.fire(canvasEvtIn, inOpt); + target.fire(config.evtIn, inOpt); + }); } }, diff --git a/src/mixins/canvas_grouping.mixin.js b/src/mixins/canvas_grouping.mixin.js index b1e95334e35..0c8424cc0c8 100644 --- a/src/mixins/canvas_grouping.mixin.js +++ b/src/mixins/canvas_grouping.mixin.js @@ -52,15 +52,22 @@ currentActiveObjects = activeSelection._objects.slice(0); if (activeSelection.contains(target)) { activeSelection.removeWithUpdate(target); - this._hoveredTarget = target; + this._hoveredTargets = {}; + this._hoveredTargets[activeSelection.__guid] = activeSelection; + this._hoveredTargetsOrdered = [activeSelection.__guid]; if (activeSelection.size() === 1) { // activate last remaining object this._setActiveObject(activeSelection.item(0), e); + this._hoveredTargets = {}; + this._hoveredTargets[target.__guid] = target; + this._hoveredTargetsOrdered = [target.__guid]; } } else { activeSelection.addWithUpdate(target); - this._hoveredTarget = activeSelection; + this._hoveredTargets = {}; + this._hoveredTargets[activeSelection.__guid] = activeSelection; + this._hoveredTargetsOrdered = [activeSelection.__guid]; } this._fireSelectionEvents(currentActiveObjects, e); }, @@ -70,7 +77,9 @@ */ _createActiveSelection: function(target, e) { var currentActives = this.getActiveObjects(), group = this._createGroup(target); - this._hoveredTarget = group; + this._hoveredTargets = {}; + this._hoveredTargets[group.__guid] = group; + this._hoveredTargetsOrdered = [group.__guid]; this._setActiveObject(group, e); this._fireSelectionEvents(currentActives, e); }, diff --git a/src/shapes/object.class.js b/src/shapes/object.class.js index 10d94f7c5c0..4f993ad0af2 100644 --- a/src/shapes/object.class.js +++ b/src/shapes/object.class.js @@ -2116,4 +2116,23 @@ */ fabric.Object.__uid = 0; + /** + * A GUID for tracking hovered targets and sub targets + * simply tracking via subTargets[index] is not reliable + */ + function generateGUID() { + // https://stackoverflow.com/questions/8012002/create-a-unique-number-with-javascript-time + return new Date().valueOf().toString(36) + Math.random().toString(36).substr(2); + // return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, function(c) { + // var r = Math.random() * 16 | 0, v = c == 'x' ? r : (r & 0x3 | 0x8); + // return v.toString(16); + // }); + } + fabric.Object.prototype.__defineGetter__('__guid', function(){ + if (!this.__myGUID) { + this.__myGUID = generateGUID(); + } + return this.__myGUID; + }); + })(typeof exports !== 'undefined' ? exports : this); diff --git a/test/unit/canvas.js b/test/unit/canvas.js index 2bca638f24b..0c334bec62e 100644 --- a/test/unit/canvas.js +++ b/test/unit/canvas.js @@ -276,9 +276,17 @@ QUnit.test('remove actual hovered target', function(assert) { var rect1 = makeRect(); canvas.add(rect1); - canvas._hoveredTarget = rect1; + canvas._hoveredTargetsOrdered.push(rect1.__guid); + canvas._hoveredTargets[rect1.__guid] = rect1; canvas.remove(rect1); - assert.equal(canvas._hoveredTarget, null, 'reference to hovered target should be removed'); + assert.equal( + canvas._hoveredTargetsOrdered.indexOf(rect1.__guid), + -1, + 'reference to hovered target should be removed'); + assert.equal( + typeof canvas._hoveredTargetsOrdered[rect1.__guid], + 'undefined', + 'reference to hovered target should be removed'); }); QUnit.test('before:selection:cleared', function(assert) { @@ -340,7 +348,8 @@ canvas.on('selection:created', function( ) { isFired = true; }); canvas.setActiveObject(rect1); canvas._createActiveSelection(rect2, {}); - assert.equal(canvas._hoveredTarget, canvas.getActiveObject(), 'the created selection is also hovered'); + assert.equal(canvas._hoveredTargetsOrdered.indexOf(canvas.getActiveObject().__guid), 0, 'the created selection is also hovered'); + assert.equal(canvas._hoveredTargets[canvas.getActiveObject().__guid], canvas.getActiveObject(), 'the created selection is also hovered'); assert.equal(isFired, true, 'selection:created fired'); canvas.off('selection:created'); }); @@ -364,7 +373,13 @@ canvas.setActiveObject(new fabric.ActiveSelection([rect1, rect2])); canvas._updateActiveSelection(rect3, {}); assert.equal(isFired, true, 'selection:updated fired'); - assert.equal(canvas._hoveredTarget, canvas.getActiveObject(), 'hovered target is updated'); + assert.equal(canvas._hoveredTargetsOrdered.indexOf(canvas.getActiveObject().__guid), + 0, + 'hovered target is updated'); + assert.equal(canvas._hoveredTargets[canvas.getActiveObject().__guid], + canvas.getActiveObject(), + 'hovered target is updated'); + canvas.off('selection:updated'); }); diff --git a/test/unit/canvas_events.js b/test/unit/canvas_events.js index 369ed91ab2e..fb3bda91ac7 100644 --- a/test/unit/canvas_events.js +++ b/test/unit/canvas_events.js @@ -537,9 +537,12 @@ var event = fabric.document.createEvent('MouseEvent'); event.initEvent('mouseenter', true, true); var c = new fabric.Canvas(); - c._hoveredTarget = new fabric.Object(); + var obj = new fabric.Object(); + c._hoveredTargets[obj.__guid] = obj; + c._hoveredTargetsOrdered = [obj.__guid]; c.upperCanvasEl.dispatchEvent(event); - assert.equal(c._hoveredTarget, null, '_hoveredTarget has been removed'); + assert.equal(canvas._hoveredTargetsOrdered.indexOf(obj.__guid), -1, 'reference to hovered target should be removed'); + assert.equal(typeof canvas._hoveredTargetsOrdered[obj.__guid], 'undefined', 'reference to hovered target should be removed'); }); QUnit.test('mouseEnter does not remove _hoveredTarget if a transform is happening', function(assert) { @@ -547,10 +550,12 @@ event.initEvent('mouseenter', true, true); var c = new fabric.Canvas(); var obj = new fabric.Object(); - c._hoveredTarget = obj; + c._hoveredTargets[obj.__guid] = obj; + c._hoveredTargetsOrdered = [obj.__guid]; c.currentTransform = {}; c.upperCanvasEl.dispatchEvent(event); - assert.equal(c._hoveredTarget, obj, '_hoveredTarget has been removed'); + assert.equal(c._hoveredTargetsOrdered.indexOf(obj.__guid), 0, '_hoveredTarget has not been removed'); + assert.equal(c._hoveredTargets[obj.__guid], obj, '_hoveredTarget has not been removed'); }); QUnit.test('mouseEnter removes __corner', function(assert) { From 3c082a92c8cf0db25b9a66ddbb2aa6e68b3fd3f4 Mon Sep 17 00:00:00 2001 From: jake downs Date: Wed, 11 Dec 2019 10:15:12 -0800 Subject: [PATCH 03/23] Revert "added a .__guid property to each object to simplify hoveredTargets and draggedoverTargets tracking" This reverts commit b6fa464c0fe2cb60bb73f8a64fc29806abe396df. # Conflicts: # src/mixins/canvas_events.mixin.js --- src/mixins/canvas_events.mixin.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/mixins/canvas_events.mixin.js b/src/mixins/canvas_events.mixin.js index c88ad098deb..86653adacad 100644 --- a/src/mixins/canvas_events.mixin.js +++ b/src/mixins/canvas_events.mixin.js @@ -803,6 +803,14 @@ target = this.findTarget(e) || null; this._setCursorFromEvent(e, target); this._fireOverOutEvents(target, e); + // handle triggering on SubTargets + this.targets.map(function(subTarget,k){ + _this._fireOverOutEvents(subTarget, e, '_hoveredTarget' + (k)); // also tried _this.targets.length - k + }); + // hoverCursor should come from top-most subtarget + this.targets.slice(0).reverse().map(function(subTarget){ + _this._setCursorFromEvent(e, subTarget); + }); } else { this._transformObject(e); @@ -817,9 +825,9 @@ * @param {Event} e Event object fired on mousemove * @private */ - _fireOverOutEvents: function(target, e) { + _fireOverOutEvents: function(target, e, targetName) { this.fireSyntheticInOutEvents(target, e, { - targetName: '_hoveredTarget', + targetName: targetName || '_hoveredTarget', canvasEvtOut: 'mouse:out', evtOut: 'mouseout', canvasEvtIn: 'mouse:over', @@ -866,6 +874,7 @@ if (outFires) { canvasEvtOut && this.fire(canvasEvtOut, outOpt); oldTarget.fire(config.evtOut, outOpt); + delete this[config.targetName]; // de-reference old target to prevent leaks } if (inFires) { canvasEvtIn && this.fire(canvasEvtIn, inOpt); From 15ab4ba315d3ba9cacf68675bbd364e611e5122d Mon Sep 17 00:00:00 2001 From: jake downs Date: Wed, 11 Dec 2019 15:14:01 -0800 Subject: [PATCH 04/23] reverting hoveredTargets to bare bones implementation --- src/canvas.class.js | 32 +++++++++----------- src/mixins/canvas_events.mixin.js | 45 ++++++++++++++++++++--------- src/mixins/canvas_grouping.mixin.js | 39 +++++++++++++++++-------- test/unit/canvas.js | 23 +++------------ test/unit/canvas_events.js | 13 +++------ 5 files changed, 81 insertions(+), 71 deletions(-) diff --git a/src/canvas.class.js b/src/canvas.class.js index da659929f81..25f04460115 100644 --- a/src/canvas.class.js +++ b/src/canvas.class.js @@ -65,10 +65,6 @@ this._initStatic(el, options); this._initInteractive(); this._createCacheCanvas(); - this._hoveredTargets = {}; - this._hoveredTargetsOrdered = []; - this._draggedoverTargets = {}; - this._draggedoverTargetsOrdered = []; }, /** @@ -1481,21 +1477,21 @@ this.fire('selection:cleared', { target: obj }); obj.fire('deselected'); } - if (this._hoveredTargets[obj.__guid]) { - var hoveredOrderedIndex = this._hoveredTargetsOrdered.indexOf(obj.__guid); - if (hoveredOrderedIndex > -1) { - this._hoveredTargetsOrdered.splice(hoveredOrderedIndex, 1); - } - this._hoveredTargets[obj.__guid] = null; - delete this._hoveredTargets[obj.__guid]; - } - if (this._draggedoverTargets[obj.__guid]) { - var draggedoverOrderedIndex = this._draggedoverTargetsOrdered.indexOf(obj.__guid); - if (draggedoverOrderedIndex > -1) { - this._draggedoverTargetsOrdered.splice(draggedoverOrderedIndex, 1); + // TODO: need a way to cleanly check if obj is one of _hoveredTargetN and de-ref it + // if (obj === this._hoveredTarget){ + // this._hoveredTarget = null; + // } + // is there a more sane way than looping through ALL properties of *this* ? + // i wanted to put them into a ._hoveredTargets[Array] + // but, that complicates the logic of fireSyntheticInOutEvents + var keys = Object.keys(this); + for (var i = 0; i < keys.length; i++){ + var key = keys[i]; + if (key.indexOf('_hoveredTarget') > -1){ + if (obj === this[key]){ + this[key] = null; + } } - this._draggedoverTargets[obj.__guid] = null; - delete this._draggedoverTargets[obj.__guid]; } this.callSuper('_onObjectRemoved', obj); }, diff --git a/src/mixins/canvas_events.mixin.js b/src/mixins/canvas_events.mixin.js index 2bb72e77b5b..0a6e02e3471 100644 --- a/src/mixins/canvas_events.mixin.js +++ b/src/mixins/canvas_events.mixin.js @@ -166,9 +166,26 @@ * @param {Event} e Event object fired on mousedown */ _onMouseOut: function(e) { + console.log('_onMouseOut',e); + + // pre-ISSUE-4115 var target = this._hoveredTarget; this.fire('mouse:out', { target: target, e: e }); target && target.fire('mouseout', { e: e }); + + // post-ISSUE-4115 + // should we really be firing mouseOut on ALL _hoveredTargets? + // maybe just the top-level one? I dunno... + var keys = Object.keys(this); + for (var i = 0; i < keys.length; i++){ + var key = keys[i]; + if (key.indexOf('_hoveredTarget') > -1){ + var target = this[key]; + this.fire('mouse:out', { target: target, e: e }); + target && target.fire('mouseout', { e: e }); + } + } + if (this._iTextInstances) { this._iTextInstances.forEach(function(obj) { if (obj.isEditing) { @@ -176,13 +193,6 @@ } }); } - var hoveredOrderedIndex = this._hoveredTargetsOrdered.indexOf(target ? target.__guid : null); - if (hoveredOrderedIndex > -1) { - // de-ref to prevent memory leaks - this._hoveredTargetsOrdered.splice(hoveredOrderedIndex,1); - this._hoveredTargets[target ? target.__guid : null] = null; - delete this._hoveredTargets[target ? target.__guid : null]; - } }, /** @@ -198,8 +208,16 @@ // side effects we added to it. if (!this.currentTransform && !this.findTarget(e)) { this.fire('mouse:over', { target: null, e: e }); - this._hoveredTargets = {}; - this._hoveredTargetsOrdered = []; + // PRE-ISSUE-4115 + // this._hoveredTarget = null; + // POST-ISSUE-4115 + var keys = Object.keys(this); + for (var i = 0; i < keys.length; i++){ + var key = keys[i]; + if (key.indexOf('_hoveredTarget') > -1){ + this[key] = null; + } + } } }, @@ -812,7 +830,7 @@ this._fireOverOutEvents(target, e); // handle triggering on SubTargets this.targets.map(function(subTarget,k){ - _this._fireOverOutEvents(subTarget, e, '_hoveredTarget' + (k)); // also tried _this.targets.length - k + _this._fireOverOutEvents(subTarget, e, '_hoveredTarget' + k); }); // hoverCursor should come from top-most subtarget this.targets.slice(0).reverse().map(function(subTarget){ @@ -871,10 +889,11 @@ var inOpt, outOpt, oldTarget = this[config.targetName], outFires, inFires, targetChanged = oldTarget !== target, canvasEvtIn = config.canvasEvtIn, canvasEvtOut = config.canvasEvtOut; if (targetChanged) { - inOpt = { e: e, target: target, previousTarget: oldTarget }; - outOpt = { e: e, target: oldTarget, nextTarget: target }; + inOpt = {e: e, target: target, previousTarget: oldTarget}; + outOpt = {e: e, target: oldTarget, nextTarget: target}; this[config.targetName] = target; - + console.log(config.targetName, target); // + } if (outFires) { canvasEvtOut && this.fire(canvasEvtOut, outOpt); oldTarget.fire(config.evtOut, outOpt); diff --git a/src/mixins/canvas_grouping.mixin.js b/src/mixins/canvas_grouping.mixin.js index 0c8424cc0c8..7ff047e2166 100644 --- a/src/mixins/canvas_grouping.mixin.js +++ b/src/mixins/canvas_grouping.mixin.js @@ -52,22 +52,38 @@ currentActiveObjects = activeSelection._objects.slice(0); if (activeSelection.contains(target)) { activeSelection.removeWithUpdate(target); - this._hoveredTargets = {}; - this._hoveredTargets[activeSelection.__guid] = activeSelection; - this._hoveredTargetsOrdered = [activeSelection.__guid]; + this._hoveredTarget = target; + // ISSUE-4115: clear out any additional hovered targets that were set? + // should we fire mouse:out on those? + var keys = Object.keys(this); + for (var i = 0; i < keys.length; i++){ + var key = keys[i]; + if (key.indexOf('_hoveredTarget') > -1){ + this[key] = null; + } + } + // ISSUE-4115: loop through this.targets and assign them as hovered as well? + // why don't we fire mouse:over here? + for (var i = 0; i < this.targets.length; i++){ + this['_hoveredTarget' + i] = this.targets[i]; + } if (activeSelection.size() === 1) { // activate last remaining object this._setActiveObject(activeSelection.item(0), e); - this._hoveredTargets = {}; - this._hoveredTargets[target.__guid] = target; - this._hoveredTargetsOrdered = [target.__guid]; } } else { activeSelection.addWithUpdate(target); - this._hoveredTargets = {}; - this._hoveredTargets[activeSelection.__guid] = activeSelection; - this._hoveredTargetsOrdered = [activeSelection.__guid]; + this._hoveredTarget = activeSelection; + // ISSUE-4115: clear out any additional hovered targets that were set? + // should we fire mouse:out on those? + var keys = Object.keys(this); + for (var i = 0; i < keys.length; i++){ + var key = keys[i]; + if (key.indexOf('_hoveredTarget') > -1){ + this[key] = null; + } + } } this._fireSelectionEvents(currentActiveObjects, e); }, @@ -77,9 +93,8 @@ */ _createActiveSelection: function(target, e) { var currentActives = this.getActiveObjects(), group = this._createGroup(target); - this._hoveredTargets = {}; - this._hoveredTargets[group.__guid] = group; - this._hoveredTargetsOrdered = [group.__guid]; + this._hoveredTarget = group; + // ISSUE 4115: should we consider subTargets here? this._setActiveObject(group, e); this._fireSelectionEvents(currentActives, e); }, diff --git a/test/unit/canvas.js b/test/unit/canvas.js index 0c334bec62e..2bca638f24b 100644 --- a/test/unit/canvas.js +++ b/test/unit/canvas.js @@ -276,17 +276,9 @@ QUnit.test('remove actual hovered target', function(assert) { var rect1 = makeRect(); canvas.add(rect1); - canvas._hoveredTargetsOrdered.push(rect1.__guid); - canvas._hoveredTargets[rect1.__guid] = rect1; + canvas._hoveredTarget = rect1; canvas.remove(rect1); - assert.equal( - canvas._hoveredTargetsOrdered.indexOf(rect1.__guid), - -1, - 'reference to hovered target should be removed'); - assert.equal( - typeof canvas._hoveredTargetsOrdered[rect1.__guid], - 'undefined', - 'reference to hovered target should be removed'); + assert.equal(canvas._hoveredTarget, null, 'reference to hovered target should be removed'); }); QUnit.test('before:selection:cleared', function(assert) { @@ -348,8 +340,7 @@ canvas.on('selection:created', function( ) { isFired = true; }); canvas.setActiveObject(rect1); canvas._createActiveSelection(rect2, {}); - assert.equal(canvas._hoveredTargetsOrdered.indexOf(canvas.getActiveObject().__guid), 0, 'the created selection is also hovered'); - assert.equal(canvas._hoveredTargets[canvas.getActiveObject().__guid], canvas.getActiveObject(), 'the created selection is also hovered'); + assert.equal(canvas._hoveredTarget, canvas.getActiveObject(), 'the created selection is also hovered'); assert.equal(isFired, true, 'selection:created fired'); canvas.off('selection:created'); }); @@ -373,13 +364,7 @@ canvas.setActiveObject(new fabric.ActiveSelection([rect1, rect2])); canvas._updateActiveSelection(rect3, {}); assert.equal(isFired, true, 'selection:updated fired'); - assert.equal(canvas._hoveredTargetsOrdered.indexOf(canvas.getActiveObject().__guid), - 0, - 'hovered target is updated'); - assert.equal(canvas._hoveredTargets[canvas.getActiveObject().__guid], - canvas.getActiveObject(), - 'hovered target is updated'); - + assert.equal(canvas._hoveredTarget, canvas.getActiveObject(), 'hovered target is updated'); canvas.off('selection:updated'); }); diff --git a/test/unit/canvas_events.js b/test/unit/canvas_events.js index fb3bda91ac7..bdde39bf07d 100644 --- a/test/unit/canvas_events.js +++ b/test/unit/canvas_events.js @@ -537,12 +537,9 @@ var event = fabric.document.createEvent('MouseEvent'); event.initEvent('mouseenter', true, true); var c = new fabric.Canvas(); - var obj = new fabric.Object(); - c._hoveredTargets[obj.__guid] = obj; - c._hoveredTargetsOrdered = [obj.__guid]; + c._hoveredTarget = new fabric.Object(); c.upperCanvasEl.dispatchEvent(event); - assert.equal(canvas._hoveredTargetsOrdered.indexOf(obj.__guid), -1, 'reference to hovered target should be removed'); - assert.equal(typeof canvas._hoveredTargetsOrdered[obj.__guid], 'undefined', 'reference to hovered target should be removed'); + assert.equal(c._hoveredTarget, null, '_hoveredTarget has been removed'); }); QUnit.test('mouseEnter does not remove _hoveredTarget if a transform is happening', function(assert) { @@ -550,12 +547,10 @@ event.initEvent('mouseenter', true, true); var c = new fabric.Canvas(); var obj = new fabric.Object(); - c._hoveredTargets[obj.__guid] = obj; - c._hoveredTargetsOrdered = [obj.__guid]; + c._hoveredTarget = obj; c.currentTransform = {}; c.upperCanvasEl.dispatchEvent(event); - assert.equal(c._hoveredTargetsOrdered.indexOf(obj.__guid), 0, '_hoveredTarget has not been removed'); - assert.equal(c._hoveredTargets[obj.__guid], obj, '_hoveredTarget has not been removed'); + assert.equal(c._hoveredTarget, obj, '_hoveredTarget has been not removed'); }); QUnit.test('mouseEnter removes __corner', function(assert) { From 38ae83084872814d4e191ef186180b26e74251d7 Mon Sep 17 00:00:00 2001 From: Andrea Bogazzi Date: Sat, 14 Dec 2019 23:43:25 +0100 Subject: [PATCH 05/23] this is an idea --- dist/fabric.js | 161 +++++++++++++++++++----------- src/canvas.class.js | 14 +++ src/mixins/canvas_events.mixin.js | 17 +++- 3 files changed, 133 insertions(+), 59 deletions(-) diff --git a/dist/fabric.js b/dist/fabric.js index e7c505b2a4d..88da6fe5a19 100644 --- a/dist/fabric.js +++ b/dist/fabric.js @@ -74,8 +74,11 @@ fabric.SHARED_ATTRIBUTES = [ */ fabric.DPI = 96; fabric.reNum = '(?:[-+]?(?:\\d+|\\d*\\.\\d+)(?:[eE][-+]?\\d+)?)'; +fabric.rePathCommand = /([-+]?((\d+\.\d+)|((\d+)|(\.\d+)))(?:[eE][-+]?\d+)?)/ig; +fabric.reNonWord = /[ \n\.,;!\?\-]/; fabric.fontPaths = { }; fabric.iMatrix = [1, 0, 0, 1, 0, 0]; +fabric.svgNS = 'http://www.w3.org/2000/svg'; /** * Pixel limit for cache canvases. 1Mpx , 4Mpx should be fine. @@ -2751,29 +2754,13 @@ fabric.CommonMethods = { * Wrapper around `console.log` (when available) * @param {*} [values] Values to log */ -fabric.log = function() { }; +fabric.log = console.log; /** * Wrapper around `console.warn` (when available) * @param {*} [values] Values to log as a warning */ -fabric.warn = function() { }; - -/* eslint-disable */ -if (typeof console !== 'undefined') { - - ['log', 'warn'].forEach(function(methodName) { - - if (typeof console[methodName] !== 'undefined' && - typeof console[methodName].apply === 'function') { - - fabric[methodName] = function() { - return console[methodName].apply(console, arguments); - }; - } - }); -} -/* eslint-enable */ +fabric.warn = console.warn; (function() { @@ -3383,7 +3370,9 @@ if (typeof console !== 'undefined') { colorAttributes = { stroke: 'strokeOpacity', fill: 'fillOpacity' - }; + }, + + fSize = 'font-size', cPath = 'clip-path'; fabric.svgValidTagNamesRegEx = getSvgRegex(svgValidTagNames); fabric.svgViewBoxElementsRegEx = getSvgRegex(svgViewBoxElements); @@ -3798,14 +3787,14 @@ if (typeof console !== 'undefined') { y = el.getAttribute('y') || 0, el2 = elementById(doc, xlink).cloneNode(true), currentTrans = (el2.getAttribute('transform') || '') + ' translate(' + x + ', ' + y + ')', - parentNode, oldLength = nodelist.length, attr, j, attrs, len; + parentNode, oldLength = nodelist.length, attr, j, attrs, len, namespace = fabric.svgNS; applyViewboxTransform(el2); if (/^svg$/i.test(el2.nodeName)) { - var el3 = el2.ownerDocument.createElement('g'); + var el3 = el2.ownerDocument.createElementNS(namespace, 'g'); for (j = 0, attrs = el2.attributes, len = attrs.length; j < len; j++) { attr = attrs.item(j); - el3.setAttribute(attr.nodeName, attr.nodeValue); + el3.setAttributeNS(namespace, attr.nodeName, attr.nodeValue); } // el2.firstChild != null while (el2.firstChild) { @@ -3950,7 +3939,7 @@ if (typeof console !== 'undefined') { (minY * scaleY + heightDiff) + ') '; parsedDim.viewboxTransform = fabric.parseTransformAttribute(matrix); if (element.nodeName === 'svg') { - el = element.ownerDocument.createElement('g'); + el = element.ownerDocument.createElementNS(fabric.svgNS, 'g'); // element.firstChild != null while (element.firstChild) { el.appendChild(element.firstChild); @@ -4175,13 +4164,21 @@ if (typeof console !== 'undefined') { }, { }); // add values parsed from style, which take precedence over attributes // (see: http://www.w3.org/TR/SVG/styling.html#UsingPresentationAttributes) - ownAttributes = extend(ownAttributes, - extend(getGlobalStylesForElement(element, svgUid), fabric.parseStyleAttribute(element))); - + var cssAttrs = extend( + getGlobalStylesForElement(element, svgUid), + fabric.parseStyleAttribute(element) + ); + ownAttributes = extend( + ownAttributes, + cssAttrs + ); + if (cssAttrs[cPath]) { + element.setAttribute(cPath, cssAttrs[cPath]); + } fontSize = parentFontSize = parentAttributes.fontSize || fabric.Text.DEFAULT_SVG_FONT_SIZE; - if (ownAttributes['font-size']) { + if (ownAttributes[fSize]) { // looks like the minimum should be 9px when dealing with ems. this is what looks like in browsers. - ownAttributes['font-size'] = fontSize = parseUnit(ownAttributes['font-size'], parentFontSize); + ownAttributes[fSize] = fontSize = parseUnit(ownAttributes[fSize], parentFontSize); } var normalizedAttr, normalizedValue, normalizedStyle = {}; @@ -4397,7 +4394,7 @@ if (typeof console !== 'undefined') { })(typeof exports !== 'undefined' ? exports : this); -fabric.ElementsParser = function(elements, callback, options, reviver, parsingOptions) { +fabric.ElementsParser = function(elements, callback, options, reviver, parsingOptions, doc) { this.elements = elements; this.callback = callback; this.options = options; @@ -4405,6 +4402,7 @@ fabric.ElementsParser = function(elements, callback, options, reviver, parsingOp this.svgUid = (options && options.svgUid) || 0; this.parsingOptions = parsingOptions; this.regexUrl = /^url\(['"]?#([^'"]+)['"]?\)/g; + this.doc = doc; }; (function(proto) { @@ -4451,7 +4449,7 @@ fabric.ElementsParser = function(elements, callback, options, reviver, parsingOp _options = obj.parsePreserveAspectRatioAttribute(el); } obj._removeTransformMatrix(_options); - _this.resolveClipPath(obj); + _this.resolveClipPath(obj, el); _this.reviver && _this.reviver(el, obj); _this.instances[index] = obj; _this.checkIfDone(); @@ -4459,12 +4457,13 @@ fabric.ElementsParser = function(elements, callback, options, reviver, parsingOp }; proto.extractPropertyDefinition = function(obj, property, storage) { - var value = obj[property]; - if (!(/^url\(/).test(value)) { + var value = obj[property], regex = this.regexUrl; + if (!regex.test(value)) { return; } - var id = this.regexUrl.exec(value)[1]; - this.regexUrl.lastIndex = 0; + regex.lastIndex = 0; + var id = regex.exec(value)[1]; + regex.lastIndex = 0; return fabric[storage][this.svgUid][id]; }; @@ -4485,12 +4484,19 @@ fabric.ElementsParser = function(elements, callback, options, reviver, parsingOp }; }; - proto.resolveClipPath = function(obj) { + proto.resolveClipPath = function(obj, usingElement) { var clipPath = this.extractPropertyDefinition(obj, 'clipPath', 'clipPaths'), element, klass, objTransformInv, container, gTransform, options; if (clipPath) { container = []; objTransformInv = fabric.util.invertTransform(obj.calcTransformMatrix()); + // move the clipPath tag as sibling to the real element that is using it + var clipPathTag = clipPath[0].parentNode; + var clipPathOwner = usingElement; + while (clipPathOwner.parentNode && clipPathOwner.getAttribute('clip-path') !== obj.clipPath) { + clipPathOwner = clipPathOwner.parentNode; + } + clipPathOwner.parentNode.appendChild(clipPathTag); for (var i = 0; i < clipPath.length; i++) { element = clipPath[i]; klass = this.findTag(element); @@ -4511,7 +4517,7 @@ fabric.ElementsParser = function(elements, callback, options, reviver, parsingOp clipPath.calcTransformMatrix() ); if (clipPath.clipPath) { - this.resolveClipPath(clipPath); + this.resolveClipPath(clipPath, clipPathOwner); } var options = fabric.util.qrDecompose(gTransform); clipPath.flipX = false; @@ -9651,6 +9657,20 @@ fabric.PatternBrush = fabric.util.createClass(fabric.PencilBrush, /** @lends fab */ fireMiddleClick: false, + /** + * Keep track of the hovered target + * @type fabric.Object + * @private + */ + _hoveredTarget: null, + + /** + * hold the list of nested targets hovered + * @type fabric.Object + * @private + */ + _hoveredTargets: [], + /** * @private */ @@ -11866,12 +11886,24 @@ fabric.PatternBrush = fabric.util.createClass(fabric.PencilBrush, /** @lends fab */ _fireOverOutEvents: function(target, e) { this.fireSyntheticInOutEvents(target, e, { - targetName: '_hoveredTarget', + oldTarget: this._hoveredTarget, canvasEvtOut: 'mouse:out', evtOut: 'mouseout', canvasEvtIn: 'mouse:over', evtIn: 'mouseover', }); + this.targets.forEach(function(_target, index) { + this.fireSyntheticInOutEvents(_target, e, { + oldTarget: this._hoveredTargets[index], + canvasEvtOut: 'mouse:out', + evtOut: 'mouseout', + canvasEvtIn: 'mouse:over', + evtIn: 'mouseover', + }); + }); + // whatever it happened, the target is not the hovered target. + this._hoveredTarget = target; + this._hoveredTargets = this.targets.concat(); }, /** @@ -11901,12 +11933,11 @@ fabric.PatternBrush = fabric.util.createClass(fabric.PencilBrush, /** @lends fab * @private */ fireSyntheticInOutEvents: function(target, e, config) { - var inOpt, outOpt, oldTarget = this[config.targetName], outFires, inFires, + var inOpt, outOpt, oldTarget = config.oldTarget, outFires, inFires, targetChanged = oldTarget !== target, canvasEvtIn = config.canvasEvtIn, canvasEvtOut = config.canvasEvtOut; if (targetChanged) { inOpt = { e: e, target: target, previousTarget: oldTarget }; outOpt = { e: e, target: oldTarget, nextTarget: target }; - this[config.targetName] = target; } inFires = target && targetChanged; outFires = oldTarget && targetChanged; @@ -14027,7 +14058,7 @@ fabric.util.object.extend(fabric.StaticCanvas.prototype, /** @lends fabric.Stati * @param {Function} alternative function to call if browser does not support lineDash */ _setLineDash: function(ctx, dashArray, alternative) { - if (!dashArray) { + if (!dashArray || dashArray.length === 0) { return; } // Spec requires the concatenation of two copies the dash list when the number of elements is odd @@ -18703,7 +18734,7 @@ fabric.util.object.extend(fabric.Object.prototype, /** @lends fabric.Object.prot coords = [], currentPath, parsed, - re = /([-+]?((\d+\.\d+)|((\d+)|(\.\d+)))(?:e[-+]?\d+)?)/ig, + re = fabric.rePathCommand, match, coordsStr; @@ -27276,15 +27307,17 @@ fabric.Image.filters.BaseFilter.fromObject = function(object, callback) { * @return {Number} Index of the beginning or end of a word */ searchWordBoundary: function(selectionStart, direction) { - var index = this._reSpace.test(this._text[selectionStart]) ? selectionStart - 1 : selectionStart, - _char = this._text[index], - reNonWord = /[ \n\.,;!\?\-]/; + var text = this._text, + index = this._reSpace.test(text[selectionStart]) ? selectionStart - 1 : selectionStart, + _char = text[index], + // wrong + reNonWord = fabric.reNonWord; - while (!reNonWord.test(_char) && index > 0 && index < this._text.length) { + while (!reNonWord.test(_char) && index > 0 && index < text.length) { index += direction; - _char = this._text[index]; + _char = text[index]; } - if (reNonWord.test(_char) && _char !== '\n') { + if (reNonWord.test(_char)) { index += direction === 1 ? 0 : 1; } return index; @@ -27981,16 +28014,32 @@ fabric.util.object.extend(fabric.IText.prototype, /** @lends fabric.IText.protot this.initClicks(); }, + /** + * Default handler for double click, select a word + */ + doubleClickHandler: function(options) { + if (!this.isEditing) { + return; + } + this.selectWord(this.getSelectionStartFromPointer(options.e)); + }, + + /** + * Default handler for triple click, select a line + */ + tripleClickHandler: function(options) { + if (!this.isEditing) { + return; + } + this.selectLine(this.getSelectionStartFromPointer(options.e)); + }, + /** * Initializes double and triple click event handlers */ initClicks: function() { - this.on('mousedblclick', function(options) { - this.selectWord(this.getSelectionStartFromPointer(options.e)); - }); - this.on('tripleclick', function(options) { - this.selectLine(this.getSelectionStartFromPointer(options.e)); - }); + this.on('mousedblclick', this.doubleClickHandler); + this.on('tripleclick', this.tripleClickHandler); }, /** @@ -28028,9 +28077,9 @@ fabric.util.object.extend(fabric.IText.prototype, /** @lends fabric.IText.protot if (!this.canvas || !this.editable || (options.e.button && options.e.button !== 1)) { return; } - if (this === this.canvas._activeObject) { - this.selected = true; - } + // we want to avoid that an object that was selected and then becomes unselectable, + // may trigger editing mode in some way. + this.selected = this === this.canvas._activeObject; }, /** diff --git a/src/canvas.class.js b/src/canvas.class.js index 94268c72cfa..a5701d79c5a 100644 --- a/src/canvas.class.js +++ b/src/canvas.class.js @@ -330,6 +330,20 @@ */ fireMiddleClick: false, + /** + * Keep track of the hovered target + * @type fabric.Object + * @private + */ + _hoveredTarget: null, + + /** + * hold the list of nested targets hovered + * @type fabric.Object + * @private + */ + _hoveredTargets: [], + /** * @private */ diff --git a/src/mixins/canvas_events.mixin.js b/src/mixins/canvas_events.mixin.js index c88ad098deb..8b92404ca29 100644 --- a/src/mixins/canvas_events.mixin.js +++ b/src/mixins/canvas_events.mixin.js @@ -819,12 +819,24 @@ */ _fireOverOutEvents: function(target, e) { this.fireSyntheticInOutEvents(target, e, { - targetName: '_hoveredTarget', + oldTarget: this._hoveredTarget, canvasEvtOut: 'mouse:out', evtOut: 'mouseout', canvasEvtIn: 'mouse:over', evtIn: 'mouseover', }); + this.targets.forEach(function(_target, index) { + this.fireSyntheticInOutEvents(_target, e, { + oldTarget: this._hoveredTargets[index], + canvasEvtOut: 'mouse:out', + evtOut: 'mouseout', + canvasEvtIn: 'mouse:over', + evtIn: 'mouseover', + }); + }); + // whatever it happened, the target is not the hovered target. + this._hoveredTarget = target; + this._hoveredTargets = this.targets.concat(); }, /** @@ -854,12 +866,11 @@ * @private */ fireSyntheticInOutEvents: function(target, e, config) { - var inOpt, outOpt, oldTarget = this[config.targetName], outFires, inFires, + var inOpt, outOpt, oldTarget = config.oldTarget, outFires, inFires, targetChanged = oldTarget !== target, canvasEvtIn = config.canvasEvtIn, canvasEvtOut = config.canvasEvtOut; if (targetChanged) { inOpt = { e: e, target: target, previousTarget: oldTarget }; outOpt = { e: e, target: oldTarget, nextTarget: target }; - this[config.targetName] = target; } inFires = target && targetChanged; outFires = oldTarget && targetChanged; From bc48c2d39a730feb023793861584884a6c7624d1 Mon Sep 17 00:00:00 2001 From: Andrea Bogazzi Date: Sat, 14 Dec 2019 23:43:35 +0100 Subject: [PATCH 06/23] this is an idea --- dist/fabric.js | 161 +++++++++++++++++-------------------------------- 1 file changed, 56 insertions(+), 105 deletions(-) diff --git a/dist/fabric.js b/dist/fabric.js index 88da6fe5a19..e7c505b2a4d 100644 --- a/dist/fabric.js +++ b/dist/fabric.js @@ -74,11 +74,8 @@ fabric.SHARED_ATTRIBUTES = [ */ fabric.DPI = 96; fabric.reNum = '(?:[-+]?(?:\\d+|\\d*\\.\\d+)(?:[eE][-+]?\\d+)?)'; -fabric.rePathCommand = /([-+]?((\d+\.\d+)|((\d+)|(\.\d+)))(?:[eE][-+]?\d+)?)/ig; -fabric.reNonWord = /[ \n\.,;!\?\-]/; fabric.fontPaths = { }; fabric.iMatrix = [1, 0, 0, 1, 0, 0]; -fabric.svgNS = 'http://www.w3.org/2000/svg'; /** * Pixel limit for cache canvases. 1Mpx , 4Mpx should be fine. @@ -2754,13 +2751,29 @@ fabric.CommonMethods = { * Wrapper around `console.log` (when available) * @param {*} [values] Values to log */ -fabric.log = console.log; +fabric.log = function() { }; /** * Wrapper around `console.warn` (when available) * @param {*} [values] Values to log as a warning */ -fabric.warn = console.warn; +fabric.warn = function() { }; + +/* eslint-disable */ +if (typeof console !== 'undefined') { + + ['log', 'warn'].forEach(function(methodName) { + + if (typeof console[methodName] !== 'undefined' && + typeof console[methodName].apply === 'function') { + + fabric[methodName] = function() { + return console[methodName].apply(console, arguments); + }; + } + }); +} +/* eslint-enable */ (function() { @@ -3370,9 +3383,7 @@ fabric.warn = console.warn; colorAttributes = { stroke: 'strokeOpacity', fill: 'fillOpacity' - }, - - fSize = 'font-size', cPath = 'clip-path'; + }; fabric.svgValidTagNamesRegEx = getSvgRegex(svgValidTagNames); fabric.svgViewBoxElementsRegEx = getSvgRegex(svgViewBoxElements); @@ -3787,14 +3798,14 @@ fabric.warn = console.warn; y = el.getAttribute('y') || 0, el2 = elementById(doc, xlink).cloneNode(true), currentTrans = (el2.getAttribute('transform') || '') + ' translate(' + x + ', ' + y + ')', - parentNode, oldLength = nodelist.length, attr, j, attrs, len, namespace = fabric.svgNS; + parentNode, oldLength = nodelist.length, attr, j, attrs, len; applyViewboxTransform(el2); if (/^svg$/i.test(el2.nodeName)) { - var el3 = el2.ownerDocument.createElementNS(namespace, 'g'); + var el3 = el2.ownerDocument.createElement('g'); for (j = 0, attrs = el2.attributes, len = attrs.length; j < len; j++) { attr = attrs.item(j); - el3.setAttributeNS(namespace, attr.nodeName, attr.nodeValue); + el3.setAttribute(attr.nodeName, attr.nodeValue); } // el2.firstChild != null while (el2.firstChild) { @@ -3939,7 +3950,7 @@ fabric.warn = console.warn; (minY * scaleY + heightDiff) + ') '; parsedDim.viewboxTransform = fabric.parseTransformAttribute(matrix); if (element.nodeName === 'svg') { - el = element.ownerDocument.createElementNS(fabric.svgNS, 'g'); + el = element.ownerDocument.createElement('g'); // element.firstChild != null while (element.firstChild) { el.appendChild(element.firstChild); @@ -4164,21 +4175,13 @@ fabric.warn = console.warn; }, { }); // add values parsed from style, which take precedence over attributes // (see: http://www.w3.org/TR/SVG/styling.html#UsingPresentationAttributes) - var cssAttrs = extend( - getGlobalStylesForElement(element, svgUid), - fabric.parseStyleAttribute(element) - ); - ownAttributes = extend( - ownAttributes, - cssAttrs - ); - if (cssAttrs[cPath]) { - element.setAttribute(cPath, cssAttrs[cPath]); - } + ownAttributes = extend(ownAttributes, + extend(getGlobalStylesForElement(element, svgUid), fabric.parseStyleAttribute(element))); + fontSize = parentFontSize = parentAttributes.fontSize || fabric.Text.DEFAULT_SVG_FONT_SIZE; - if (ownAttributes[fSize]) { + if (ownAttributes['font-size']) { // looks like the minimum should be 9px when dealing with ems. this is what looks like in browsers. - ownAttributes[fSize] = fontSize = parseUnit(ownAttributes[fSize], parentFontSize); + ownAttributes['font-size'] = fontSize = parseUnit(ownAttributes['font-size'], parentFontSize); } var normalizedAttr, normalizedValue, normalizedStyle = {}; @@ -4394,7 +4397,7 @@ fabric.warn = console.warn; })(typeof exports !== 'undefined' ? exports : this); -fabric.ElementsParser = function(elements, callback, options, reviver, parsingOptions, doc) { +fabric.ElementsParser = function(elements, callback, options, reviver, parsingOptions) { this.elements = elements; this.callback = callback; this.options = options; @@ -4402,7 +4405,6 @@ fabric.ElementsParser = function(elements, callback, options, reviver, parsingOp this.svgUid = (options && options.svgUid) || 0; this.parsingOptions = parsingOptions; this.regexUrl = /^url\(['"]?#([^'"]+)['"]?\)/g; - this.doc = doc; }; (function(proto) { @@ -4449,7 +4451,7 @@ fabric.ElementsParser = function(elements, callback, options, reviver, parsingOp _options = obj.parsePreserveAspectRatioAttribute(el); } obj._removeTransformMatrix(_options); - _this.resolveClipPath(obj, el); + _this.resolveClipPath(obj); _this.reviver && _this.reviver(el, obj); _this.instances[index] = obj; _this.checkIfDone(); @@ -4457,13 +4459,12 @@ fabric.ElementsParser = function(elements, callback, options, reviver, parsingOp }; proto.extractPropertyDefinition = function(obj, property, storage) { - var value = obj[property], regex = this.regexUrl; - if (!regex.test(value)) { + var value = obj[property]; + if (!(/^url\(/).test(value)) { return; } - regex.lastIndex = 0; - var id = regex.exec(value)[1]; - regex.lastIndex = 0; + var id = this.regexUrl.exec(value)[1]; + this.regexUrl.lastIndex = 0; return fabric[storage][this.svgUid][id]; }; @@ -4484,19 +4485,12 @@ fabric.ElementsParser = function(elements, callback, options, reviver, parsingOp }; }; - proto.resolveClipPath = function(obj, usingElement) { + proto.resolveClipPath = function(obj) { var clipPath = this.extractPropertyDefinition(obj, 'clipPath', 'clipPaths'), element, klass, objTransformInv, container, gTransform, options; if (clipPath) { container = []; objTransformInv = fabric.util.invertTransform(obj.calcTransformMatrix()); - // move the clipPath tag as sibling to the real element that is using it - var clipPathTag = clipPath[0].parentNode; - var clipPathOwner = usingElement; - while (clipPathOwner.parentNode && clipPathOwner.getAttribute('clip-path') !== obj.clipPath) { - clipPathOwner = clipPathOwner.parentNode; - } - clipPathOwner.parentNode.appendChild(clipPathTag); for (var i = 0; i < clipPath.length; i++) { element = clipPath[i]; klass = this.findTag(element); @@ -4517,7 +4511,7 @@ fabric.ElementsParser = function(elements, callback, options, reviver, parsingOp clipPath.calcTransformMatrix() ); if (clipPath.clipPath) { - this.resolveClipPath(clipPath, clipPathOwner); + this.resolveClipPath(clipPath); } var options = fabric.util.qrDecompose(gTransform); clipPath.flipX = false; @@ -9657,20 +9651,6 @@ fabric.PatternBrush = fabric.util.createClass(fabric.PencilBrush, /** @lends fab */ fireMiddleClick: false, - /** - * Keep track of the hovered target - * @type fabric.Object - * @private - */ - _hoveredTarget: null, - - /** - * hold the list of nested targets hovered - * @type fabric.Object - * @private - */ - _hoveredTargets: [], - /** * @private */ @@ -11886,24 +11866,12 @@ fabric.PatternBrush = fabric.util.createClass(fabric.PencilBrush, /** @lends fab */ _fireOverOutEvents: function(target, e) { this.fireSyntheticInOutEvents(target, e, { - oldTarget: this._hoveredTarget, + targetName: '_hoveredTarget', canvasEvtOut: 'mouse:out', evtOut: 'mouseout', canvasEvtIn: 'mouse:over', evtIn: 'mouseover', }); - this.targets.forEach(function(_target, index) { - this.fireSyntheticInOutEvents(_target, e, { - oldTarget: this._hoveredTargets[index], - canvasEvtOut: 'mouse:out', - evtOut: 'mouseout', - canvasEvtIn: 'mouse:over', - evtIn: 'mouseover', - }); - }); - // whatever it happened, the target is not the hovered target. - this._hoveredTarget = target; - this._hoveredTargets = this.targets.concat(); }, /** @@ -11933,11 +11901,12 @@ fabric.PatternBrush = fabric.util.createClass(fabric.PencilBrush, /** @lends fab * @private */ fireSyntheticInOutEvents: function(target, e, config) { - var inOpt, outOpt, oldTarget = config.oldTarget, outFires, inFires, + var inOpt, outOpt, oldTarget = this[config.targetName], outFires, inFires, targetChanged = oldTarget !== target, canvasEvtIn = config.canvasEvtIn, canvasEvtOut = config.canvasEvtOut; if (targetChanged) { inOpt = { e: e, target: target, previousTarget: oldTarget }; outOpt = { e: e, target: oldTarget, nextTarget: target }; + this[config.targetName] = target; } inFires = target && targetChanged; outFires = oldTarget && targetChanged; @@ -14058,7 +14027,7 @@ fabric.util.object.extend(fabric.StaticCanvas.prototype, /** @lends fabric.Stati * @param {Function} alternative function to call if browser does not support lineDash */ _setLineDash: function(ctx, dashArray, alternative) { - if (!dashArray || dashArray.length === 0) { + if (!dashArray) { return; } // Spec requires the concatenation of two copies the dash list when the number of elements is odd @@ -18734,7 +18703,7 @@ fabric.util.object.extend(fabric.Object.prototype, /** @lends fabric.Object.prot coords = [], currentPath, parsed, - re = fabric.rePathCommand, + re = /([-+]?((\d+\.\d+)|((\d+)|(\.\d+)))(?:e[-+]?\d+)?)/ig, match, coordsStr; @@ -27307,17 +27276,15 @@ fabric.Image.filters.BaseFilter.fromObject = function(object, callback) { * @return {Number} Index of the beginning or end of a word */ searchWordBoundary: function(selectionStart, direction) { - var text = this._text, - index = this._reSpace.test(text[selectionStart]) ? selectionStart - 1 : selectionStart, - _char = text[index], - // wrong - reNonWord = fabric.reNonWord; + var index = this._reSpace.test(this._text[selectionStart]) ? selectionStart - 1 : selectionStart, + _char = this._text[index], + reNonWord = /[ \n\.,;!\?\-]/; - while (!reNonWord.test(_char) && index > 0 && index < text.length) { + while (!reNonWord.test(_char) && index > 0 && index < this._text.length) { index += direction; - _char = text[index]; + _char = this._text[index]; } - if (reNonWord.test(_char)) { + if (reNonWord.test(_char) && _char !== '\n') { index += direction === 1 ? 0 : 1; } return index; @@ -28014,32 +27981,16 @@ fabric.util.object.extend(fabric.IText.prototype, /** @lends fabric.IText.protot this.initClicks(); }, - /** - * Default handler for double click, select a word - */ - doubleClickHandler: function(options) { - if (!this.isEditing) { - return; - } - this.selectWord(this.getSelectionStartFromPointer(options.e)); - }, - - /** - * Default handler for triple click, select a line - */ - tripleClickHandler: function(options) { - if (!this.isEditing) { - return; - } - this.selectLine(this.getSelectionStartFromPointer(options.e)); - }, - /** * Initializes double and triple click event handlers */ initClicks: function() { - this.on('mousedblclick', this.doubleClickHandler); - this.on('tripleclick', this.tripleClickHandler); + this.on('mousedblclick', function(options) { + this.selectWord(this.getSelectionStartFromPointer(options.e)); + }); + this.on('tripleclick', function(options) { + this.selectLine(this.getSelectionStartFromPointer(options.e)); + }); }, /** @@ -28077,9 +28028,9 @@ fabric.util.object.extend(fabric.IText.prototype, /** @lends fabric.IText.protot if (!this.canvas || !this.editable || (options.e.button && options.e.button !== 1)) { return; } - // we want to avoid that an object that was selected and then becomes unselectable, - // may trigger editing mode in some way. - this.selected = this === this.canvas._activeObject; + if (this === this.canvas._activeObject) { + this.selected = true; + } }, /** From f3bb7edc711e22e943dab5139055fac7a94094d9 Mon Sep 17 00:00:00 2001 From: jake downs Date: Sat, 14 Dec 2019 19:11:59 -0800 Subject: [PATCH 07/23] adjustments to PR #6032 + added a _this var for targets.forEach loop closure + changed targetName to oldTarget in _fireEnterLeaveEvents config object + added additional loop to call `fireSyntheticInOutEvents` when `this._hoveredTargets.length` is larger than `this.targets.length` --- src/mixins/canvas_events.mixin.js | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/mixins/canvas_events.mixin.js b/src/mixins/canvas_events.mixin.js index 8b92404ca29..b39d4ab47aa 100644 --- a/src/mixins/canvas_events.mixin.js +++ b/src/mixins/canvas_events.mixin.js @@ -818,6 +818,7 @@ * @private */ _fireOverOutEvents: function(target, e) { + var _this = this; this.fireSyntheticInOutEvents(target, e, { oldTarget: this._hoveredTarget, canvasEvtOut: 'mouse:out', @@ -826,14 +827,27 @@ evtIn: 'mouseover', }); this.targets.forEach(function(_target, index) { - this.fireSyntheticInOutEvents(_target, e, { - oldTarget: this._hoveredTargets[index], + _this.fireSyntheticInOutEvents(_target, e, { + oldTarget: _this._hoveredTargets[index], canvasEvtOut: 'mouse:out', evtOut: 'mouseout', canvasEvtIn: 'mouse:over', evtIn: 'mouseover', }); }); + var remainingTargetsLength = this._hoveredTargets.length - this.targets.length; + if (remainingTargetsLength > 0){ + for (var i = 0; i < remainingTargetsLength; i++){ + var _remainingTargetIndex = i + this.targets.length - 1; + this.fireSyntheticInOutEvents(null, e, { + oldTarget: this._hoveredTargets[_remainingTargetIndex], + canvasEvtOut: 'mouse:out', + evtOut: 'mouseout', + canvasEvtIn: 'mouse:over', + evtIn: 'mouseover', + }); + } + } // whatever it happened, the target is not the hovered target. this._hoveredTarget = target; this._hoveredTargets = this.targets.concat(); @@ -847,7 +861,7 @@ */ _fireEnterLeaveEvents: function(target, e) { this.fireSyntheticInOutEvents(target, e, { - targetName: '_draggedoverTarget', + oldTarget: '_draggedoverTarget', evtOut: 'dragleave', evtIn: 'dragenter', }); From 4ab6f2a482966bc9f993e7b81c269b99290469d9 Mon Sep 17 00:00:00 2001 From: jake downs Date: Sat, 14 Dec 2019 19:46:39 -0800 Subject: [PATCH 08/23] adjust additional _hoveredTarget management + reference new ._hoveredTargets array instead of Canvas._hoveredTargetN properties --- src/canvas.class.js | 20 ++++--------------- src/mixins/canvas_events.mixin.js | 30 +++++++---------------------- src/mixins/canvas_grouping.mixin.js | 25 ++---------------------- 3 files changed, 13 insertions(+), 62 deletions(-) diff --git a/src/canvas.class.js b/src/canvas.class.js index 339a7df2a97..de7a2733000 100644 --- a/src/canvas.class.js +++ b/src/canvas.class.js @@ -339,7 +339,7 @@ /** * hold the list of nested targets hovered - * @type fabric.Object + * @type Array[fabric.Object] * @private */ _hoveredTargets: [], @@ -1491,21 +1491,9 @@ this.fire('selection:cleared', { target: obj }); obj.fire('deselected'); } - // TODO: need a way to cleanly check if obj is one of _hoveredTargetN and de-ref it - // if (obj === this._hoveredTarget){ - // this._hoveredTarget = null; - // } - // is there a more sane way than looping through ALL properties of *this* ? - // i wanted to put them into a ._hoveredTargets[Array] - // but, that complicates the logic of fireSyntheticInOutEvents - var keys = Object.keys(this); - for (var i = 0; i < keys.length; i++){ - var key = keys[i]; - if (key.indexOf('_hoveredTarget') > -1){ - if (obj === this[key]){ - this[key] = null; - } - } + if (obj === this._hoveredTarget){ + this._hoveredTarget = null; + this._hoveredTargets = []; } this.callSuper('_onObjectRemoved', obj); }, diff --git a/src/mixins/canvas_events.mixin.js b/src/mixins/canvas_events.mixin.js index 8beff6a0c8e..f84a4694202 100644 --- a/src/mixins/canvas_events.mixin.js +++ b/src/mixins/canvas_events.mixin.js @@ -168,23 +168,15 @@ _onMouseOut: function(e) { console.log('_onMouseOut',e); - // pre-ISSUE-4115 var target = this._hoveredTarget; this.fire('mouse:out', { target: target, e: e }); target && target.fire('mouseout', { e: e }); - // post-ISSUE-4115 - // should we really be firing mouseOut on ALL _hoveredTargets? - // maybe just the top-level one? I dunno... - var keys = Object.keys(this); - for (var i = 0; i < keys.length; i++){ - var key = keys[i]; - if (key.indexOf('_hoveredTarget') > -1){ - var target = this[key]; - this.fire('mouse:out', { target: target, e: e }); - target && target.fire('mouseout', { e: e }); - } - } + var _this = this; + this._hoveredTargets.forEach(function(_target){ + _this.fire('mouse:out', { target: target, e: e }); + _target && target.fire('mouseout', { e: e }); + }); if (this._iTextInstances) { this._iTextInstances.forEach(function(obj) { @@ -208,16 +200,8 @@ // side effects we added to it. if (!this.currentTransform && !this.findTarget(e)) { this.fire('mouse:over', { target: null, e: e }); - // PRE-ISSUE-4115 - // this._hoveredTarget = null; - // POST-ISSUE-4115 - var keys = Object.keys(this); - for (var i = 0; i < keys.length; i++){ - var key = keys[i]; - if (key.indexOf('_hoveredTarget') > -1){ - this[key] = null; - } - } + this._hoveredTarget = null; + this._hoveredTargets = []; } }, diff --git a/src/mixins/canvas_grouping.mixin.js b/src/mixins/canvas_grouping.mixin.js index 7ff047e2166..73dc5ae7d02 100644 --- a/src/mixins/canvas_grouping.mixin.js +++ b/src/mixins/canvas_grouping.mixin.js @@ -53,20 +53,7 @@ if (activeSelection.contains(target)) { activeSelection.removeWithUpdate(target); this._hoveredTarget = target; - // ISSUE-4115: clear out any additional hovered targets that were set? - // should we fire mouse:out on those? - var keys = Object.keys(this); - for (var i = 0; i < keys.length; i++){ - var key = keys[i]; - if (key.indexOf('_hoveredTarget') > -1){ - this[key] = null; - } - } - // ISSUE-4115: loop through this.targets and assign them as hovered as well? - // why don't we fire mouse:over here? - for (var i = 0; i < this.targets.length; i++){ - this['_hoveredTarget' + i] = this.targets[i]; - } + this._hoveredTargets = this.targets; if (activeSelection.size() === 1) { // activate last remaining object this._setActiveObject(activeSelection.item(0), e); @@ -75,15 +62,7 @@ else { activeSelection.addWithUpdate(target); this._hoveredTarget = activeSelection; - // ISSUE-4115: clear out any additional hovered targets that were set? - // should we fire mouse:out on those? - var keys = Object.keys(this); - for (var i = 0; i < keys.length; i++){ - var key = keys[i]; - if (key.indexOf('_hoveredTarget') > -1){ - this[key] = null; - } - } + this._hoveredTargets = this.targets; } this._fireSelectionEvents(currentActiveObjects, e); }, From f24f61fbb5426b1cb05074b632b78c36104f1d91 Mon Sep 17 00:00:00 2001 From: jake downs Date: Sat, 14 Dec 2019 19:47:27 -0800 Subject: [PATCH 09/23] move _this to where it's needed --- src/mixins/canvas_events.mixin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mixins/canvas_events.mixin.js b/src/mixins/canvas_events.mixin.js index f84a4694202..d0533d734e1 100644 --- a/src/mixins/canvas_events.mixin.js +++ b/src/mixins/canvas_events.mixin.js @@ -835,7 +835,6 @@ * @private */ _fireOverOutEvents: function(target, e) { - var _this = this; this.fireSyntheticInOutEvents(target, e, { oldTarget: this._hoveredTarget, canvasEvtOut: 'mouse:out', @@ -843,6 +842,7 @@ canvasEvtIn: 'mouse:over', evtIn: 'mouseover', }); + var _this = this; this.targets.forEach(function(_target, index) { _this.fireSyntheticInOutEvents(_target, e, { oldTarget: _this._hoveredTargets[index], From bd6590d8e73af26e603459d48a140223d9877fc1 Mon Sep 17 00:00:00 2001 From: jake downs Date: Sat, 14 Dec 2019 20:31:54 -0800 Subject: [PATCH 10/23] more cleanup of lingering stuff from earlier commits --- src/mixins/canvas_events.mixin.js | 13 +++++-------- src/mixins/canvas_grouping.mixin.js | 6 ++++-- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/mixins/canvas_events.mixin.js b/src/mixins/canvas_events.mixin.js index d0533d734e1..e83ea7e5d70 100644 --- a/src/mixins/canvas_events.mixin.js +++ b/src/mixins/canvas_events.mixin.js @@ -170,6 +170,7 @@ var target = this._hoveredTarget; this.fire('mouse:out', { target: target, e: e }); + this._hoveredTarget = null; target && target.fire('mouseout', { e: e }); var _this = this; @@ -177,6 +178,7 @@ _this.fire('mouse:out', { target: target, e: e }); _target && target.fire('mouseout', { e: e }); }); + this._hoveredTargets = []; if (this._iTextInstances) { this._iTextInstances.forEach(function(obj) { @@ -812,14 +814,6 @@ target = this.findTarget(e) || null; this._setCursorFromEvent(e, target); this._fireOverOutEvents(target, e); - // handle triggering on SubTargets - this.targets.map(function(subTarget,k){ - _this._fireOverOutEvents(subTarget, e, '_hoveredTarget' + k); - }); - // hoverCursor should come from top-most subtarget - this.targets.slice(0).reverse().map(function(subTarget){ - _this._setCursorFromEvent(e, subTarget); - }); } else { this._transformObject(e); @@ -886,6 +880,7 @@ /** * Manage the synthetic in/out events for the fabric objects on the canvas + * @param {Fabric.Object} target the target where the target from the supported events * @param {Event} e Event object fired * @param {Object} config configuration for the function to work * @param {String} config.targetName property on the canvas where the old target is stored @@ -902,6 +897,8 @@ inOpt = { e: e, target: target, previousTarget: oldTarget }; outOpt = { e: e, target: oldTarget, nextTarget: target }; } + inFires = target && targetChanged; + outFires = oldTarget && targetChanged; if (outFires) { canvasEvtOut && this.fire(canvasEvtOut, outOpt); oldTarget.fire(config.evtOut, outOpt); diff --git a/src/mixins/canvas_grouping.mixin.js b/src/mixins/canvas_grouping.mixin.js index 73dc5ae7d02..a639f8f66f4 100644 --- a/src/mixins/canvas_grouping.mixin.js +++ b/src/mixins/canvas_grouping.mixin.js @@ -53,7 +53,7 @@ if (activeSelection.contains(target)) { activeSelection.removeWithUpdate(target); this._hoveredTarget = target; - this._hoveredTargets = this.targets; + this._hoveredTargets = this.targets.concat(); if (activeSelection.size() === 1) { // activate last remaining object this._setActiveObject(activeSelection.item(0), e); @@ -62,7 +62,7 @@ else { activeSelection.addWithUpdate(target); this._hoveredTarget = activeSelection; - this._hoveredTargets = this.targets; + this._hoveredTargets = this.targets.concat(); } this._fireSelectionEvents(currentActiveObjects, e); }, @@ -74,6 +74,8 @@ var currentActives = this.getActiveObjects(), group = this._createGroup(target); this._hoveredTarget = group; // ISSUE 4115: should we consider subTargets here? + // this._hoveredTargets = []; + // this._hoveredTargets = this.targets.concat(); this._setActiveObject(group, e); this._fireSelectionEvents(currentActives, e); }, From 7d40b4d68cbedef440869779d41f9a1b41298f92 Mon Sep 17 00:00:00 2001 From: jake downs Date: Sat, 14 Dec 2019 20:33:05 -0800 Subject: [PATCH 11/23] fix linter error --- src/mixins/canvas_events.mixin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mixins/canvas_events.mixin.js b/src/mixins/canvas_events.mixin.js index e83ea7e5d70..b5eff3f006c 100644 --- a/src/mixins/canvas_events.mixin.js +++ b/src/mixins/canvas_events.mixin.js @@ -788,7 +788,7 @@ __onMouseMove: function (e) { this._handleEvent(e, 'move:before'); this._cacheTransformEventData(e); - var target, pointer, _this = this; + var target, pointer; if (this.isDrawingMode) { this._onMouseMoveInDrawingMode(e); From b4251d0f516f66750959ff1399dd474bf907b1af Mon Sep 17 00:00:00 2001 From: jake downs Date: Sat, 14 Dec 2019 20:56:46 -0800 Subject: [PATCH 12/23] targets: default to empty array null check before calling .fire on oldTarget/target --- src/canvas.class.js | 6 ++++++ src/mixins/canvas_events.mixin.js | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/canvas.class.js b/src/canvas.class.js index de7a2733000..ceed40f04da 100644 --- a/src/canvas.class.js +++ b/src/canvas.class.js @@ -330,6 +330,12 @@ */ fireMiddleClick: false, + /** + * Keep track of the subTargets for Mouse Events + * @type Array[fabric.Object] + */ + targets: [], + /** * Keep track of the hovered target * @type fabric.Object diff --git a/src/mixins/canvas_events.mixin.js b/src/mixins/canvas_events.mixin.js index b5eff3f006c..19583b0daa2 100644 --- a/src/mixins/canvas_events.mixin.js +++ b/src/mixins/canvas_events.mixin.js @@ -901,11 +901,11 @@ outFires = oldTarget && targetChanged; if (outFires) { canvasEvtOut && this.fire(canvasEvtOut, outOpt); - oldTarget.fire(config.evtOut, outOpt); + oldTarget && oldTarget.fire(config.evtOut, outOpt); } if (inFires) { canvasEvtIn && this.fire(canvasEvtIn, inOpt); - target.fire(config.evtIn, inOpt); + target && target.fire(config.evtIn, inOpt); } }, From 41978ddc964d87e70d8756f5f1896db6923a0c0c Mon Sep 17 00:00:00 2001 From: jake downs Date: Sun, 15 Dec 2019 12:19:08 -0800 Subject: [PATCH 13/23] correction to jsdoc array of fabric.Object type declaration --- src/canvas.class.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/canvas.class.js b/src/canvas.class.js index ceed40f04da..eb39c6cb068 100644 --- a/src/canvas.class.js +++ b/src/canvas.class.js @@ -332,7 +332,7 @@ /** * Keep track of the subTargets for Mouse Events - * @type Array[fabric.Object] + * @type fabric.Object[] */ targets: [], @@ -345,7 +345,7 @@ /** * hold the list of nested targets hovered - * @type Array[fabric.Object] + * @type fabric.Object[] * @private */ _hoveredTargets: [], From deb64f039a8f43aa526b2f6aa088a179f76cc454 Mon Sep 17 00:00:00 2001 From: jake downs Date: Sun, 15 Dec 2019 12:26:08 -0800 Subject: [PATCH 14/23] remove genuine unique id GUID getter from object class --- src/shapes/object.class.js | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/src/shapes/object.class.js b/src/shapes/object.class.js index 4f993ad0af2..13996116650 100644 --- a/src/shapes/object.class.js +++ b/src/shapes/object.class.js @@ -2115,24 +2115,4 @@ * @type Number */ fabric.Object.__uid = 0; - - /** - * A GUID for tracking hovered targets and sub targets - * simply tracking via subTargets[index] is not reliable - */ - function generateGUID() { - // https://stackoverflow.com/questions/8012002/create-a-unique-number-with-javascript-time - return new Date().valueOf().toString(36) + Math.random().toString(36).substr(2); - // return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, function(c) { - // var r = Math.random() * 16 | 0, v = c == 'x' ? r : (r & 0x3 | 0x8); - // return v.toString(16); - // }); - } - fabric.Object.prototype.__defineGetter__('__guid', function(){ - if (!this.__myGUID) { - this.__myGUID = generateGUID(); - } - return this.__myGUID; - }); - })(typeof exports !== 'undefined' ? exports : this); From 426e68cb407bced5b8205be211339fcdb7de3631 Mon Sep 17 00:00:00 2001 From: jake downs Date: Sun, 15 Dec 2019 12:26:44 -0800 Subject: [PATCH 15/23] simplify & optimize for minification --- src/mixins/canvas_events.mixin.js | 33 +++++++------------------------ 1 file changed, 7 insertions(+), 26 deletions(-) diff --git a/src/mixins/canvas_events.mixin.js b/src/mixins/canvas_events.mixin.js index 19583b0daa2..ec9e2272eb5 100644 --- a/src/mixins/canvas_events.mixin.js +++ b/src/mixins/canvas_events.mixin.js @@ -829,37 +829,18 @@ * @private */ _fireOverOutEvents: function(target, e) { - this.fireSyntheticInOutEvents(target, e, { - oldTarget: this._hoveredTarget, - canvasEvtOut: 'mouse:out', - evtOut: 'mouseout', - canvasEvtIn: 'mouse:over', - evtIn: 'mouseover', - }); - var _this = this; - this.targets.forEach(function(_target, index) { + var _this = this, _hoveredTarget = this._hoveredTarget, + _hoveredTargets = this._hoveredTargets, targets = this.targets, + diff = _hoveredTargets.length - targets.length; + [target].concat(targets, new Array(diff > 0 ? diff : 0)).forEach(function(_target, index) { _this.fireSyntheticInOutEvents(_target, e, { - oldTarget: _this._hoveredTargets[index], + oldTarget: index === 0 ? _hoveredTarget : _hoveredTargets[index - 1], canvasEvtOut: 'mouse:out', evtOut: 'mouseout', canvasEvtIn: 'mouse:over', evtIn: 'mouseover', }); }); - var remainingTargetsLength = this._hoveredTargets.length - this.targets.length; - if (remainingTargetsLength > 0){ - for (var i = 0; i < remainingTargetsLength; i++){ - var _remainingTargetIndex = i + this.targets.length - 1; - this.fireSyntheticInOutEvents(null, e, { - oldTarget: this._hoveredTargets[_remainingTargetIndex], - canvasEvtOut: 'mouse:out', - evtOut: 'mouseout', - canvasEvtIn: 'mouse:over', - evtIn: 'mouseover', - }); - } - } - // whatever it happened, the target is not the hovered target. this._hoveredTarget = target; this._hoveredTargets = this.targets.concat(); }, @@ -901,11 +882,11 @@ outFires = oldTarget && targetChanged; if (outFires) { canvasEvtOut && this.fire(canvasEvtOut, outOpt); - oldTarget && oldTarget.fire(config.evtOut, outOpt); + oldTarget.fire(config.evtOut, outOpt); } if (inFires) { canvasEvtIn && this.fire(canvasEvtIn, inOpt); - target && target.fire(config.evtIn, inOpt); + target.fire(config.evtIn, inOpt); } }, From 0cd41961d47a39153bb322bbcf725fc839ed77ac Mon Sep 17 00:00:00 2001 From: jake downs Date: Sun, 15 Dec 2019 12:33:37 -0800 Subject: [PATCH 16/23] updated _fireEnterLeaveEvents to include subTargets --- src/mixins/canvas_events.mixin.js | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/mixins/canvas_events.mixin.js b/src/mixins/canvas_events.mixin.js index ec9e2272eb5..f6c61d74a98 100644 --- a/src/mixins/canvas_events.mixin.js +++ b/src/mixins/canvas_events.mixin.js @@ -852,11 +852,17 @@ * @private */ _fireEnterLeaveEvents: function(target, e) { - this.fireSyntheticInOutEvents(target, e, { - oldTarget: '_draggedoverTarget', - evtOut: 'dragleave', - evtIn: 'dragenter', + var _this = this, _draggedoverTarget = this._draggedoverTarget, + _hoveredTargets = this._hoveredTargets, targets = this.targets, + diff = _hoveredTargets.length - targets.length; + [target].concat(targets, new Array(diff > 0 ? diff : 0)).forEach(function(_target, index) { + _this.fireSyntheticInOutEvents(_target, e, { + oldTarget: index === 0 ? _draggedoverTarget : _hoveredTargets[index - 1], + evtOut: 'dragleave', + evtIn: 'dragenter', + }); }); + this._draggedoverTarget = target; }, /** From 1909d1f49ee1c147475e6d71d68299126a33c5b0 Mon Sep 17 00:00:00 2001 From: jake downs Date: Sun, 15 Dec 2019 13:12:01 -0800 Subject: [PATCH 17/23] adjusted _setCursorFromEvent to account for subTargets --- src/mixins/canvas_events.mixin.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/mixins/canvas_events.mixin.js b/src/mixins/canvas_events.mixin.js index f6c61d74a98..41b8d88d5b4 100644 --- a/src/mixins/canvas_events.mixin.js +++ b/src/mixins/canvas_events.mixin.js @@ -1047,6 +1047,13 @@ && target._findTargetCorner(this.getPointer(e, true)); if (!corner) { + if (target.subTargetCheck){ + // hoverCursor should come from top-most subTarget, + // so we walk the array backwards + this.targets.concat().reverse().map(function(_target){ + hoverCursor = _target.hoverCursor || hoverCursor; + }); + } this.setCursor(hoverCursor); } else { From 74831428001d603668350fd4d5b54e7795ef0db7 Mon Sep 17 00:00:00 2001 From: jake downs Date: Sun, 15 Dec 2019 13:12:30 -0800 Subject: [PATCH 18/23] stub in unit test for #4115 --- test/unit/canvas_events.js | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/test/unit/canvas_events.js b/test/unit/canvas_events.js index bdde39bf07d..989f366ad4d 100644 --- a/test/unit/canvas_events.js +++ b/test/unit/canvas_events.js @@ -1,5 +1,7 @@ (function() { + var SUB_TARGETS_JSON = '{"version":"' + fabric.version + '","objects":[{"type":"activeSelection","left":-152,"top":656.25,"width":356.5,"height":356.5,"scaleX":0.45,"scaleY":0.45,"objects":[]},{"type":"group","left":11,"top":6,"width":511.5,"height":511.5,"objects":[{"type":"rect","left":-255.75,"top":-255.75,"width":50,"height":50,"fill":"#6ce798","scaleX":10.03,"scaleY":10.03,"opacity":0.8},{"type":"group","left":-179.75,"top":22,"width":356.5,"height":356.5,"scaleX":0.54,"scaleY":0.54,"objects":[{"type":"rect","left":-178.25,"top":-178.25,"width":50,"height":50,"fill":"#4862cc","scaleX":6.99,"scaleY":6.99,"opacity":0.8},{"type":"group","left":-163.25,"top":-161.25,"width":177.5,"height":177.5,"objects":[{"type":"rect","left":-88.75,"top":-88.75,"width":50,"height":50,"fill":"#5fe909","scaleX":3.48,"scaleY":3.48,"opacity":0.8},{"type":"rect","left":-59.75,"top":-68.75,"width":50,"height":50,"fill":"#f3529c","opacity":0.8},{"type":"triangle","left":36.03,"top":-38.12,"width":50,"height":50,"fill":"#c1124e","angle":39.07,"opacity":0.8},{"type":"rect","left":-65.75,"top":17.25,"width":50,"height":50,"fill":"#9c5120","opacity":0.8}]},{"type":"group","left":-34.25,"top":-31.25,"width":177.5,"height":177.5,"scaleX":1.08,"scaleY":1.08,"objects":[{"type":"rect","left":-88.75,"top":-88.75,"width":50,"height":50,"fill":"#5fe909","scaleX":3.48,"scaleY":3.48,"opacity":0.8},{"type":"rect","left":-59.75,"top":-68.75,"width":50,"height":50,"fill":"#f3529c","opacity":0.8},{"type":"triangle","left":36.03,"top":-38.12,"width":50,"height":50,"fill":"#c1124e","angle":39.07,"opacity":0.8},{"type":"rect","left":-65.75,"top":17.25,"width":50,"height":50,"fill":"#9c5120","opacity":0.8}]}]},{"type":"group","left":-202.75,"top":-228.5,"width":356.5,"height":356.5,"scaleX":0.61,"scaleY":0.61,"objects":[{"type":"rect","left":-178.25,"top":-178.25,"width":50,"height":50,"fill":"#4862cc","scaleX":6.99,"scaleY":6.99,"opacity":0.8},{"type":"group","left":-163.25,"top":-161.25,"width":177.5,"height":177.5,"objects":[{"type":"rect","left":-88.75,"top":-88.75,"width":50,"height":50,"fill":"#5fe909","scaleX":3.48,"scaleY":3.48,"opacity":0.8},{"type":"rect","left":-59.75,"top":-68.75,"width":50,"height":50,"fill":"#f3529c","opacity":0.8},{"type":"triangle","left":36.03,"top":-38.12,"width":50,"height":50,"fill":"#c1124e","angle":39.07,"opacity":0.8},{"type":"rect","left":-65.75,"top":17.25,"width":50,"height":50,"fill":"#9c5120","opacity":0.8}]},{"type":"group","left":-34.25,"top":-31.25,"width":177.5,"height":177.5,"scaleX":1.08,"scaleY":1.08,"objects":[{"type":"rect","left":-88.75,"top":-88.75,"width":50,"height":50,"fill":"#5fe909","scaleX":3.48,"scaleY":3.48,"opacity":0.8},{"type":"rect","left":-59.75,"top":-68.75,"width":50,"height":50,"fill":"#f3529c","opacity":0.8},{"type":"triangle","left":36.03,"top":-38.12,"width":50,"height":50,"fill":"#c1124e","angle":39.07,"opacity":0.8},{"type":"rect","left":-65.75,"top":17.25,"width":50,"height":50,"fill":"#9c5120","opacity":0.8}]}]},{"type":"group","left":138.3,"top":-90.22,"width":356.5,"height":356.5,"scaleX":0.42,"scaleY":0.42,"angle":62.73,"objects":[{"type":"rect","left":-178.25,"top":-178.25,"width":50,"height":50,"fill":"#4862cc","scaleX":6.99,"scaleY":6.99,"opacity":0.8},{"type":"group","left":-163.25,"top":-161.25,"width":177.5,"height":177.5,"objects":[{"type":"rect","left":-88.75,"top":-88.75,"width":50,"height":50,"fill":"#5fe909","scaleX":3.48,"scaleY":3.48,"opacity":0.8},{"type":"rect","left":-59.75,"top":-68.75,"width":50,"height":50,"fill":"#f3529c","opacity":0.8},{"type":"triangle","left":36.03,"top":-38.12,"width":50,"height":50,"fill":"#c1124e","angle":39.07,"opacity":0.8},{"type":"rect","left":-65.75,"top":17.25,"width":50,"height":50,"fill":"#9c5120","opacity":0.8}]},{"type":"group","left":-34.25,"top":-31.25,"width":177.5,"height":177.5,"scaleX":1.08,"scaleY":1.08,"objects":[{"type":"rect","left":-88.75,"top":-88.75,"width":50,"height":50,"fill":"#5fe909","scaleX":3.48,"scaleY":3.48,"opacity":0.8},{"type":"rect","left":-59.75,"top":-68.75,"width":50,"height":50,"fill":"#f3529c","opacity":0.8},{"type":"triangle","left":36.03,"top":-38.12,"width":50,"height":50,"fill":"#c1124e","angle":39.07,"opacity":0.8},{"type":"rect","left":-65.75,"top":17.25,"width":50,"height":50,"fill":"#9c5120","opacity":0.8}]}]}]}]}'; + var canvas = this.canvas = new fabric.Canvas(null, {enableRetinaScaling: false, width: 600, height: 600}); var upperCanvasEl = canvas.upperCanvasEl; @@ -481,6 +483,39 @@ }); }); + QUnit.test('Fabric mouseover, mouseout events fire for subTargets when subTargetCheck is enabled', function(assert){ + var counterOver = 0, counterOut = 0, canvas = new fabric.Canvas(); + function setSubTargetCheckRecursive(obj) { + if (obj._objects) { + obj._objects.forEach(setSubTargetCheckRecursive); + } + // TODO: maybe this property could've been baked into the JSON? + // not sure if loadFromJSON accounts for it tho + obj.subTargetCheck = true; + obj.on('mouseover', function() { + counterOver++; + }); + obj.on('mouseout', function() { + counterOut++; + }); + } + canvas.loadFromJSON(SUB_TARGETS_JSON, function() { + var activeSelection = new fabric.ActiveSelection(canvas.getObjects(), { + canvas: canvas + }); + canvas.setActiveObject(activeSelection); + setSubTargetCheckRecursive(activeSelection); + // perform MouseOver event on deepest nested subTarget + assert.equal(counterOver, 4, 'mouseover fabric event fired 4 times for primary hoveredTarget & subTargets'); + assert.equal(canvas._hoveredTarget, activeSelection, 'activeSelection is _hoveredTarget'); + assert.equal(canvas._hoveredTargets.length, 3, '3 additional subTargets are captured as _hoveredTargets'); + // perform MouseOut even on all hoveredTargets + assert.equal(counterOver, 4, 'mouseout fabric event fired 4 times for primary hoveredTarget & subTargets'); + assert.equal(canvas._hoveredTarget, null, '_hoveredTarget has been set to null'); + assert.equal(canvas._hoveredTargets.length, 0, '_hoveredTargets array is empty'); + }); + }); + ['MouseDown', 'MouseMove', 'MouseOut', 'MouseEnter', 'MouseWheel', 'DoubleClick'].forEach(function(eventType) { QUnit.test('avoid multiple registration - ' + eventType, function(assert) { var funcName = '_on' + eventType; From c4e53ce46a9b0a989da8fc0dcc7354af4114d2c0 Mon Sep 17 00:00:00 2001 From: jake downs Date: Mon, 16 Dec 2019 07:37:48 -0800 Subject: [PATCH 19/23] revert change in _onDragOver, call to _fireEnterLeaveEvents should be single target --- src/mixins/canvas_events.mixin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mixins/canvas_events.mixin.js b/src/mixins/canvas_events.mixin.js index 41b8d88d5b4..92b9665a59f 100644 --- a/src/mixins/canvas_events.mixin.js +++ b/src/mixins/canvas_events.mixin.js @@ -242,7 +242,7 @@ _onDragOver: function(e) { e.preventDefault(); var target = this._simpleEventHandler('dragover', e); - this._fireEnterLeaveEvents([target].concat(this.targets), e); + this._fireEnterLeaveEvents(target, e); }, /** From 3826d2eab0954c6c0884437cdf1ce9a5f58b0275 Mon Sep 17 00:00:00 2001 From: jake downs Date: Tue, 17 Dec 2019 22:45:39 -0800 Subject: [PATCH 20/23] updates after running tests + _fireOverOutEvents: fill diff array with nulls + _onMouseOut: remove debug console.log + updated description for subTargetCheck property to expand what it affects + updated subTargetCheck mouseover/mouseout event test case + added TODO notes for new tests for "mousemove: subTargetCheck: setCursorFromEvent considers subTargets" --- src/mixins/canvas_events.mixin.js | 7 +++---- src/shapes/group.class.js | 2 +- test/unit/canvas_events.js | 24 +++++++++++++++++++----- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/mixins/canvas_events.mixin.js b/src/mixins/canvas_events.mixin.js index 92b9665a59f..74f2ba7ad53 100644 --- a/src/mixins/canvas_events.mixin.js +++ b/src/mixins/canvas_events.mixin.js @@ -166,8 +166,6 @@ * @param {Event} e Event object fired on mousedown */ _onMouseOut: function(e) { - console.log('_onMouseOut',e); - var target = this._hoveredTarget; this.fire('mouse:out', { target: target, e: e }); this._hoveredTarget = null; @@ -831,8 +829,9 @@ _fireOverOutEvents: function(target, e) { var _this = this, _hoveredTarget = this._hoveredTarget, _hoveredTargets = this._hoveredTargets, targets = this.targets, - diff = _hoveredTargets.length - targets.length; - [target].concat(targets, new Array(diff > 0 ? diff : 0)).forEach(function(_target, index) { + diff = _hoveredTargets.length - targets.length, + diffArrayLength = diff > 0 ? diff : 0; + [target].concat(targets, new Array(diffArrayLength).fill(null)).forEach(function(_target, index) { _this.fireSyntheticInOutEvents(_target, e, { oldTarget: index === 0 ? _hoveredTarget : _hoveredTargets[index - 1], canvasEvtOut: 'mouse:out', diff --git a/src/shapes/group.class.js b/src/shapes/group.class.js index 2b622ec6245..e4a92375348 100644 --- a/src/shapes/group.class.js +++ b/src/shapes/group.class.js @@ -35,7 +35,7 @@ strokeWidth: 0, /** - * Indicates if click events should also check for subtargets + * Indicates if click, mouseover, mouseout events & hoverCursor should also check for subtargets * @type Boolean * @default */ diff --git a/test/unit/canvas_events.js b/test/unit/canvas_events.js index 989f366ad4d..cfbc5c6f95e 100644 --- a/test/unit/canvas_events.js +++ b/test/unit/canvas_events.js @@ -489,8 +489,6 @@ if (obj._objects) { obj._objects.forEach(setSubTargetCheckRecursive); } - // TODO: maybe this property could've been baked into the JSON? - // not sure if loadFromJSON accounts for it tho obj.subTargetCheck = true; obj.on('mouseover', function() { counterOver++; @@ -505,17 +503,33 @@ }); canvas.setActiveObject(activeSelection); setSubTargetCheckRecursive(activeSelection); - // perform MouseOver event on deepest nested subTarget + + // perform MouseOver event on a deeply nested subTarget + var moveEvent = fabric.document.createEvent('HTMLEvents'); + moveEvent.initEvent('mousemove', true, true); + var target = canvas.item(1); + canvas.targets = [ + target.item(1), + target.item(1).item(1), + target.item(1).item(1).item(1) + ]; + canvas._fireOverOutEvents(target, moveEvent); assert.equal(counterOver, 4, 'mouseover fabric event fired 4 times for primary hoveredTarget & subTargets'); - assert.equal(canvas._hoveredTarget, activeSelection, 'activeSelection is _hoveredTarget'); + assert.equal(canvas._hoveredTarget, target, 'activeSelection is _hoveredTarget'); assert.equal(canvas._hoveredTargets.length, 3, '3 additional subTargets are captured as _hoveredTargets'); + // perform MouseOut even on all hoveredTargets - assert.equal(counterOver, 4, 'mouseout fabric event fired 4 times for primary hoveredTarget & subTargets'); + canvas.targets = []; + canvas._fireOverOutEvents(null, moveEvent); + assert.equal(counterOut, 4, 'mouseout fabric event fired 4 times for primary hoveredTarget & subTargets'); assert.equal(canvas._hoveredTarget, null, '_hoveredTarget has been set to null'); assert.equal(canvas._hoveredTargets.length, 0, '_hoveredTargets array is empty'); }); }); + // TODO: QUnit.test('mousemove: subTargetCheck: setCursorFromEvent considers subTargets') + // TODO: QUnit.test('mousemove: subTargetCheck: setCursorFromEvent considers subTargets in reverse order, so the top-most subTarget's .hoverCursor takes precedence') + ['MouseDown', 'MouseMove', 'MouseOut', 'MouseEnter', 'MouseWheel', 'DoubleClick'].forEach(function(eventType) { QUnit.test('avoid multiple registration - ' + eventType, function(assert) { var funcName = '_on' + eventType; From 3714348269ebb6e7ab739959ed186272208414b1 Mon Sep 17 00:00:00 2001 From: jake downs Date: Tue, 17 Dec 2019 22:58:56 -0800 Subject: [PATCH 21/23] rm unused var --- src/mixins/canvas_events.mixin.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/mixins/canvas_events.mixin.js b/src/mixins/canvas_events.mixin.js index 74f2ba7ad53..abdad5a86fd 100644 --- a/src/mixins/canvas_events.mixin.js +++ b/src/mixins/canvas_events.mixin.js @@ -829,9 +829,8 @@ _fireOverOutEvents: function(target, e) { var _this = this, _hoveredTarget = this._hoveredTarget, _hoveredTargets = this._hoveredTargets, targets = this.targets, - diff = _hoveredTargets.length - targets.length, - diffArrayLength = diff > 0 ? diff : 0; - [target].concat(targets, new Array(diffArrayLength).fill(null)).forEach(function(_target, index) { + diff = _hoveredTargets.length - targets.length; + [target].concat(targets, new Array(diff > 0 ? diff : 0).fill(null)).forEach(function(_target, index) { _this.fireSyntheticInOutEvents(_target, e, { oldTarget: index === 0 ? _hoveredTarget : _hoveredTargets[index - 1], canvasEvtOut: 'mouse:out', From dbaf781eb48feeaf32a96845daa6657f18d457c1 Mon Sep 17 00:00:00 2001 From: jake downs Date: Wed, 18 Dec 2019 08:39:25 -0800 Subject: [PATCH 22/23] replace Array.fill with for loop (IE support) --- src/mixins/canvas_events.mixin.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/mixins/canvas_events.mixin.js b/src/mixins/canvas_events.mixin.js index abdad5a86fd..a4e18c0818b 100644 --- a/src/mixins/canvas_events.mixin.js +++ b/src/mixins/canvas_events.mixin.js @@ -829,8 +829,13 @@ _fireOverOutEvents: function(target, e) { var _this = this, _hoveredTarget = this._hoveredTarget, _hoveredTargets = this._hoveredTargets, targets = this.targets, - diff = _hoveredTargets.length - targets.length; - [target].concat(targets, new Array(diff > 0 ? diff : 0).fill(null)).forEach(function(_target, index) { + diff = _hoveredTargets.length - targets.length, + diffArrayLength = diff > 0 ? diff : 0, + diffArray = []; + for (var i = 0; i < diffArrayLength; i++){ + diffArray.push(null); + } + [target].concat(targets, diffArray).forEach(function(_target, index) { _this.fireSyntheticInOutEvents(_target, e, { oldTarget: index === 0 ? _hoveredTarget : _hoveredTargets[index - 1], canvasEvtOut: 'mouse:out', From 06e9c7e46482815d28ddbd40c4c89418d7327221 Mon Sep 17 00:00:00 2001 From: jake downs Date: Wed, 18 Dec 2019 08:46:21 -0800 Subject: [PATCH 23/23] update _fireEnterLeaveEvents to match _fireOverOutEvents --- src/mixins/canvas_events.mixin.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/mixins/canvas_events.mixin.js b/src/mixins/canvas_events.mixin.js index a4e18c0818b..ae99d86905d 100644 --- a/src/mixins/canvas_events.mixin.js +++ b/src/mixins/canvas_events.mixin.js @@ -857,8 +857,13 @@ _fireEnterLeaveEvents: function(target, e) { var _this = this, _draggedoverTarget = this._draggedoverTarget, _hoveredTargets = this._hoveredTargets, targets = this.targets, - diff = _hoveredTargets.length - targets.length; - [target].concat(targets, new Array(diff > 0 ? diff : 0)).forEach(function(_target, index) { + diff = _hoveredTargets.length - targets.length, + diffArrayLength = diff > 0 ? diff : 0, + diffArray = []; + for (var i = 0; i < diffArrayLength; i++){ + diffArray.push(null); + } + [target].concat(targets, diffArray).forEach(function(_target, index) { _this.fireSyntheticInOutEvents(_target, e, { oldTarget: index === 0 ? _draggedoverTarget : _hoveredTargets[index - 1], evtOut: 'dragleave',