refactor(#9498): refactor endCurrentTransform

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## [next]
 
+- fix(#9498): exception due to wrong type, refactor endCurrentTransform [#9500](https://github.com/fabricjs/fabric.js/pull/9500)
 - fix(StaticCanvas): StaticCanvas setDimensions typings [#9618](https://github.com/fabricjs/fabric.js/pull/9618)
 - refactor(): Align gradient with class registry usage, part of #9144 [#9627](https://github.com/fabricjs/fabric.js/pull/9627)
 - refactor(): Align shadow with class registry, part of #9144 [#9626](https://github.com/fabricjs/fabric.js/pull/9626)

src/EventTypeDefs.ts

@@ -106,6 +106,7 @@ export type TModificationEvents =
 
 export interface ModifiedEvent<E extends Event = TPointerEvent> {
   e?: E;
+  aborted: boolean;
   transform: Transform;
   target: FabricObject;
   action?: string;

src/canvas/Canvas.ts

@@ -798,7 +798,7 @@ export class Canvas extends SelectableCanvas implements CanvasOptions {
     }
     let shouldRender = false;
     if (transform) {
-      this._finalizeCurrentTransform(e);
+      this.endCurrentTransform(e, false);
       shouldRender = transform.actionPerformed;
     }
     if (!isClick) {
@@ -832,7 +832,6 @@ export class Canvas extends SelectableCanvas implements CanvasOptions {
           mouseUpHandler.call(control, e, transform!, pointer.x, pointer.y);
         }
       }
-      target.isMoving = false;
     }
     // if we are ending up a transform on a different control or a new object
     // fire the original mouse up from the corner that started the transform
@@ -860,9 +859,8 @@ export class Canvas extends SelectableCanvas implements CanvasOptions {
         );
     }
     this._setCursorFromEvent(e, target);
-    this._handleEvent(e, 'up');
+    this._handleEvent(e, 'up', { ...this.createEventData(e, 'up'), transform });
     this._groupSelector = null;
-    this._currentTransform = null;
     // reset the target information about which corner is selected
     target && (target.__corner = undefined);
     if (shouldRender) {
@@ -888,35 +886,47 @@ export class Canvas extends SelectableCanvas implements CanvasOptions {
     return options;
   }
 
+  protected createEventData<T extends TPointerEventNames>(
+    e: TPointerEvent,
+    eventType: T
+  ) {
+    const target = this._target;
+    const subTargets = this.targets || [];
+    return {
+      e,
+      target,
+      subTargets,
+      ...getEventPoints(this, e),
+      transform: this._currentTransform,
+      ...(eventType === 'up:before' || eventType === 'up'
+        ? {
+            isClick: this._isClick,
+            currentTarget: this.findTarget(e),
+            // set by the preceding `findTarget` call
+            currentSubTargets: this.targets,
+          }
+        : {}),
+    } as CanvasEvents[`mouse:${T}`];
+  }
+
   /**
    * @private
    * Handle event firing for target and subtargets
    * @param {TPointerEvent} e event from mouse
    * @param {TPointerEventNames} eventType
    */
-  _handleEvent<T extends TPointerEventNames>(e: TPointerEvent, eventType: T) {
-    const target = this._target,
-      targets = this.targets || [],
-      options: CanvasEvents[`mouse:${T}`] = {
-        e,
-        target,
-        subTargets: targets,
-        ...getEventPoints(this, e),
-        transform: this._currentTransform,
-        ...(eventType === 'up:before' || eventType === 'up'
-          ? {
-              isClick: this._isClick,
-              currentTarget: this.findTarget(e),
-              // set by the preceding `findTarget` call
-              currentSubTargets: this.targets,
-            }
-          : {}),
-      } as CanvasEvents[`mouse:${T}`];
-    this.fire(`mouse:${eventType}`, options);
+  _handleEvent<T extends TPointerEventNames>(
+    e: TPointerEvent,
+    eventType: T,
+    eventData = this.createEventData(e, eventType)
+  ) {
+    const { target, subTargets = [] } = eventData;
+    this.fire(`mouse:${eventType}`, eventData);
     // this may be a little be more complicated of what we want to handle
-    target && target.fire(`mouse${eventType}`, options);
-    for (let i = 0; i < targets.length; i++) {
-      targets[i] !== target && targets[i].fire(`mouse${eventType}`, options);
+    target && target.fire(`mouse${eventType}`, eventData);
+    for (let i = 0; i < subTargets.length; i++) {
+      subTargets[i] !== target &&
+        subTargets[i].fire(`mouse${eventType}`, eventData);
     }
   }
 
@@ -1315,7 +1325,6 @@ export class Canvas extends SelectableCanvas implements CanvasOptions {
       actionPerformed = actionHandler(e, transform, x, y);
     }
     if (action === 'drag' && actionPerformed) {
-      transform.target.isMoving = true;
       this.setCursor(transform.target.moveCursor || this.moveCursor);
     }
     transform.actionPerformed = transform.actionPerformed || actionPerformed;

src/canvas/SelectableCanvas.ts

@@ -4,6 +4,7 @@
 import { FabricObject } from '../shapes/Object/FabricObject';
 import type {
   CanvasEvents,
+  ModifiedEvent,
   ModifierKey,
   TOptionalModifierKey,
   TPointerEvent,
@@ -571,7 +572,6 @@
    * @private
    * @param {Event} e Event object
    * @param {FabricObject} target
-   * @param {boolean} [alreadySelected] pass true to setup the active control
    */
   _setupCurrentTransform(
     e: TPointerEvent,
@@ -638,6 +638,35 @@
     });
   }
 
+  /**
+   * @private
+   * @param {Event} e send the mouse event that generate the finalize down, so it can be used in the event
+   * @param {boolean} aborted flag passed down to the `modified` event indicating if the interaction was aborted
+   */
+  endCurrentTransform(e?: TPointerEvent, aborted = true) {
+    const transform = this._currentTransform;
+    if (!transform) {
+      return;
+    }
+
+    const { target, action, actionPerformed } = transform;
+    const options: ModifiedEvent = {
+      e,
+      aborted,
+      target,
+      transform,
+      action,
+    };
+
+    target.setCoords();
+    this._currentTransform = null;
+
+    if (actionPerformed) {
+      this.fire('object:modified', options);
+      target.fire('modified', options);
+    }
+  }
+
   /**
    * Set the cursor type of the canvas element
    * @param {String} value Cursor type of the canvas element.
@@ -1157,7 +1186,7 @@
         return false;
       }
       if (this._currentTransform && this._currentTransform.target === obj) {
-        this.endCurrentTransform(e);
+        this.endCurrentTransform(e, true);
       }
       this._activeObject = undefined;
       return true;
@@ -1193,7 +1222,7 @@
    * because of some other event ( a press of key combination, or something that block the user UX )
    * @param {Event} [e] send the mouse event that generate the finalize down, so it can be used in the event
    */
   endCurrentTransform(e?: TPointerEvent) {
     const transform = this._currentTransform;
     this._finalizeCurrentTransform(e);
     if (transform && transform.target) {

src/canvas/canvas_gestures.mixin.ts

@@ -153,9 +153,7 @@ Object.assign(Canvas.prototype, {
    */
   _scaleObjectBy: function (s, e) {
     const t = this._currentTransform;
-    const target = t.target;
     t.gestureScale = s;
-    target._scaling = true;
     return scalingEqually(e, t, 0, 0);
   },
 

src/shapes/ActiveSelection.ts

@@ -193,7 +193,9 @@ export class ActiveSelection extends Group {
     childrenOverride?: ControlRenderingStyleOverride
   ) {
     ctx.save();
-    ctx.globalAlpha = this.isMoving ? this.borderOpacityWhenMoving : 1;
+    ctx.globalAlpha = this.isTransforming('drag')
+      ? this.borderOpacityWhenMoving
+      : 1;
     super._renderControls(ctx, styleOverride);
     const options = {
       hasControls: false,

src/shapes/Image.ts

@@ -586,7 +586,11 @@ export class FabricImage<
    */
   _render(ctx: CanvasRenderingContext2D) {
     ctx.imageSmoothingEnabled = this.imageSmoothing;
-    if (this.isMoving !== true && this.resizeFilter && this._needsResize()) {
+    if (
+      !this.isTransforming('drag') &&
+      this.resizeFilter &&
+      this._needsResize()
+    ) {
       this.applyResizeFilters();
     }
     this._stroke(ctx);

src/shapes/Object/InteractiveObject.ts

@@ -116,22 +116,6 @@ export class InteractiveFabricObject<
    */
   declare controls: TControlSet;
 
-  /**
-   * internal boolean to signal the code that the object is
-   * part of the move action.
-   */
-  declare isMoving?: boolean;
-
-  /**
-   * A boolean used from the gesture module to keep tracking of a scaling
-   * action when there is no scaling transform in place.
-   * This is an edge case and is used twice in all codebase.
-   * Probably added to keep track of some performance issues
-   * @TODO use git blame to investigate why it was added
-   * DON'T USE IT. WE WILL TRY TO REMOVE IT
-   */
-  declare _scaling?: boolean;
-
   declare canvas?: Canvas;
 
   static ownDefaults: Record<string, any> = interactiveObjectDefaultValues;
@@ -178,6 +162,13 @@ export class InteractiveFabricObject<
       : undefined;
   }
 
+  isTransforming(action: string) {
+    const transform = this.canvas?._currentTransform;
+    return (
+      transform && transform.action === action && transform.actionPerformed
+    );
+  }
+
   /**
    * Determines which corner is under the mouse cursor, represented by `pointer`.
    * This function is return a corner only if the object is the active one.
@@ -415,7 +406,9 @@ export class InteractiveFabricObject<
     ctx.translate(options.translateX, options.translateY);
     ctx.lineWidth = 1 * this.borderScaleFactor;
     if (!this.group) {
-      ctx.globalAlpha = this.isMoving ? this.borderOpacityWhenMoving : 1;
+      ctx.globalAlpha = this.isTransforming('drag')
+        ? this.borderOpacityWhenMoving
+        : 1;
     }
     if (this.flipX) {
       options.angle -= 180;

test/unit/canvas.js

@@ -1418,10 +1418,8 @@
     canvas.setActiveObject(target);
     canvas._setupCurrentTransform(e, target, true);
     assert.ok(canvas._currentTransform, 'transform should be set');
-    target.isMoving = true;
     canvas._discardActiveObject();
     assert.ok(!canvas._currentTransform, 'transform should be cleared');
-    assert.ok(!target.isMoving, 'moving flag should have been negated');
     assert.equal(canvas.getActiveObject(), null);
   });