From a3a780414a0734e7cfae5262e5f4fe3edd06b57d Mon Sep 17 00:00:00 2001
From: Steve Ruiz <steveruizok@gmail.com>
Date: Thu, 31 Aug 2023 10:48:39 +0200
Subject: [PATCH] [fix] arrows bind to locked shapes (#1833)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This PR fixes a bug where arrows would bind to locked shapes.

### Change Type

- [x] `patch` — Bug fix

### Test Plan

1. Lock a shape.
2. Confirm that an arrow can neither begin bound to the shape
3. Confirm that an arrow cannot bind to the shape

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

---------

Co-authored-by: Mitja Bezenšek <mitja.bezensek@gmail.com>
---
 .../src/lib/shapes/arrow/ArrowShapeUtil.tsx   |  4 +-
 .../lib/shapes/arrow/toolStates/Pointing.ts   |  4 +-
 .../tldraw/src/test/arrows-megabus.test.ts    | 45 +++++++++++++++++++
 3 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx b/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx
index 44ce0f524..4ac4d19f0 100644
--- a/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx
+++ b/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx
@@ -239,10 +239,12 @@ export class ArrowShapeUtil extends ShapeUtil<TLArrowShape> {
 		const point = this.editor.getShapePageTransform(shape.id)!.applyToPoint(handle)
 
 		const target = this.editor.getShapeAtPoint(point, {
-			filter: (shape) => this.editor.getShapeUtil(shape).canBind(shape),
 			hitInside: true,
 			hitFrameInside: true,
 			margin: 0,
+			filter: (targetShape) => {
+				return !targetShape.isLocked && this.editor.getShapeUtil(targetShape).canBind(targetShape)
+			},
 		})
 
 		if (!target) {
diff --git a/packages/tldraw/src/lib/shapes/arrow/toolStates/Pointing.ts b/packages/tldraw/src/lib/shapes/arrow/toolStates/Pointing.ts
index 1f0f8580b..bc4ca76f9 100644
--- a/packages/tldraw/src/lib/shapes/arrow/toolStates/Pointing.ts
+++ b/packages/tldraw/src/lib/shapes/arrow/toolStates/Pointing.ts
@@ -11,7 +11,9 @@ export class Pointing extends StateNode {
 		this.didTimeout = false
 
 		const target = this.editor.getShapeAtPoint(this.editor.inputs.currentPagePoint, {
-			filter: (shape) => this.editor.getShapeUtil(shape).canBind(shape),
+			filter: (targetShape) => {
+				return !targetShape.isLocked && this.editor.getShapeUtil(targetShape).canBind(targetShape)
+			},
 			margin: 0,
 			hitInside: true,
 		})
diff --git a/packages/tldraw/src/test/arrows-megabus.test.ts b/packages/tldraw/src/test/arrows-megabus.test.ts
index d97f182e3..6ee58ca05 100644
--- a/packages/tldraw/src/test/arrows-megabus.test.ts
+++ b/packages/tldraw/src/test/arrows-megabus.test.ts
@@ -169,6 +169,14 @@ describe('When binding an arrow to a shape', () => {
 		expect(arrow().props.end.type).toBe('point')
 	})
 
+	it('does not bind when the shape is locked', () => {
+		editor.toggleLock(editor.currentPageShapes)
+		editor.setCurrentTool('arrow')
+		editor.pointerDown(0, 50)
+		editor.pointerMove(100, 50)
+		expect(arrow().props.end.type).toBe('point')
+	})
+
 	it('should use timer on keyup when using control key to skip binding', () => {
 		editor.setCurrentTool('arrow')
 		editor.pointerDown(0, 50)
@@ -217,6 +225,20 @@ describe('When shapes are overlapping', () => {
 		expect(arrow().props.end).toMatchObject({ boundShapeId: ids.box4 })
 	})
 
+	it('does not bind when shapes are locked', () => {
+		editor.toggleLock([ids.box1, ids.box2, ids.box4])
+		editor.setCurrentTool('arrow')
+		editor.pointerDown(0, 50)
+		editor.pointerMove(125, 50) // over box1 only
+		expect(arrow().props.end).toMatchObject({ type: 'point' }) // box 1 is locked!
+		editor.pointerMove(175, 50) // box2 is higher
+		expect(arrow().props.end).toMatchObject({ type: 'point' }) // box 2 is locked! box1 is locked!
+		editor.pointerMove(225, 50) // box3 is higher
+		expect(arrow().props.end).toMatchObject({ boundShapeId: ids.box3 })
+		editor.pointerMove(275, 50) // box4 is higher
+		expect(arrow().props.end).toMatchObject({ boundShapeId: ids.box3 }) // box 4 is locked!
+	})
+
 	it('binds to the highest shape or to the first filled shape', () => {
 		editor.updateShapes([
 			{ id: ids.box1, type: 'geo', props: { fill: 'solid' } },
@@ -445,6 +467,29 @@ describe('When starting an arrow inside of multiple shapes', () => {
 		})
 	})
 
+	it('skips locked shape when starting an arrow over shapes', () => {
+		editor.toggleLock([ids.box2])
+		editor.sendToBack([ids.box2])
+		// box1 is bigger and is above box2
+
+		editor.setCurrentTool('arrow')
+		editor.pointerDown(25, 25)
+		expect(editor.currentPageShapes.length).toBe(2)
+		expect(arrow()).toBe(null)
+		editor.pointerMove(30, 30)
+		expect(editor.currentPageShapes.length).toBe(3)
+		expect(arrow()).toMatchObject({
+			props: {
+				start: {
+					boundShapeId: ids.box1, // not box 2!
+				},
+				end: {
+					boundShapeId: ids.box1, // not box 2
+				},
+			},
+		})
+	})
+
 	it('starts a filled shape if it is above the hollow shape', () => {
 		// box2 - small, hollow
 		// box1 - big, filled