Maintain bindings whilst translating arrows (#2424)

This diff tries to maintain bindings whilst translating arrows. It looks
at where the terminal of the arrow ends up, and if it's still over the
same shape, it updates the binding to a precise one at that location
rather than removing the binding entirely.

![Kapture 2024-01-08 at 18 22
12](https://github.com/tldraw/tldraw/assets/1489520/b97ce5d9-ac02-456e-aaa6-ffe06825ed1d)

### Change Type

- [x] `minor` — New feature

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

### Test Plan

1. Create an arrow with bindings
2. Move the arrow (translation, stacking, nudging, distribution, etc)
3. Make sure that the end point of the arrow remains bound if
appropriate

- [x] Unit Tests


### Release Notes

- You can now move arrows without them becoming unattached the shapes
they're pointing to

---------

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

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

@@ -5076,6 +5076,35 @@ export class Editor extends EventEmitter<TLEventMap> {
 		return this
 	}
 
+	private getChangesToTranslateShape(initialShape: TLShape, newShapeCoords: VecLike): TLShape {
+		let workingShape = initialShape
+		const util = this.getShapeUtil(initialShape)
+
+		workingShape = applyPartialToShape(
+			workingShape,
+			util.onTranslateStart?.(workingShape) ?? undefined
+		)
+
+		workingShape = applyPartialToShape(workingShape, {
+			id: initialShape.id,
+			type: initialShape.type,
+			x: newShapeCoords.x,
+			y: newShapeCoords.y,
+		})
+
+		workingShape = applyPartialToShape(
+			workingShape,
+			util.onTranslate?.(initialShape, workingShape) ?? undefined
+		)
+
+		workingShape = applyPartialToShape(
+			workingShape,
+			util.onTranslateEnd?.(initialShape, workingShape) ?? undefined
+		)
+
+		return workingShape
+	}
+
 	/**
 	 * Move shapes by a delta.
 	 *
@@ -5103,32 +5132,12 @@ export class Editor extends EventEmitter<TLEventMap> {
 		const changes: TLShapePartial[] = []
 
 		for (const id of ids) {
-			const shape = this.getShape(id)
+			const shape = this.getShape(id)!
-
-			if (!shape) {
-				throw Error(`Could not find a shape with the id ${id}.`)
-			}
-
 			const localDelta = Vec.Cast(offset)
 			const parentTransform = this.getShapeParentTransform(shape)
 			if (parentTransform) localDelta.rot(-parentTransform.rotation())
 
-			const translateStartChanges = this.getShapeUtil(shape).onTranslateStart?.(shape)
+			changes.push(this.getChangesToTranslateShape(shape, localDelta.add(shape)))
-
-			changes.push(
-				translateStartChanges
-					? {
-							...translateStartChanges,
-							x: shape.x + localDelta.x,
-							y: shape.y + localDelta.y,
-					  }
-					: {
-							id,
-							x: shape.x + localDelta.x,
-							y: shape.y + localDelta.y,
-							type: shape.type,
-					  }
-			)
 		}
 
 		this.updateShapes(changes, {
@@ -5989,22 +5998,7 @@ export class Editor extends EventEmitter<TLEventMap> {
 				? Vec.Rot(delta, -this.getShapePageTransform(parent)!.decompose().rotation)
 				: delta
 
-			const translateChanges = this.getShapeUtil(shape).onTranslateStart?.(shape)
+			changes.push(this.getChangesToTranslateShape(shape, Vec.Add(shape, localDelta)))
-
-			changes.push(
-				translateChanges
-					? {
-							...translateChanges,
-							x: shape.x + localDelta.x,
-							y: shape.y + localDelta.y,
-					  }
-					: {
-							id: shape.id,
-							type: shape.type,
-							x: shape.x + localDelta.x,
-							y: shape.y + localDelta.y,
-					  }
-			)
 		})
 
 		this.updateShapes(changes)
@@ -6082,20 +6076,8 @@ export class Editor extends EventEmitter<TLEventMap> {
 				const localDelta = parent
 					? Vec.Rot(delta, -this.getShapePageTransform(parent)!.rotation())
 					: delta
-				const translateStartChanges = this.getShapeUtil(shape).onTranslateStart?.(shape)
 
-				changes.push(
+				changes.push(this.getChangesToTranslateShape(shape, Vec.Add(shape, localDelta)))
-					translateStartChanges
-						? {
-								...translateStartChanges,
-								[val]: shape[val] + localDelta[val],
-						  }
-						: {
-								id: shape.id,
-								type: shape.type,
-								[val]: shape[val] + localDelta[val],
-						  }
-				)
 			})
 
 		this.updateShapes(changes)
@@ -7017,47 +6999,7 @@ export class Editor extends EventEmitter<TLEventMap> {
 				partials.map((partial) => {
 					const prev = snapshots[partial.id]
 					if (!prev) return null
-					let newRecord = null as null | TLShape
+					return applyPartialToShape(prev, partial)
-					for (const [k, v] of Object.entries(partial)) {
-						if (v === undefined) continue
-						switch (k) {
-							case 'id':
-							case 'type':
-								continue
-							default: {
-								if (v !== (prev as any)[k]) {
-									if (!newRecord) {
-										newRecord = { ...prev }
-									}
-
-									if (k === 'props') {
-										// props property
-										const nextProps = { ...prev.props } as JsonObject
-										for (const [propKey, propValue] of Object.entries(v as object)) {
-											if (propValue !== undefined) {
-												nextProps[propKey] = propValue
-											}
-										}
-										newRecord!.props = nextProps
-									} else if (k === 'meta') {
-										// meta property
-										const nextMeta = { ...prev.meta } as JsonObject
-										for (const [metaKey, metaValue] of Object.entries(v as object)) {
-											if (metaValue !== undefined) {
-												nextMeta[metaKey] = metaValue
-											}
-										}
-										newRecord!.meta = nextMeta
-									} else {
-										// base property
-										;(newRecord as any)[k] = v
-									}
-								}
-							}
-						}
-					}
-
-					return newRecord ?? prev
 				})
 			)
 
@@ -8987,3 +8929,48 @@ function alertMaxShapes(editor: Editor, pageId = editor.getCurrentPageId()) {
 	const name = editor.getPage(pageId)!.name
 	editor.emit('max-shapes', { name, pageId, count: MAX_SHAPES_PER_PAGE })
 }
+
+function applyPartialToShape<T extends TLShape>(prev: T, partial?: TLShapePartial<T>): T {
+	if (!partial) return prev
+	let next = null as null | T
+	for (const [k, v] of Object.entries(partial)) {
+		if (v === undefined) continue
+		switch (k) {
+			case 'id':
+			case 'type':
+				continue
+			default: {
+				if (v !== (prev as any)[k]) {
+					if (!next) {
+						next = { ...prev }
+					}
+
+					if (k === 'props') {
+						// props property
+						const nextProps = { ...prev.props } as JsonObject
+						for (const [propKey, propValue] of Object.entries(v as object)) {
+							if (propValue !== undefined) {
+								nextProps[propKey] = propValue
+							}
+						}
+						next!.props = nextProps
+					} else if (k === 'meta') {
+						// meta property
+						const nextMeta = { ...prev.meta } as JsonObject
+						for (const [metaKey, metaValue] of Object.entries(v as object)) {
+							if (metaValue !== undefined) {
+								nextMeta[metaKey] = metaValue
+							}
+						}
+						next!.meta = nextMeta
+					} else {
+						// base property
+						;(next as any)[k] = v
+					}
+				}
+			}
+		}
+	}
+
+	return next ?? prev
+}

packages/tldraw/api-report.md

@@ -81,6 +81,7 @@ import { TLOnEditEndHandler } from '@tldraw/editor';
 import { TLOnHandleChangeHandler } from '@tldraw/editor';
 import { TLOnResizeEndHandler } from '@tldraw/editor';
 import { TLOnResizeHandler } from '@tldraw/editor';
+import { TLOnTranslateHandler } from '@tldraw/editor';
 import { TLOnTranslateStartHandler } from '@tldraw/editor';
 import { TLParentId } from '@tldraw/editor';
 import { TLPointerEvent } from '@tldraw/editor';
@@ -163,6 +164,8 @@ export class ArrowShapeUtil extends ShapeUtil<TLArrowShape> {
     // (undocumented)
     onResize: TLOnResizeHandler<TLArrowShape>;
     // (undocumented)
+    onTranslate?: TLOnTranslateHandler<TLArrowShape>;
+    // (undocumented)
     onTranslateStart: TLOnTranslateStartHandler<TLArrowShape>;
     // (undocumented)
     static props: {

packages/tldraw/api/api.json

@@ -1163,6 +1163,50 @@
               "isProtected": false,
               "isAbstract": false
             },
+            {
+              "kind": "Property",
+              "canonicalReference": "@tldraw/tldraw!ArrowShapeUtil#onTranslate:member",
+              "docComment": "",
+              "excerptTokens": [
+                {
+                  "kind": "Content",
+                  "text": "onTranslate?: "
+                },
+                {
+                  "kind": "Reference",
+                  "text": "TLOnTranslateHandler",
+                  "canonicalReference": "@tldraw/editor!TLOnTranslateHandler:type"
+                },
+                {
+                  "kind": "Content",
+                  "text": "<"
+                },
+                {
+                  "kind": "Reference",
+                  "text": "TLArrowShape",
+                  "canonicalReference": "@tldraw/tlschema!TLArrowShape:type"
+                },
+                {
+                  "kind": "Content",
+                  "text": ">"
+                },
+                {
+                  "kind": "Content",
+                  "text": ";"
+                }
+              ],
+              "isReadonly": false,
+              "isOptional": true,
+              "releaseTag": "Public",
+              "name": "onTranslate",
+              "propertyTypeTokenRange": {
+                "startIndex": 1,
+                "endIndex": 5
+              },
+              "isStatic": false,
+              "isProtected": false,
+              "isAbstract": false
+            },
             {
               "kind": "Property",
               "canonicalReference": "@tldraw/tldraw!ArrowShapeUtil#onTranslateStart:member",

packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.test.ts

@@ -131,22 +131,6 @@ describe('When translating a bound shape', () => {
 })
 
 describe('When translating the arrow', () => {
-	it('unbinds all handles if neither bound shape is not also translating', () => {
-		editor.select(ids.arrow1)
-		editor.pointerDown(200, 200, { target: 'shape', shape: editor.getShape(ids.arrow1)! })
-		editor.pointerMove(200, 190)
-		editor.expectShapeToMatch({
-			id: ids.arrow1,
-			type: 'arrow',
-			x: 150,
-			y: 140,
-			props: {
-				start: { type: 'point', x: 0, y: 0 },
-				end: { type: 'point', x: 200, y: 200 },
-			},
-		})
-	})
-
 	it('retains all handles if either bound shape is also translating', () => {
 		editor.select(ids.arrow1, ids.box2)
 		expect(editor.getSelectionPageBounds()).toMatchObject({
@@ -196,12 +180,15 @@ describe('Other cases when arrow are moved', () => {
 			},
 		})
 
-		// unbinds when only the arrow is selected (not its bound shapes)
+		// when only the arrow is selected, we keep the binding but make it precise:
 		editor.select(ids.arrow1)
 		editor.nudgeShapes(editor.getSelectedShapeIds(), { x: 0, y: -1 })
 
 		expect(editor.getShape(ids.arrow1)).toMatchObject({
-			props: { start: { type: 'point' }, end: { type: 'point' } },
+			props: {
+				start: { type: 'binding', boundShapeId: ids.box1, isPrecise: true },
+				end: { type: 'binding', boundShapeId: ids.box2, isPrecise: true },
+			},
 		})
 	})
 
@@ -220,7 +207,7 @@ describe('Other cases when arrow are moved', () => {
 			},
 		})
 
-		// unbinds when only the arrow is selected (not its bound shapes)
+		// maintains bindings if they would still be over the same shape (but makes them precise), but unbinds others
 		editor.select(ids.arrow1, ids.box3)
 		editor.alignShapes(editor.getSelectedShapeIds(), 'top')
 		jest.advanceTimersByTime(1000)
@@ -228,7 +215,8 @@ describe('Other cases when arrow are moved', () => {
 		expect(editor.getShape(ids.arrow1)).toMatchObject({
 			props: {
 				start: {
-					type: 'point',
+					type: 'binding',
+					isPrecise: true,
 				},
 				end: {
 					type: 'point',

packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx

@@ -10,6 +10,7 @@ import {
 	SvgExportContext,
 	TLArrowShape,
 	TLArrowShapeArrowheadStyle,
+	TLArrowShapeProps,
 	TLDefaultColorStyle,
 	TLDefaultColorTheme,
 	TLDefaultFillStyle,
@@ -17,6 +18,7 @@ import {
 	TLOnEditEndHandler,
 	TLOnHandleChangeHandler,
 	TLOnResizeHandler,
+	TLOnTranslateHandler,
 	TLOnTranslateStartHandler,
 	TLShapePartial,
 	TLShapeUtilCanvasSvgDef,
@@ -27,6 +29,8 @@ import {
 	deepCopy,
 	getArrowTerminalsInArrowSpace,
 	getDefaultColorTheme,
+	mapObjectMapValues,
+	objectMapEntries,
 	toDomPrecision,
 	useIsEditing,
 } from '@tldraw/editor'
@@ -342,6 +346,9 @@ export class ArrowShapeUtil extends ShapeUtil<TLArrowShape> {
 			shape.props.start.type === 'binding' ? shape.props.start.boundShapeId : null
 		const endBindingId = shape.props.end.type === 'binding' ? shape.props.end.boundShapeId : null
 
+		const terminalsInArrowSpace = getArrowTerminalsInArrowSpace(this.editor, shape)
+		const shapePageTransform = this.editor.getShapePageTransform(shape.id)!
+
 		// If at least one bound shape is in the selection, do nothing;
 		// If no bound shapes are in the selection, unbind any bound shapes
 
@@ -357,25 +364,91 @@ export class ArrowShapeUtil extends ShapeUtil<TLArrowShape> {
 			return
 		}
 
-		const { start, end } = getArrowTerminalsInArrowSpace(this.editor, shape)
+		let result = shape
 
-		return {
+		// When we start translating shapes, record where their bindings were in page space so we
-			id: shape.id,
+		// can maintain them as we translate the arrow
-			type: shape.type,
+		shapeAtTranslationStart.set(shape, {
-			props: {
+			pagePosition: shapePageTransform.applyToPoint(shape),
-				...shape.props,
+			terminalBindings: mapObjectMapValues(terminalsInArrowSpace, (terminalName, point) => {
-				start: {
+				const terminal = shape.props[terminalName]
-					type: 'point',
+				if (terminal.type !== 'binding') return null
-					x: start.x,
+				return {
-					y: start.y,
+					binding: terminal,
-				},
+					shapePosition: point,
-				end: {
+					pagePosition: shapePageTransform.applyToPoint(point),
-					type: 'point',
+				}
-					x: end.x,
+			}),
-					y: end.y,
+		})
-				},
+
-			},
+		for (const handleName of ['start', 'end'] as const) {
+			const terminal = shape.props[handleName]
+			if (terminal.type !== 'binding') continue
+			result = {
+				...shape,
+				props: { ...shape.props, [handleName]: { ...terminal, isPrecise: true } },
+			}
 		}
+
+		return result
+	}
+
+	override onTranslate?: TLOnTranslateHandler<TLArrowShape> = (initialShape, shape) => {
+		const atTranslationStart = shapeAtTranslationStart.get(initialShape)
+		if (!atTranslationStart) return
+
+		const shapePageTransform = this.editor.getShapePageTransform(shape.id)!
+		const pageDelta = Vec.Sub(
+			shapePageTransform.applyToPoint(shape),
+			atTranslationStart.pagePosition
+		)
+
+		let result = shape
+		for (const [terminalName, terminalBinding] of objectMapEntries(
+			atTranslationStart.terminalBindings
+		)) {
+			if (!terminalBinding) continue
+
+			const newPagePoint = Vec.Add(terminalBinding.pagePosition, Vec.Mul(pageDelta, 0.5))
+			const newTarget = this.editor.getShapeAtPoint(newPagePoint, {
+				hitInside: true,
+				hitFrameInside: true,
+				margin: 0,
+				filter: (targetShape) => {
+					return !targetShape.isLocked && this.editor.getShapeUtil(targetShape).canBind(targetShape)
+				},
+			})
+
+			if (newTarget?.id === terminalBinding.binding.boundShapeId) {
+				const targetBounds = Box.ZeroFix(this.editor.getShapeGeometry(newTarget).bounds)
+				const pointInTargetSpace = this.editor.getPointInShapeSpace(newTarget, newPagePoint)
+				const normalizedAnchor = {
+					x: (pointInTargetSpace.x - targetBounds.minX) / targetBounds.width,
+					y: (pointInTargetSpace.y - targetBounds.minY) / targetBounds.height,
+				}
+				result = {
+					...result,
+					props: {
+						...result.props,
+						[terminalName]: { ...terminalBinding.binding, isPrecise: true, normalizedAnchor },
+					},
+				}
+			} else {
+				result = {
+					...result,
+					props: {
+						...result.props,
+						[terminalName]: {
+							type: 'point',
+							x: terminalBinding.shapePosition.x,
+							y: terminalBinding.shapePosition.y,
+						},
+					},
+				}
+			}
+		}
+
+		return result
 	}
 
 	override onResize: TLOnResizeHandler<TLArrowShape> = (shape, info) => {
@@ -499,6 +572,7 @@ export class ArrowShapeUtil extends ShapeUtil<TLArrowShape> {
 				'select.idle',
 				'select.pointing_handle',
 				'select.dragging_handle',
+				'select.translating',
 				'arrow.dragging'
 			) && !this.editor.getInstanceState().isReadonly
 
@@ -1036,3 +1110,18 @@ function getArrowheadSvgPath(
 		return path
 	}
 }
+
+const shapeAtTranslationStart = new WeakMap<
+	TLArrowShape,
+	{
+		pagePosition: Vec
+		terminalBindings: Record<
+			'start' | 'end',
+			{
+				pagePosition: Vec
+				shapePosition: Vec
+				binding: Extract<TLArrowShapeProps['start'], { type: 'binding' }>
+			} | null
+		>
+	}
+>()

packages/tldraw/src/test/arrows-megabus.test.tsx

@@ -1,5 +1,6 @@
-import { TLArrowShape, Vec, createShapeId } from '@tldraw/editor'
+import { TLArrowShape, TLShapeId, Vec, createShapeId } from '@tldraw/editor'
 import { TestEditor } from './TestEditor'
+import { TL } from './test-jsx'
 
 let editor: TestEditor
 
@@ -650,3 +651,84 @@ describe('When binding an arrow to an ancestor', () => {
 		expect(arrow.props.end.isPrecise).toBe(true)
 	})
 })
+
+describe('Moving a bound arrow', () => {
+	function setup() {
+		editor.createShapesFromJsx([
+			<TL.geo id={ids.box1} x={0} y={0} w={200} h={200} />,
+			<TL.geo id={ids.box2} x={300} y={0} w={200} h={200} />,
+		])
+	}
+
+	function expectBound(handle: 'start' | 'end', boundShapeId: TLShapeId) {
+		expect(editor.getOnlySelectedShape()).toMatchObject({
+			props: { [handle]: { type: 'binding', boundShapeId } },
+		})
+	}
+
+	function expectUnbound(handle: 'start' | 'end') {
+		expect(editor.getOnlySelectedShape()).toMatchObject({
+			props: { [handle]: { type: 'point' } },
+		})
+	}
+
+	it('keeps the start of the arrow bound to the original shape as it moves', () => {
+		setup()
+
+		// draw an arrow pointing down from box1
+		editor.setCurrentTool('arrow').pointerDown(100, 100).pointerMove(100, 300).pointerUp(100, 300)
+		expectBound('start', ids.box1)
+		expectUnbound('end')
+
+		// start translating it:
+		editor.setCurrentTool('select').pointerDown(100, 200)
+
+		// arrow should stay bound to box1 as long as its end is within it:
+		editor.pointerMove(150, 200)
+		expectBound('start', ids.box1)
+		expectUnbound('end')
+
+		// arrow becomes unbound when its end is outside of box1:
+		editor.pointerMove(250, 200)
+		expectUnbound('start')
+		expectUnbound('end')
+
+		// arrow remains unbound when its end is inside of box2:
+		editor.pointerMove(350, 200)
+		expectUnbound('start')
+		expectUnbound('end')
+
+		// arrow becomes re-bound to box1 when it goes back inside box1:
+		editor.pointerMove(100, 200)
+		expectBound('start', ids.box1)
+		expectUnbound('end')
+	})
+
+	it('keeps the end of the arrow bound to the original shape as it moves', () => {
+		setup()
+
+		// draw an arrow pointing from box1 to box2
+		editor.setCurrentTool('arrow').pointerDown(100, 100).pointerMove(400, 200).pointerUp(400, 200)
+		expectBound('start', ids.box1)
+		expectBound('end', ids.box2)
+
+		// start translating it:
+		const center = editor.getShapePageBounds(editor.getOnlySelectedShape()!)!.center
+		editor.setCurrentTool('select').pointerDown(center.x, center.y)
+
+		// arrow should stay bound to box2 as long as its end is within it:
+		editor.pointerMove(center.x + 50, center.y)
+		expectBound('start', ids.box1)
+		expectBound('end', ids.box2)
+
+		// arrow becomes unbound when its end is outside of box2:
+		editor.pointerMove(center.x + 200, 200)
+		expectUnbound('start')
+		expectUnbound('end')
+
+		// arrow becomes re-bound to box2 when it goes back inside box2:
+		editor.pointerMove(center.x + 50, center.y)
+		expectBound('start', ids.box1)
+		expectBound('end', ids.box2)
+	})
+})