diff --git a/CHANGELOG.md b/CHANGELOG.md index fe6bbae6675..b9ba11bb6e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [next] +- fix(Canvas): Fix searchPossibleTargets for non-interactive nested targets [#9762](https://github.com/fabricjs/fabric.js/pull/9762) - test(): Rename svg tests [#9775](https://github.com/fabricjs/fabric.js/pull/9775) - refactor(): `_findTargetCorner` is now called `findControl` and returns the key and the control and the coordinates [#9668](https://github.com/fabricjs/fabric.js/pull/9668) - feat(LayoutManager): Handle the case of activeSelection with objects inside different groups [#9651](https://github.com/fabricjs/fabric.js/pull/9651) diff --git a/src/canvas/SelectableCanvas.spec.ts b/src/canvas/SelectableCanvas.spec.ts index 05030452ecb..5c1f2578903 100644 --- a/src/canvas/SelectableCanvas.spec.ts +++ b/src/canvas/SelectableCanvas.spec.ts @@ -303,6 +303,137 @@ describe('Selectable Canvas', () => { }); }); + describe('searchPossibleTargets', () => { + test('the target returned will stop at the first non interactive container', () => { + const object = new FabricObject({ + id: 'a', + left: 0, + top: 0, + width: 10, + height: 10, + padding: 0, + strokeWidth: 0, + }); + const groupB = new Group([object], { + id: 'b', + interactive: true, + subTargetCheck: true, + }); + const groupC = new Group([groupB], { + id: 'c', + interactive: false, + subTargetCheck: true, + }); + const groupD = new Group([groupC], { + id: 'd', + interactive: true, + subTargetCheck: true, + }); + const canvas = new Canvas(undefined, { renderOnAddRemove: false }); + canvas.add(groupD); + const target = canvas.searchPossibleTargets( + canvas.getObjects(), + groupD.getCenterPoint() + ); + expect(target).toBe(groupC); + expect(canvas.targets.map((obj) => obj.id)).toEqual(['a', 'b', 'c']); + }); + test('a interactive group covered by a non interactive group wont be selected', () => { + const object = new FabricObject({ + id: 'a', + left: 0, + top: 0, + width: 10, + height: 10, + padding: 0, + strokeWidth: 0, + }); + const groupB = new Group([object], { + id: 'b', + interactive: true, + subTargetCheck: true, + }); + const groupC = new Group([groupB], { + id: 'c', + interactive: false, + subTargetCheck: true, + }); + const groupD = new Group([groupC], { + id: 'd', + interactive: true, + subTargetCheck: true, + }); + const groupE = new Group([groupD], { + id: 'e', + interactive: true, + subTargetCheck: true, + }); + const canvas = new Canvas(undefined, { renderOnAddRemove: false }); + canvas.add(groupE); + const target = canvas.searchPossibleTargets( + canvas.getObjects(), + groupD.getCenterPoint() + ); + expect(target).toBe(groupC); + expect(canvas.targets.map((obj) => obj.id)).toEqual(['a', 'b', 'c', 'd']); + }); + + test('nested non interactive groups with subTargetCheck', () => { + const object = new FabricObject({ + left: 0, + top: 0, + width: 10, + height: 10, + padding: 0, + strokeWidth: 0, + }); + + const object2 = new FabricObject({ + left: 20, + top: 0, + width: 10, + height: 10, + padding: 0, + strokeWidth: 0, + }); + + const object3 = new FabricObject({ + left: 40, + top: 0, + width: 10, + height: 10, + padding: 0, + strokeWidth: 0, + }); + + const nestedGroup = new Group([object2, object3], { + interactive: false, + subTargetCheck: true, + }); + + const canvas = new Canvas(undefined, { renderOnAddRemove: false }); + const group = new Group([object, nestedGroup], { + interactive: true, + subTargetCheck: true, + }); + canvas.add(group); + + const object2Position = object2.getCenterPoint(); + const target = canvas.searchPossibleTargets( + canvas.getObjects(), + object2Position + ); + expect(target).toBe(nestedGroup); + + nestedGroup.set({ interactive: true }); + const nestedTarget = canvas.searchPossibleTargets( + canvas.getObjects(), + object2Position + ); + expect(nestedTarget).toBe(object2); + }); + }); + describe('setupCurrentTransform', () => { test.each( ['tl', 'mt', 'tr', 'mr', 'br', 'mb', 'bl', 'ml', 'mtr'] diff --git a/src/canvas/SelectableCanvas.ts b/src/canvas/SelectableCanvas.ts index 019731c1069..7a3fce244d9 100644 --- a/src/canvas/SelectableCanvas.ts +++ b/src/canvas/SelectableCanvas.ts @@ -188,7 +188,7 @@ export class SelectableCanvas declare fireMiddleClick: boolean; /** - * Keep track of the subTargets for Mouse Events + * Keep track of the subTargets for Mouse Events, ordered bottom up from innermost nested subTarget * @type FabricObject[] */ targets: FabricObject[] = []; @@ -860,15 +860,31 @@ export class SelectableCanvas pointer: Point ): FabricObject | undefined { const target = this._searchPossibleTargets(objects, pointer); - // if we found something in this.targets, and the group is interactive, return that subTarget + + // if we found something in this.targets, and the group is interactive, return the innermost subTarget + // that is still interactive // TODO: reverify why interactive. the target should be returned always, but selected only // if interactive. - return target && + if ( + target && isCollection(target) && target.interactive && this.targets[0] - ? this.targets[0] - : target; + ) { + /** targets[0] is the innermost nested target, but it could be inside non interactive groups and so not a selection target */ + const targets = this.targets; + for (let i = targets.length - 1; i > 0; i--) { + const t = targets[i]; + if (!(isCollection(t) && t.interactive)) { + // one of the subtargets was not interactive. that is the last subtarget we can return. + // we can't dig more deep; + return t; + } + } + return targets[0]; + } + + return target; } /**