Skip to content

Commit 6f8851f

Browse files
authored
perf(arrows): use reactive hooks for arrow handle display and editing state (tldraw#8167)
Previously, any change to any shape would cause all arrows to render. We should be careful with any reactive call to getOnlySelectedShape, because it will call when the selected shape is changing. If we need to react to a change in which shape is selected, we should use getOnlySelectedShapeId. In order to reduce unnecessary re-renders of arrow components, this PR changes a call to `getOnlySelectedShape()?.id` to `getOnlySelectedShapeId`. To avoid an extra render when editing shape changes, it also wraps `shouldDisplayHandles` and `isEditing` checks in `useValue` hooks for finer-grained reactivity. It also refactors `getArrowLabelPosition` to accept `isEditing` as a parameter rather than computing it internally, avoiding redundant reactive lookups. ### Change type - [x] `improvement` ### Test plan 1. Create arrows between shapes 2. Select an arrow — handles should display correctly 3. Double-click an arrow label to edit — label positioning should work as before 4. Deselect and verify handles disappear - [ ] Unit tests - [ ] End to end tests ### Release notes - Improve arrow component rendering performance with finer-grained reactivity <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches arrow rendering reactivity (selection/editing state and label positioning), so regressions could show up as missing handles or mispositioned labels, but the changes are localized to UI/perf code. > > **Overview** > Reduces unnecessary arrow re-renders by switching arrow component state derivations to fine-grained `useValue` subscriptions (e.g. `shouldDisplayHandles`, selection, and editing state) and by preferring `getOnlySelectedShapeId()` over `getOnlySelectedShape()` where only the id is needed. > > Refactors `getArrowLabelPosition` to accept an explicit `isEditing` flag and updates all call sites (geometry, component render, and SVG export) to avoid redundant reactive lookups when computing label bounds. Also updates touch-end handling in `useCanvasEvents` to use `editor.getEditingShapeId()`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit e04558e. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 535ee94 commit 6f8851f

3 files changed

Lines changed: 44 additions & 27 deletions

File tree

packages/editor/src/lib/hooks/useCanvasEvents.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ export function useCanvasEvents() {
7878
// check that e.target is an HTMLElement
7979
if (!(e.target instanceof HTMLElement)) return
8080

81-
const editingShapeId = editor.getEditingShape()?.id
81+
const editingShapeId = editor.getEditingShapeId()
8282
if (
8383
// if the target is not inside the editing shape
8484
!(editingShapeId && e.target.closest(`[data-shape-id="${editingShapeId}"]`)) &&

packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx

Lines changed: 42 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
/* eslint-disable react-hooks/rules-of-hooks */
12
import {
23
Arc2d,
34
Box,
@@ -45,6 +46,7 @@ import {
4546
useEditor,
4647
useIsEditing,
4748
useSharedSafeId,
49+
useValue,
4850
} from '@tldraw/editor'
4951
import React, { useMemo } from 'react'
5052
import { updateArrowTerminal } from '../../bindings/arrow/ArrowBindingUtil'
@@ -220,7 +222,7 @@ export class ArrowShapeUtil extends ShapeUtil<TLArrowShape> {
220222

221223
let labelGeom
222224
if (isEditing || !isEmptyRichText(shape.props.richText)) {
223-
const labelPosition = getArrowLabelPosition(this.editor, shape)
225+
const labelPosition = getArrowLabelPosition(this.editor, shape, isEditing)
224226
if (debugFlags.debugGeometry.get()) {
225227
debugGeom.push(...labelPosition.debugGeom)
226228
}
@@ -755,33 +757,50 @@ export class ArrowShapeUtil extends ShapeUtil<TLArrowShape> {
755757
}
756758

757759
component(shape: TLArrowShape) {
758-
// eslint-disable-next-line react-hooks/rules-of-hooks
760+
const { editor } = this
761+
759762
const theme = useDefaultColorTheme()
760-
const onlySelectedShape = this.editor.getOnlySelectedShape()
761-
const shouldDisplayHandles =
762-
this.editor.isInAny(
763-
'select.idle',
764-
'select.pointing_handle',
765-
'select.dragging_handle',
766-
'select.translating',
767-
'arrow.dragging'
768-
) && !this.editor.getIsReadonly()
769763

770-
const info = getArrowInfo(this.editor, shape)
764+
const shouldDisplayHandles = useValue(
765+
'should display handles',
766+
() => {
767+
const { editor } = this
768+
return (
769+
!editor.getIsReadonly() &&
770+
editor.getOnlySelectedShapeId() === shape.id &&
771+
editor.isInAny(
772+
'select.idle',
773+
'select.pointing_handle',
774+
'select.dragging_handle',
775+
'select.translating',
776+
'arrow.dragging'
777+
)
778+
)
779+
},
780+
[editor, shape.id]
781+
)
782+
783+
const isSelected = useValue(
784+
'is selected',
785+
() => editor.getOnlySelectedShape()?.id === shape.id,
786+
[editor, shape.id]
787+
)
788+
789+
const isEditing = useValue('is editing', () => editor.getEditingShapeId() === shape.id, [
790+
editor,
791+
shape.id,
792+
])
793+
794+
const info = getArrowInfo(editor, shape)
771795
if (!info?.isValid) return null
772796

773-
const labelPosition = getArrowLabelPosition(this.editor, shape)
774-
const isSelected = shape.id === this.editor.getOnlySelectedShapeId()
775-
const isEditing = this.editor.getEditingShapeId() === shape.id
797+
const labelPosition = getArrowLabelPosition(editor, shape, isEditing)
776798
const showArrowLabel = isEditing || !isEmptyRichText(shape.props.richText)
777799

778800
return (
779801
<>
780802
<SVGContainer style={{ minWidth: 50, minHeight: 50 }}>
781-
<ArrowSvg
782-
shape={shape}
783-
shouldDisplayHandles={shouldDisplayHandles && onlySelectedShape?.id === shape.id}
784-
/>
803+
<ArrowSvg shape={shape} shouldDisplayHandles={shouldDisplayHandles} />
785804
{shape.props.kind === 'elbow' && debugFlags.debugElbowArrows.get() && (
786805
<ElbowArrowDebug arrow={shape} />
787806
)}
@@ -798,7 +817,7 @@ export class ArrowShapeUtil extends ShapeUtil<TLArrowShape> {
798817
labelColor={getColorValue(theme, shape.props.labelColor, 'solid')}
799818
richText={shape.props.richText}
800819
textWidth={labelPosition.box.w - ARROW_LABEL_PADDING * 2 * shape.props.scale}
801-
isSelected={isSelected}
820+
isSelected={isSelected} // does this HAVE to be isSelected? or isOnlySelected?
802821
padding={0}
803822
showTextOutline={this.options.showTextOutline}
804823
style={{
@@ -811,9 +830,8 @@ export class ArrowShapeUtil extends ShapeUtil<TLArrowShape> {
811830
}
812831

813832
indicator(shape: TLArrowShape) {
814-
// eslint-disable-next-line react-hooks/rules-of-hooks
815833
const isEditing = useIsEditing(shape.id)
816-
// eslint-disable-next-line react-hooks/rules-of-hooks
834+
817835
const clipPathId = useSharedSafeId(shape.id + '_clip')
818836

819837
const info = getArrowInfo(this.editor, shape)
@@ -1072,7 +1090,7 @@ export class ArrowShapeUtil extends ShapeUtil<TLArrowShape> {
10721090
verticalAlign="middle"
10731091
labelColor={getColorValue(theme, shape.props.labelColor, 'solid')}
10741092
richText={shape.props.richText}
1075-
bounds={getArrowLabelPosition(this.editor, shape)
1093+
bounds={getArrowLabelPosition(this.editor, shape, false)
10761094
.box.clone()
10771095
.expandBy(-ARROW_LABEL_PADDING * shape.props.scale)}
10781096
padding={0}
@@ -1185,7 +1203,7 @@ const ArrowSvg = track(function ArrowSvg({
11851203
})
11861204
}
11871205

1188-
const labelPosition = getArrowLabelPosition(editor, shape)
1206+
const labelPosition = getArrowLabelPosition(editor, shape, isEditing)
11891207

11901208
const clipStartArrowhead = !(info.start.arrowhead === 'none' || info.start.arrowhead === 'arrow')
11911209
const clipEndArrowhead = !(info.end.arrowhead === 'none' || info.end.arrowhead === 'arrow')

packages/tldraw/src/lib/shapes/arrow/arrowLabel.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,8 +226,7 @@ interface ArrowheadInfo {
226226
hasStartArrowhead: boolean
227227
hasEndArrowhead: boolean
228228
}
229-
export function getArrowLabelPosition(editor: Editor, shape: TLArrowShape) {
230-
const isEditing = editor.getEditingShapeId() === shape.id
229+
export function getArrowLabelPosition(editor: Editor, shape: TLArrowShape, isEditing: boolean) {
231230
if (!isEditing && isEmptyRichText(shape.props.richText)) {
232231
// Short-circuit for empty labels.
233232
const bodyGeom = getArrowBodyGeometry(editor, shape)

0 commit comments

Comments
 (0)