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 all commits
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
23 changes: 22 additions & 1 deletion src/canvas.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,26 @@
*/
fireMiddleClick: false,

/**
* Keep track of the subTargets for Mouse Events
* @type fabric.Object[]
*/
targets: [],

/**
* 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
*/
Expand Down Expand Up @@ -1477,8 +1497,9 @@
this.fire('selection:cleared', { target: obj });
obj.fire('deselected');
}
if (this._hoveredTarget === obj) {
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
62 changes: 50 additions & 12 deletions src/mixins/canvas_events.mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,14 @@
this.fire('mouse:out', { target: target, e: e });
this._hoveredTarget = null;
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 });
});
this._hoveredTargets = [];

if (this._iTextInstances) {
this._iTextInstances.forEach(function(obj) {
if (obj.isEditing) {
Expand All @@ -193,6 +201,7 @@
if (!this.currentTransform && !this.findTarget(e)) {
this.fire('mouse:over', { target: null, e: e });
this._hoveredTarget = null;
this._hoveredTargets = [];
}
},

Expand Down Expand Up @@ -818,13 +827,25 @@
* @private
*/
_fireOverOutEvents: function(target, e) {
this.fireSyntheticInOutEvents(target, e, {
targetName: '_hoveredTarget',
canvasEvtOut: 'mouse:out',
evtOut: 'mouseout',
canvasEvtIn: 'mouse:over',
evtIn: 'mouseover',
var _this = this, _hoveredTarget = this._hoveredTarget,
_hoveredTargets = this._hoveredTargets, targets = this.targets,
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',
evtOut: 'mouseout',
canvasEvtIn: 'mouse:over',
evtIn: 'mouseover',
});
});
this._hoveredTarget = target;
this._hoveredTargets = this.targets.concat();
},

/**
Expand All @@ -834,11 +855,22 @@
* @private
*/
_fireEnterLeaveEvents: function(target, e) {
this.fireSyntheticInOutEvents(target, e, {
targetName: '_draggedoverTarget',
evtOut: 'dragleave',
evtIn: 'dragenter',
var _this = this, _draggedoverTarget = this._draggedoverTarget,
_hoveredTargets = this._hoveredTargets, targets = this.targets,
diff = _hoveredTargets.length - targets.length,
diffArrayLength = diff > 0 ? diff : 0,
diffArray = [];
for (var i = 0; i < diffArrayLength; i++){
diffArray.push(null);
}
Copy link
Member

Choose a reason for hiding this comment

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

oh i see, the array with 3 undefined will not iterate. Well ok this is fine, if will find away to have it more terse, it can be a follow up.

image

the length is 7 but forEach will not care

Copy link
Contributor Author

@jakedowns jakedowns Dec 18, 2019

Choose a reason for hiding this comment

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

yeah forEach ignores the empties :/ interested to see what you come up with that's shorter than that. I looked at an Array.prototype.fill polyfill, but it was way more lines than a 3-line for loop.

[target].concat(targets, diffArray).forEach(function(_target, index) {
_this.fireSyntheticInOutEvents(_target, e, {
oldTarget: index === 0 ? _draggedoverTarget : _hoveredTargets[index - 1],
evtOut: 'dragleave',
evtIn: 'dragenter',
});
});
this._draggedoverTarget = target;
},

/**
Expand All @@ -854,12 +886,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;
jakedowns marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -1024,6 +1055,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 {
Expand Down
5 changes: 5 additions & 0 deletions src/mixins/canvas_grouping.mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
if (activeSelection.contains(target)) {
activeSelection.removeWithUpdate(target);
this._hoveredTarget = target;
this._hoveredTargets = this.targets.concat();
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 @@ -61,6 +62,7 @@
else {
activeSelection.addWithUpdate(target);
this._hoveredTarget = activeSelection;
this._hoveredTargets = this.targets.concat();
}
this._fireSelectionEvents(currentActiveObjects, e);
},
Expand All @@ -71,6 +73,9 @@
_createActiveSelection: function(target, e) {
var currentActives = this.getActiveObjects(), group = this._createGroup(target);
this._hoveredTarget = group;
// ISSUE 4115: should we consider subTargets here?
jakedowns marked this conversation as resolved.
Show resolved Hide resolved
// this._hoveredTargets = [];
// this._hoveredTargets = this.targets.concat();
this._setActiveObject(group, e);
this._fireSelectionEvents(currentActives, e);
},
Expand Down
2 changes: 1 addition & 1 deletion src/shapes/group.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
1 change: 0 additions & 1 deletion src/shapes/object.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -2115,5 +2115,4 @@
* @type Number
*/
fabric.Object.__uid = 0;

})(typeof exports !== 'undefined' ? exports : this);
51 changes: 50 additions & 1 deletion test/unit/canvas_events.js
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -481,6 +483,53 @@
});
});

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);
}
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 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, target, 'activeSelection is _hoveredTarget');
assert.equal(canvas._hoveredTargets.length, 3, '3 additional subTargets are captured as _hoveredTargets');

// perform MouseOut even on all hoveredTargets
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;
Expand Down Expand Up @@ -550,7 +599,7 @@
c._hoveredTarget = obj;
c.currentTransform = {};
c.upperCanvasEl.dispatchEvent(event);
assert.equal(c._hoveredTarget, obj, '_hoveredTarget has been removed');
assert.equal(c._hoveredTarget, obj, '_hoveredTarget has been not removed');
});

QUnit.test('mouseEnter removes __corner', function(assert) {
Expand Down