Fixed point binding for simple arrows when the arrow doesn't point to the element

This commit is contained in:
Mark Tolmacs
2025-06-25 18:44:29 +02:00
parent b0ac15381b
commit 7de5d29b79
3 changed files with 172 additions and 46 deletions

View File

@ -404,7 +404,6 @@ export const maybeSuggestBindingsForLinearElementAtCoords = (
Array.from( Array.from(
pointerCoords.reduce( pointerCoords.reduce(
(acc: Set<NonDeleted<ExcalidrawBindableElement>>, coords) => { (acc: Set<NonDeleted<ExcalidrawBindableElement>>, coords) => {
const p = pointFrom<GlobalPoint>(coords.x, coords.y);
const hoveredBindableElement = getHoveredElementForBinding( const hoveredBindableElement = getHoveredElementForBinding(
coords, coords,
scene.getNonDeletedElements(), scene.getNonDeletedElements(),
@ -413,19 +412,10 @@ export const maybeSuggestBindingsForLinearElementAtCoords = (
isElbowArrow(linearElement), isElbowArrow(linearElement),
isElbowArrow(linearElement), isElbowArrow(linearElement),
); );
const pointIsInside =
hoveredBindableElement != null &&
hitElementItself({
point: p,
element: hoveredBindableElement,
elementsMap,
threshold: 0, // TODO: Not ideal, should be calculated from the same source
});
if ( if (
hoveredBindableElement != null && hoveredBindableElement != null &&
((pointIsInside && (oppositeBindingBoundElement?.id === hoveredBindableElement.id ||
oppositeBindingBoundElement?.id === hoveredBindableElement.id) ||
!isLinearElementSimpleAndAlreadyBound( !isLinearElementSimpleAndAlreadyBound(
linearElement, linearElement,
oppositeBindingBoundElement?.id, oppositeBindingBoundElement?.id,
@ -504,6 +494,11 @@ export const bindLinearElement = (
} }
const elementsMap = scene.getNonDeletedElementsMap(); const elementsMap = scene.getNonDeletedElementsMap();
const edgePoint = LinearElementEditor.getPointAtIndexGlobalCoordinates(
linearElement,
startOrEnd === "start" ? 0 : -1,
elementsMap,
);
let binding: PointBinding | FixedPointBinding; let binding: PointBinding | FixedPointBinding;
if (isElbowArrow(linearElement)) { if (isElbowArrow(linearElement)) {
@ -525,36 +520,67 @@ export const bindLinearElement = (
elementsMap, elementsMap,
), ),
}; };
} else if (
hitElementItself({
point: edgePoint,
element: hoveredElement,
elementsMap,
threshold: 0, // TODO: Not ideal, should be calculated from the same source
})
) {
// Use FixedPoint binding when the arrow endpoint is inside the shape
binding = {
elementId: hoveredElement.id,
focus: 0,
gap: 0,
...calculateFixedPointForNonElbowArrowBinding(
linearElement,
hoveredElement,
startOrEnd,
elementsMap,
),
};
} else { } else {
// For non-elbow arrows, check if the endpoint is inside the shape // For non-elbow arrows, extend the last segment and check intersection
const edgePoint = LinearElementEditor.getPointAtIndexGlobalCoordinates( const adjacentPoint = LinearElementEditor.getPointAtIndexGlobalCoordinates(
linearElement, linearElement,
startOrEnd === "start" ? 0 : -1, startOrEnd === "start" ? 1 : -2,
elementsMap, elementsMap,
); );
const extendedDirection = vectorScale(
if ( vectorNormalize(
hitElementItself({ vectorFromPoint(
point: edgePoint, pointFrom(
element: hoveredElement, edgePoint[0] - adjacentPoint[0],
elementsMap, edgePoint[1] - adjacentPoint[1],
threshold: 0, // TODO: Not ideal, should be calculated from the same source ),
})
) {
// Use FixedPoint binding when the arrow endpoint is inside the shape
binding = {
elementId: hoveredElement.id,
focus: 0,
gap: 0,
...calculateFixedPointForNonElbowArrowBinding(
linearElement,
hoveredElement,
startOrEnd,
elementsMap,
), ),
}; ),
} else { Math.max(hoveredElement.width, hoveredElement.height) * 2,
// Use traditional focus/gap binding when the endpoint is outside the shape );
const intersector = lineSegment(
edgePoint,
pointFromVector(
vectorFromPoint(
pointFrom(
edgePoint[0] + extendedDirection[0],
edgePoint[1] + extendedDirection[1],
),
),
),
);
// Check if this extended segment intersects the bindable element
const intersections = intersectElementWithLineSegment(
hoveredElement,
elementsMap,
intersector,
);
const intersectsElement = intersections.length > 0;
if (intersectsElement) {
// Use traditional focus/gap binding when the extended segment intersects
binding = { binding = {
elementId: hoveredElement.id, elementId: hoveredElement.id,
...normalizePointBinding( ...normalizePointBinding(
@ -567,6 +593,19 @@ export const bindLinearElement = (
hoveredElement, hoveredElement,
), ),
}; };
} else {
// Use FixedPoint binding when the extended segment doesn't intersect
binding = {
elementId: hoveredElement.id,
focus: 0,
gap: 0,
...calculateFixedPointForNonElbowArrowBinding(
linearElement,
hoveredElement,
startOrEnd,
elementsMap,
),
};
} }
} }

View File

@ -8,7 +8,13 @@ import { Excalidraw, isLinearElement } from "@excalidraw/excalidraw";
import { API } from "@excalidraw/excalidraw/tests/helpers/api"; import { API } from "@excalidraw/excalidraw/tests/helpers/api";
import { UI, Pointer, Keyboard } from "@excalidraw/excalidraw/tests/helpers/ui"; import { UI, Pointer, Keyboard } from "@excalidraw/excalidraw/tests/helpers/ui";
import { fireEvent, render } from "@excalidraw/excalidraw/tests/test-utils"; import {
act,
fireEvent,
render,
} from "@excalidraw/excalidraw/tests/test-utils";
import { defaultLang, setLanguage } from "@excalidraw/excalidraw/i18n";
import { getTransformHandles } from "../src/transformHandles"; import { getTransformHandles } from "../src/transformHandles";
import { import {
@ -73,8 +79,9 @@ describe("element binding", () => {
expect(arrow.startBinding).toEqual({ expect(arrow.startBinding).toEqual({
elementId: rect.id, elementId: rect.id,
focus: expect.toBeNonNaNNumber(), focus: 0,
gap: expect.toBeNonNaNNumber(), gap: 0,
fixedPoint: expect.arrayContaining([1.1, 0]),
}); });
// Move the end point to the overlapping binding position // Move the end point to the overlapping binding position
@ -85,13 +92,15 @@ describe("element binding", () => {
// Both the start and the end points should be bound // Both the start and the end points should be bound
expect(arrow.startBinding).toEqual({ expect(arrow.startBinding).toEqual({
elementId: rect.id, elementId: rect.id,
focus: expect.toBeNonNaNNumber(), focus: 0,
gap: expect.toBeNonNaNNumber(), gap: 0,
fixedPoint: expect.arrayContaining([1.1, 0]),
}); });
expect(arrow.endBinding).toEqual({ expect(arrow.endBinding).toEqual({
elementId: rect.id, elementId: rect.id,
focus: expect.toBeNonNaNNumber(), focus: 0,
gap: expect.toBeNonNaNNumber(), gap: 0,
fixedPoint: expect.arrayContaining([1.1, 0]),
}); });
}); });
@ -190,9 +199,9 @@ describe("element binding", () => {
// Test sticky connection // Test sticky connection
expect(API.getSelectedElement().type).toBe("arrow"); expect(API.getSelectedElement().type).toBe("arrow");
Keyboard.keyPress(KEYS.ARROW_RIGHT); Keyboard.keyPress(KEYS.ARROW_RIGHT);
expect(arrow.endBinding?.elementId).toBe(rectangle.id); expect(arrow.endBinding?.elementId).not.toBe(rectangle.id);
Keyboard.keyPress(KEYS.ARROW_LEFT); Keyboard.keyPress(KEYS.ARROW_LEFT);
expect(arrow.endBinding?.elementId).toBe(rectangle.id); expect(arrow.endBinding?.elementId).not.toBe(rectangle.id);
// Sever connection // Sever connection
expect(API.getSelectedElement().type).toBe("arrow"); expect(API.getSelectedElement().type).toBe("arrow");
@ -750,3 +759,82 @@ describe("Fixed-point arrow binding", () => {
expect(arrow.points[2][1]).toBe(originalInnerPoint2[1]); expect(arrow.points[2][1]).toBe(originalInnerPoint2[1]);
}); });
}); });
describe("line segment extension binding", () => {
beforeEach(async () => {
mouse.reset();
await act(() => {
return setLanguage(defaultLang);
});
await render(<Excalidraw handleKeyboardGlobally={true} />);
});
it("should use point binding when extended segment intersects element", () => {
// Create a rectangle that will be intersected by the extended arrow segment
const rect = API.createElement({
type: "rectangle",
x: 100,
y: 100,
width: 100,
height: 100,
});
API.setElements([rect]);
// Draw an arrow that points at the rectangle (extended segment will intersect)
UI.clickTool("arrow");
mouse.downAt(0, 0); // Start point
mouse.moveTo(120, 95); // End point - arrow direction points toward rectangle
mouse.up();
const arrow = API.getSelectedElement() as ExcalidrawLinearElement;
// Should create a normal point binding since the extended line segment
// from the last arrow segment intersects the rectangle
expect(arrow.endBinding?.elementId).toBe(rect.id);
expect(arrow.endBinding).toHaveProperty("focus");
expect(arrow.endBinding).toHaveProperty("gap");
expect(arrow.endBinding).not.toHaveProperty("fixedPoint");
});
it("should use fixed point binding when extended segment misses element", () => {
// Create a rectangle positioned so the extended arrow segment will miss it
const rect = API.createElement({
type: "rectangle",
x: 100,
y: 100,
width: 100,
height: 100,
});
API.setElements([rect]);
// Draw an arrow that doesn't point at the rectangle (extended segment will miss)
UI.clickTool("arrow");
mouse.reset();
mouse.downAt(125, 93); // Start point
mouse.moveTo(175, 93); // End point - arrow direction is horizontal, misses rectangle
mouse.up();
const arrow = API.getSelectedElement() as ExcalidrawLinearElement;
// Should create a fixed point binding since the extended line segment
// from the last arrow segment misses the rectangle
expect(arrow.startBinding?.elementId).toBe(rect.id);
expect(arrow.startBinding).toHaveProperty("fixedPoint");
expect(
(arrow.startBinding as FixedPointBinding).fixedPoint[0],
).toBeGreaterThanOrEqual(0);
expect(
(arrow.startBinding as FixedPointBinding).fixedPoint[0],
).toBeLessThanOrEqual(1);
expect(
(arrow.startBinding as FixedPointBinding).fixedPoint[1],
).toBeLessThanOrEqual(0);
expect(
(arrow.startBinding as FixedPointBinding).fixedPoint[1],
).toBeLessThanOrEqual(1);
expect(arrow.endBinding).toBe(null);
});
});

View File

@ -5923,7 +5923,6 @@ class App extends React.Component<AppProps, AppState> {
this.scene, this.scene,
this.state.zoom, this.state.zoom,
this.scene.getNonDeletedElementsMap(), this.scene.getNonDeletedElementsMap(),
//this.state.startBoundElement,
), ),
}); });
} else { } else {