diff --git a/CHANGELOG.md b/CHANGELOG.md index c68cb4a3362..9d322dac44e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [next] +- fix(): avoid resetting Group transform by default [#9525](https://github.com/fabricjs/fabric.js/pull/9525) - refactor(): Change how LayoutManager handles restoring groups [#9522](https://github.com/fabricjs/fabric.js/pull/9522) - fix(BaseConfiguration): set `devicePixelRatio` from window [#9470](https://github.com/fabricjs/fabric.js/pull/9470) - fix(): bubble dirty flag for group only when true [#9540](https://github.com/fabricjs/fabric.js/pull/9540) diff --git a/src/LayoutManager/LayoutManager.spec.ts b/src/LayoutManager/LayoutManager.spec.ts index 4209cfa2fb8..1fd2321a26e 100644 --- a/src/LayoutManager/LayoutManager.spec.ts +++ b/src/LayoutManager/LayoutManager.spec.ts @@ -474,10 +474,10 @@ describe('Layout Manager', () => { const canvasFire = jest.fn(); target.canvas = { fire: canvasFire }; - const shouldResetTransform = jest - .spyOn(manager.strategy, 'shouldResetTransform') + const onAfterLayout = jest + .spyOn(manager.strategy, 'onAfterLayout') .mockImplementation(() => { - lifecycle.push(shouldResetTransform); + lifecycle.push(onAfterLayout); }); const context: StrictLayoutContext = { @@ -500,11 +500,11 @@ describe('Layout Manager', () => { manager['onAfterLayout'](context, layoutResult); expect(lifecycle).toEqual([ - shouldResetTransform, + onAfterLayout, targetFire, ...(bubbles ? [parentPerformLayout] : []), ]); - expect(shouldResetTransform).toBeCalledWith(context); + expect(onAfterLayout).toBeCalledWith(context); expect(targetFire).toBeCalledWith('layout:after', { context, result: layoutResult, @@ -526,24 +526,21 @@ 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); + test.each([ + { strategy: new FitContentLayout(), shouldReset: true }, + { strategy: new FixedLayout(), shouldReset: false }, + ])('reset target transform %s', ({ strategy, shouldReset }) => { + const target = new Group([]); target.left = 50; - const manager = new LayoutManager(); - jest - .spyOn(manager.strategy, 'shouldResetTransform') - .mockImplementation(() => { - return reset; - }); + const manager = new LayoutManager(strategy); const context: StrictLayoutContext = { bubbles: true, strategy: manager.strategy, type: LAYOUT_TYPE_REMOVED, target, - targets, + targets: [], prevStrategy: undefined, stopPropagation() { this.bubbles = false; @@ -551,7 +548,7 @@ describe('Layout Manager', () => { }; manager['onAfterLayout'](context); - expect(target.left).toBe(reset ? 0 : 50); + expect(target.left).toBe(shouldReset ? 0 : 50); }); test('bubbling', () => { diff --git a/src/LayoutManager/LayoutManager.ts b/src/LayoutManager/LayoutManager.ts index c2de920f0b1..0c2bbf3b0f3 100644 --- a/src/LayoutManager/LayoutManager.ts +++ b/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'; @@ -101,8 +100,11 @@ export class LayoutManager { } protected onBeforeLayout(context: StrictLayoutContext) { - const { target } = context; + const { target, strategy } = context; const { canvas } = target; + + strategy.onBeforeLayout(context); + // handle layout triggers subscription if ( // subscribe only if instance is the target's `layoutManager` @@ -253,11 +255,7 @@ export class LayoutManager { ...bubblingContext } = context; const { canvas } = target; - if (strategy.shouldResetTransform(context)) { - resetObjectTransform(target); - target.left = 0; - target.top = 0; - } + strategy.onAfterLayout(context); // fire layout event (event will fire only for layouts after initialization layout) target.fire('layout:after', { diff --git a/src/LayoutManager/LayoutStrategies/FitContentLayout.ts b/src/LayoutManager/LayoutStrategies/FitContentLayout.ts index f90d443684e..d2569950b14 100644 --- a/src/LayoutManager/LayoutStrategies/FitContentLayout.ts +++ b/src/LayoutManager/LayoutStrategies/FitContentLayout.ts @@ -1,3 +1,4 @@ +import { resetObjectTransform } from '../../util/misc/objectTransforms'; import type { StrictLayoutContext } from '../types'; import { LayoutStrategy } from './LayoutStrategy'; @@ -15,4 +16,12 @@ export class FitContentLayout extends LayoutStrategy { shouldPerformLayout(context: StrictLayoutContext) { return true; } + + onAfterLayout({ target }: StrictLayoutContext): void { + if (target.size() === 0) { + resetObjectTransform(target); + target.left = 0; + target.top = 0; + } + } } diff --git a/src/LayoutManager/LayoutStrategies/LayoutStrategy.ts b/src/LayoutManager/LayoutStrategies/LayoutStrategy.ts index a7bb9bf0e95..f79947e8d47 100644 --- a/src/LayoutManager/LayoutStrategies/LayoutStrategy.ts +++ b/src/LayoutManager/LayoutStrategies/LayoutStrategy.ts @@ -61,11 +61,20 @@ export abstract class LayoutStrategy { return result.size; } + /** + * called from the `onBeforeLayout` hook + */ + // eslint-disable-next-line @typescript-eslint/no-unused-vars + onBeforeLayout(context: StrictLayoutContext) { + // Noop + } + /** * called from the `onAfterLayout` hook */ - shouldResetTransform(context: StrictLayoutContext) { - return context.target.size() === 0; + // eslint-disable-next-line @typescript-eslint/no-unused-vars + onAfterLayout(context: StrictLayoutContext) { + // Noop } /**