[Snapping 2/5] Fix line-handle mid-point snapping (#2831)

Currently, only the end handles of the line tool snap. It should be all
of them.

Line handles work kind of weirdly at the moment: instead of just storing
the positions, we store full `TLHandle` objects complete with IDs,
`canSnap`/`canBind` properties, etc. Currently, all the handles get
written to the store with `canSnap: false`, when really it should be up
to the shape util to decide which handles are snappable.

This diff replaces the current handles map (from arbitrary ID to
`TLHandle`) with just the data we need: a map from index to x/y. The
extra information that the `Editor` needs for `TLHandle` is hydrated at
runtime (with `canSnap` set to `true` this time!)

Fixes TLD-2200

This PR is part of a series - please don't merge it until the things
before it have landed!
1. #2827 
2. #2831 (you are here)
3. #2793
4. #2841
5. #2845

### Change Type

- [x] `major` — Breaking change


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

### Test Plan

1. Create a funky line shape on tldraw.com
2. Paste it into staging and make sure it comes across ok
3. Make some funky line shape in staging - make sure you use dragging,
mid-point creation, and shift-clicking

- [x] Unit Tests

### Release Notes

- Simplify the contents of `TLLineShape.props.handles`

packages/editor/src/lib/editor/managers/SnapManager/HandleSnaps.ts

@@ -38,7 +38,6 @@ export class HandleSnaps {
 		let minDistance = snapThreshold
 		let nearestPoint: Vec | null = null
 		let C: VecModel, D: VecModel, nearest: Vec, distance: number
-
 		const allSegments = [...outlinesInPageSpace, ...additionalSegments]
 		for (const outline of allSegments) {
 			for (let i = 0; i < outline.length - 1; i++) {

packages/tldraw/api-report.md

@@ -921,7 +921,7 @@ export class LineShapeUtil extends ShapeUtil<TLLineShape> {
         dash: EnumStyleProp<"dashed" | "dotted" | "draw" | "solid">;
         size: EnumStyleProp<"l" | "m" | "s" | "xl">;
         spline: EnumStyleProp<"cubic" | "line">;
-        handles: DictValidator<string, TLHandle>;
+        handles: DictValidator<IndexKey, VecModel>;
     };
     // (undocumented)
     toSvg(shape: TLLineShape): SVGGElement;

packages/tldraw/api/api.json

@@ -10994,12 +10994,21 @@
                 },
                 {
                   "kind": "Content",
-                  "text": "<string, "
+                  "text": "<import(\"@tldraw/editor\")."
                 },
                 {
                   "kind": "Reference",
-                  "text": "TLHandle",
-                  "canonicalReference": "@tldraw/tlschema!TLHandle:interface"
+                  "text": "IndexKey",
+                  "canonicalReference": "@tldraw/utils!IndexKey:type"
+                },
+                {
+                  "kind": "Content",
+                  "text": ", import(\"@tldraw/editor\")."
+                },
+                {
+                  "kind": "Reference",
+                  "text": "VecModel",
+                  "canonicalReference": "@tldraw/tlschema!VecModel:interface"
                 },
                 {
                   "kind": "Content",
@@ -11016,7 +11025,7 @@
               "name": "props",
               "propertyTypeTokenRange": {
                 "startIndex": 1,
-                "endIndex": 14
+                "endIndex": 16
               },
               "isStatic": true,
               "isProtected": false,

packages/tldraw/src/lib/shapes/line/LineShapeTool.test.ts

@@ -66,8 +66,8 @@ describe('When dragging the line', () => {
 			y: 0,
 			props: {
 				handles: {
-					start: { id: 'start', index: 'a1', type: 'vertex', x: 0, y: 0 },
-					end: { id: 'end', index: 'a2', type: 'vertex', x: 10, y: 10 },
+					a1: { x: 0, y: 0 },
+					a2: { x: 10, y: 10 },
 				},
 			},
 		})

packages/tldraw/src/lib/shapes/line/LineShapeUtil.test.tsx

@@ -1,5 +1,7 @@
 import { IndexKey, TLGeoShape, TLLineShape, createShapeId, deepCopy } from '@tldraw/editor'
 import { TestEditor } from '../../../test/TestEditor'
+import { TL } from '../../../test/test-jsx'
+import { LineShapeUtil } from './LineShapeUtil'
 
 jest.mock('nanoid', () => {
 	let i = 0
@@ -24,19 +26,11 @@ beforeEach(() => {
 				y: 150,
 				props: {
 					handles: {
-						start: {
-							id: 'start',
-							type: 'vertex',
-							canBind: false,
-							index: 'a1',
+						a1: {
 							x: 0,
 							y: 0,
 						},
-						end: {
-							id: 'end',
-							type: 'vertex',
-							canBind: false,
-							index: 'a2',
+						a2: {
 							x: 100,
 							y: 100,
 						},
@@ -75,39 +69,103 @@ describe('Translating', () => {
 	})
 })
 
-it('create new handle', () => {
-	editor.select(id)
+describe('Mid-point handles', () => {
+	it('create new handle', () => {
+		editor.select(id)
 
-	const shape = editor.getShape<TLLineShape>(id)!
-	editor.pointerDown(200, 200, {
-		target: 'handle',
-		shape,
-		handle: {
-			id: 'mid-0',
-			type: 'create',
-			index: 'a1V' as IndexKey,
-			x: 50,
-			y: 50,
-		},
-	})
-	editor.pointerMove(349, 349).pointerMove(350, 350) // Move handle by 150, 150
-	editor.pointerUp()
+		const shape = editor.getShape<TLLineShape>(id)!
+		editor.pointerDown(200, 200, {
+			target: 'handle',
+			shape,
+			handle: {
+				id: 'mid-0',
+				type: 'create',
+				index: 'a1V' as IndexKey,
+				x: 50,
+				y: 50,
+			},
+		})
+		editor.pointerMove(349, 349).pointerMove(350, 350) // Move handle by 150, 150
+		editor.pointerUp()
 
-	editor.expectShapeToMatch({
-		id: id,
-		props: {
-			handles: {
-				...shape.props.handles,
-				'handle:a1V': {
-					id: 'handle:a1V',
-					type: 'vertex',
-					canBind: false,
-					index: 'a1V',
-					x: 200,
-					y: 200,
+		editor.expectShapeToMatch({
+			id: id,
+			props: {
+				handles: {
+					...shape.props.handles,
+					a1V: { x: 200, y: 200 },
 				},
 			},
-		},
+		})
+	})
+
+	it('allows snapping with mid-point handles', () => {
+		editor.createShapesFromJsx([<TL.geo x={200} y={200} w={100} h={100} />])
+
+		editor.select(id)
+
+		const shape = editor.getShape<TLLineShape>(id)!
+		const util = editor.getShapeUtil('line') as LineShapeUtil
+		editor
+			.pointerDown(200, 200, {
+				target: 'handle',
+				shape,
+				handle: util.getHandles(shape).find((h) => h.id === 'mid-0')!,
+			})
+			.pointerMove(198, 230, undefined, { ctrlKey: true })
+
+		expect(editor.snaps.getIndicators()).toHaveLength(1)
+		editor.expectShapeToMatch({
+			id: id,
+			props: {
+				handles: {
+					...shape.props.handles,
+					a1V: { x: 50, y: 80 },
+				},
+			},
+		})
+	})
+
+	it('allows snapping with created mid-point handles', () => {
+		editor.createShapesFromJsx([<TL.geo x={200} y={200} w={100} h={100} />])
+		editor.select(id)
+
+		const getShape = () => editor.getShape<TLLineShape>(id)!
+		const util = editor.getShapeUtil('line') as LineShapeUtil
+
+		// use a mid-point handle to create a new handle
+		editor
+			.pointerDown(200, 200, {
+				target: 'handle',
+				shape: getShape(),
+				handle: util.getHandles(getShape()).find((h) => h.id === 'mid-0')!,
+			})
+			.pointerMove(230, 200)
+			.pointerMove(200, 200)
+			.pointerUp()
+
+		// 3 actual points, plus 2 mid-points:
+		expect(util.getHandles(getShape())).toHaveLength(5)
+
+		// now, try dragging the newly created handle. it should still snap:
+		editor
+			.pointerDown(200, 200, {
+				target: 'handle',
+				shape: getShape(),
+				handle: util.getHandles(getShape()).find((h) => h.id === 'a1V')!,
+			})
+			.pointerMove(198, 230, undefined, { ctrlKey: true })
+
+		expect(editor.snaps.getIndicators()).toHaveLength(1)
+		editor.expectShapeToMatch({
+			id: id,
+			props: {
+				handles: {
+					...getShape().props.handles,
+					a1V: { x: 50, y: 80 },
+				},
+			},
+		})
 	})
 })
 

packages/tldraw/src/lib/shapes/line/LineShapeUtil.tsx

@@ -1,7 +1,6 @@
 /* eslint-disable react-hooks/rules-of-hooks */
 import {
 	CubicSpline2d,
-	IndexKey,
 	Polyline2d,
 	SVGContainer,
 	ShapeUtil,
@@ -14,8 +13,10 @@ import {
 	deepCopy,
 	getDefaultColorTheme,
 	getIndexBetween,
+	getIndices,
 	lineShapeMigrations,
 	lineShapeProps,
+	objectMapEntries,
 	sortByIndex,
 } from '@tldraw/editor'
 
@@ -45,27 +46,18 @@ export class LineShapeUtil extends ShapeUtil<TLLineShape> {
 	override hideSelectionBoundsBg = () => true
 
 	override getDefaultProps(): TLLineShape['props'] {
+		const [startIndex, endIndex] = getIndices(2)
 		return {
 			dash: 'draw',
 			size: 'm',
 			color: 'black',
 			spline: 'line',
 			handles: {
-				start: {
-					id: 'start',
-					type: 'vertex',
-					canBind: false,
-					canSnap: true,
-					index: 'a1' as IndexKey,
+				[startIndex]: {
 					x: 0,
 					y: 0,
 				},
-				end: {
-					id: 'end',
-					type: 'vertex',
-					canBind: false,
-					canSnap: true,
-					index: 'a2' as IndexKey,
+				[endIndex]: {
 					x: 0.1,
 					y: 0.1,
 				},
@@ -84,7 +76,18 @@ export class LineShapeUtil extends ShapeUtil<TLLineShape> {
 
 			const spline = getGeometryForLineShape(shape)
 
-			const sortedHandles = Object.values(handles).sort(sortByIndex)
+			const sortedHandles = objectMapEntries(handles)
+				.map(
+					([index, handle]): TLHandle => ({
+						id: index,
+						index,
+						...handle,
+						type: 'vertex',
+						canBind: false,
+						canSnap: true,
+					})
+				)
+				.sort(sortByIndex)
 			const results = sortedHandles.slice()
 
 			// Add "create" handles between each vertex handle
@@ -99,6 +102,8 @@ export class LineShapeUtil extends ShapeUtil<TLLineShape> {
 					index,
 					x: point.x,
 					y: point.y,
+					canSnap: true,
+					canBind: false,
 				})
 			}
 
@@ -118,9 +123,9 @@ export class LineShapeUtil extends ShapeUtil<TLLineShape> {
 
 		const handles = deepCopy(shape.props.handles)
 
-		Object.values(shape.props.handles).forEach(({ id, x, y }) => {
-			handles[id].x = x * scaleX
-			handles[id].y = y * scaleY
+		objectMapEntries(shape.props.handles).forEach(([index, { x, y }]) => {
+			handles[index].x = x * scaleX
+			handles[index].y = y * scaleY
 		})
 
 		return {
@@ -131,45 +136,16 @@ export class LineShapeUtil extends ShapeUtil<TLLineShape> {
 	}
 
 	override onHandleDrag: TLOnHandleDragHandler<TLLineShape> = (shape, { handle }) => {
-		const next = deepCopy(shape)
-
-		switch (handle.id) {
-			case 'start':
-			case 'end': {
-				next.props.handles[handle.id] = {
-					...next.props.handles[handle.id],
-					x: handle.x,
-					y: handle.y,
-				}
-				break
-			}
-
-			default: {
-				const id = 'handle:' + handle.index
-				const existing = shape.props.handles[id]
-
-				if (existing) {
-					next.props.handles[id] = {
-						...existing,
-						x: handle.x,
-						y: handle.y,
-					}
-				} else {
-					next.props.handles[id] = {
-						id,
-						type: 'vertex',
-						canBind: false,
-						index: handle.index,
-						x: handle.x,
-						y: handle.y,
-					}
-				}
-
-				break
-			}
+		return {
+			...shape,
+			props: {
+				...shape.props,
+				handles: {
+					...shape.props.handles,
+					[handle.index]: { x: handle.x, y: handle.y },
+				},
+			},
 		}
-
-		return next
 	}
 
 	component(shape: TLLineShape) {
@@ -409,7 +385,10 @@ export class LineShapeUtil extends ShapeUtil<TLLineShape> {
 /** @public */
 export function getGeometryForLineShape(shape: TLLineShape): CubicSpline2d | Polyline2d {
 	const { spline, handles } = shape.props
-	const handlePoints = Object.values(handles).sort(sortByIndex).map(Vec.From)
+	const handlePoints = objectMapEntries(handles)
+		.map(([index, position]) => ({ index, ...position }))
+		.sort(sortByIndex)
+		.map(Vec.From)
 
 	switch (spline) {
 		case 'cubic': {

packages/tldraw/src/lib/shapes/line/__snapshots__/LineShapeUtil.test.tsx.snap

@@ -12,22 +12,14 @@ exports[`Misc resizes: line shape after resize 1`] = `
     "color": "black",
     "dash": "draw",
     "handles": {
-      "end": {
-        "canBind": false,
-        "id": "end",
-        "index": "a2",
-        "type": "vertex",
-        "x": 100,
-        "y": 700,
-      },
-      "start": {
-        "canBind": false,
-        "id": "start",
-        "index": "a1",
-        "type": "vertex",
+      "a1": {
         "x": 0,
         "y": 0,
       },
+      "a2": {
+        "x": 100,
+        "y": 700,
+      },
     },
     "size": "m",
     "spline": "line",

packages/tldraw/src/lib/shapes/line/toolStates/Pointing.ts

@@ -3,11 +3,11 @@ import {
 	Mat,
 	StateNode,
 	TLEventHandlers,
-	TLHandle,
 	TLInterruptEvent,
 	TLLineShape,
 	TLShapeId,
 	Vec,
+	VecModel,
 	createShapeId,
 	getIndexAbove,
 	last,
@@ -51,7 +51,7 @@ export class Pointing extends StateNode {
 				new Vec(this.shape.x, this.shape.y)
 			)
 
-			let nextEndHandleIndex: IndexKey, nextEndHandleId: string, nextEndHandle: TLHandle
+			let nextEndHandleIndex: IndexKey, nextEndHandle: VecModel
 
 			const nextPoint = Vec.Sub(currentPagePoint, shapePagePoint)
 
@@ -61,29 +61,22 @@ export class Pointing extends StateNode {
 			) {
 				// If the end handle is too close to the previous end handle, we'll just extend the previous end handle
 				nextEndHandleIndex = endHandle.index
-				nextEndHandleId = endHandle.id
 				nextEndHandle = {
-					...endHandle,
 					x: nextPoint.x + 0.1,
 					y: nextPoint.y + 0.1,
 				}
 			} else {
 				// Otherwise, we'll create a new end handle
 				nextEndHandleIndex = getIndexAbove(endHandle.index)
-				nextEndHandleId = 'handle:' + nextEndHandleIndex
 				nextEndHandle = {
-					id: nextEndHandleId,
-					type: 'vertex',
-					index: nextEndHandleIndex,
 					x: nextPoint.x + 0.1,
 					y: nextPoint.y + 0.1,
-					canBind: false,
 				}
 			}
 
 			const nextHandles = structuredClone(this.shape.props.handles)
 
-			nextHandles[nextEndHandle.id] = nextEndHandle
+			nextHandles[nextEndHandleIndex] = nextEndHandle
 
 			this.editor.updateShapes([
 				{

packages/tlschema/api-report.md

@@ -674,7 +674,7 @@ export const lineShapeProps: {
     dash: EnumStyleProp<"dashed" | "dotted" | "draw" | "solid">;
     size: EnumStyleProp<"l" | "m" | "s" | "xl">;
     spline: EnumStyleProp<"cubic" | "line">;
-    handles: T.DictValidator<string, TLHandle>;
+    handles: T.DictValidator<IndexKey, VecModel>;
 };
 
 // @public (undocumented)

packages/tlschema/api/api.json

@@ -2765,12 +2765,21 @@
             },
             {
               "kind": "Content",
-              "text": "<string, import(\"../misc/TLHandle\")."
+              "text": "<import(\"@tldraw/utils\")."
             },
             {
               "kind": "Reference",
-              "text": "TLHandle",
-              "canonicalReference": "@tldraw/tlschema!TLHandle:interface"
+              "text": "IndexKey",
+              "canonicalReference": "@tldraw/utils!IndexKey:type"
+            },
+            {
+              "kind": "Content",
+              "text": ", import(\"../misc/geometry-types\")."
+            },
+            {
+              "kind": "Reference",
+              "text": "VecModel",
+              "canonicalReference": "@tldraw/tlschema!VecModel:interface"
             },
             {
               "kind": "Content",
@@ -2783,7 +2792,7 @@
           "name": "lineShapeProps",
           "variableTypeTokenRange": {
             "startIndex": 1,
-            "endIndex": 14
+            "endIndex": 16
           }
         },
         {

packages/tlschema/src/migrations.test.ts

@@ -1844,6 +1844,99 @@ describe('Add duplicate props to instance', () => {
 	})
 })
 
+describe('Remove extra handle props', () => {
+	const { up, down } = lineShapeMigrations.migrators[lineShapeVersions.RemoveExtraHandleProps]
+	it('up works as expected', () => {
+		expect(
+			up({
+				props: {
+					handles: {
+						start: {
+							id: 'start',
+							type: 'vertex',
+							canBind: false,
+							canSnap: true,
+							index: 'a1',
+							x: 0,
+							y: 0,
+						},
+						end: {
+							id: 'end',
+							type: 'vertex',
+							canBind: false,
+							canSnap: true,
+							index: 'a2',
+							x: 190,
+							y: -62,
+						},
+						'handle:a1V': {
+							id: 'handle:a1V',
+							type: 'vertex',
+							canBind: false,
+							index: 'a1V',
+							x: 76,
+							y: 60,
+						},
+					},
+				},
+			})
+		).toEqual({
+			props: {
+				handles: {
+					a1: { x: 0, y: 0 },
+					a1V: { x: 76, y: 60 },
+					a2: { x: 190, y: -62 },
+				},
+			},
+		})
+	})
+	it('down works as expected', () => {
+		expect(
+			down({
+				props: {
+					handles: {
+						a1: { x: 0, y: 0 },
+						a1V: { x: 76, y: 60 },
+						a2: { x: 190, y: -62 },
+					},
+				},
+			})
+		).toEqual({
+			props: {
+				handles: {
+					start: {
+						id: 'start',
+						type: 'vertex',
+						canBind: false,
+						canSnap: true,
+						index: 'a1',
+						x: 0,
+						y: 0,
+					},
+					end: {
+						id: 'end',
+						type: 'vertex',
+						canBind: false,
+						canSnap: true,
+						index: 'a2',
+						x: 190,
+						y: -62,
+					},
+					'handle:a1V': {
+						id: 'handle:a1V',
+						type: 'vertex',
+						canBind: false,
+						canSnap: true,
+						index: 'a1V',
+						x: 76,
+						y: 60,
+					},
+				},
+			},
+		})
+	})
+})
+
 /* ---  PUT YOUR MIGRATIONS TESTS ABOVE HERE --- */
 
 for (const migrator of allMigrators) {

packages/tlschema/src/misc/TLHandle.ts

@@ -1,5 +1,4 @@
 import { IndexKey } from '@tldraw/utils'
-import { T } from '@tldraw/validate'
 import { SetValue } from '../util-types'
 
 /**
@@ -29,14 +28,3 @@ export interface TLHandle {
 	x: number
 	y: number
 }
-
-/** @internal */
-export const handleValidator: T.Validator<TLHandle> = T.object({
-	id: T.string,
-	type: T.setEnum(TL_HANDLE_TYPES),
-	canBind: T.boolean.optional(),
-	canSnap: T.boolean.optional(),
-	index: T.indexKey,
-	x: T.number,
-	y: T.number,
-})

packages/tlschema/src/shapes/TLLineShape.ts

@@ -1,7 +1,7 @@
 import { defineMigrations } from '@tldraw/store'
-import { deepCopy } from '@tldraw/utils'
+import { deepCopy, objectMapFromEntries, sortByIndex } from '@tldraw/utils'
 import { T } from '@tldraw/validate'
-import { handleValidator } from '../misc/TLHandle'
+import { vecModelValidator } from '../misc/geometry-types'
 import { StyleProp } from '../styles/StyleProp'
 import { DefaultColorStyle } from '../styles/TLColorStyle'
 import { DefaultDashStyle } from '../styles/TLDashStyle'
@@ -23,7 +23,7 @@ export const lineShapeProps = {
 	dash: DefaultDashStyle,
 	size: DefaultSizeStyle,
 	spline: LineShapeSplineStyle,
-	handles: T.dict(T.string, handleValidator),
+	handles: T.dict(T.indexKey, vecModelValidator),
 }
 
 /** @public */
@@ -35,11 +35,12 @@ export type TLLineShape = TLBaseShape<'line', TLLineShapeProps>
 /** @internal */
 export const lineShapeVersions = {
 	AddSnapHandles: 1,
+	RemoveExtraHandleProps: 2,
 } as const
 
 /** @internal */
 export const lineShapeMigrations = defineMigrations({
-	currentVersion: lineShapeVersions.AddSnapHandles,
+	currentVersion: lineShapeVersions.RemoveExtraHandleProps,
 	migrators: {
 		[lineShapeVersions.AddSnapHandles]: {
 			up: (record: any) => {
@@ -57,5 +58,54 @@ export const lineShapeMigrations = defineMigrations({
 				return { ...record, props: { ...record.props, handles } }
 			},
 		},
+		[lineShapeVersions.RemoveExtraHandleProps]: {
+			up: (record: any) => {
+				return {
+					...record,
+					props: {
+						...record.props,
+						handles: objectMapFromEntries(
+							Object.values(record.props.handles).map((handle: any) => [
+								handle.index,
+								{
+									x: handle.x,
+									y: handle.y,
+								},
+							])
+						),
+					},
+				}
+			},
+			down: (record: any) => {
+				const handles = Object.entries(record.props.handles)
+					.map(([index, handle]: any) => ({ index, ...handle }))
+					.sort(sortByIndex)
+
+				return {
+					...record,
+					props: {
+						...record.props,
+						handles: Object.fromEntries(
+							handles.map((handle, i) => {
+								const id =
+									i === 0 ? 'start' : i === handles.length - 1 ? 'end' : `handle:${handle.index}`
+								return [
+									id,
+									{
+										id,
+										type: 'vertex',
+										canBind: false,
+										canSnap: true,
+										index: handle.index,
+										x: handle.x,
+										y: handle.y,
+									},
+								]
+							})
+						),
+					},
+				}
+			},
+		},
 	},
 })