From 7d699a749f6b384910a1e4361d477790f0658262 Mon Sep 17 00:00:00 2001 From: Steve Ruiz Date: Fri, 1 Dec 2023 22:34:12 +0100 Subject: [PATCH] [improvements] arrows x enclosing shapes x precision. (#2265) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR makes several improvements to the behavior of arrows as they relate to precision and container relationships. - an arrow's terminals are always "true" and are never snapped to { x: .5, y: .5 } as they were previously when not precise - instead, a new `isPrecise` boolean is added to the arrow terminal - when an arrow terminal renders "imprecisely" it will be placed to the center of the bound shape - when an arrow terminal renders "precisely" it will be placed at the normalized location within the bound shape ![Kapture 2023-11-29 at 23 12 12](https://github.com/tldraw/tldraw/assets/23072548/e94e1594-75fa-4c94-86f3-7d911bf25f7f) The logic now is... - if the user has indicated precision by "pausing" while drawing the arrow, it will be precise - otherwise... - if both of an arrow's terminals are bound to the same shape, both will be precise - if a terminal is bound to a shape that contains the shape that its opposite terminal is bound to, it will be precise - if a terminal is bound to a shape that contains the shape that its opposite terminal is bound to, it will be precise - or else it will be imprecise If the spatial relationships change, the precision may change as well. Fixes https://github.com/tldraw/tldraw/issues/2204 Note: a previous version of this PR was based around ancestry but that's not actually important. ### Change Type - [x] `minor` — New feature ### Test Plan 1. Draw an arrow between a frame and its descendant 2. Draw an arrow inside of a shape to another shape contained within the bounds of the big shape 3. Vis versa 4. Vis versa - [x] Unit Tests ### Release Notes - Improves the logic about when to draw "precise" arrows between the center of bound shapes. --- packages/editor/src/lib/editor/Editor.ts | 4 +- .../shapes/shared/arrow/curved-arrow.ts | 37 +++-- .../lib/editor/shapes/shared/arrow/shared.ts | 74 +++++++++- .../shapes/shared/arrow/straight-arrow.ts | 54 +++++-- packages/tldraw/api-report.md | 2 + packages/tldraw/api/api.json | 8 +- .../lib/shapes/arrow/ArrowShapeTool.test.ts | 11 +- .../lib/shapes/arrow/ArrowShapeUtil.test.ts | 8 + .../src/lib/shapes/arrow/ArrowShapeUtil.tsx | 48 +++--- .../SelectTool/childStates/DraggingHandle.ts | 2 +- .../SelectTool/children/DraggingHandle.ts | 2 +- .../hooks/clipboard/pasteExcalidrawContent.ts | 2 + .../src/test/arrowBindingsIndex.test.tsx | 1 + .../tldraw/src/test/arrows-megabus.test.ts | 95 ++++++++++-- .../src/test/commands/clipboard.test.ts | 4 + .../src/test/commands/deleteShapes.test.ts | 2 + packages/tldraw/src/test/duplicate.test.ts | 8 + packages/tldraw/src/test/flipShapes.test.ts | 6 + packages/tldraw/src/test/translating.test.ts | 4 + packages/tlschema/api-report.md | 2 + packages/tlschema/api/api.json | 4 +- packages/tlschema/src/migrations.test.ts | 137 +++++++++++++++++- packages/tlschema/src/shapes/TLArrowShape.ts | 60 +++++++- 23 files changed, 488 insertions(+), 87 deletions(-) diff --git a/packages/editor/src/lib/editor/Editor.ts b/packages/editor/src/lib/editor/Editor.ts index a677a180a..b4c128523 100644 --- a/packages/editor/src/lib/editor/Editor.ts +++ b/packages/editor/src/lib/editor/Editor.ts @@ -4353,9 +4353,7 @@ export class Editor extends EventEmitter { /** @internal */ @computed private _getShapeMaskCache(): ComputedCache { return this.store.createComputedCache('pageMaskCache', (shape) => { - if (isPageId(shape.parentId)) { - return undefined - } + if (isPageId(shape.parentId)) return undefined const frameAncestors = this.getShapeAncestors(shape.id).filter((shape) => this.isShapeOfType(shape, 'frame') diff --git a/packages/editor/src/lib/editor/shapes/shared/arrow/curved-arrow.ts b/packages/editor/src/lib/editor/shapes/shared/arrow/curved-arrow.ts index 9ce2d4938..3194b46eb 100644 --- a/packages/editor/src/lib/editor/shapes/shared/arrow/curved-arrow.ts +++ b/packages/editor/src/lib/editor/shapes/shared/arrow/curved-arrow.ts @@ -19,6 +19,7 @@ import { WAY_TOO_BIG_ARROW_BEND_FACTOR, getArrowTerminalsInArrowSpace, getBoundShapeInfoForTerminal, + getBoundShapeRelationships, } from './shared' import { getStraightArrowInfo } from './straight-arrow' @@ -94,6 +95,8 @@ export function getCurvedArrowInfo( let offsetA = 0 let offsetB = 0 + let minLength = MIN_ARROW_LENGTH + if (startShapeInfo && !startShapeInfo.isExact) { const startInPageSpace = Matrix2d.applyToPoint(arrowPageTransform, tempA) const centerInPageSpace = Matrix2d.applyToPoint(arrowPageTransform, handleArc.center) @@ -151,12 +154,13 @@ export function getCurvedArrowInfo( startShapeInfo.didIntersect = true if (arrowheadStart !== 'none') { - offsetA = - BOUND_ARROW_OFFSET + + const strokeOffset = STROKE_SIZES[shape.props.size] / 2 + ('size' in startShapeInfo.shape.props ? STROKE_SIZES[startShapeInfo.shape.props.size] / 2 : 0) + offsetA = BOUND_ARROW_OFFSET + strokeOffset + minLength += strokeOffset } } } @@ -224,10 +228,11 @@ export function getCurvedArrowInfo( endShapeInfo.didIntersect = true if (arrowheadEnd !== 'none') { - offsetB = - BOUND_ARROW_OFFSET + + const strokeOffset = STROKE_SIZES[shape.props.size] / 2 + ('size' in endShapeInfo.shape.props ? STROKE_SIZES[endShapeInfo.shape.props.size] / 2 : 0) + offsetB = BOUND_ARROW_OFFSET + strokeOffset + minLength += strokeOffset } } } @@ -258,7 +263,7 @@ export function getCurvedArrowInfo( } const distAB = Vec2d.Dist(tA, tB) - if (distAB < MIN_ARROW_LENGTH) { + if (distAB < minLength) { if (offsetA !== 0 && offsetB !== 0) { offsetA *= -1.5 offsetB *= -1.5 @@ -267,10 +272,7 @@ export function getCurvedArrowInfo( } else if (offsetB !== 0) { offsetB *= -2 } else { - if (distAB < 10) { - if (startShapeInfo) offsetA = -(10 - distAB) - else if (endShapeInfo) offsetB = -(10 - distAB) - } + // noop } } @@ -292,14 +294,17 @@ export function getCurvedArrowInfo( aCB = Vec2d.Angle(handleArc.center, tempB) // angle center -> b dAB = distFn(aCA, aCB) // angle distance between a and b lAB = dAB * handleArc.radius // length of arc between a and b + const relationship = getBoundShapeRelationships( + editor, + startShapeInfo.shape.id, + endShapeInfo.shape.id + ) - if (startShapeInfo.shape === endShapeInfo.shape) { - if (lAB < 100) { - tempA.setTo(a) - tempB.setTo(b) - tempC.setTo(c) - } - } else { + if (relationship === 'double-bound' && lAB < 30) { + tempA.setTo(a) + tempB.setTo(b) + tempC.setTo(c) + } else if (relationship === 'safe') { if (startShapeInfo && !startShapeInfo.didIntersect) { tempA.setTo(a) } diff --git a/packages/editor/src/lib/editor/shapes/shared/arrow/shared.ts b/packages/editor/src/lib/editor/shapes/shared/arrow/shared.ts index cdc5b2990..729b29483 100644 --- a/packages/editor/src/lib/editor/shapes/shared/arrow/shared.ts +++ b/packages/editor/src/lib/editor/shapes/shared/arrow/shared.ts @@ -1,4 +1,4 @@ -import { TLArrowShape, TLArrowShapeTerminal, TLShape } from '@tldraw/tlschema' +import { TLArrowShape, TLArrowShapeTerminal, TLShape, TLShapeId } from '@tldraw/tlschema' import { Matrix2d } from '../../../../primitives/Matrix2d' import { Vec2d } from '../../../../primitives/Vec2d' import { Group2d } from '../../../../primitives/geometry/Group2d' @@ -48,7 +48,8 @@ export function getBoundShapeInfoForTerminal( export function getArrowTerminalInArrowSpace( editor: Editor, arrowPageTransform: Matrix2d, - terminal: TLArrowShapeTerminal + terminal: TLArrowShapeTerminal, + forceImprecise: boolean ) { if (terminal.type === 'point') { return Vec2d.From(terminal) @@ -64,7 +65,14 @@ export function getArrowTerminalInArrowSpace( // the bound shape and transform it to page space, then transform // it to arrow space const { point, size } = editor.getShapeGeometry(boundShape).bounds - const shapePoint = Vec2d.Add(point, Vec2d.MulV(terminal.normalizedAnchor, size)) + const shapePoint = Vec2d.Add( + point, + Vec2d.MulV( + // if the parent is the bound shape, then it's ALWAYS precise + terminal.isPrecise || forceImprecise ? terminal.normalizedAnchor : { x: 0.5, y: 0.5 }, + size + ) + ) const pagePoint = Matrix2d.applyToPoint(editor.getShapePageTransform(boundShape)!, shapePoint) const arrowPoint = Matrix2d.applyToPoint(Matrix2d.Inverse(arrowPageTransform), pagePoint) return arrowPoint @@ -75,14 +83,39 @@ export function getArrowTerminalInArrowSpace( export function getArrowTerminalsInArrowSpace(editor: Editor, shape: TLArrowShape) { const arrowPageTransform = editor.getShapePageTransform(shape)! - const start = getArrowTerminalInArrowSpace(editor, arrowPageTransform, shape.props.start) - const end = getArrowTerminalInArrowSpace(editor, arrowPageTransform, shape.props.end) + let startBoundShapeId: TLShapeId | undefined + let endBoundShapeId: TLShapeId | undefined + + if (shape.props.start.type === 'binding' && shape.props.end.type === 'binding') { + startBoundShapeId = shape.props.start.boundShapeId + endBoundShapeId = shape.props.end.boundShapeId + } + + const boundShapeRelationships = getBoundShapeRelationships( + editor, + startBoundShapeId, + endBoundShapeId + ) + + const start = getArrowTerminalInArrowSpace( + editor, + arrowPageTransform, + shape.props.start, + boundShapeRelationships === 'double-bound' || boundShapeRelationships === 'start-contains-end' + ) + + const end = getArrowTerminalInArrowSpace( + editor, + arrowPageTransform, + shape.props.end, + boundShapeRelationships === 'double-bound' || boundShapeRelationships === 'end-contains-start' + ) return { start, end } } /** @internal */ -export const MIN_ARROW_LENGTH = 32 +export const MIN_ARROW_LENGTH = 10 /** @internal */ export const BOUND_ARROW_OFFSET = 10 /** @internal */ @@ -95,3 +128,32 @@ export const STROKE_SIZES: Record = { l: 5, xl: 10, } + +/** + * Get the relationships for an arrow that has two bound shape terminals. + * If the arrow has only one bound shape, then it is always "safe" to apply + * standard offsets and precision behavior. If the shape is bound to the same + * shape on both ends, then that is an exception. If one of the shape's + * terminals is bound to a shape that contains / is contained by the shape that + * is bound to the other terminal, then that is also an exception. + * + * @param editor - the editor instance + * @param startShapeId - the bound shape from the arrow's start + * @param endShapeId - the bound shape from the arrow's end + * + * @internal */ +export function getBoundShapeRelationships( + editor: Editor, + startShapeId?: TLShapeId, + endShapeId?: TLShapeId +) { + if (!startShapeId || !endShapeId) return 'safe' + if (startShapeId === endShapeId) return 'double-bound' + const startBounds = editor.getShapePageBounds(startShapeId) + const endBounds = editor.getShapePageBounds(endShapeId) + if (startBounds && endBounds) { + if (startBounds.contains(endBounds)) return 'start-contains-end' + if (endBounds.contains(startBounds)) return 'end-contains-start' + } + return 'safe' +} diff --git a/packages/editor/src/lib/editor/shapes/shared/arrow/straight-arrow.ts b/packages/editor/src/lib/editor/shapes/shared/arrow/straight-arrow.ts index 11a13b5b6..838f63181 100644 --- a/packages/editor/src/lib/editor/shapes/shared/arrow/straight-arrow.ts +++ b/packages/editor/src/lib/editor/shapes/shared/arrow/straight-arrow.ts @@ -15,6 +15,7 @@ import { STROKE_SIZES, getArrowTerminalsInArrowSpace, getBoundShapeInfoForTerminal, + getBoundShapeRelationships, } from './shared' export function getStraightArrowInfo(editor: Editor, shape: TLArrowShape): TLArrowInfo { @@ -72,11 +73,20 @@ export function getStraightArrowInfo(editor: Editor, shape: TLArrowShape): TLArr let offsetA = 0 let offsetB = 0 + let strokeOffsetA = 0 + let strokeOffsetB = 0 + let minLength = MIN_ARROW_LENGTH const isSelfIntersection = startShapeInfo && endShapeInfo && startShapeInfo.shape === endShapeInfo.shape + const relationship = + startShapeInfo && endShapeInfo + ? getBoundShapeRelationships(editor, startShapeInfo.shape.id, endShapeInfo.shape.id) + : 'safe' + if ( + relationship === 'safe' && startShapeInfo && endShapeInfo && !isSelfIntersection && @@ -86,6 +96,7 @@ export function getStraightArrowInfo(editor: Editor, shape: TLArrowShape): TLArr if (endShapeInfo.didIntersect && !startShapeInfo.didIntersect) { // ...and if only the end shape intersected, then make it // a short arrow ending at the end shape intersection. + if (startShapeInfo.isClosed) { a.setTo(b.clone().add(uAB.clone().mul(MIN_ARROW_LENGTH))) } @@ -103,43 +114,58 @@ export function getStraightArrowInfo(editor: Editor, shape: TLArrowShape): TLArr const didFlip = !Vec2d.Equals(u, uAB) // If the arrow is bound non-exact to a start shape and the - // start point has an arrowhead offset the start point + // start point has an arrowhead, then offset the start point if (!isSelfIntersection) { - if (startShapeInfo && arrowheadStart !== 'none' && !startShapeInfo.isExact) { - offsetA = - BOUND_ARROW_OFFSET + + if ( + relationship !== 'start-contains-end' && + startShapeInfo && + arrowheadStart !== 'none' && + !startShapeInfo.isExact + ) { + strokeOffsetA = STROKE_SIZES[shape.props.size] / 2 + ('size' in startShapeInfo.shape.props ? STROKE_SIZES[startShapeInfo.shape.props.size] / 2 : 0) + offsetA = BOUND_ARROW_OFFSET + strokeOffsetA + minLength += strokeOffsetA } // If the arrow is bound non-exact to an end shape and the // end point has an arrowhead offset the end point - if (endShapeInfo && arrowheadEnd !== 'none' && !endShapeInfo.isExact) { - offsetB = - BOUND_ARROW_OFFSET + + if ( + relationship !== 'end-contains-start' && + endShapeInfo && + arrowheadEnd !== 'none' && + !endShapeInfo.isExact + ) { + strokeOffsetB = STROKE_SIZES[shape.props.size] / 2 + ('size' in endShapeInfo.shape.props ? STROKE_SIZES[endShapeInfo.shape.props.size] / 2 : 0) + offsetB = BOUND_ARROW_OFFSET + strokeOffsetB + minLength += strokeOffsetB } } + // Adjust offsets if the length of the arrow is too small + const tA = a.clone().add(u.clone().mul(offsetA * (didFlip ? -1 : 1))) const tB = b.clone().sub(u.clone().mul(offsetB * (didFlip ? -1 : 1))) const distAB = Vec2d.Dist(tA, tB) - if (distAB < MIN_ARROW_LENGTH) { + + if (distAB < minLength) { if (offsetA !== 0 && offsetB !== 0) { + // both bound + offset offsetA *= -1.5 offsetB *= -1.5 } else if (offsetA !== 0) { - offsetA *= -2 + // start bound + offset + offsetA *= -1 } else if (offsetB !== 0) { - offsetB *= -2 + // end bound + offset + offsetB *= -1 } else { - if (distAB < 10) { - if (startShapeInfo) offsetA = -(10 - distAB) - else if (endShapeInfo) offsetB = -(10 - distAB) - } + // noop, its just a really short arrow } } diff --git a/packages/tldraw/api-report.md b/packages/tldraw/api-report.md index 6ca16fe84..1c8e83aa4 100644 --- a/packages/tldraw/api-report.md +++ b/packages/tldraw/api-report.md @@ -178,6 +178,7 @@ export class ArrowShapeUtil extends ShapeUtil { boundShapeId: TLShapeId; normalizedAnchor: Vec2dModel; isExact: boolean; + isPrecise: boolean; }>; point: ObjectValidator< { type: "point"; @@ -191,6 +192,7 @@ export class ArrowShapeUtil extends ShapeUtil { boundShapeId: TLShapeId; normalizedAnchor: Vec2dModel; isExact: boolean; + isPrecise: boolean; }>; point: ObjectValidator< { type: "point"; diff --git a/packages/tldraw/api/api.json b/packages/tldraw/api/api.json index 42cd82216..7a02a2da7 100644 --- a/packages/tldraw/api/api.json +++ b/packages/tldraw/api/api.json @@ -1317,7 +1317,7 @@ }, { "kind": "Content", - "text": ";\n normalizedAnchor: " + "text": ";\n normalizedAnchor: import(\"@tldraw/editor\")." }, { "kind": "Reference", @@ -1326,7 +1326,7 @@ }, { "kind": "Content", - "text": ";\n isExact: boolean;\n }>;\n point: import(\"@tldraw/editor\")." + "text": ";\n isExact: boolean;\n isPrecise: boolean;\n }>;\n point: import(\"@tldraw/editor\")." }, { "kind": "Reference", @@ -1362,7 +1362,7 @@ }, { "kind": "Content", - "text": ";\n normalizedAnchor: " + "text": ";\n normalizedAnchor: import(\"@tldraw/editor\")." }, { "kind": "Reference", @@ -1371,7 +1371,7 @@ }, { "kind": "Content", - "text": ";\n isExact: boolean;\n }>;\n point: import(\"@tldraw/editor\")." + "text": ";\n isExact: boolean;\n isPrecise: boolean;\n }>;\n point: import(\"@tldraw/editor\")." }, { "kind": "Reference", diff --git a/packages/tldraw/src/lib/shapes/arrow/ArrowShapeTool.test.ts b/packages/tldraw/src/lib/shapes/arrow/ArrowShapeTool.test.ts index fb92ebf82..7a8052451 100644 --- a/packages/tldraw/src/lib/shapes/arrow/ArrowShapeTool.test.ts +++ b/packages/tldraw/src/lib/shapes/arrow/ArrowShapeTool.test.ts @@ -150,6 +150,7 @@ describe('When pointing a start shape', () => { type: 'binding', isExact: false, normalizedAnchor: { x: 0.5, y: 0.5 }, // center! + isPrecise: false, boundShapeId: ids.box3, }, end: { type: 'point', x: 0, y: 125 }, @@ -191,6 +192,7 @@ describe('When pointing an end shape', () => { type: 'binding', isExact: false, normalizedAnchor: { x: 0.5, y: 0.5 }, // center! + isPrecise: false, boundShapeId: ids.box3, }, }, @@ -223,6 +225,7 @@ describe('When pointing an end shape', () => { type: 'binding', isExact: false, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, boundShapeId: ids.box3, }, }, @@ -243,6 +246,7 @@ describe('When pointing an end shape', () => { type: 'binding', isExact: false, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: true, boundShapeId: ids.box3, }, }, @@ -280,7 +284,8 @@ describe('When pointing an end shape', () => { end: { type: 'binding', isExact: false, - normalizedAnchor: { x: 0.5, y: 0.5 }, // center! + normalizedAnchor: { x: 0.25, y: 0.25 }, // center! + isPrecise: false, boundShapeId: ids.box2, }, }, @@ -330,7 +335,8 @@ describe('When pointing an end shape', () => { end: { type: 'binding', isExact: false, - normalizedAnchor: { x: 0.5, y: 0.5 }, + normalizedAnchor: { x: 0.4, y: 0.4 }, + isPrecise: false, boundShapeId: ids.box3, }, }, @@ -373,6 +379,7 @@ describe('When pointing an end shape', () => { type: 'binding', isExact: false, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: true, boundShapeId: ids.box3, }, }, diff --git a/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.test.ts b/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.test.ts index bf19d1499..098961172 100644 --- a/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.test.ts +++ b/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.test.ts @@ -47,12 +47,14 @@ beforeEach(() => { isExact: false, boundShapeId: ids.box1, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, }, end: { type: 'binding', isExact: false, boundShapeId: ids.box2, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, }, }, }, @@ -80,12 +82,14 @@ describe('When translating a bound shape', () => { isExact: false, boundShapeId: ids.box1, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, }, end: { type: 'binding', isExact: false, boundShapeId: ids.box2, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, }, }, }) @@ -112,12 +116,14 @@ describe('When translating a bound shape', () => { isExact: false, boundShapeId: ids.box1, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, }, end: { type: 'binding', isExact: false, boundShapeId: ids.box2, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, }, }, }) @@ -162,12 +168,14 @@ describe('When translating the arrow', () => { isExact: false, boundShapeId: ids.box1, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, }, end: { type: 'binding', isExact: false, boundShapeId: ids.box2, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, }, }, }) diff --git a/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx b/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx index 3392d064e..e0175e4f7 100644 --- a/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx +++ b/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx @@ -22,7 +22,6 @@ import { TLShapeUtilCanvasSvgDef, TLShapeUtilFlag, Vec2d, - Vec2dModel, arrowShapeMigrations, arrowShapeProps, deepCopy, @@ -281,16 +280,6 @@ export class ArrowShapeUtil extends ShapeUtil { } } - if (precise) { - // Turn off precision if we're within a certain distance to the center of the shape. - // Funky math but we want the snap distance to be 4 at the minimum and either - // 16 or 15% of the smaller dimension of the target shape, whichever is smaller - precise = - Vec2d.Dist(pointInTargetSpace, targetBounds.center) > - Math.max(4, Math.min(Math.min(targetBounds.width, targetBounds.height) * 0.15, 16)) / - this.editor.getZoomLevel() - } - if (!isPrecise) { if (!targetGeometry.isClosed) { precise = true @@ -302,21 +291,36 @@ export class ArrowShapeUtil extends ShapeUtil { if ( otherHandle.type === 'binding' && target.id === otherHandle.boundShapeId && - Vec2d.Equals(otherHandle.normalizedAnchor, { x: 0.5, y: 0.5 }) + otherHandle.isPrecise ) { precise = true } } + const normalizedAnchor = { + x: (pointInTargetSpace.x - targetBounds.minX) / targetBounds.width, + y: (pointInTargetSpace.y - targetBounds.minY) / targetBounds.height, + } + + if (precise) { + // Turn off precision if we're within a certain distance to the center of the shape. + // Funky math but we want the snap distance to be 4 at the minimum and either + // 16 or 15% of the smaller dimension of the target shape, whichever is smaller + if ( + Vec2d.Dist(pointInTargetSpace, targetBounds.center) < + Math.max(4, Math.min(Math.min(targetBounds.width, targetBounds.height) * 0.15, 16)) / + this.editor.getZoomLevel() + ) { + normalizedAnchor.x = 0.5 + normalizedAnchor.y = 0.5 + } + } + next.props[handleId] = { type: 'binding', boundShapeId: target.id, - normalizedAnchor: precise - ? { - x: (pointInTargetSpace.x - targetBounds.minX) / targetBounds.width, - y: (pointInTargetSpace.y - targetBounds.minY) / targetBounds.height, - } - : { x: 0.5, y: 0.5 }, + normalizedAnchor: normalizedAnchor, + isPrecise: precise, isExact: this.editor.inputs.altKey, } @@ -542,7 +546,7 @@ export class ArrowShapeUtil extends ShapeUtil { shape.props.start.type === 'binding' ? shape.props.start.isExact ? '' - : isPrecise(shape.props.start.normalizedAnchor) + : shape.props.start.isPrecise ? 'url(#arrowhead-cross)' : 'url(#arrowhead-dot)' : '' @@ -551,7 +555,7 @@ export class ArrowShapeUtil extends ShapeUtil { shape.props.end.type === 'binding' ? shape.props.end.isExact ? '' - : isPrecise(shape.props.end.normalizedAnchor) + : shape.props.end.isPrecise ? 'url(#arrowhead-cross)' : 'url(#arrowhead-dot)' : '' @@ -1030,7 +1034,3 @@ function getArrowheadSvgPath( return path } } - -function isPrecise(normalizedAnchor: Vec2dModel) { - return normalizedAnchor.x !== 0.5 || normalizedAnchor.y !== 0.5 -} diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/DraggingHandle.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/DraggingHandle.ts index 740d3f6a8..425966bd0 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/DraggingHandle.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/DraggingHandle.ts @@ -98,7 +98,7 @@ export class DraggingHandle extends StateNode { if (initialTerminal?.type === 'binding') { this.editor.setHintingShapes([initialTerminal.boundShapeId]) - this.isPrecise = !Vec2d.Equals(initialTerminal.normalizedAnchor, { x: 0.5, y: 0.5 }) + this.isPrecise = initialTerminal.isPrecise if (this.isPrecise) { this.isPreciseId = initialTerminal.boundShapeId } else { diff --git a/packages/tldraw/src/lib/tools/SelectTool/children/DraggingHandle.ts b/packages/tldraw/src/lib/tools/SelectTool/children/DraggingHandle.ts index 740d3f6a8..425966bd0 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/children/DraggingHandle.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/children/DraggingHandle.ts @@ -98,7 +98,7 @@ export class DraggingHandle extends StateNode { if (initialTerminal?.type === 'binding') { this.editor.setHintingShapes([initialTerminal.boundShapeId]) - this.isPrecise = !Vec2d.Equals(initialTerminal.normalizedAnchor, { x: 0.5, y: 0.5 }) + this.isPrecise = initialTerminal.isPrecise if (this.isPrecise) { this.isPreciseId = initialTerminal.boundShapeId } else { diff --git a/packages/tldraw/src/lib/ui/hooks/clipboard/pasteExcalidrawContent.ts b/packages/tldraw/src/lib/ui/hooks/clipboard/pasteExcalidrawContent.ts index 583b8e755..e6526e36b 100644 --- a/packages/tldraw/src/lib/ui/hooks/clipboard/pasteExcalidrawContent.ts +++ b/packages/tldraw/src/lib/ui/hooks/clipboard/pasteExcalidrawContent.ts @@ -242,6 +242,7 @@ export async function pasteExcalidrawContent(editor: Editor, clipboard: any, poi type: 'binding', boundShapeId: startTargetId, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, isExact: false, } : { @@ -254,6 +255,7 @@ export async function pasteExcalidrawContent(editor: Editor, clipboard: any, poi type: 'binding', boundShapeId: endTargetId, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, isExact: false, } : { diff --git a/packages/tldraw/src/test/arrowBindingsIndex.test.tsx b/packages/tldraw/src/test/arrowBindingsIndex.test.tsx index 1a151545a..e361d24f3 100644 --- a/packages/tldraw/src/test/arrowBindingsIndex.test.tsx +++ b/packages/tldraw/src/test/arrowBindingsIndex.test.tsx @@ -281,6 +281,7 @@ describe('arrowBindingsIndex', () => { isExact: false, boundShapeId: box3, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, }, }, }, diff --git a/packages/tldraw/src/test/arrows-megabus.test.ts b/packages/tldraw/src/test/arrows-megabus.test.ts index 782808951..cf9ba5b51 100644 --- a/packages/tldraw/src/test/arrows-megabus.test.ts +++ b/packages/tldraw/src/test/arrows-megabus.test.ts @@ -156,6 +156,7 @@ describe('When binding an arrow to a shape', () => { type: 'binding', boundShapeId: ids.box1, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: true, // enclosed }) editor.pointerMove(250, 50) expect(arrow().props.end.type).toBe('point') @@ -343,9 +344,10 @@ describe('When starting an arrow inside of multiple shapes', () => { boundShapeId: ids.box1, normalizedAnchor: { // bound to the center, imprecise! - x: 0.5, - y: 0.5, + x: 0.2, + y: 0.2, }, + isPrecise: false, }, end: { type: 'binding', @@ -425,8 +427,8 @@ describe('When starting an arrow inside of multiple shapes', () => { type: 'binding', boundShapeId: ids.box2, normalizedAnchor: { - x: 0.6, - y: 0.6, + x: 0.55, + y: 0.5, }, }, }, @@ -459,8 +461,8 @@ describe('When starting an arrow inside of multiple shapes', () => { type: 'binding', boundShapeId: ids.box2, normalizedAnchor: { - x: 0.6, - y: 0.6, + x: 0.55, + y: 0.5, }, }, }, @@ -519,9 +521,10 @@ describe('When starting an arrow inside of multiple shapes', () => { type: 'binding', boundShapeId: ids.box1, normalizedAnchor: { - x: 0.5, - y: 0.5, + x: 0.25, + y: 0.25, }, + isPrecise: false, }, end: { type: 'binding', @@ -563,8 +566,8 @@ describe('When starting an arrow inside of multiple shapes', () => { type: 'binding', boundShapeId: ids.box2, normalizedAnchor: { - x: 0.6, - y: 0.6, + x: 0.55, + y: 0.5, }, }, }, @@ -575,3 +578,75 @@ describe('When starting an arrow inside of multiple shapes', () => { it.todo( 'after creating an arrow while tool lock is enabled, pressing enter will begin editing that shape' ) + +describe('When binding an arrow to an ancestor', () => { + it('binds precisely from child to parent', () => { + const ids = { + frame: createShapeId(), + box1: createShapeId(), + } + + editor.createShapes([ + { + id: ids.frame, + type: 'frame', + }, + { + id: ids.box1, + type: 'geo', + parentId: ids.frame, + }, + ]) + + editor.setCurrentTool('arrow') + editor.pointerMove(25, 25) + editor.pointerDown() + editor.pointerMove(150, 50) + editor.pointerUp() + + const arrow = editor.getCurrentPageShapes().find((s) => s.type === 'arrow') as TLArrowShape + if (!arrow) throw Error('No arrow') + if (arrow.props.start.type !== 'binding') throw Error('no binding') + if (arrow.props.end.type !== 'binding') throw Error('no binding') + + expect(arrow.props.start.boundShapeId).toBe(ids.box1) + expect(arrow.props.end.boundShapeId).toBe(ids.frame) + expect(arrow.props.start.isPrecise).toBe(false) + expect(arrow.props.end.isPrecise).toBe(true) + }) + + it('binds precisely from parent to child', () => { + const ids = { + frame: createShapeId(), + box1: createShapeId(), + } + + editor.createShapes([ + { + id: ids.frame, + type: 'frame', + }, + { + id: ids.box1, + type: 'geo', + parentId: ids.frame, + }, + ]) + + editor.setCurrentTool('arrow') + editor.pointerMove(150, 50) + editor.pointerDown() + editor.pointerMove(25, 25) + editor.pointerUp() + + const arrow = editor.getCurrentPageShapes().find((s) => s.type === 'arrow') as TLArrowShape + if (!arrow) throw Error('No arrow') + if (arrow.props.start.type !== 'binding') throw Error('no binding') + if (arrow.props.end.type !== 'binding') throw Error('no binding') + + expect(arrow.props.start.boundShapeId).toBe(ids.frame) + expect(arrow.props.end.boundShapeId).toBe(ids.box1) + expect(arrow.props.start.isPrecise).toBe(false) + expect(arrow.props.end.isPrecise).toBe(true) + }) +}) diff --git a/packages/tldraw/src/test/commands/clipboard.test.ts b/packages/tldraw/src/test/commands/clipboard.test.ts index b963b1efc..92ef92ca9 100644 --- a/packages/tldraw/src/test/commands/clipboard.test.ts +++ b/packages/tldraw/src/test/commands/clipboard.test.ts @@ -215,12 +215,14 @@ describe('When copying and pasting', () => { boundShapeId: ids.box1, isExact: false, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, }, end: { type: 'binding', boundShapeId: ids.box2, isExact: false, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, }, }, }, @@ -397,12 +399,14 @@ describe('When copying and pasting', () => { boundShapeId: ids.box1, isExact: false, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, }, end: { type: 'binding', boundShapeId: ids.box2, isExact: false, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, }, }, }, diff --git a/packages/tldraw/src/test/commands/deleteShapes.test.ts b/packages/tldraw/src/test/commands/deleteShapes.test.ts index 00c67593c..411fe794b 100644 --- a/packages/tldraw/src/test/commands/deleteShapes.test.ts +++ b/packages/tldraw/src/test/commands/deleteShapes.test.ts @@ -34,12 +34,14 @@ beforeEach(() => { isExact: false, boundShapeId: ids.box1, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, }, end: { type: 'binding', isExact: false, boundShapeId: ids.box2, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, }, }, }, diff --git a/packages/tldraw/src/test/duplicate.test.ts b/packages/tldraw/src/test/duplicate.test.ts index 2e1432edc..e0991c4b9 100644 --- a/packages/tldraw/src/test/duplicate.test.ts +++ b/packages/tldraw/src/test/duplicate.test.ts @@ -34,12 +34,14 @@ it('creates new bindings for arrows when pasting', async () => { boundShapeId: ids.box1, isExact: false, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, }, end: { type: 'binding', boundShapeId: ids.box2, isExact: false, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, }, }, }, @@ -119,12 +121,14 @@ describe('When duplicating shapes that include arrows', () => { normalizedAnchor: { x: 0.75, y: 0.75 }, boundShapeId: box1, isExact: false, + isPrecise: true, }, end: { type: 'binding', normalizedAnchor: { x: 0.25, y: 0.25 }, boundShapeId: box1, isExact: false, + isPrecise: true, }, }, }, @@ -140,12 +144,14 @@ describe('When duplicating shapes that include arrows', () => { normalizedAnchor: { x: 0.75, y: 0.75 }, boundShapeId: box1, isExact: false, + isPrecise: true, }, end: { type: 'binding', normalizedAnchor: { x: 0.25, y: 0.25 }, boundShapeId: box1, isExact: false, + isPrecise: true, }, }, }, @@ -161,12 +167,14 @@ describe('When duplicating shapes that include arrows', () => { normalizedAnchor: { x: 0.75, y: 0.75 }, boundShapeId: box1, isExact: false, + isPrecise: true, }, end: { type: 'binding', normalizedAnchor: { x: 0.25, y: 0.25 }, boundShapeId: box3, isExact: false, + isPrecise: true, }, }, }, diff --git a/packages/tldraw/src/test/flipShapes.test.ts b/packages/tldraw/src/test/flipShapes.test.ts index 10132336d..81239cf62 100644 --- a/packages/tldraw/src/test/flipShapes.test.ts +++ b/packages/tldraw/src/test/flipShapes.test.ts @@ -503,12 +503,14 @@ describe('When flipping shapes that include arrows', () => { normalizedAnchor: { x: 0.75, y: 0.75 }, boundShapeId: box1, isExact: false, + isPrecise: true, }, end: { type: 'binding', normalizedAnchor: { x: 0.25, y: 0.25 }, boundShapeId: box1, isExact: false, + isPrecise: true, }, }, }, @@ -524,12 +526,14 @@ describe('When flipping shapes that include arrows', () => { normalizedAnchor: { x: 0.75, y: 0.75 }, boundShapeId: box1, isExact: false, + isPrecise: true, }, end: { type: 'binding', normalizedAnchor: { x: 0.25, y: 0.25 }, boundShapeId: box1, isExact: false, + isPrecise: true, }, }, }, @@ -545,12 +549,14 @@ describe('When flipping shapes that include arrows', () => { normalizedAnchor: { x: 0.75, y: 0.75 }, boundShapeId: box1, isExact: false, + isPrecise: true, }, end: { type: 'binding', normalizedAnchor: { x: 0.25, y: 0.25 }, boundShapeId: box3, isExact: false, + isPrecise: true, }, }, }, diff --git a/packages/tldraw/src/test/translating.test.ts b/packages/tldraw/src/test/translating.test.ts index e63f83a30..b16945acf 100644 --- a/packages/tldraw/src/test/translating.test.ts +++ b/packages/tldraw/src/test/translating.test.ts @@ -1595,12 +1595,14 @@ describe('translating a shape with a bound shape', () => { isExact: false, boundShapeId: ids.box1, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, }, end: { type: 'binding', isExact: false, boundShapeId: ids.box2, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, }, }, }, @@ -1631,12 +1633,14 @@ describe('translating a shape with a bound shape', () => { isExact: false, boundShapeId: ids.box1, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, }, end: { type: 'binding', isExact: false, boundShapeId: ids.box2, normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, }, }, }, diff --git a/packages/tlschema/api-report.md b/packages/tlschema/api-report.md index d2f8973f3..19cc9eae0 100644 --- a/packages/tlschema/api-report.md +++ b/packages/tlschema/api-report.md @@ -43,6 +43,7 @@ export const arrowShapeProps: { boundShapeId: TLShapeId; normalizedAnchor: Vec2dModel; isExact: boolean; + isPrecise: boolean; }>; point: T.ObjectValidator<{ type: "point"; @@ -56,6 +57,7 @@ export const arrowShapeProps: { boundShapeId: TLShapeId; normalizedAnchor: Vec2dModel; isExact: boolean; + isPrecise: boolean; }>; point: T.ObjectValidator<{ type: "point"; diff --git a/packages/tlschema/api/api.json b/packages/tlschema/api/api.json index f256cd0b7..e6d98bd2b 100644 --- a/packages/tlschema/api/api.json +++ b/packages/tlschema/api/api.json @@ -355,7 +355,7 @@ }, { "kind": "Content", - "text": ";\n isExact: boolean;\n }>;\n point: " + "text": ";\n isExact: boolean;\n isPrecise: boolean;\n }>;\n point: " }, { "kind": "Reference", @@ -400,7 +400,7 @@ }, { "kind": "Content", - "text": ";\n isExact: boolean;\n }>;\n point: " + "text": ";\n isExact: boolean;\n isPrecise: boolean;\n }>;\n point: " }, { "kind": "Reference", diff --git a/packages/tlschema/src/migrations.test.ts b/packages/tlschema/src/migrations.test.ts index 7231451dd..9c71473d6 100644 --- a/packages/tlschema/src/migrations.test.ts +++ b/packages/tlschema/src/migrations.test.ts @@ -11,7 +11,7 @@ import { instancePageStateMigrations, instancePageStateVersions } from './record import { pointerMigrations, pointerVersions } from './records/TLPointer' import { instancePresenceMigrations, instancePresenceVersions } from './records/TLPresence' import { TLShape, rootShapeMigrations, rootShapeVersions } from './records/TLShape' -import { arrowShapeMigrations } from './shapes/TLArrowShape' +import { ArrowMigrationVersions, arrowShapeMigrations } from './shapes/TLArrowShape' import { bookmarkShapeMigrations } from './shapes/TLBookmarkShape' import { drawShapeMigrations } from './shapes/TLDrawShape' import { embedShapeMigrations } from './shapes/TLEmbedShape' @@ -1623,6 +1623,141 @@ describe('add scribbles to TLInstance', () => { }) }) +describe('add isPrecise to arrow handles', () => { + const { up, down } = arrowShapeMigrations.migrators[ArrowMigrationVersions.AddIsPrecise] + + test('up works as expected', () => { + expect( + up({ + props: { + start: { + type: 'point', + }, + end: { + type: 'binding', + normalizedAnchor: { x: 0.5, y: 0.5 }, + }, + }, + }) + ).toEqual({ + props: { + start: { + type: 'point', + }, + end: { + type: 'binding', + normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, + }, + }, + }) + expect( + up({ + props: { + start: { + type: 'point', + }, + end: { + type: 'binding', + normalizedAnchor: { x: 0.15, y: 0.15 }, + }, + }, + }) + ).toEqual({ + props: { + start: { + type: 'point', + }, + end: { + type: 'binding', + normalizedAnchor: { x: 0.15, y: 0.15 }, + isPrecise: true, + }, + }, + }) + }) + + test('down works as expected', () => { + expect( + down({ + props: { + start: { + type: 'point', + }, + end: { + type: 'binding', + normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: true, + }, + }, + }) + ).toEqual({ + props: { + start: { + type: 'point', + }, + end: { + type: 'binding', + normalizedAnchor: { x: 0.5, y: 0.5 }, + }, + }, + }) + + expect( + down({ + props: { + start: { + type: 'point', + }, + end: { + type: 'binding', + normalizedAnchor: { x: 0.25, y: 0.25 }, + isPrecise: true, + }, + }, + }) + ).toEqual({ + props: { + start: { + type: 'point', + }, + end: { + type: 'binding', + normalizedAnchor: { x: 0.25, y: 0.25 }, + }, + }, + }) + + expect( + down({ + props: { + start: { + type: 'binding', + normalizedAnchor: { x: 0.5, y: 0.5 }, + isPrecise: false, + }, + end: { + type: 'binding', + normalizedAnchor: { x: 0.15, y: 0.15 }, + isPrecise: false, + }, + }, + }) + ).toEqual({ + props: { + start: { + type: 'binding', + normalizedAnchor: { x: 0.5, y: 0.5 }, + }, + end: { + type: 'binding', + normalizedAnchor: { x: 0.5, y: 0.5 }, + }, + }, + }) + }) +}) + /* --- PUT YOUR MIGRATIONS TESTS ABOVE HERE --- */ for (const migrator of allMigrators) { diff --git a/packages/tlschema/src/shapes/TLArrowShape.ts b/packages/tlschema/src/shapes/TLArrowShape.ts index f02af3a38..5c8a9dac5 100644 --- a/packages/tlschema/src/shapes/TLArrowShape.ts +++ b/packages/tlschema/src/shapes/TLArrowShape.ts @@ -43,6 +43,7 @@ const ArrowShapeTerminal = T.union('type', { boundShapeId: shapeIdValidator, normalizedAnchor: vec2dModelValidator, isExact: T.boolean, + isPrecise: T.boolean, }), point: T.object({ type: T.literal('point'), @@ -76,15 +77,16 @@ export type TLArrowShapeProps = ShapePropsType /** @public */ export type TLArrowShape = TLBaseShape<'arrow', TLArrowShapeProps> -const Versions = { +export const ArrowMigrationVersions = { AddLabelColor: 1, + AddIsPrecise: 2, } as const /** @internal */ export const arrowShapeMigrations = defineMigrations({ - currentVersion: Versions.AddLabelColor, + currentVersion: ArrowMigrationVersions.AddIsPrecise, migrators: { - [Versions.AddLabelColor]: { + [ArrowMigrationVersions.AddLabelColor]: { up: (record) => { return { ...record, @@ -102,5 +104,57 @@ export const arrowShapeMigrations = defineMigrations({ } }, }, + [ArrowMigrationVersions.AddIsPrecise]: { + up: (record) => { + const { start, end } = record.props + return { + ...record, + props: { + ...record.props, + start: + (start as TLArrowShapeTerminal).type === 'binding' + ? { + ...start, + isPrecise: !( + start.normalizedAnchor.x === 0.5 && start.normalizedAnchor.y === 0.5 + ), + } + : start, + end: + (end as TLArrowShapeTerminal).type === 'binding' + ? { + ...end, + isPrecise: !(end.normalizedAnchor.x === 0.5 && end.normalizedAnchor.y === 0.5), + } + : end, + }, + } + }, + down: (record: any) => { + const { start, end } = record.props + const nStart = { ...start } + const nEnd = { ...end } + if (nStart.type === 'binding') { + if (!nStart.isPrecise) { + nStart.normalizedAnchor = { x: 0.5, y: 0.5 } + } + delete nStart.isPrecise + } + if (nEnd.type === 'binding') { + if (!nEnd.isPrecise) { + nEnd.normalizedAnchor = { x: 0.5, y: 0.5 } + } + delete nEnd.isPrecise + } + return { + ...record, + props: { + ...record.props, + start: nStart, + end: nEnd, + }, + } + }, + }, }, })