diff --git a/CHANGELOG.md b/CHANGELOG.md index 1304d81e372..544949c2072 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [next] +- refactor(Canvas): `_setupCurrentTransform` => `setupCurrentTransform` fixing control mousedown unexpected behavior [#9842](https://github.com/fabricjs/fabric.js/pull/9842) - ci(): Add Jest coverage to the report [#9836](https://github.com/fabricjs/fabric.js/pull/9836) - test(): Add cursor animation testing and migrate some easy one to jest [#9829](https://github.com/fabricjs/fabric.js/pull/9829) - fix(Group, Controls): Fix interactive group actions when negative scaling is involved [#9811](https://github.com/fabricjs/fabric.js/pull/9811) diff --git a/src/canvas/Canvas.ts b/src/canvas/Canvas.ts index ba0a0efbd78..ad77db57938 100644 --- a/src/canvas/Canvas.ts +++ b/src/canvas/Canvas.ts @@ -1049,24 +1049,44 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { if (target.selectable && target.activeOn === 'down') { this.setActiveObject(target, e); } - const handle = target.findControl( - this.getViewportPoint(e), - isTouchEvent(e) - ); - if (target === this._activeObject && (handle || !grouped)) { - this._setupCurrentTransform(e, target, alreadySelected); - const control = handle ? handle.control : undefined, - pointer = this.getScenePoint(e), - mouseDownHandler = - control && control.getMouseDownHandler(e, target, control); - mouseDownHandler && + const isSelected = target === this._activeObject; + const controlContext = + isSelected && + alreadySelected && + target.findControl(this.getViewportPoint(e), isTouchEvent(e)); + const transformContext = + controlContext || + (isSelected && !grouped && ({ key: 'drag' } as const)); + + if (transformContext) { + const transform = this.setupCurrentTransform( + e, + target, + transformContext + ); + this.fire('before:transform', { + e, + transform, + }); + + const mouseDownHandler = + controlContext && + controlContext.control.getMouseDownHandler( + e, + target, + controlContext.control + ); + + if (mouseDownHandler) { + const pointer = this.getScenePoint(e); mouseDownHandler.call( - control, + controlContext.control, e, - this._currentTransform!, + transform, pointer.x, pointer.y ); + } } } // we clear `_objectsToRender` in case of a change in order to repopulate it at rendering diff --git a/src/canvas/SelectableCanvas.spec.ts b/src/canvas/SelectableCanvas.spec.ts index 5c1f2578903..9984d4207cd 100644 --- a/src/canvas/SelectableCanvas.spec.ts +++ b/src/canvas/SelectableCanvas.spec.ts @@ -2,6 +2,7 @@ import { FabricObject } from '../shapes/Object/FabricObject'; import { Point } from '../Point'; import { Canvas } from './Canvas'; import { Group } from '../shapes/Group'; +import { Rect } from '../shapes/Rect'; describe('Selectable Canvas', () => { describe('_pointIsInObjectSelectionArea', () => { @@ -460,7 +461,7 @@ describe('Selectable Canvas', () => { canvas.on('before:transform', spy); const setupCurrentTransformSpy = jest.spyOn( canvas, - '_setupCurrentTransform' + 'setupCurrentTransform' ); const { @@ -478,7 +479,85 @@ describe('Selectable Canvas', () => { expect(canvas._currentTransform).toBeDefined(); expect(canvas._currentTransform).toHaveProperty('target', object); expect(canvas._currentTransform).toHaveProperty('corner', controlKey); + expect(setupCurrentTransformSpy).toHaveReturnedWith( + canvas._currentTransform + ); expect(spy).toHaveBeenCalledTimes(1); }); + + describe('setting current transform', () => { + const rectOptions = { + left: 75, + top: 75, + width: 50, + height: 50, + }; + const eAt100 = new MouseEvent('mousedown', { + clientX: 100, + clientY: 100, + }); + const eOverTLControl = new MouseEvent('mousedown', { + clientX: 69, + clientY: 69, + }); + const eOverMLControl = new MouseEvent('mousedown', { + clientX: 69, + clientY: 95, + shiftKey: true, + }); + + test.each([ + { e: eAt100, name: '100x100' }, + { e: eOverTLControl, name: 'over TL' }, + { e: eOverMLControl, name: 'over skew ML' }, + ])('event $name', ({ e }) => { + const canvas = new Canvas(); + const rect = new Rect(rectOptions); + jest.spyOn(rect, 'toJSON').mockReturnValue('rect'); + canvas.add(rect); + canvas.setActiveObject(rect); + expect( + canvas.setupCurrentTransform( + e, + rect, + rect.findControl(canvas.getViewportPoint(e)) || { + key: 'drag', + } + ) + ).toMatchSnapshot(); + }); + + it('control mousedown handler should be called only if current transform uses it', () => { + const canvas = new Canvas(); + const rect = new Rect(rectOptions); + jest.spyOn(rect, 'toJSON').mockReturnValue('rect'); + canvas.add(rect); + canvas.setActiveObject(rect); + const hit = rect.findControl(canvas.getViewportPoint(eOverTLControl)); + + expect(hit).toBeDefined(); + expect(hit?.key).toBe('tl'); + const controlDownSpy = jest.spyOn(hit!.control, 'getMouseDownHandler'); + const spy = jest.spyOn(canvas, 'setupCurrentTransform'); + jest.spyOn(canvas, 'findTarget').mockReturnValue(rect); + + canvas.getSelectionElement().dispatchEvent(eOverTLControl); + expect(spy).toHaveBeenCalledTimes(1); + expect(spy).toHaveBeenCalledWith(eOverTLControl, rect, hit); + expect(controlDownSpy).toHaveBeenCalledTimes(1); + + spy.mockClear(); + controlDownSpy.mockClear(); + + canvas.discardActiveObject(); + + canvas.getSelectionElement().dispatchEvent(eOverTLControl); + expect(spy).toHaveBeenCalledTimes(1); + expect(spy).toHaveBeenCalledWith(eOverTLControl, rect, { + key: 'drag', + }); + expect(controlDownSpy).not.toHaveBeenCalled(); + }); + }); }); }); diff --git a/src/canvas/SelectableCanvas.ts b/src/canvas/SelectableCanvas.ts index 7a3fce244d9..b2ac170d20a 100644 --- a/src/canvas/SelectableCanvas.ts +++ b/src/canvas/SelectableCanvas.ts @@ -1,5 +1,4 @@ import { dragHandler } from '../controls/drag'; -import { getActionFromCorner } from '../controls/util'; import { Point } from '../Point'; import { FabricObject } from '../shapes/Object/FabricObject'; import type { @@ -37,6 +36,7 @@ import type { CanvasOptions } from './CanvasOptions'; import { canvasDefaults } from './CanvasOptions'; import { Intersection } from '../Intersection'; import { isActiveSelection } from '../util/typeAssertions'; +import type { Control } from '../controls/Control'; /** * Canvas class @@ -568,16 +568,31 @@ export class SelectableCanvas } /** - * @private + * Setup state used by the canvas to execute object transforms via the control API * @param {Event} e Event object * @param {FabricObject} target - * @param {boolean} [alreadySelected] pass true to setup the active control + * @param [transformContext] pass {@link FabricObject.findControl}, `{ key: 'drag' }` to setup dragging + * or undefined to block transforming */ - _setupCurrentTransform( + setupCurrentTransform( e: TPointerEvent, target: FabricObject, - alreadySelected: boolean - ): void { + transformContext?: + | { key: string; control: Control } + | { key: 'drag'; control?: never } + ): Transform { + const key = transformContext?.key || ''; + const control = transformContext?.control; + const action = control?.getActionName(e, control, target) ?? key, + altKey = e[this.centeredKey as ModifierKey], + origin = this._shouldCenterTransform(target, action, altKey) + ? ({ x: CENTER, y: CENTER } as const) + : this._getOriginFromCorner(target, key); + + /** + * relative to target's containing coordinate plane + * both agree on every point + **/ const pointer = target.group ? // transform pointer to target's containing coordinate plane sendPointToPlane( @@ -586,56 +601,63 @@ export class SelectableCanvas target.group.calcTransformMatrix() ) : this.getScenePoint(e); - const { key: corner = '', control } = target.getActiveControl() || {}, - actionHandler = - alreadySelected && control - ? control.getActionHandler(e, target, control)?.bind(control) - : dragHandler, - action = getActionFromCorner(alreadySelected, corner, e, target), - altKey = e[this.centeredKey as ModifierKey], - origin = this._shouldCenterTransform(target, action, altKey) - ? ({ x: CENTER, y: CENTER } as const) - : this._getOriginFromCorner(target, corner), - /** - * relative to target's containing coordinate plane - * both agree on every point - **/ - transform: Transform = { - target: target, - action, - actionHandler, - actionPerformed: false, - corner, - scaleX: target.scaleX, - scaleY: target.scaleY, - skewX: target.skewX, - skewY: target.skewY, - offsetX: pointer.x - target.left, - offsetY: pointer.y - target.top, + const transform: Transform = { + target: target, + action, + actionHandler: control + ? control.getActionHandler(e, target, control)?.bind(control) + : key === 'drag' + ? dragHandler + : undefined, + actionPerformed: false, + corner: control ? key : '', + scaleX: target.scaleX, + scaleY: target.scaleY, + skewX: target.skewX, + skewY: target.skewY, + offsetX: pointer.x - target.left, + offsetY: pointer.y - target.top, + originX: origin.x, + originY: origin.y, + ex: pointer.x, + ey: pointer.y, + lastX: pointer.x, + lastY: pointer.y, + theta: degreesToRadians(target.angle), + width: target.width, + height: target.height, + shiftKey: e.shiftKey, + altKey, + original: { + ...saveObjectTransform(target), originX: origin.x, originY: origin.y, - ex: pointer.x, - ey: pointer.y, - lastX: pointer.x, - lastY: pointer.y, - theta: degreesToRadians(target.angle), - width: target.width, - height: target.height, - shiftKey: e.shiftKey, - altKey, - original: { - ...saveObjectTransform(target), - originX: origin.x, - originY: origin.y, - }, - }; + }, + }; - this._currentTransform = transform; + return (this._currentTransform = transform); + } - this.fire('before:transform', { + /** + * @deprecated use {@link setupCurrentTransform} and fire your own `before:transform` event if you need to + * @param {Event} e Event object + * @param {FabricObject} target + * @param {boolean} [activateControl] pass true to setup the active control + */ + _setupCurrentTransform( + e: TPointerEvent, + target: FabricObject, + activateControl: boolean + ) { + const transform = this.setupCurrentTransform( e, - transform, - }); + target, + (activateControl && + target.findControl(this.getViewportPoint(e), isTouchEvent(e))) || { + key: 'drag', + } + ); + this.fire('before:transform', { e, transform }); } /** diff --git a/src/canvas/__snapshots__/SelectableCanvas.spec.ts.snap b/src/canvas/__snapshots__/SelectableCanvas.spec.ts.snap new file mode 100644 index 00000000000..e96cbb6e373 --- /dev/null +++ b/src/canvas/__snapshots__/SelectableCanvas.spec.ts.snap @@ -0,0 +1,121 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Selectable Canvas setupCurrentTransform setting current transform event 100x100 1`] = ` +{ + "action": "drag", + "actionHandler": [Function], + "actionPerformed": false, + "altKey": false, + "corner": "", + "ex": 100, + "ey": 100, + "height": 50, + "lastX": 100, + "lastY": 100, + "offsetX": 25, + "offsetY": 25, + "originX": "left", + "originY": "top", + "original": { + "angle": 0, + "flipX": false, + "flipY": false, + "left": 75, + "originX": "left", + "originY": "top", + "scaleX": 1, + "scaleY": 1, + "skewX": 0, + "skewY": 0, + "top": 75, + }, + "scaleX": 1, + "scaleY": 1, + "shiftKey": false, + "skewX": 0, + "skewY": 0, + "target": "rect", + "theta": 0, + "width": 50, +} +`; + +exports[`Selectable Canvas setupCurrentTransform setting current transform event over TL 1`] = ` +{ + "action": "scale", + "actionHandler": [Function], + "actionPerformed": false, + "altKey": false, + "corner": "tl", + "ex": 69, + "ey": 69, + "height": 50, + "lastX": 69, + "lastY": 69, + "offsetX": -6, + "offsetY": -6, + "originX": "right", + "originY": "bottom", + "original": { + "angle": 0, + "flipX": false, + "flipY": false, + "left": 75, + "originX": "right", + "originY": "bottom", + "scaleX": 1, + "scaleY": 1, + "skewX": 0, + "skewY": 0, + "top": 75, + }, + "scaleX": 1, + "scaleY": 1, + "shiftKey": false, + "skewX": 0, + "skewY": 0, + "target": "rect", + "theta": 0, + "width": 50, +} +`; + +exports[`Selectable Canvas setupCurrentTransform setting current transform event over skew ML 1`] = ` +{ + "action": "skewY", + "actionHandler": [Function], + "actionPerformed": false, + "altKey": false, + "corner": "ml", + "ex": 69, + "ey": 95, + "height": 50, + "lastX": 69, + "lastY": 95, + "offsetX": -6, + "offsetY": 20, + "originX": "right", + "originY": "top", + "original": { + "angle": 0, + "flipX": false, + "flipY": false, + "left": 75, + "originX": "right", + "originY": "top", + "scaleX": 1, + "scaleY": 1, + "skewX": 0, + "skewY": 0, + "top": 75, + }, + "scaleX": 1, + "scaleY": 1, + "shiftKey": true, + "skewX": 0, + "skewY": 0, + "target": "rect", + "theta": 0, + "width": 50, +} +`; diff --git a/src/controls/polyControl.spec.ts b/src/controls/polyControl.spec.ts index 987912224bb..7b26ee4dc65 100644 --- a/src/controls/polyControl.spec.ts +++ b/src/controls/polyControl.spec.ts @@ -19,10 +19,10 @@ describe('polyControl', () => { canvas .getSelectionElement() .dispatchEvent(new MouseEvent('mousedown', { clientX: 50, clientY: 50 })); - canvas._setupCurrentTransform( + canvas.setupCurrentTransform( new MouseEvent('mousedown', { clientX: 50, clientY: 50 }), poly, - true + poly.findControl(new Point(50, 50)) ); document.dispatchEvent( new MouseEvent('mousemove', { clientX: 55, clientY: 55 }) diff --git a/src/controls/scale.test.ts b/src/controls/scale.test.ts index 99b500ace9b..7bd6b803c27 100644 --- a/src/controls/scale.test.ts +++ b/src/controls/scale.test.ts @@ -50,8 +50,11 @@ const createZeroThickRectangleScalingItems = ( canvas.setActiveObject(target); target.__corner = usedCorner; - canvas._setupCurrentTransform(mouseDown, target, false); - const transform = canvas._currentTransform!; + const transform = canvas.setupCurrentTransform( + mouseDown, + target, + target.getActiveControl() + ); const pointer = canvas.getScenePoint(moveEvent); // return items used by our action handler diff --git a/src/controls/util.ts b/src/controls/util.ts index c372c1a2612..d8fc1f05f06 100644 --- a/src/controls/util.ts +++ b/src/controls/util.ts @@ -1,5 +1,4 @@ import type { - TPointerEvent, Transform, TransformAction, BasicTransformEvent, @@ -17,25 +16,6 @@ import { CENTER } from '../constants'; export const NOT_ALLOWED_CURSOR = 'not-allowed'; -/** - * @param {Boolean} alreadySelected true if target is already selected - * @param {String} corner a string representing the corner ml, mr, tl ... - * @param {Event} e Event object - * @param {FabricObject} [target] inserted back to help overriding. Unused - */ -export const getActionFromCorner = ( - alreadySelected: boolean, - corner: string | undefined, - e: TPointerEvent, - target: FabricObject -) => { - if (!corner || !alreadySelected) { - return 'drag'; - } - const control = target.controls[corner]; - return control.getActionName(e, control, target); -}; - /** * Checks if transform is centered * @param {Object} transform transform data diff --git a/test/unit/canvas.js b/test/unit/canvas.js index 73b8d9ba760..bf642fb22e2 100644 --- a/test/unit/canvas.js +++ b/test/unit/canvas.js @@ -1416,7 +1416,7 @@ var target = makeRect(); canvas.add(target); canvas.setActiveObject(target); - canvas._setupCurrentTransform(e, target, true); + canvas.setupCurrentTransform(e, target, target.findControl(new fabric.Point(5,5))); assert.ok(canvas._currentTransform, 'transform should be set'); target.isMoving = true; canvas._discardActiveObject(); @@ -1629,125 +1629,6 @@ assert.equal(canvas.getHeight(), 500, 'Should be as the backstore only value'); }); - QUnit.test('setupCurrentTransform', function(assert) { - assert.ok(typeof canvas._setupCurrentTransform === 'function'); - - var rect = new fabric.Rect({ left: 75, top: 75, width: 50, height: 50 }); - canvas.add(rect); - var canvasOffset = canvas.calcOffset(); - var eventStub = { - clientX: canvasOffset.left + 100, - clientY: canvasOffset.top + 100, - target: canvas.upperCanvasEl - }; - canvas.setActiveObject(rect); - const targetCorner = rect.findControl( - canvas.getViewportPoint(eventStub) - ); - rect.__corner = targetCorner ? targetCorner.key : undefined; - canvas._setupCurrentTransform(eventStub, rect); - var t = canvas._currentTransform; - assert.equal(t.target, rect, 'should have rect as a target'); - assert.equal(t.action, 'drag', 'should target inside rect and setup drag'); - assert.equal(t.corner, 0, 'no corner selected'); - assert.equal(t.originX, rect.originX, 'no origin change for drag'); - assert.equal(t.originY, rect.originY, 'no origin change for drag'); - - eventStub = { - clientX: canvasOffset.left + rect.oCoords.tl.corner.tl.x + 1, - clientY: canvasOffset.top + rect.oCoords.tl.corner.tl.y + 1, - target: canvas.upperCanvasEl - }; - rect.__corner = rect.findControl( - canvas.getViewportPoint(eventStub) - ).key; - canvas._setupCurrentTransform(eventStub, rect, false); - t = canvas._currentTransform; - assert.equal(t.target, rect, 'should have rect as a target'); - assert.equal(t.action, 'drag', 'should setup drag since the object was not selected'); - assert.equal(t.corner, 'tl', 'tl selected'); - assert.equal(t.shiftKey, undefined, 'shift was not pressed'); - - var alreadySelected = true; - rect.__corner = rect.findControl( - canvas.getViewportPoint(eventStub) - ).key; - canvas._setupCurrentTransform(eventStub, rect, alreadySelected); - t = canvas._currentTransform; - assert.equal(t.target, rect, 'should have rect as a target'); - assert.equal(t.action, 'scale', 'should target a corner and setup scale'); - assert.equal(t.corner, 'tl', 'tl selected'); - assert.equal(t.originX, 'right', 'origin in opposite direction'); - assert.equal(t.originY, 'bottom', 'origin in opposite direction'); - assert.equal(t.shiftKey, undefined, 'shift was not pressed'); - - eventStub = { - clientX: canvasOffset.left + rect.left - 2, - clientY: canvasOffset.top + rect.top + rect.height / 2, - target: canvas.upperCanvasEl, - shiftKey: true - }; - rect.__corner = rect.findControl( - canvas.getViewportPoint(eventStub) - ).key; - canvas._setupCurrentTransform(eventStub, rect, alreadySelected); - t = canvas._currentTransform; - assert.equal(t.target, rect, 'should have rect as a target'); - assert.equal(t.action, 'skewY', 'should target a corner and setup skew'); - assert.equal(t.shiftKey, true, 'shift was pressed'); - assert.equal(t.corner, 'ml', 'ml selected'); - assert.equal(t.originX, 'right', 'origin in opposite direction'); - - // to be replaced with new api test - // eventStub = { - // clientX: canvasOffset.left + rect.oCoords.mtr.x, - // clientY: canvasOffset.top + rect.oCoords.mtr.y, - // target: canvas.upperCanvasEl, - // }; - // canvas._setupCurrentTransform(eventStub, rect, alreadySelected); - // t = canvas._currentTransform; - // assert.equal(t.target, rect, 'should have rect as a target'); - // assert.equal(t.action, 'mtr', 'should target a corner and setup rotate'); - // assert.equal(t.corner, 'mtr', 'mtr selected'); - // assert.equal(t.originX, 'center', 'origin in center'); - // assert.equal(t.originY, 'center', 'origin in center'); - // canvas._currentTransform = false; - }); - - // QUnit.test('_rotateObject', function(assert) { - // assert.ok(typeof canvas._rotateObject === 'function'); - // var rect = new fabric.Rect({ left: 75, top: 75, width: 50, height: 50 }); - // canvas.add(rect); - // var canvasEl = canvas.getElement(), - // canvasOffset = fabric.util.getElementOffset(canvasEl); - // var eventStub = { - // clientX: canvasOffset.left + rect.oCoords.mtr.x, - // clientY: canvasOffset.top + rect.oCoords.mtr.y, - // target: canvas.upperCanvasEl, - // }; - // canvas._setupCurrentTransform(eventStub, rect); - // var rotated = canvas._rotateObject(30, 30, 'equally'); - // assert.equal(rotated, true, 'return true if a rotation happened'); - // rotated = canvas._rotateObject(30, 30); - // assert.equal(rotated, false, 'return true if no rotation happened'); - // }); - // - // QUnit.test('_rotateObject do not change origins', function(assert) { - // assert.ok(typeof canvas._rotateObject === 'function'); - // var rect = new fabric.Rect({ left: 75, top: 75, width: 50, height: 50, originX: 'right', originY: 'bottom' }); - // canvas.add(rect); - // var canvasEl = canvas.getElement(), - // canvasOffset = fabric.util.getElementOffset(canvasEl); - // var eventStub = { - // clientX: canvasOffset.left + rect.oCoords.mtr.x, - // clientY: canvasOffset.top + rect.oCoords.mtr.y, - // target: canvas.upperCanvasEl, - // }; - // canvas._setupCurrentTransform(eventStub, rect); - // assert.equal(rect.originX, 'right'); - // assert.equal(rect.originY, 'bottom'); - // }); - QUnit.skip('fxRemove', function(assert) { var done = assert.async(); assert.ok(typeof canvas.fxRemove === 'function');