feat(LayoutManager): Handle the case of activeSelection with objects inside different groups

.codesandbox/templates/vanilla/src/index.ts

@@ -1,6 +1,6 @@
 import * as fabric from 'fabric';
 import './styles.css';
-import { testCase } from './testcases/fixedLayoutGroup';
+import { testCase } from './testcases/nestedSelections';
 
 const el = document.getElementById('canvas');
 const canvas = (window.canvas = new fabric.Canvas(el));

.codesandbox/templates/vanilla/src/testcases/nestedSelections.ts

@@ -28,6 +28,12 @@ export function testCase(canvas: fabric.Canvas, objectCaching = true) {
     }
   );
   canvas.add(text, g);
+  g.clone().then((clone) => {
+    clone.set({
+      backgroundColor: 'red',
+    });
+    canvas.add(clone);
+  });
   g.clone().then((clone) => {
     clone.item(2).set({ text: 'Edit me\nclip path layout' });
     clone.set({
@@ -67,7 +73,7 @@ export function testCase(canvas: fabric.Canvas, objectCaching = true) {
       .set({ text: 'Edit me\nabsolute positioned\nclip path layout' });
     clone.set({
       objectCaching,
-      backgroundColor: '#0dcaf0',
+      backgroundColor: 'cyan',
       clipPath: new fabric.Circle({
         radius: 110,
         originX: 'center',

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## [next]
 
+- feat(LayoutManager): Handle the case of activeSelection with objects inside different groups [#9651](https://github.com/fabricjs/fabric.js/pull/9651)
+
 ## [6.0.0-beta20]
 
 - chore(TS): minor changes to typescript notation to be compatible with a 5.3.3 [#9725](https://github.com/fabricjs/fabric.js/pull/9725)

e2e/tests/selection/multi-selection-in-groups/index.spec.ts

@@ -0,0 +1,32 @@
+import { expect, test } from '@playwright/test';
+import setup from '../../../setup';
+import { CanvasUtil } from '../../../utils/CanvasUtil';
+import { ObjectUtil } from '../../../utils/ObjectUtil';
+import type { Circle } from 'fabric';
+
+setup();
+
+test('Activeselection across interactive groups', async ({ page }) => {
+  const canvasUtil = new CanvasUtil(page);
+  const circle1Util = new ObjectUtil<Circle>(page, 'circle1');
+  const circle2Util = new ObjectUtil<Circle>(page, 'circle2');
+  const circle3Util = new ObjectUtil<Circle>(page, 'circle3');
+  const circle4Util = new ObjectUtil<Circle>(page, 'circle4');
+  expect(await canvasUtil.screenshot()).toMatchSnapshot({
+    name: 'initial-layout.png',
+  });
+  await canvasUtil.makeActiveSelectionWith([
+    circle1Util,
+    circle2Util,
+    circle3Util,
+    circle4Util,
+  ]);
+  expect(await canvasUtil.screenshot()).toMatchSnapshot({
+    name: 'initial-layout-with-activeSelection.png',
+  });
+  const { x, y } = await circle1Util.getObjectCenter();
+  await canvasUtil.clickAndDrag({ x, y }, { x: 30, y: y + 100 }, 80);
+  expect(await canvasUtil.screenshot()).toMatchSnapshot({
+    name: 'after-moving-objects.png',
+  });
+});

e2e/tests/selection/multi-selection-in-groups/index.ts

@@ -0,0 +1,76 @@
+/**
+ * Runs in the **BROWSER**
+ * Imports are defined in 'e2e/imports.ts'
+ */
+
+import { Rect, Circle, Group, ClipPathLayout } from 'fabric';
+import { beforeAll } from '../../test';
+
+beforeAll(async (canvas) => {
+  canvas.setDimensions({ width: 450, height: 450 });
+
+  canvas.preserveObjectStacking = true;
+  const circle1 = new Circle({ left: 100, top: 50, radius: 50 });
+  const g = new Group(
+    [
+      new Rect({
+        top: 200,
+        width: 50,
+        height: 50,
+        fill: 'red',
+        opacity: 0.3,
+      }),
+      circle1,
+    ],
+    {
+      backgroundColor: 'blue',
+      subTargetCheck: true,
+      interactive: true,
+    }
+  );
+  canvas.add(g);
+  const clone1 = await g.clone();
+  clone1.set({
+    top: clone1.top + 200,
+    left: clone1.left + 200,
+    backgroundColor: 'red',
+  });
+  const circle2 = clone1.item(1);
+  canvas.add(clone1);
+  const clone3 = await g.clone();
+  clone3.set({
+    top: clone3.top + 200,
+    backgroundColor: 'yellow',
+    clipPath: new Circle({
+      radius: 110,
+      originX: 'center',
+      originY: 'center',
+      group: clone3,
+    }),
+  });
+  clone3.layoutManager.strategy = new ClipPathLayout();
+  clone3.triggerLayout();
+  const circle3 = clone3.item(1);
+  canvas.add(clone3);
+
+  const clone4 = await g.clone();
+  clone4.set({
+    left: clone4.left + 200,
+    backgroundColor: 'cyan',
+    clipPath: new Circle({
+      radius: 110,
+      originX: 'center',
+      originY: 'center',
+      absolutePositioned: true,
+      left: 250,
+      top: 150,
+      skewX: 20,
+    }),
+  });
+  clone4.layoutManager.strategy = new ClipPathLayout();
+  clone4.triggerLayout();
+  const circle4 = clone4.item(1);
+  canvas.insertAt(0, clone4);
+
+  return { circle1, circle2, circle3, circle4 };
+});

e2e/utils/CanvasUtil.ts

@@ -1,7 +1,8 @@
 import type { JSHandle } from '@playwright/test';
 import { type LocatorScreenshotOptions, type Page } from '@playwright/test';
-import type { Canvas } from 'fabric';
+import type { Canvas, XY } from 'fabric';
 import os from 'node:os';
+import type { ObjectUtil } from './ObjectUtil';
 
 export class CanvasUtil {
   constructor(readonly page: Page, readonly selector = '#canvas') {}
@@ -10,6 +11,28 @@ export class CanvasUtil {
     return this.page.click(`canvas_top=${this.selector}`, clickProperties);
   }
 
+  async makeActiveSelectionWith(objects: ObjectUtil[]) {
+    for (const objectUtil of objects) {
+      this.page.click(`canvas_top=${this.selector}`, {
+        modifiers: ['Shift'],
+        position: await objectUtil.getObjectCenter(),
+      });
+    }
+  }
+
+  async clickAndDrag(point: XY, dragTo: XY, steps = 20) {
+    await this.page.mouse.move(point.x, point.y);
+    await this.page.mouse.down({
+      button: 'left',
+    });
+    await this.page.mouse.move(dragTo.x, dragTo.y, {
+      steps,
+    });
+    await this.page.mouse.up({
+      button: 'left',
+    });
+  }
+
   press(keyPressed: Parameters<Page['press']>[1]) {
     return this.page.keyboard.press(keyPressed, { delay: 200 });
   }

src/LayoutManager/ActiveSelectionLayoutManager.spec.ts

@@ -0,0 +1,148 @@
+import type { TModificationEvents } from '../EventTypeDefs';
+import { ActiveSelection } from '../shapes/ActiveSelection';
+import { Group } from '../shapes/Group';
+import { FabricObject } from '../shapes/Object/FabricObject';
+import { ActiveSelectionLayoutManager } from './ActiveSelectionLayoutManager';
+
+describe('ActiveSelectionLayoutManager', () => {
+  describe('onBeforeLayout', () => {
+    describe('triggers', () => {
+      const triggers: ('modified' | TModificationEvents | 'changed')[] = [
+        'modified',
+        'moving',
+        'resizing',
+        'rotating',
+        'scaling',
+        'skewing',
+        'changed',
+        'modifyPoly',
+      ];
+
+      it('should subscribe activeSelection that contains object', () => {
+        const manager = new ActiveSelectionLayoutManager();
+        const object = new FabricObject();
+        const group = new Group([object], {
+          interactive: true,
+          subTargetCheck: true,
+        });
+        const as = new ActiveSelection([object], { layoutManager: manager });
+        const objectOn = jest.spyOn(object, 'on');
+        const objectOff = jest.spyOn(object, 'off');
+        const asOn = jest.spyOn(as, 'on');
+        manager.subscribeTargets({
+          targets: [object],
+          target: as,
+        });
+        expect(objectOn).not.toHaveBeenCalled();
+        expect(objectOff).not.toHaveBeenCalled();
+        expect(asOn).toHaveBeenCalledTimes(triggers.length);
+        expect(objectOff).not.toHaveBeenCalled();
+      });
+
+      it('a subscribed activeSelection should trigger layout on the object parent once per parent', () => {
+        const manager = new ActiveSelectionLayoutManager();
+        const object = new FabricObject();
+        const object2 = new FabricObject();
+        const object3 = new FabricObject();
+        const object4 = new FabricObject();
+        const group = new Group([object, object2], {
+          interactive: true,
+          subTargetCheck: true,
+        });
+        const group2 = new Group([object3, object4], {
+          interactive: true,
+          subTargetCheck: true,
+        });
+        const as = new ActiveSelection([object, object2, object3, object4], {
+          layoutManager: manager,
+        });
+        const asPerformLayout = jest.spyOn(manager, 'performLayout');
+        const groupPerformLayout = jest.spyOn(
+          group.layoutManager,
+          'performLayout'
+        );
+        const groupPerformLayout2 = jest.spyOn(
+          group2.layoutManager,
+          'performLayout'
+        );
+        groupPerformLayout.mockClear();
+        groupPerformLayout2.mockClear();
+        asPerformLayout.mockClear();
+        expect(group.layoutManager['_subscriptions'].get(as)).toBeDefined();
+        expect(group2.layoutManager['_subscriptions'].get(as)).toBeDefined();
+        expect(manager['_subscriptions'].size).toBe(0);
+
+        const event = { foo: 'bar' };
+        triggers.forEach((trigger) => as.fire(trigger, event));
+        expect(asPerformLayout).not.toHaveBeenCalled();
+        expect(groupPerformLayout.mock.calls).toMatchObject([
+          [
+            {
+              e: event,
+              target: group,
+              trigger: 'modified',
+              type: 'object_modified',
+            },
+          ],
+          ...triggers.slice(1).map((trigger) => [
+            {
+              e: event,
+              target: group,
+              trigger,
+              type: 'object_modifying',
+            },
+          ]),
+        ]);
+        expect(groupPerformLayout).toHaveBeenCalledTimes(triggers.length);
+        expect(groupPerformLayout2).toHaveBeenCalledTimes(triggers.length);
+
+        as.remove(object);
+        groupPerformLayout.mockClear();
+        groupPerformLayout2.mockClear();
+        asPerformLayout.mockClear();
+
+        triggers.forEach((trigger) => as.fire(trigger, event));
+        expect(asPerformLayout).not.toHaveBeenCalled();
+        expect(groupPerformLayout.mock.calls).toMatchObject([
+          [
+            {
+              e: event,
+              target: group,
+              trigger: 'modified',
+              type: 'object_modified',
+            },
+          ],
+          ...triggers.slice(1).map((trigger) => [
+            {
+              e: event,
+              target: group,
+              trigger,
+              type: 'object_modifying',
+            },
+          ]),
+        ]);
+        expect(groupPerformLayout).toHaveBeenCalledTimes(triggers.length);
+        expect(groupPerformLayout2).toHaveBeenCalledTimes(triggers.length);
+
+        groupPerformLayout.mockClear();
+        groupPerformLayout2.mockClear();
+        asPerformLayout.mockClear();
+
+        as.remove(object2);
+        expect(group.layoutManager['_subscriptions'].get(as)).toBeUndefined();
+        expect(group2.layoutManager['_subscriptions'].get(as)).toBeDefined();
+        as.removeAll();
+        expect(group2.layoutManager['_subscriptions'].get(as)).toBeUndefined();
+
+        groupPerformLayout.mockClear();
+        groupPerformLayout2.mockClear();
+        asPerformLayout.mockClear();
+
+        triggers.forEach((trigger) => as.fire(trigger, event));
+        expect(groupPerformLayout).not.toHaveBeenCalled();
+        expect(groupPerformLayout2).not.toHaveBeenCalled();
+        expect(asPerformLayout).not.toHaveBeenCalled();
+      });
+    });
+  });
+});

src/LayoutManager/ActiveSelectionLayoutManager.ts

@@ -0,0 +1,54 @@
+import { LayoutManager } from './LayoutManager';
+import type { RegistrationContext, StrictLayoutContext } from './types';
+import type { Group } from '../shapes/Group';
+
+/**
+ * Today the LayoutManager class also takes care of subscribing event handlers
+ * to update the group layout when the group is interactive and a transform is applied
+ * to a child object.
+ * The ActiveSelection is never interactive, but it could contain objects from
+ * groups that are.
+ * The standard LayoutManager would subscribe the children of the activeSelection to
+ * perform layout changes to the active selection itself, what we need instead is that
+ * the transformation applied to the active selection will trigger changes to the
+ * original group of the children ( the one referenced under the parent property )
+ * This subclass of the LayoutManager has a single duty to fill the gap of this difference.`
+ */
+export class ActiveSelectionLayoutManager extends LayoutManager {
+  subscribeTargets(
+    context: RegistrationContext & Partial<StrictLayoutContext>
+  ): void {
+    const activeSelection = context.target;
+    const parents = context.targets.reduce((parents, target) => {
+      target.parent && parents.add(target.parent);
+      return parents;
+    }, new Set<Group>());
+    parents.forEach((parent) => {
+      parent.layoutManager.subscribeTargets({
+        target: parent,
+        targets: [activeSelection],
+      });
+    });
+  }
+
+  /**
+   * unsubscribe from parent only if all its children were deselected
+   */
+  unsubscribeTargets(
+    context: RegistrationContext & Partial<StrictLayoutContext>
+  ): void {
+    const activeSelection = context.target;
+    const selectedObjects = activeSelection.getObjects();
+    const parents = context.targets.reduce((parents, target) => {
+      target.parent && parents.add(target.parent);
+      return parents;
+    }, new Set<Group>());
+    parents.forEach((parent) => {
+      !selectedObjects.some((object) => object.parent === parent) &&
+        parent.layoutManager.unsubscribeTargets({
+          target: parent,
+          targets: [activeSelection],
+        });
+    });
+  }
+}