From 231354d93c521c12071105fce1ae486c96aa862d Mon Sep 17 00:00:00 2001 From: alex Date: Sat, 13 Jan 2024 20:09:05 +0000 Subject: [PATCH] Maintain bindings whilst translating arrows (#2424) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- packages/editor/src/lib/editor/Editor.ts | 171 ++++++++---------- packages/tldraw/api-report.md | 3 + packages/tldraw/api/api.json | 44 +++++ .../lib/shapes/arrow/ArrowShapeUtil.test.ts | 28 +-- .../src/lib/shapes/arrow/ArrowShapeUtil.tsx | 123 +++++++++++-- ...egabus.test.ts => arrows-megabus.test.tsx} | 84 ++++++++- 6 files changed, 323 insertions(+), 130 deletions(-) rename packages/tldraw/src/test/{arrows-megabus.test.ts => arrows-megabus.test.tsx} (87%) diff --git a/packages/editor/src/lib/editor/Editor.ts b/packages/editor/src/lib/editor/Editor.ts index c94901fbf..ebfc0f2f6 100644 --- a/packages/editor/src/lib/editor/Editor.ts +++ b/packages/editor/src/lib/editor/Editor.ts @@ -5076,6 +5076,35 @@ export class Editor extends EventEmitter { 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 { const changes: TLShapePartial[] = [] for (const id of ids) { - const shape = this.getShape(id) - - if (!shape) { - throw Error(`Could not find a shape with the id ${id}.`) - } - + const shape = this.getShape(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( - 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, - } - ) + changes.push(this.getChangesToTranslateShape(shape, localDelta.add(shape))) } this.updateShapes(changes, { @@ -5989,22 +5998,7 @@ export class Editor extends EventEmitter { ? Vec.Rot(delta, -this.getShapePageTransform(parent)!.decompose().rotation) : delta - const translateChanges = this.getShapeUtil(shape).onTranslateStart?.(shape) - - 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, - } - ) + changes.push(this.getChangesToTranslateShape(shape, Vec.Add(shape, localDelta))) }) this.updateShapes(changes) @@ -6082,20 +6076,8 @@ export class Editor extends EventEmitter { const localDelta = parent ? Vec.Rot(delta, -this.getShapePageTransform(parent)!.rotation()) : delta - const translateStartChanges = this.getShapeUtil(shape).onTranslateStart?.(shape) - changes.push( - translateStartChanges - ? { - ...translateStartChanges, - [val]: shape[val] + localDelta[val], - } - : { - id: shape.id, - type: shape.type, - [val]: shape[val] + localDelta[val], - } - ) + changes.push(this.getChangesToTranslateShape(shape, Vec.Add(shape, localDelta))) }) this.updateShapes(changes) @@ -7017,47 +6999,7 @@ export class Editor extends EventEmitter { partials.map((partial) => { const prev = snapshots[partial.id] if (!prev) return null - let newRecord = null as null | TLShape - 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 + return applyPartialToShape(prev, partial) }) ) @@ -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(prev: T, partial?: TLShapePartial): 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 +} diff --git a/packages/tldraw/api-report.md b/packages/tldraw/api-report.md index bfbefde80..1e09ce06d 100644 --- a/packages/tldraw/api-report.md +++ b/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 { // (undocumented) onResize: TLOnResizeHandler; // (undocumented) + onTranslate?: TLOnTranslateHandler; + // (undocumented) onTranslateStart: TLOnTranslateStartHandler; // (undocumented) static props: { diff --git a/packages/tldraw/api/api.json b/packages/tldraw/api/api.json index 762853bab..2ea7613a2 100644 --- a/packages/tldraw/api/api.json +++ b/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", diff --git a/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.test.ts b/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.test.ts index b306069ad..31d6bf6e8 100644 --- a/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.test.ts +++ b/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', diff --git a/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx b/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx index 8334720b9..99a54ef61 100644 --- a/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx +++ b/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 { 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 { return } - const { start, end } = getArrowTerminalsInArrowSpace(this.editor, shape) + let result = shape - return { - id: shape.id, - type: shape.type, - props: { - ...shape.props, - start: { - type: 'point', - x: start.x, - y: start.y, - }, - end: { - type: 'point', - x: end.x, - y: end.y, - }, - }, + // When we start translating shapes, record where their bindings were in page space so we + // can maintain them as we translate the arrow + shapeAtTranslationStart.set(shape, { + pagePosition: shapePageTransform.applyToPoint(shape), + terminalBindings: mapObjectMapValues(terminalsInArrowSpace, (terminalName, point) => { + const terminal = shape.props[terminalName] + if (terminal.type !== 'binding') return null + return { + binding: terminal, + shapePosition: point, + pagePosition: shapePageTransform.applyToPoint(point), + } + }), + }) + + 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 = (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 = (shape, info) => { @@ -499,6 +572,7 @@ export class ArrowShapeUtil extends ShapeUtil { '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 + } | null + > + } +>() diff --git a/packages/tldraw/src/test/arrows-megabus.test.ts b/packages/tldraw/src/test/arrows-megabus.test.tsx similarity index 87% rename from packages/tldraw/src/test/arrows-megabus.test.ts rename to packages/tldraw/src/test/arrows-megabus.test.tsx index 4cfba92e4..cd86941a7 100644 --- a/packages/tldraw/src/test/arrows-megabus.test.ts +++ b/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([ + , + , + ]) + } + + 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) + }) +})