Add select option to Editor.groupShapes and Editor.ungroupShapes (#3690)

This PR adds an (optional) options argument to the `Editor.groupShapes`
and `Editor.ungroupShapes` methods.

Taha's original PR:

This PR seeks to make it easier to disentangle shape selection logic
from the grouping shapes logic. This is one of two approaches we could
take to this problem.

Other PR is here: https://github.com/tldraw/tldraw/pull/3691

I think this is a better method because it doesn't require consumers of
the library with their own custom actions to change the way their own
grouping logic works. As evidenced by all the tests failing on the other
PR.

### Change Type

<!-- ❗ Please select a 'Scope' label ❗️ -->

- [x] `sdk` — Changes the tldraw SDK
- [ ] `dotcom` — Changes the tldraw.com web app
- [ ] `docs` — Changes to the documentation, examples, or templates.
- [ ] `vs code` — Changes to the vscode plugin
- [ ] `internal` — Does not affect user-facing stuff

<!-- ❗ Please select a 'Type' label ❗️ -->

- [ ] `bugfix` — Bug fix
- [ ] `feature` — New feature
- [x] `improvement` — Improving existing features
- [ ] `chore` — Updating dependencies, other boring stuff
- [ ] `galaxy brain` — Architectural changes
- [ ] `tests` — Changes to any test code
- [ ] `tools` — Changes to infrastructure, CI, internal scripts,
debugging tools, etc.
- [ ] `dunno` — I don't know


### Test Plan

1. Add a step-by-step description of how to test your PR here.
2.

- [ ] Unit Tests
- [ ] End to end tests

### Release Notes

- Add a brief release note for your PR here.

---------

Co-authored-by: Steve Ruiz <steveruizok@gmail.com>

packages/editor/api-report.md

@@ -986,7 +986,15 @@ export class Editor extends EventEmitter<TLEventMap> {
     getViewportScreenBounds(): Box;
     getViewportScreenCenter(): Vec;
     getZoomLevel(): number;
-    groupShapes(shapes: TLShape[] | TLShapeId[], groupId?: TLShapeId): this;
+    groupShapes(shapes: TLShape[], options?: Partial<{
+        groupId: TLShapeId;
+        select: boolean;
+    }>): this;
+    // (undocumented)
+    groupShapes(ids: TLShapeId[], options?: Partial<{
+        groupId: TLShapeId;
+        select: boolean;
+    }>): this;
     hasAncestor(shape: TLShape | TLShapeId | undefined, ancestorId: TLShapeId): boolean;
     readonly history: HistoryManager<TLRecord>;
     inputs: {
@@ -1100,9 +1108,13 @@ export class Editor extends EventEmitter<TLEventMap> {
     readonly textMeasure: TextManager;
     toggleLock(shapes: TLShape[] | TLShapeId[]): this;
     undo(): this;
-    ungroupShapes(ids: TLShapeId[]): this;
+    ungroupShapes(ids: TLShapeId[], options?: Partial<{
+        select: boolean;
+    }>): this;
     // (undocumented)
-    ungroupShapes(ids: TLShape[]): this;
+    ungroupShapes(shapes: TLShape[], options?: Partial<{
+        select: boolean;
+    }>): this;
     updateAssets(assets: TLAssetPartial[]): this;
     // (undocumented)
     updateBinding<B extends TLBinding = TLBinding>(partial: TLBindingUpdate<B>): this;

packages/editor/src/lib/editor/Editor.ts

@@ -6922,12 +6922,25 @@ export class Editor extends EventEmitter<TLEventMap> {
 	/**
 	 * Create a group containing the provided shapes.
 	 *
+	 * @example
+	 * ```ts
+	 * editor.groupShapes([myShape, myOtherShape])
+	 * editor.groupShapes([myShape, myOtherShape], { groupId: myGroupId, select: false })
+	 * ```
+	 *
 	 * @param shapes - The shapes (or shape ids) to group. Defaults to the selected shapes.
-	 * @param groupId - The id of the group to create.
+	 * @param options - An options object.
 	 *
 	 * @public
 	 */
-	groupShapes(shapes: TLShapeId[] | TLShape[], groupId = createShapeId()): this {
+	groupShapes(shapes: TLShape[], options?: Partial<{ groupId: TLShapeId; select: boolean }>): this
+	groupShapes(ids: TLShapeId[], options?: Partial<{ groupId: TLShapeId; select: boolean }>): this
+	groupShapes(
+		shapes: TLShapeId[] | TLShape[],
+		options = {} as Partial<{ groupId: TLShapeId; select: boolean }>
+	): this {
+		const { groupId = createShapeId(), select = true } = options
+
 		if (!Array.isArray(shapes)) {
 			throw Error('Editor.groupShapes: must provide an array of shapes or shape ids')
 		}
@@ -6977,7 +6990,10 @@ export class Editor extends EventEmitter<TLEventMap> {
 				},
 			])
 			this.reparentShapes(sortedShapeIds, groupId)
-			this.select(groupId)
+			if (select) {
+				// the select option determines whether the grouped shapes' children are selected
+				this.select(groupId)
+			}
 		})
 
 		return this
@@ -6986,15 +7002,25 @@ export class Editor extends EventEmitter<TLEventMap> {
 	/**
 	 * Ungroup some shapes.
 	 *
-	 * @param ids - Ids of the shapes to ungroup. Defaults to the selected shapes.
+	 * @example
+	 * ```ts
+	 * editor.ungroupShapes([myGroup, myOtherGroup])
+	 * editor.ungroupShapes([myGroup], { select: false })
+	 * ```
+	 *
+	 * @param shapes - The group shapes (or shape ids) to ungroup.
+	 * @param options - An options object.
 	 *
 	 * @public
 	 */
-	ungroupShapes(ids: TLShapeId[]): this
+	ungroupShapes(ids: TLShapeId[], options?: Partial<{ select: boolean }>): this
-	ungroupShapes(ids: TLShape[]): this
+	ungroupShapes(shapes: TLShape[], options?: Partial<{ select: boolean }>): this
-	ungroupShapes(_ids: TLShapeId[] | TLShape[]) {
+	ungroupShapes(shapes: TLShapeId[] | TLShape[], options = {} as Partial<{ select: boolean }>) {
+		const { select = true } = options
 		const ids =
-			typeof _ids[0] === 'string' ? (_ids as TLShapeId[]) : (_ids as TLShape[]).map((s) => s.id)
+			typeof shapes[0] === 'string'
+				? (shapes as TLShapeId[])
+				: (shapes as TLShape[]).map((s) => s.id)
 		if (this.getInstanceState().isReadonly) return this
 		if (ids.length === 0) return this
 
@@ -7012,11 +7038,9 @@ export class Editor extends EventEmitter<TLEventMap> {
 		const idsToSelect = new Set<TLShapeId>()
 
 		// Get all groups in the selection
-		const shapes = compact(ids.map((id) => this.getShape(id)))
-
 		const groups: TLGroupShape[] = []
 
-		shapes.forEach((shape) => {
+		compact(ids.map((id) => this.getShape(id))).forEach((shape) => {
 			if (this.isShapeOfType<TLGroupShape>(shape, 'group')) {
 				groups.push(shape)
 			} else {
@@ -7041,7 +7065,11 @@ export class Editor extends EventEmitter<TLEventMap> {
 			}
 
 			this.deleteShapes(groups.map((group) => group.id))
-			this.select(...idsToSelect)
+
+			if (select) {
+				// the select option determines whether the ungrouped shapes' children are selected
+				this.select(...idsToSelect)
+			}
 		})
 
 		return this

packages/tldraw/src/lib/utils/tldr/buildFromV1Document.ts

@@ -498,7 +498,7 @@ export function buildFromV1Document(editor: Editor, document: LegacyTldrawDocume
 				v1GroupShapeIdsToV1ChildIds.forEach((v1ChildIds, v1GroupId) => {
 					const v2ChildShapeIds = v1ChildIds.map((id) => v1ShapeIdsToV2ShapeIds.get(id)!)
 					const v2GroupId = v1ShapeIdsToV2ShapeIds.get(v1GroupId)!
-					editor.groupShapes(v2ChildShapeIds, v2GroupId)
+					editor.groupShapes(v2ChildShapeIds, { groupId: v2GroupId })
 
 					const v1Group = v1Page.shapes[v1GroupId]
 					const rotation = coerceNumber(v1Group.rotation)

packages/tldraw/src/test/EraserTool.test.ts

@@ -153,7 +153,7 @@ describe('When clicking', () => {
 	})
 
 	it("Erases a group when clicking on the group's child", () => {
-		editor.groupShapes([ids.box2, ids.box3], ids.group1)
+		editor.groupShapes([ids.box2, ids.box3], { groupId: ids.group1 })
 		editor.setCurrentTool('eraser')
 
 		const shapesBeforeCount = editor.getCurrentPageShapes().length
@@ -174,7 +174,7 @@ describe('When clicking', () => {
 	})
 
 	it('Does not erase a group when clicking on the group itself', () => {
-		editor.groupShapes([ids.box2, ids.box3], ids.group1)
+		editor.groupShapes([ids.box2, ids.box3], { groupId: ids.group1 })
 		editor.setCurrentTool('eraser')
 
 		const shapesBeforeCount = editor.getCurrentPageShapes().length
@@ -340,7 +340,7 @@ describe('When clicking and dragging', () => {
 	})
 
 	it('Excludes a group if it was hovered when the drag started', () => {
-		editor.groupShapes([ids.box2, ids.box3], ids.group1)
+		editor.groupShapes([ids.box2, ids.box3], { groupId: ids.group1 })
 		editor.setCurrentTool('eraser')
 		editor.expectToBeIn('eraser.idle')
 		editor.pointerDown(275, 275) // in between box2 AND box3, so over of the new group

packages/tldraw/src/test/frames.test.ts

@@ -917,7 +917,7 @@ describe('When dragging a shape inside a group inside a frame', () => {
 
 		expect(editor.getSelectedShapeIds()).toHaveLength(2)
 
-		editor.groupShapes(editor.getSelectedShapeIds(), ids.group1)
+		editor.groupShapes(editor.getSelectedShapeIds(), { groupId: ids.group1 })
 
 		expect(editor.getShape(ids.box1)!.parentId).toBe(ids.group1)
 
@@ -937,7 +937,7 @@ describe('When dragging a shape inside a group inside a frame', () => {
 
 		expect(editor.getSelectedShapeIds()).toHaveLength(2)
 
-		editor.groupShapes(editor.getSelectedShapeIds(), ids.group1)
+		editor.groupShapes(editor.getSelectedShapeIds(), { groupId: ids.group1 })
 
 		expect(editor.getShape(ids.box1)!.parentId).toBe(ids.group1)
 

packages/tldraw/src/test/groups.test.ts

@@ -876,7 +876,7 @@ describe('right clicking in detail', () => {
 			box(ids.boxB, 20, 0, 10, 10, 'solid'),
 			box(ids.boxC, 0, 50, 10, 10, 'none'),
 		])
-		editor.groupShapes([ids.boxA, ids.boxB], groupId)
+		editor.groupShapes([ids.boxA, ids.boxB], { groupId: groupId })
 		editor.selectNone()
 	})
 
@@ -908,9 +908,9 @@ describe('the select tool', () => {
 			box(ids.boxC, 60, 0, 10, 10),
 			box(ids.boxD, 90, 0, 10, 10),
 		])
-		editor.groupShapes([ids.boxA, ids.boxB], groupAId)
+		editor.groupShapes([ids.boxA, ids.boxB], { groupId: groupAId })
-		editor.groupShapes([ids.boxC, ids.boxD], groupBId)
+		editor.groupShapes([ids.boxC, ids.boxD], { groupId: groupBId })
-		editor.groupShapes([groupAId, groupBId], groupCId)
+		editor.groupShapes([groupAId, groupBId], { groupId: groupCId })
 		editor.selectNone()
 	})
 
@@ -1969,7 +1969,7 @@ describe('Group opacity', () => {
 describe('Grouping / ungrouping locked shapes', () => {
 	it('keeps locked shapes locked when grouped', () => {
 		editor.createShapes([box(ids.boxA, 0, 0), box(ids.boxB, 200, 0)])
-		editor.groupShapes([ids.boxA, ids.boxB], ids.groupA)
+		editor.groupShapes([ids.boxA, ids.boxB], { groupId: ids.groupA })
 		editor.select(ids.boxA)
 
 		// Lock boxA

packages/tldraw/src/test/selection-omnibus.test.ts

@@ -967,7 +967,7 @@ describe('Selects inside of groups', () => {
 			{ id: ids.box1, type: 'geo', x: 0, y: 0, props: { w: 100, h: 100 } },
 			{ id: ids.box2, type: 'geo', x: 200, y: 0, props: { w: 100, h: 100, fill: 'solid' } },
 		])
-		editor.groupShapes([ids.box1, ids.box2], ids.group1)
+		editor.groupShapes([ids.box1, ids.box2], { groupId: ids.group1 })
 		editor.selectNone()
 	})
 
@@ -1288,7 +1288,7 @@ describe('when shift+selecting a group', () => {
 				{ id: ids.box3, type: 'geo', x: 400, y: 0, props: { fill: 'solid' } },
 				{ id: ids.box4, type: 'geo', x: 600, y: 0 },
 			])
-			.groupShapes([ids.box2, ids.box3], ids.group1)
+			.groupShapes([ids.box2, ids.box3], { groupId: ids.group1 })
 			.select(ids.box1)
 	})
 
@@ -1370,9 +1370,9 @@ describe('When children / descendants of a group are selected', () => {
 				{ id: ids.box4, type: 'geo', x: 600, y: 0 },
 				{ id: ids.box5, type: 'geo', x: 800, y: 0 },
 			])
-			.groupShapes([ids.box1, ids.box2], ids.group1)
+			.groupShapes([ids.box1, ids.box2], { groupId: ids.group1 })
-			.groupShapes([ids.box3, ids.box4], ids.group2)
+			.groupShapes([ids.box3, ids.box4], { groupId: ids.group2 })
-			.groupShapes([ids.group1, ids.group2], ids.group3)
+			.groupShapes([ids.group1, ids.group2], { groupId: ids.group3 })
 			.selectNone()
 	})
 
@@ -1445,8 +1445,8 @@ describe('When pressing the enter key with groups selected', () => {
 				{ id: ids.box4, type: 'geo', x: 600, y: 0 },
 				{ id: ids.box5, type: 'geo', x: 800, y: 0 },
 			])
-			.groupShapes([ids.box1, ids.box2], ids.group1)
+			.groupShapes([ids.box1, ids.box2], { groupId: ids.group1 })
-			.groupShapes([ids.box3, ids.box4], ids.group2)
+			.groupShapes([ids.box3, ids.box4], { groupId: ids.group2 })
 	})
 
 	it('selects the children of the groups on enter up', () => {
@@ -1460,7 +1460,7 @@ describe('When pressing the enter key with groups selected', () => {
 	})
 
 	it('repeats children of the groups on enter up', () => {
-		editor.groupShapes([ids.group1, ids.group2], ids.group3)
+		editor.groupShapes([ids.group1, ids.group2], { groupId: ids.group3 })
 		editor.select(ids.group3)
 		expect(editor.getSelectedShapeIds()).toEqual([ids.group3])
 		editor.keyDown('Enter').keyUp('Enter')
@@ -1527,7 +1527,7 @@ describe('When double clicking an editable shape', () => {
 
 	it('starts editing a child of a group on triple (not double!) click', () => {
 		editor.createShape({ id: ids.box2, type: 'geo', x: 300, y: 0 })
-		editor.groupShapes([ids.box1, ids.box2], ids.group1)
+		editor.groupShapes([ids.box1, ids.box2], { groupId: ids.group1 })
 		editor.selectNone()
 		editor.pointerMove(50, 50).click() // clicks on the shape label
 		expect(editor.getSelectedShapeIds()).toEqual([ids.group1])
@@ -1552,7 +1552,7 @@ describe('shift brushes to add to the selection', () => {
 				{ id: ids.box3, type: 'geo', x: 400, y: 0 },
 				{ id: ids.box4, type: 'geo', x: 600, y: 200 },
 			])
-			.groupShapes([ids.box3, ids.box4], ids.group1)
+			.groupShapes([ids.box3, ids.box4], { groupId: ids.group1 })
 	})
 
 	it('does not select when brushing into margin', () => {
@@ -1669,7 +1669,7 @@ describe('scribble brushes to add to the selection', () => {
 	})
 
 	it('selects a group when scribble is colliding with the groups child shape', () => {
-		editor.groupShapes([ids.box3, ids.box4], ids.group1)
+		editor.groupShapes([ids.box3, ids.box4], { groupId: ids.group1 })
 		editor.pointerMove(650, -50)
 		editor.keyDown('Alt')
 		editor.pointerDown()

packages/tldraw/src/test/translating.test.ts

@@ -323,7 +323,7 @@ describe('When cloning...', () => {
 
 	it('Clones twice', () => {
 		const groupId = createShapeId('g')
-		editor.groupShapes([ids.box1, ids.box2], groupId)
+		editor.groupShapes([ids.box1, ids.box2], { groupId: groupId })
 		const count1 = editor.getCurrentPageShapes().length
 
 		editor.pointerDown(50, 50, { shape: editor.getShape(groupId)!, target: 'shape' })