From 8cd8117acf4edf03d5b1a3ae1e91fe2de6a71b6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mime=20=C4=8Cuvalo?= Date: Tue, 6 Feb 2024 14:42:35 +0000 Subject: [PATCH] arrows: update cursor only when in Select mode (#2742) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cursor was updating for the arrow label even when in other tools (e.g. Draw) and it should only be updating when in Select mode. That addresses this issue: https://github.com/tldraw/tldraw/issues/2750 Also, there was a problem with arrows and zero length arrows and labels. The problem was actually in `Vec.ts` where we were dividing by zero. Addresses this bug https://github.com/tldraw/tldraw/issues/2749 ### Change Type - [x] `patch` — Bug fix ### Test Plan 1. Make sure that the cursor is only applicable to the Select tool. ### Release Notes - Cursor tweak for arrow labels. --- packages/editor/editor.css | 4 -- .../shapes/shared/arrow/curved-arrow.ts | 4 +- .../shapes/shared/arrow/straight-arrow.ts | 4 +- .../editor/src/lib/primitives/Vec.test.ts | 4 ++ .../tldraw/src/lib/shapes/arrow/arrowheads.ts | 4 ++ .../lib/tools/SelectTool/childStates/Idle.ts | 48 +++++++++++++------ .../childStates/PointingArrowLabel.ts | 7 +-- 7 files changed, 48 insertions(+), 27 deletions(-) diff --git a/packages/editor/editor.css b/packages/editor/editor.css index eb011ecb7..be068e168 100644 --- a/packages/editor/editor.css +++ b/packages/editor/editor.css @@ -1143,10 +1143,6 @@ input, align-items: center; } -.tl-arrow-label__inner p { - cursor: var(--tl-cursor-grab); -} - .tl-arrow-label p, .tl-arrow-label textarea { margin: 0px; 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 352eac392..43e63580f 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 @@ -37,7 +37,9 @@ export function getCurvedArrowInfo( const terminalsInArrowSpace = getArrowTerminalsInArrowSpace(editor, shape) const med = Vec.Med(terminalsInArrowSpace.start, terminalsInArrowSpace.end) // point between start and end - const u = Vec.Sub(terminalsInArrowSpace.end, terminalsInArrowSpace.start).uni() // unit vector between start and end + const distance = Vec.Sub(terminalsInArrowSpace.end, terminalsInArrowSpace.start) + // Check for divide-by-zero before we call uni() + const u = Vec.Len(distance) ? distance.uni() : Vec.From(distance) // unit vector between start and end const middle = Vec.Add(med, u.per().mul(-bend)) // middle handle const startShapeInfo = getBoundShapeInfoForTerminal(editor, shape.props.start) 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 65e791699..c1ffb90dc 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 @@ -109,7 +109,9 @@ export function getStraightArrowInfo(editor: Editor, shape: TLArrowShape): TLArr } } - const u = Vec.Sub(b, a).uni() + const distance = Vec.Sub(b, a) + // Check for divide-by-zero before we call uni() + const u = Vec.Len(distance) ? distance.uni() : Vec.From(distance) const didFlip = !Vec.Equals(u, uAB) // If the arrow is bound non-exact to a start shape and the diff --git a/packages/editor/src/lib/primitives/Vec.test.ts b/packages/editor/src/lib/primitives/Vec.test.ts index 884271284..ff07ad80e 100644 --- a/packages/editor/src/lib/primitives/Vec.test.ts +++ b/packages/editor/src/lib/primitives/Vec.test.ts @@ -143,6 +143,10 @@ describe('Vec.Uni', () => { expect(Vec.Uni(new Vec(0, 10))).toMatchObject(new Vec(0, 1)) expect(Vec.Uni(new Vec(10, 10))).toMatchObject(new Vec(0.7071067811865475, 0.7071067811865475)) }) + + it('Divide-by-zero spits out NaN (at the moment)', () => { + expect(Vec.Uni(new Vec(0, 0))).toMatchObject(new Vec(NaN, NaN)) + }) }) describe('Vec.Tan', () => { diff --git a/packages/tldraw/src/lib/shapes/arrow/arrowheads.ts b/packages/tldraw/src/lib/shapes/arrow/arrowheads.ts index 99bbbe16f..9e79db4e2 100644 --- a/packages/tldraw/src/lib/shapes/arrow/arrowheads.ts +++ b/packages/tldraw/src/lib/shapes/arrow/arrowheads.ts @@ -33,6 +33,10 @@ function getArrowPoints( : ints[0] } + if (isNaN(P0.x) || isNaN(P0.y)) { + P0 = info.start.point + } + return { point: PT, int: P0, diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/Idle.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/Idle.ts index 2975f11e1..1c03059ae 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/Idle.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/Idle.ts @@ -34,6 +34,13 @@ export class Idle extends StateNode { override onPointerMove: TLEventHandlers['onPointerMove'] = () => { updateHoveredId(this.editor) + + const hitShape = this.editor.getHoveredShape() + if (this.isOverArrowLabelTest(hitShape)) { + this.editor.setCursor({ type: 'pointer', rotation: 0 }) + } else { + this.editor.setCursor({ type: 'default', rotation: 0 }) + } } override onPointerDown: TLEventHandlers['onPointerDown'] = (info) => { @@ -93,21 +100,10 @@ export class Idle extends StateNode { } case 'shape': { const { shape } = info - const pointInShapeSpace = this.editor.getPointInShapeSpace( - shape, - this.editor.inputs.currentPagePoint - ) - // todo: Extract into general hit test for arrows - if (this.editor.isShapeOfType(shape, 'arrow')) { - // How should we handle multiple labels? Do shapes ever have multiple labels? - const labelGeometry = this.editor.getShapeGeometry(shape).children[1] - // Knowing what we know about arrows... if the shape has no text in its label, - // then the label geometry should not be there. - if (labelGeometry && pointInPolygon(pointInShapeSpace, labelGeometry.vertices)) { - // We're moving the label on a shape. - this.parent.transition('pointing_arrow_label', info) - break - } + if (this.isOverArrowLabelTest(shape)) { + // We're moving the label on a shape. + this.parent.transition('pointing_arrow_label', info) + break } if (this.editor.isShapeOrAncestorLocked(shape)) { @@ -499,6 +495,28 @@ export class Idle extends StateNode { isDarwin = window.navigator.userAgent.toLowerCase().indexOf('mac') > -1 + isOverArrowLabelTest(shape: TLShape | undefined) { + if (!shape) return false + + const pointInShapeSpace = this.editor.getPointInShapeSpace( + shape, + this.editor.inputs.currentPagePoint + ) + + // todo: Extract into general hit test for arrows + if (this.editor.isShapeOfType(shape, 'arrow')) { + // How should we handle multiple labels? Do shapes ever have multiple labels? + const labelGeometry = this.editor.getShapeGeometry(shape).children[1] + // Knowing what we know about arrows... if the shape has no text in its label, + // then the label geometry should not be there. + if (labelGeometry && pointInPolygon(pointInShapeSpace, labelGeometry.vertices)) { + return true + } + } + + return false + } + handleDoubleClickOnCanvas(info: TLClickEventInfo) { // Create text shape and transition to editing_shape if (this.editor.getInstanceState().isReadonly) return diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingArrowLabel.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingArrowLabel.ts index f99dfa5d0..556b38f98 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingArrowLabel.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingArrowLabel.ts @@ -24,12 +24,7 @@ export class PointingArrowLabel extends StateNode { } private updateCursor() { - this.editor.updateInstanceState({ - cursor: { - type: 'grabbing', - rotation: 0, - }, - }) + this.editor.setCursor({ type: 'grabbing', rotation: 0 }) } override onEnter = (