Fix frames not preserving shape order (#2928)

It looks like enclosing shapes with a new frame did not preserve the
order of the shapes. Also makes framing inside of frames work.

Solves https://github.com/tldraw/tldraw/issues/2892

Before:


https://github.com/tldraw/tldraw/assets/2523721/90da4fc0-92a1-49fe-b658-73842c4ef4c2


After:


https://github.com/tldraw/tldraw/assets/2523721/0558d22e-8216-4d84-8a89-7dd049c37974

### Change Type

- [x] `patch` — Bug fix
- [ ] `minor` — New feature
- [ ] `major` — Breaking change
- [ ] `dependencies` — Changes to package dependencies[^1]
- [ ] `documentation` — Changes to the documentation only[^2]
- [ ] `tests` — Changes to any test code only[^2]
- [ ] `internal` — Any other changes that don't affect the published
package[^2]
- [ ] I don't know

[^1]: publishes a `patch` release, for devDependencies use `internal`
[^2]: will not publish a new version

### Test Plan

1. Add a few shapes.
2. Make sure to change some of their order (example in the gif puts the
last created shape to the back)
3. Create a new frame that encloses these shapes.
4. The order of the shapes should be preseved.

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

### Release Notes

- Fix an issue when framing shapes did not preserve the original order
of the shapes.

packages/tldraw/src/lib/shapes/frame/FrameShapeTool.ts

@@ -13,18 +13,20 @@ export class FrameShapeTool extends BaseBoxShapeTool {
 		const shapesToAddToFrame: TLShapeId[] = []
 		const ancestorIds = this.editor.getShapeAncestors(shape).map((shape) => shape.id)
 
-		this.editor.getCurrentPageShapes().map((pageShape) => {
+		this.editor.getSortedChildIdsForParent(shape.parentId).map((siblingShapeId) => {
+			const siblingShape = this.editor.getShape(siblingShapeId)
+			if (!siblingShape) return
 			// We don't want to frame the frame itself
-			if (pageShape.id === shape.id) return
+			if (siblingShape.id === shape.id) return
-			if (pageShape.isLocked) return
+			if (siblingShape.isLocked) return
 
-			const pageShapeBounds = this.editor.getShapePageBounds(pageShape)
+			const pageShapeBounds = this.editor.getShapePageBounds(siblingShape)
 			if (!pageShapeBounds) return
 
 			// Frame shape encloses page shape
 			if (bounds.contains(pageShapeBounds)) {
-				if (canEnclose(pageShape, ancestorIds, shape)) {
+				if (canEnclose(siblingShape, ancestorIds, shape)) {
-					shapesToAddToFrame.push(pageShape.id)
+					shapesToAddToFrame.push(siblingShape.id)
 				}
 			}
 		})

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

@@ -784,6 +784,44 @@ describe('frame shapes', () => {
 		expect(newRectB.x).toBe(300)
 		expect(newRectB.y).toBe(300)
 	})
+
+	it('preserves the order of shapes when enclosing over them', () => {
+		const rectAId = createRect({ pos: [100, 100], size: [100, 100] })
+		const rectBId = createRect({ pos: [300, 300], size: [100, 100] })
+		const pageId = editor.getCurrentPageId()
+		expect(editor.getSortedChildIdsForParent(pageId)).toStrictEqual([rectAId, rectBId])
+
+		// Create the frame that encloses both rects
+		let frameId = dragCreateFrame({ down: [0, 0], move: [700, 700], up: [700, 700] })
+
+		// The order should be the same as before
+		expect(editor.getSortedChildIdsForParent(frameId)).toStrictEqual([rectAId, rectBId])
+
+		removeFrame(editor, [frameId])
+		expect(editor.getSortedChildIdsForParent(pageId)).toStrictEqual([rectAId, rectBId])
+
+		// Now let's push the second rect to the back
+		editor.sendToBack([rectBId])
+		expect(editor.getSortedChildIdsForParent(pageId)).toStrictEqual([rectBId, rectAId])
+
+		frameId = dragCreateFrame({ down: [0, 0], move: [700, 700], up: [700, 700] })
+		expect(editor.getSortedChildIdsForParent(frameId)).toStrictEqual([rectBId, rectAId])
+	})
+
+	it('allows us to frame inside of frames', () => {
+		const rectAId = createRect({ pos: [100, 100], size: [100, 100] })
+		const rectBId = createRect({ pos: [300, 300], size: [100, 100] })
+		const pageId = editor.getCurrentPageId()
+		expect(editor.getSortedChildIdsForParent(pageId)).toStrictEqual([rectAId, rectBId])
+
+		const outsideFrameId = dragCreateFrame({ down: [0, 0], move: [700, 700], up: [700, 700] })
+		expect(editor.getSortedChildIdsForParent(outsideFrameId)).toStrictEqual([rectAId, rectBId])
+
+		// Create a frame inside the frame
+		const insideFrameId = dragCreateFrame({ down: [50, 50], move: [600, 600], up: [600, 600] })
+		expect(editor.getSortedChildIdsForParent(insideFrameId)).toStrictEqual([rectAId, rectBId])
+		expect(editor.getSortedChildIdsForParent(outsideFrameId)).toStrictEqual([insideFrameId])
+	})
 })
 
 test('arrows bound to a shape within a group within a frame are reparented if the group is moved outside of the frame', () => {