feat(LayoutManager): BREAKING remove shouldResetTransform logic from layoutManager

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## [next]
 
+- BREAKING BETA(LayoutManager): rm `shouldResetTransform` [#9581](https://github.com/fabricjs/fabric.js/pull/9581)
 - refactor(): rm active selection ref [#9561](https://github.com/fabricjs/fabric.js/pull/9561)
 - feat(Next.js sandbox): simpler canvas hook [#9577](https://github.com/fabricjs/fabric.js/pull/9577)
 - fix(): fix modify polygon points with zero sized polygons ( particular case of axis oriented lines ) [#9575](https://github.com/fabricjs/fabric.js/pull/9575)

src/LayoutManager/LayoutManager.spec.ts

@@ -305,19 +305,27 @@ describe('Layout Manager', () => {
         relativeCorrection: new Point(-30, -40),
       });
 
-      const target = new Group([], { scaleX: 2, scaleY: 0.5, angle: 30 });
+      const rect = new FabricObject({ width: 50, height: 50 });
+      const target = new Group([rect], { scaleX: 2, scaleY: 0.5, angle: 30 });
 
       const context: StrictLayoutContext = {
         bubbles: true,
         strategy: manager.strategy,
         target,
+        targets: [rect],
         ...options,
         stopPropagation() {
           this.bubbles = false;
         },
       };
 
-      expect(manager['getLayoutResult'](context)).toMatchSnapshot();
+      expect(manager['getLayoutResult'](context)).toMatchSnapshot({
+        cloneDeepWith: (value: any) => {
+          if (value instanceof Point) {
+            return new Point(Math.round(value.x), Math.round(value.y));
+          }
+        },
+      });
     });
   });
 
@@ -475,12 +483,6 @@ describe('Layout Manager', () => {
         const canvasFire = jest.fn();
         target.canvas = { fire: canvasFire };
 
-        const shouldResetTransform = jest
-          .spyOn(manager.strategy, 'shouldResetTransform')
-          .mockImplementation(() => {
-            lifecycle.push(shouldResetTransform);
-          });
-
         const context: StrictLayoutContext = {
           bubbles,
           strategy: manager.strategy,
@@ -501,11 +503,9 @@ describe('Layout Manager', () => {
         manager['onAfterLayout'](context, layoutResult);
 
         expect(lifecycle).toEqual([
-          shouldResetTransform,
           targetFire,
           ...(bubbles ? [parentPerformLayout] : []),
         ]);
-        expect(shouldResetTransform).toBeCalledWith(context);
         expect(targetFire).toBeCalledWith('layout:after', {
           context,
           result: layoutResult,
@@ -527,34 +527,6 @@ describe('Layout Manager', () => {
       }
     );
 
-    test.each([true, false])('reset target transform %s', (reset) => {
-      const targets = [new Group([new FabricObject()]), new FabricObject()];
-      const target = new Group(targets);
-      target.left = 50;
-
-      const manager = new LayoutManager();
-      jest
-        .spyOn(manager.strategy, 'shouldResetTransform')
-        .mockImplementation(() => {
-          return reset;
-        });
-
-      const context: StrictLayoutContext = {
-        bubbles: true,
-        strategy: manager.strategy,
-        type: LAYOUT_TYPE_REMOVED,
-        target,
-        targets,
-        prevStrategy: undefined,
-        stopPropagation() {
-          this.bubbles = false;
-        },
-      };
-      manager['onAfterLayout'](context);
-
-      expect(target.left).toBe(reset ? 0 : 50);
-    });
-
     test('bubbling', () => {
       const manager = new LayoutManager();
       const manager2 = new LayoutManager();

src/LayoutManager/LayoutManager.ts

@@ -4,7 +4,6 @@ import { CENTER, iMatrix } from '../constants';
 import type { Group } from '../shapes/Group';
 import type { FabricObject } from '../shapes/Object/FabricObject';
 import { invertTransform } from '../util/misc/matrix';
-import { resetObjectTransform } from '../util/misc/objectTransforms';
 import { resolveOrigin } from '../util/misc/resolveOrigin';
 import { FitContentLayout } from './LayoutStrategies/FitContentLayout';
 import type { LayoutStrategy } from './LayoutStrategies/LayoutStrategy';
@@ -262,11 +261,6 @@ export class LayoutManager {
       ...bubblingContext
     } = context;
     const { canvas } = target;
-    if (strategy.shouldResetTransform(context)) {
-      resetObjectTransform(target);
-      target.left = 0;
-      target.top = 0;
-    }
 
     //  fire layout event (event will fire only for layouts after initialization layout)
     target.fire('layout:after', {

src/LayoutManager/LayoutStrategies/LayoutStrategy.ts

@@ -61,13 +61,6 @@ export abstract class LayoutStrategy {
     return result.size;
   }
 
-  /**
-   * called from the `onAfterLayout` hook
-   */
-  shouldResetTransform(context: StrictLayoutContext) {
-    return context.target.size() === 0;
-  }
-
   /**
    * Override this method to customize layout.
    */

src/LayoutManager/__snapshots__/LayoutManager.spec.ts.snap

@@ -7,12 +7,12 @@ exports[`Layout Manager getLayoutResult imperative trigger 1`] = `
     "y": 100,
   },
   "offset": Point {
-    "x": -70,
-    "y": -120,
+    "x": -42,
+    "y": -113,
   },
   "prevCenter": Point {
-    "x": 0,
-    "y": 0,
+    "x": 38,
+    "y": 37,
   },
   "result": {
     "center": Point {

src/shapes/ActiveSelection.spec.ts

@@ -13,36 +13,7 @@ describe('ActiveSelection', () => {
     );
   });
 
-  it('clearing active selection objects resets transform', () => {
-    const obj = new FabricObject({
-      left: 100,
-      top: 100,
-      width: 100,
-      height: 100,
-    });
-    const selection = new ActiveSelection([obj], {
-      left: 200,
-      top: 200,
-      angle: 45,
-      skewX: 0.5,
-      skewY: -0.5,
-    });
-    selection.remove(obj);
-    expect(selection).toMatchObject({
-      left: 0,
-      top: 0,
-      angle: 0,
-      scaleX: 1,
-      scaleY: 1,
-      skewX: 0,
-      skewY: 0,
-      flipX: false,
-      flipY: false,
-      _objects: [],
-    });
-  });
-
-  it('deselect removes all objects and resets transform', () => {
+  it('deselect removes all objects', () => {
     const selection = new ActiveSelection([], {
       left: 200,
       top: 100,
@@ -52,9 +23,9 @@ describe('ActiveSelection', () => {
     selection.onDeselect();
     expect(spy).toHaveBeenCalled();
     expect(selection).toMatchObject({
-      left: 0,
-      top: 0,
-      angle: 0,
+      left: 200,
+      top: 100,
+      angle: 45,
       scaleX: 1,
       scaleY: 1,
       skewX: 0,
@@ -64,11 +35,11 @@ describe('ActiveSelection', () => {
       _objects: [],
     });
     selection.add(new FabricObject({ left: 50, top: 50, strokeWidth: 0 }));
-    expect(selection.item(0).getCenterPoint()).toEqual({ x: 50, y: 50 });
+    const { x, y } = selection.item(0).getCenterPoint();
+    expect({ x: Math.round(x), y: Math.round(y) }).toEqual({ x: 50, y: 50 });
   });
 
-  // remove skip once #9152 is merged
-  it.skip('should not set coords in the constructor', () => {
+  it('should not set coords in the constructor', () => {
     const spy = jest.spyOn(ActiveSelection.prototype, 'setCoords');
     new ActiveSelection([
       new FabricObject({

src/shapes/ActiveSelection.ts

@@ -146,9 +146,7 @@ export class ActiveSelection extends Group {
   }
 
   /**
-   * If returns true, deselection is cancelled.
-   * @since 2.0.0
-   * @return {Boolean} [cancel]
+   * @override remove all objects
    */
   onDeselect() {
     this.removeAll();