[fix] Arrow bindings + labels (#558)

* Fix label colors for arrow / triangle shapes in dark mode

* Fix bouncy arrow label, add tests, remove redundant call to updateBindings, remove redundant call to updateArrowBinding

packages/tldraw/src/state/TLDR.ts

@@ -243,52 +243,50 @@ export class TLDR {
     return Array.from(visited.values())
   }
 
-  static updateBindings(
+  // static updateBindings(
-    data: TDSnapshot,
+  //   data: TDSnapshot,
-    id: string,
+  //   id: string,
-    beforeShapes: Record<string, Partial<TDShape>> = {},
+  //   beforeShapes: Record<string, Partial<TDShape>> = {},
-    afterShapes: Record<string, Partial<TDShape>> = {},
+  //   afterShapes: Record<string, Partial<TDShape>> = {},
-    pageId: string
+  //   pageId: string
-  ): TDSnapshot {
+  // ): TDSnapshot {
-    const page = { ...TLDR.getPage(data, pageId) }
+  //   const page = { ...TLDR.getPage(data, pageId) }
-    return Object.values(page.bindings)
+  //   return Object.values(page.bindings)
-      .filter((binding) => binding.fromId === id || binding.toId === id)
+  //     .filter((binding) => binding.fromId === id || binding.toId === id)
-      .reduce((cTDSnapshot, binding) => {
+  //     .reduce((cTDSnapshot, binding) => {
-        let oppositeShape: TDShape | undefined = undefined
+  //       let oppositeShape: TDShape | undefined = undefined
 
-        if (!beforeShapes[binding.fromId]) {
+  //       if (!beforeShapes[binding.fromId]) {
-          const arrowShape = TLDR.getShape<ArrowShape>(cTDSnapshot, binding.fromId, pageId)
+  //         const arrowShape = TLDR.getShape<ArrowShape>(cTDSnapshot, binding.fromId, pageId)
-          beforeShapes[binding.fromId] = Utils.deepClone(arrowShape)
+  //         beforeShapes[binding.fromId] = Utils.deepClone(arrowShape)
-          const oppositeHandle = arrowShape.handles[binding.handleId === 'start' ? 'end' : 'start']
+  //         const oppositeHandle = arrowShape.handles[binding.handleId === 'start' ? 'end' : 'start']
-          if (oppositeHandle.bindingId) {
+  //         if (oppositeHandle.bindingId) {
-            const oppositeBinding = page.bindings[oppositeHandle.bindingId]
+  //           const oppositeBinding = page.bindings[oppositeHandle.bindingId]
-            oppositeShape = TLDR.getShape(data, oppositeBinding.toId, data.appState.currentPageId)
+  //           oppositeShape = TLDR.getShape(data, oppositeBinding.toId, data.appState.currentPageId)
-          }
+  //         }
-        }
+  //       }
 
-        if (!beforeShapes[binding.toId]) {
+  //       if (!beforeShapes[binding.toId]) {
-          beforeShapes[binding.toId] = Utils.deepClone(
+  //         beforeShapes[binding.toId] = Utils.deepClone(
-            TLDR.getShape(cTDSnapshot, binding.toId, pageId)
+  //           TLDR.getShape(cTDSnapshot, binding.toId, pageId)
-          )
+  //         )
-        }
+  //       }
 
-        // TLDR.onBindingChange(
+  //       // Hmm
-        //   TLDR.getShape(cTDSnapshot, binding.fromId, pageId),
+
-        //   binding,
+  //       // updateArrowBindings ?
-        //   TLDR.getShape(cTDSnapshot, binding.toId, pageId),
+
-        //   oppositeShape
+  //       afterShapes[binding.fromId] = Utils.deepClone(
+  //         TLDR.getShape(cTDSnapshot, binding.fromId, pageId)
   //       )
 
-        afterShapes[binding.fromId] = Utils.deepClone(
+  //       afterShapes[binding.toId] = Utils.deepClone(
-          TLDR.getShape(cTDSnapshot, binding.fromId, pageId)
+  //         TLDR.getShape(cTDSnapshot, binding.toId, pageId)
-        )
+  //       )
-        afterShapes[binding.toId] = Utils.deepClone(
-          TLDR.getShape(cTDSnapshot, binding.toId, pageId)
-        )
 
-        return cTDSnapshot
+  //       return cTDSnapshot
-      }, data)
+  //     }, data)
-  }
+  // }
 
   static getLinkedShapeIds(
     data: TDSnapshot,
@@ -463,14 +461,11 @@ export class TLDR {
         },
       },
     })
-    const dataWithBindingChanges = ids.reduce<TDSnapshot>((cTDSnapshot, id) => {
-      return TLDR.updateBindings(cTDSnapshot, id, beforeShapes, afterShapes, pageId)
-    }, dataWithMutations)
 
     return {
       before: beforeShapes,
       after: afterShapes,
-      data: dataWithBindingChanges,
+      data: dataWithMutations,
     }
   }
 

packages/tldraw/src/state/TldrawApp.ts

@@ -363,6 +363,8 @@ export class TldrawApp extends StateManager<TDSnapshot> {
           // Get bindings related to the changed shapes
           const bindingsToUpdate = TLDR.getRelatedBindings(next, Object.keys(changedShapes), pageId)
 
+          const visitedShapes = new Set<ArrowShape>()
+
           // Update all of the bindings we've just collected
           bindingsToUpdate.forEach((binding) => {
             if (!page.bindings[binding.id]) {
@@ -377,15 +379,19 @@ export class TldrawApp extends StateManager<TDSnapshot> {
               return
             }
 
+            if (visitedShapes.has(fromShape)) {
+              return
+            }
+
             // We only need to update the binding's "from" shape (an arrow)
             const fromDelta = TLDR.updateArrowBindings(page, fromShape)
+            visitedShapes.add(fromShape)
 
             if (fromDelta) {
               const nextShape = {
                 ...fromShape,
                 ...fromDelta,
-              } as TDShape
+              } as ArrowShape
-
               page.shapes[fromShape.id] = nextShape
             }
           })
@@ -775,11 +781,12 @@ export class TldrawApp extends StateManager<TDSnapshot> {
           },
         },
       }
+      const page = next.document.pages[pageId]
 
       // Get bindings related to the changed shapes
       const bindingsToUpdate = TLDR.getRelatedBindings(next, Object.keys(nextShapes), pageId)
 
-      const page = next.document.pages[pageId]
+      const visitedShapes = new Set<ArrowShape>()
 
       // Update all of the bindings we've just collected
       bindingsToUpdate.forEach((binding) => {
@@ -789,8 +796,13 @@ export class TldrawApp extends StateManager<TDSnapshot> {
 
         const fromShape = page.shapes[binding.fromId] as ArrowShape
 
+        if (visitedShapes.has(fromShape)) {
+          return
+        }
+
         // We only need to update the binding's "from" shape (an arrow)
         const fromDelta = TLDR.updateArrowBindings(page, fromShape)
+        visitedShapes.add(fromShape)
 
         if (fromDelta) {
           const nextShape = {

packages/tldraw/src/state/sessions/ArrowSession/ArrowSession.spec.ts

@@ -286,7 +286,7 @@ describe('When creating with an arrow session', () => {
     expect(arrow.handles?.end.bindingId).toBeDefined()
     expect(arrow.point).toStrictEqual([116, 116])
     expect(arrow.handles.start.point).toStrictEqual([84, 84])
-    expect(arrow.handles.end.point).toStrictEqual([0, 0])
+    expect(arrow.handles.end.point).toStrictEqual([-0, -0])
 
     // Drag the shape away by [10,10]
     app.movePointer([50, 50]).pointShape('arrow1', [50, 50]).movePointer([60, 60]).stopPointing()
@@ -295,7 +295,7 @@ describe('When creating with an arrow session', () => {
     expect(arrow.point).toStrictEqual([126, 126])
     // The handles should be in the same place
     expect(arrow.handles.start.point).toStrictEqual([84, 84])
-    expect(arrow.handles.end.point).toStrictEqual([0, 0])
+    expect(arrow.handles.end.point).toStrictEqual([-0, -0])
     // The bindings should have been removed
     expect(app.bindings.length).toBe(0)
     expect(arrow.handles.start.bindingId).toBe(undefined)

packages/tldraw/src/state/shapes/ArrowUtil/ArrowUtil.spec.tsx

@@ -1,3 +1,6 @@
+import Vec from '@tldraw/vec'
+import { TldrawTestApp } from '~test'
+import { ArrowShape, SessionType, TDShapeType } from '~types'
 import { Arrow } from '..'
 
 describe('Arrow shape', () => {
@@ -5,3 +8,36 @@ describe('Arrow shape', () => {
     expect(Arrow.create({ id: 'arrow' })).toMatchSnapshot('arrow')
   })
 })
+
+describe('When the arrow has a label...', () => {
+  it("Positions a straight arrow's label in the center of the bounding box", () => {
+    const app = new TldrawTestApp()
+      .resetDocument()
+      .createShapes(
+        { type: TDShapeType.Rectangle, id: 'target1', point: [0, 0], size: [100, 100] },
+        { type: TDShapeType.Arrow, id: 'arrow1', point: [200, 200] }
+      )
+      .select('arrow1')
+      .movePointer([200, 200])
+      .startSession(SessionType.Arrow, 'arrow1', 'start')
+      .movePointer([55, 55])
+    expect(app.bindings[0]).toMatchObject({
+      fromId: 'arrow1',
+      toId: 'target1',
+      point: [0.5, 0.5],
+    })
+    function getOffset() {
+      const shape = app.getShape<ArrowShape>('arrow1')
+      const bounds = Arrow.getBounds(shape)
+      const offset = Vec.sub(
+        shape.handles.bend.point,
+        Vec.toFixed([bounds.width / 2, bounds.height / 2])
+      )
+      return offset
+    }
+    expect(getOffset()).toMatchObject([0, 0])
+    app.select('target1')
+    app.nudge([0, 1])
+    expect(getOffset()).toMatchObject([0, 0])
+  })
+})

packages/tldraw/src/state/shapes/ArrowUtil/ArrowUtil.tsx

@@ -94,7 +94,7 @@ export class ArrowUtil extends TDShapeUtil<T, E> {
       } = shape
       const isStraightLine = Vec.dist(bend.point, Vec.toFixed(Vec.med(start.point, end.point))) < 1
       const font = getFontStyle(style)
-      const styles = getShapeStyle(style)
+      const styles = getShapeStyle(style, meta.isDarkMode)
       const labelSize = label || isEditing ? getTextLabelSize(label, font) : [0, 0]
       const bounds = this.getBounds(shape)
       const dist = React.useMemo(() => {
@@ -112,7 +112,10 @@ export class ArrowUtil extends TDShapeUtil<T, E> {
       )
       const offset = React.useMemo(() => {
         const bounds = this.getBounds(shape)
-        const offset = Vec.sub(shape.handles.bend.point, [bounds.width / 2, bounds.height / 2])
+        const offset = Vec.sub(
+          shape.handles.bend.point,
+          Vec.toFixed([bounds.width / 2, bounds.height / 2])
+        )
         return offset
       }, [shape, scale])
       const handleLabelChange = React.useCallback(
@@ -153,7 +156,7 @@ export class ArrowUtil extends TDShapeUtil<T, E> {
                   rx={4 * scale}
                   ry={4 * scale}
                   fill="black"
-                  opacity={Math.max(scale, 0.9)}
+                  opacity={1}
                 />
               </mask>
             </defs>

packages/tldraw/src/state/shapes/TDShapeUtil.tsx

@@ -159,19 +159,6 @@ export abstract class TDShapeUtil<T extends TDShape, E extends Element = any> ex
 
   onChildrenChange?: (shape: T, children: TDShape[]) => Partial<T> | void
 
-  // onBindingChange?: (
-  //   shape: T,
-  //   binding: TDBinding,
-  //   target: TDShape,
-  //   targetBounds: TLBounds,
-  //   targetExpandedBounds: TLBounds,
-  //   targetCenter: number[],
-  //   oppositeShape?: TDShape,
-  //   oppositeShapeTargetBounds?: TLBounds,
-  //   oppositeShapeTargetExpandedBounds?: TLBounds,
-  //   oppositeShapeTargetCenter?: number[]
-  // ) => Partial<T> | void
-
   onHandleChange?: (shape: T, handles: Partial<T['handles']>) => Partial<T> | void
 
   onRightPointHandle?: (

packages/tldraw/src/state/shapes/TriangleUtil/TriangleUtil.tsx

@@ -72,7 +72,7 @@ export class TriangleUtil extends TDShapeUtil<T, E> {
     ) => {
       const { id, label = '', size, style, labelPoint = LABEL_POINT } = shape
       const font = getFontStyle(style)
-      const styles = getShapeStyle(style)
+      const styles = getShapeStyle(style, meta.isDarkMode)
       const Component = style.dash === DashStyle.Draw ? DrawTriangle : DashedTriangle
       const handleLabelChange = React.useCallback(
         (label: string) => onShapeChange?.({ id, label }),

packages/tldraw/src/state/shapes/shared/TextLabel.tsx

@@ -3,7 +3,6 @@ import { stopPropagation } from '~components/stopPropagation'
 import { GHOSTED_OPACITY, LETTER_SPACING } from '~constants'
 import { TLDR } from '~state/TLDR'
 import { styled } from '~styles'
-import { TextAreaUtils } from '.'
 import { getTextLabelSize } from './getTextSize'
 import { useTextKeyboardEvents } from './useTextKeyboardEvents'
 
@@ -231,5 +230,8 @@ const TextArea = styled('textarea', {
   background: '$boundsBg',
   userSelect: 'text',
   WebkitUserSelect: 'text',
+  fontSmooth: 'always',
+  WebkitFontSmoothing: 'subpixel-antialiased',
+  MozOsxFontSmoothing: 'auto',
   ...commonTextWrapping,
 })

packages/vec/CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## 1.6.1
+
+- Change `vec.toFixed` to always round to two decimal places. This drops the second parameter, which was previously available for custom precisions, but which was defaulted to 2 (its current behavior).
+
 ## 1.4.3
 
 - Update README

packages/vec/src/index.ts

@@ -368,12 +368,11 @@ export class Vec {
   }
 
   /**
-   * Round a vector to the a given precision.
+   * Round a vector to two decimal places.
    * @param a
-   * @param d
    */
-  static toFixed = (a: number[], d = 2): number[] => {
+  static toFixed = (a: number[]): number[] => {
-    return a.map((v) => +v.toFixed(d))
+    return a.map((v) => Math.round(v * 100) / 100)
   }
 
   /**