Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ISSUE-4115 - triggers in/out events for sub targets #6013

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
54c5923
Fixes #4115 - triggers in/out events for sub targets; and calls _setC…
jakedowns Dec 3, 2019
b6fa464
added a .__guid property to each object to simplify hoveredTargets an…
jakedowns Dec 4, 2019
3c082a9
Revert "added a .__guid property to each object to simplify hoveredTa…
jakedowns Dec 11, 2019
5fbf4a4
Merge branch 'master' into ISSUE-4115-mouse-over-out-sub-targets
jakedowns Dec 11, 2019
15ab4ba
reverting hoveredTargets to bare bones implementation
jakedowns Dec 11, 2019
38ae830
this is an idea
asturur Dec 14, 2019
bc48c2d
this is an idea
asturur Dec 14, 2019
f3bb7ed
adjustments to PR #6032
jakedowns Dec 15, 2019
69647aa
Merge branch 'sub-targeting-mouse-in-out' into ISSUE-4115-mouse-over-…
jakedowns Dec 15, 2019
4ab6f2a
adjust additional _hoveredTarget management
jakedowns Dec 15, 2019
f24f61f
move _this to where it's needed
jakedowns Dec 15, 2019
bd6590d
more cleanup of lingering stuff from earlier commits
jakedowns Dec 15, 2019
7d40b4d
fix linter error
jakedowns Dec 15, 2019
b4251d0
targets: default to empty array
jakedowns Dec 15, 2019
41978dd
correction to jsdoc array of fabric.Object type declaration
jakedowns Dec 15, 2019
deb64f0
remove genuine unique id GUID getter from object class
jakedowns Dec 15, 2019
426e68c
simplify & optimize for minification
jakedowns Dec 15, 2019
0cd4196
updated _fireEnterLeaveEvents to include subTargets
jakedowns Dec 15, 2019
1909d1f
adjusted _setCursorFromEvent to account for subTargets
jakedowns Dec 15, 2019
7483142
stub in unit test for #4115
jakedowns Dec 15, 2019
c4e53ce
revert change in _onDragOver, call to _fireEnterLeaveEvents should be…
jakedowns Dec 16, 2019
3826d2e
updates after running tests
jakedowns Dec 18, 2019
3714348
rm unused var
jakedowns Dec 18, 2019
dbaf781
replace Array.fill with for loop (IE support)
jakedowns Dec 18, 2019
06e9c7e
update _fireEnterLeaveEvents to match _fireOverOutEvents
jakedowns Dec 18, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 4 additions & 16 deletions src/canvas.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@

/**
* hold the list of nested targets hovered
* @type fabric.Object
* @type Array[fabric.Object]
jakedowns marked this conversation as resolved.
Show resolved Hide resolved
* @private
*/
_hoveredTargets: [],
Expand Down Expand Up @@ -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 = [];
jakedowns marked this conversation as resolved.
Show resolved Hide resolved
}
this.callSuper('_onObjectRemoved', obj);
},
Expand Down
30 changes: 7 additions & 23 deletions src/mixins/canvas_events.mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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 = [];
}
},

Expand Down
25 changes: 2 additions & 23 deletions src/mixins/canvas_grouping.mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
jakedowns marked this conversation as resolved.
Show resolved Hide resolved
if (activeSelection.size() === 1) {
// activate last remaining object
this._setActiveObject(activeSelection.item(0), e);
Expand All @@ -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);
},
Expand Down