Skip to content

Commit

Permalink
patch(): Control#shouldActivate
Browse files Browse the repository at this point in the history
  • Loading branch information
ShaMan123 committed May 19, 2023
1 parent e4847c7 commit a4c0f62
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 10 deletions.
8 changes: 8 additions & 0 deletions src/controls/Control.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,14 @@ export class Control {
*/
declare mouseUpHandler?: ControlActionHandler;

shouldActivate(controlKey: string, fabricObject: InteractiveFabricObject) {
// TODO: locking logic can be handled here instead of in the control handler logic

This comment has been minimized.

Copy link
@asturur

asturur May 19, 2023

Member

mmm this TODO isn't doable.
shouldActivate with the current parameter does not know what the actionHandler will do.
So you don't know if is lockScaling, lockSkewing or whatever, you are not passin the current trasnfrom either so how woudl we know how to add some lock logic here?

This comment has been minimized.

Copy link
@ShaMan123

ShaMan123 May 19, 2023

Author Contributor

Right
I was thinking of subclasses of controls
But it seems I am moving away of subclassing so perhaps we remove this todo or add an arg.
What do you think?
I want to deprecate all the lockXXX props. Plain and simple mess, not real functionality.

return (
fabricObject.canvas?.getActiveObject() === fabricObject &&
fabricObject.isControlVisible(controlKey)
);
}

/**
* Returns control actionHandler
* @param {Event} eventData the native mouse event
Expand Down
11 changes: 3 additions & 8 deletions src/shapes/Object/InteractiveObject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,11 +185,7 @@ export class InteractiveFabricObject<
* @return {String} corner code (tl, tr, bl, br, etc.), or an empty string if nothing is found.
*/
_findTargetCorner(pointer: Point, forTouch = false): string {
if (
!this.hasControls ||
!this.canvas ||
(this.canvas._activeObject as unknown as this) !== this
) {
if (!this.hasControls || !this.canvas) {
return '';
}

Expand All @@ -198,10 +194,8 @@ export class InteractiveFabricObject<
const cornerEntries = Object.entries(this.controlCoords);
for (let i = cornerEntries.length - 1; i >= 0; i--) {
const [key, coord] = cornerEntries[i];
if (!this.isControlVisible(key)) {
continue;
}
if (
this.controls[key].shouldActivate(key, this) &&
PlaneBBox.build(
forTouch ? coord.touchCorner : coord.corner
).containsPoint(pointer)
Expand All @@ -210,6 +204,7 @@ export class InteractiveFabricObject<
return key;
}
}

return '';
}

Expand Down
25 changes: 23 additions & 2 deletions test/unit/object_interactivity.js
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@
assert.ok(typeof cObj._findTargetCorner === 'function', '_findTargetCorner should exist');
cObj.setCoords();
cObj.canvas = {
_activeObject: cObj
getActiveObject() { return cObj }
};
assert.equal(cObj._findTargetCorner(cObj.controlCoords.br.position), 'br');
assert.equal(cObj._findTargetCorner(cObj.controlCoords.tl.position), 'tl');
Expand All @@ -223,7 +223,7 @@
var cObj = new fabric.Object({ top: 10, left: 10, width: 30, height: 30, strokeWidth: 0 });
cObj.setCoords();
cObj.canvas = {
_activeObject: cObj
getActiveObject() { return cObj }
};
var pointNearBr = {
x: cObj.controlCoords.br.position.x + cObj.cornerSize / 3,
Expand All @@ -239,6 +239,27 @@
assert.equal(cObj._findTargetCorner(pointNearBr, false), false, 'not touch event touchCornerSize/3 near br returns false');
});

QUnit.test('_findTargetCorner for non active object', function (assert) {
var cObj = new fabric.Object({ top: 10, left: 10, width: 30, height: 30, strokeWidth: 0 });
assert.ok(typeof cObj._findTargetCorner === 'function', '_findTargetCorner should exist');
cObj.setCoords();
cObj.canvas = {
getActiveObject() { return }
};
assert.equal(cObj._findTargetCorner(cObj.oCoords.mtr), '', 'object is not active');
});

QUnit.test('_findTargetCorner for non visible control', function (assert) {
var cObj = new fabric.Object({ top: 10, left: 10, width: 30, height: 30, strokeWidth: 0 });
assert.ok(typeof cObj._findTargetCorner === 'function', '_findTargetCorner should exist');
cObj.setCoords();
cObj.canvas = {
getActiveObject() { return cObj }
};
cObj.isControlVisible = () => false;
assert.equal(cObj._findTargetCorner(cObj.oCoords.mtr), '', 'object is not active');
});

QUnit.test.skip('_calculateCurrentDimensions', function(assert) {
var cObj = new fabric.Object({ width: 10, height: 15, strokeWidth: 0 }), dim;
assert.ok(typeof cObj._calculateCurrentDimensions === 'function', '_calculateCurrentDimensions should exist');
Expand Down

0 comments on commit a4c0f62

Please sign in to comment.