Skip to content

Arrow bend translation #770

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
trangdoan982 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Arrow bend translation #770

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions apps/obsidian/src/components/canvas/shapes/DiscourseRelationShape.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,8 +474,13 @@ export class DiscourseRelationUtil extends ShapeUtil<DiscourseRelationShape> {

const bindings = getArrowBindings(this.editor, shape);

// If both ends are bound, convert translation to bend changes instead of moving the arrow
if (bindings.start && bindings.end) {
// Check if other shapes are also being translated
const selectedShapeIds = this.editor.getSelectedShapeIds();
const onlyRelationSelected = selectedShapeIds.length === 1 && selectedShapeIds[0] === shape.id;

// If both ends are bound AND only the relation is selected, convert translation to bend changes
// If other shapes are also selected, do a simple translation instead
if (bindings.start && bindings.end && onlyRelationSelected) {
Comment on lines +477 to +483
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Missing early return in onTranslate causes bindings to be removed when multi-selecting bound arrows with their nodes

When a DiscourseRelationShape with both ends bound is translated as part of a multi-shape selection (e.g., the arrow plus its two bound nodes), the arrow's bindings are incorrectly modified or removed.

Root Cause: onTranslateStart always sets translation data for both-ends-bound case, but onTranslate now falls through to wrong code path

In onTranslateStart (line 606), when both ends are bound, the method always stores shapeAtTranslationStart data and returns early — regardless of whether the bound shapes are also selected:

// Line 606-622
if (bindings.start && bindings.end) {
  shapeAtTranslationStart.set(shape, { ... });
  return; // Always returns here
}

This check at line 606 runs before the check at line 630 that would normally detect that bound shapes are in the selection and return without setting shapeAtTranslationStart.

Then in onTranslate, the new PR code at line 483 checks onlyRelationSelected. When other shapes are also selected, onlyRelationSelected is false, so the bend-change block is skipped. Execution falls through to the "normal translation behavior" code path (lines 535-591).

This normal path iterates over atTranslationStart.terminalBindings and for each terminal:

  1. Calculates newPagePoint = terminalBinding.pagePosition + pageDelta * 0.5 (line 547-549)
  2. Calls getShapeAtPoint(newPagePoint) to find the target (line 551)
  3. If the target isn't found at that point, calls removeArrowBinding() (line 585-589)

When the bound nodes are also being translated (moving by the full pageDelta), searching at pagePosition + pageDelta * 0.5 (only half the delta) will likely miss the target shapes. This causes removeArrowBinding to fire, breaking the arrow connections.

Impact: Selecting and moving a group of connected nodes and arrows together will disconnect the arrows from their nodes, which is the opposite of the intended fix.

Suggested change
// Check if other shapes are also being translated
const selectedShapeIds = this.editor.getSelectedShapeIds();
const onlyRelationSelected = selectedShapeIds.length === 1 && selectedShapeIds[0] === shape.id;
// If both ends are bound AND only the relation is selected, convert translation to bend changes
// If other shapes are also selected, do a simple translation instead
if (bindings.start && bindings.end && onlyRelationSelected) {
// Check if other shapes are also being translated
const selectedShapeIds = this.editor.getSelectedShapeIds();
const onlyRelationSelected = selectedShapeIds.length === 1 && selectedShapeIds[0] === shape.id;
// If both ends are bound AND only the relation is selected, convert translation to bend changes
// If other shapes are also selected, allow simple translation (tldraw handles moving bound shapes)
if (bindings.start && bindings.end) {
if (!onlyRelationSelected) return;
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

const shapePageTransform = this.editor.getShapePageTransform(shape.id);
const pageDelta = Vec.Sub(
shapePageTransform.applyToPoint(shape),
Expand Down