From 8ca4cf32605343c1631ed7ef57f0c9d87f4c23b1 Mon Sep 17 00:00:00 2001 From: David Luzar <5153846+dwelle@users.noreply.github.com> Date: Thu, 19 Sep 2024 15:46:36 +0200 Subject: [PATCH] feat: flip arrowheads if only arrow(s) selected (#8525) Co-authored-by: Mark Tolmacs --- .../excalidraw/actions/actionFlip.test.tsx | 254 +++++++++++++----- packages/excalidraw/actions/actionFlip.ts | 24 +- packages/excalidraw/tests/helpers/api.ts | 12 + 3 files changed, 222 insertions(+), 68 deletions(-) diff --git a/packages/excalidraw/actions/actionFlip.test.tsx b/packages/excalidraw/actions/actionFlip.test.tsx index 0a1b9f41d5..c8a6239cdf 100644 --- a/packages/excalidraw/actions/actionFlip.test.tsx +++ b/packages/excalidraw/actions/actionFlip.test.tsx @@ -3,87 +3,209 @@ import { Excalidraw } from "../index"; import { render } from "../tests/test-utils"; import { API } from "../tests/helpers/api"; import { point } from "../../math"; -import { actionFlipHorizontal } from "./actionFlip"; +import { actionFlipHorizontal, actionFlipVertical } from "./actionFlip"; const { h } = window; -const testElements = [ - API.createElement({ - type: "rectangle", - id: "rec1", - x: 1046, - y: 541, - width: 100, - height: 100, - boundElements: [ - { - id: "arr", +describe("flipping re-centers selection", () => { + it("elbow arrow touches group selection side yet it remains in place after multiple moves", async () => { + const elements = [ + API.createElement({ + type: "rectangle", + id: "rec1", + x: 100, + y: 100, + width: 100, + height: 100, + boundElements: [{ id: "arr", type: "arrow" }], + }), + API.createElement({ + type: "rectangle", + id: "rec2", + x: 220, + y: 250, + width: 100, + height: 100, + boundElements: [{ id: "arr", type: "arrow" }], + }), + API.createElement({ type: "arrow", - }, - ], - }), - API.createElement({ - type: "rectangle", - id: "rec2", - x: 1169, - y: 777, - width: 102, - height: 115, - boundElements: [ - { id: "arr", - type: "arrow", - }, - ], - }), - API.createElement({ - type: "arrow", - id: "arrow", - x: 1103.0717787616313, - y: 536.8531862198708, - width: 159.68539325842903, - height: 333.0396003698186, - startBinding: { - elementId: "rec1", - focus: 0.1366906474820229, - gap: 5.000000000000057, - fixedPoint: [0.5683453237410123, -0.05014327585315258], - }, - endBinding: { - elementId: "rec2", - focus: 0.0014925373134265828, - gap: 5, - fixedPoint: [-0.04862325174825108, 0.4992537313432874], - }, - points: [ - point(0, 0), - point(0, -35), - point(-97.80898876404626, -35), - point(-97.80898876404626, 298.0396003698186), - point(61.87640449438277, 298.0396003698186), - ], - elbowed: true, - }), -]; + x: 149.9, + y: 95, + width: 156, + height: 239.9, + startBinding: { + elementId: "rec1", + focus: 0, + gap: 5, + fixedPoint: [0.49, -0.05], + }, + endBinding: { + elementId: "rec2", + focus: 0, + gap: 5, + fixedPoint: [-0.05, 0.49], + }, + startArrowhead: null, + endArrowhead: "arrow", + points: [ + point(0, 0), + point(0, -35), + point(-90.9, -35), + point(-90.9, 204.9), + point(65.1, 204.9), + ], + elbowed: true, + }), + ]; + await render(); -describe("flipping action", () => { - it("flip re-centers the selection even after multiple flip actions", async () => { - await render(); - - API.setSelectedElements(testElements); + API.setSelectedElements(elements); expect(Object.keys(h.state.selectedElementIds).length).toBe(3); API.executeAction(actionFlipHorizontal); API.executeAction(actionFlipHorizontal); API.executeAction(actionFlipHorizontal); + API.executeAction(actionFlipHorizontal); const rec1 = h.elements.find((el) => el.id === "rec1"); - expect(rec1?.x).toBeCloseTo(1113.78, 0); - expect(rec1?.y).toBeCloseTo(541, 0); + expect(rec1?.x).toBeCloseTo(100); + expect(rec1?.y).toBeCloseTo(100); const rec2 = h.elements.find((el) => el.id === "rec2"); - expect(rec2?.x).toBeCloseTo(988.72, 0); - expect(rec2?.y).toBeCloseTo(777, 0); + expect(rec2?.x).toBeCloseTo(220); + expect(rec2?.y).toBeCloseTo(250); + }); +}); + +describe("flipping arrowheads", () => { + beforeEach(async () => { + await render(); + }); + + it("flipping bound arrow should flip arrowheads only", () => { + const rect = API.createElement({ + type: "rectangle", + boundElements: [{ type: "arrow", id: "arrow1" }], + }); + const arrow = API.createElement({ + type: "arrow", + id: "arrow1", + startArrowhead: "arrow", + endArrowhead: null, + endBinding: { + elementId: rect.id, + focus: 0.5, + gap: 5, + }, + }); + + API.setElements([rect, arrow]); + API.setSelectedElements([arrow]); + + expect(API.getElement(arrow).startArrowhead).toBe("arrow"); + expect(API.getElement(arrow).endArrowhead).toBe(null); + + API.executeAction(actionFlipHorizontal); + expect(API.getElement(arrow).startArrowhead).toBe(null); + expect(API.getElement(arrow).endArrowhead).toBe("arrow"); + + API.executeAction(actionFlipHorizontal); + expect(API.getElement(arrow).startArrowhead).toBe("arrow"); + expect(API.getElement(arrow).endArrowhead).toBe(null); + + API.executeAction(actionFlipVertical); + expect(API.getElement(arrow).startArrowhead).toBe(null); + expect(API.getElement(arrow).endArrowhead).toBe("arrow"); + }); + + it("flipping bound arrow should flip arrowheads only 2", () => { + const rect = API.createElement({ + type: "rectangle", + boundElements: [{ type: "arrow", id: "arrow1" }], + }); + const rect2 = API.createElement({ + type: "rectangle", + boundElements: [{ type: "arrow", id: "arrow1" }], + }); + const arrow = API.createElement({ + type: "arrow", + id: "arrow1", + startArrowhead: "arrow", + endArrowhead: "circle", + startBinding: { + elementId: rect.id, + focus: 0.5, + gap: 5, + }, + endBinding: { + elementId: rect2.id, + focus: 0.5, + gap: 5, + }, + }); + + API.setElements([rect, rect2, arrow]); + API.setSelectedElements([arrow]); + + expect(API.getElement(arrow).startArrowhead).toBe("arrow"); + expect(API.getElement(arrow).endArrowhead).toBe("circle"); + + API.executeAction(actionFlipHorizontal); + expect(API.getElement(arrow).startArrowhead).toBe("circle"); + expect(API.getElement(arrow).endArrowhead).toBe("arrow"); + + API.executeAction(actionFlipVertical); + expect(API.getElement(arrow).startArrowhead).toBe("arrow"); + expect(API.getElement(arrow).endArrowhead).toBe("circle"); + }); + + it("flipping unbound arrow shouldn't flip arrowheads", () => { + const arrow = API.createElement({ + type: "arrow", + id: "arrow1", + startArrowhead: "arrow", + endArrowhead: "circle", + }); + + API.setElements([arrow]); + API.setSelectedElements([arrow]); + + expect(API.getElement(arrow).startArrowhead).toBe("arrow"); + expect(API.getElement(arrow).endArrowhead).toBe("circle"); + + API.executeAction(actionFlipHorizontal); + expect(API.getElement(arrow).startArrowhead).toBe("arrow"); + expect(API.getElement(arrow).endArrowhead).toBe("circle"); + }); + + it("flipping bound arrow shouldn't flip arrowheads if selected alongside non-arrow eleemnt", () => { + const rect = API.createElement({ + type: "rectangle", + boundElements: [{ type: "arrow", id: "arrow1" }], + }); + const arrow = API.createElement({ + type: "arrow", + id: "arrow1", + startArrowhead: "arrow", + endArrowhead: null, + endBinding: { + elementId: rect.id, + focus: 0.5, + gap: 5, + }, + }); + + API.setElements([rect, arrow]); + API.setSelectedElements([rect, arrow]); + + expect(API.getElement(arrow).startArrowhead).toBe("arrow"); + expect(API.getElement(arrow).endArrowhead).toBe(null); + + API.executeAction(actionFlipHorizontal); + expect(API.getElement(arrow).startArrowhead).toBe("arrow"); + expect(API.getElement(arrow).endArrowhead).toBe(null); }); }); diff --git a/packages/excalidraw/actions/actionFlip.ts b/packages/excalidraw/actions/actionFlip.ts index 45ca7a298c..6b75b8facd 100644 --- a/packages/excalidraw/actions/actionFlip.ts +++ b/packages/excalidraw/actions/actionFlip.ts @@ -2,6 +2,7 @@ import { register } from "./register"; import { getSelectedElements } from "../scene"; import { getNonDeletedElements } from "../element"; import type { + ExcalidrawArrowElement, ExcalidrawElbowArrowElement, ExcalidrawElement, NonDeleted, @@ -19,9 +20,13 @@ import { import { updateFrameMembershipOfSelectedElements } from "../frame"; import { flipHorizontal, flipVertical } from "../components/icons"; import { StoreAction } from "../store"; -import { isElbowArrow, isLinearElement } from "../element/typeChecks"; +import { + isArrowElement, + isElbowArrow, + isLinearElement, +} from "../element/typeChecks"; import { mutateElbowArrow } from "../element/routing"; -import { mutateElement } from "../element/mutateElement"; +import { mutateElement, newElementWith } from "../element/mutateElement"; export const actionFlipHorizontal = register({ name: "flipHorizontal", @@ -112,6 +117,21 @@ const flipElements = ( flipDirection: "horizontal" | "vertical", app: AppClassProperties, ): ExcalidrawElement[] => { + if ( + selectedElements.every( + (element) => + isArrowElement(element) && (element.startBinding || element.endBinding), + ) + ) { + return selectedElements.map((element) => { + const _element = element as ExcalidrawArrowElement; + return newElementWith(_element, { + startArrowhead: _element.endArrowhead, + endArrowhead: _element.startArrowhead, + }); + }); + } + const { minX, minY, maxX, maxY, midX, midY } = getCommonBoundingBox(selectedElements); diff --git a/packages/excalidraw/tests/helpers/api.ts b/packages/excalidraw/tests/helpers/api.ts index d98a908f7a..b7dc6e10d6 100644 --- a/packages/excalidraw/tests/helpers/api.ts +++ b/packages/excalidraw/tests/helpers/api.ts @@ -129,6 +129,10 @@ export class API { expect(API.getSelectedElements().length).toBe(0); }; + static getElement = (element: T): T => { + return h.app.scene.getElementsMapIncludingDeleted().get(element.id) as T || element; + } + static createElement = < T extends Exclude = "rectangle", >({ @@ -186,6 +190,12 @@ export class API { endBinding?: T extends "arrow" ? ExcalidrawArrowElement["endBinding"] | ExcalidrawElbowArrowElement["endBinding"] : never; + startArrowhead?: T extends "arrow" + ? ExcalidrawArrowElement["startArrowhead"] | ExcalidrawElbowArrowElement["startArrowhead"] + : never; + endArrowhead?: T extends "arrow" + ? ExcalidrawArrowElement["endArrowhead"] | ExcalidrawElbowArrowElement["endArrowhead"] + : never; elbowed?: boolean; }): T extends "arrow" | "line" ? ExcalidrawLinearElement @@ -342,6 +352,8 @@ export class API { if (element.type === "arrow") { element.startBinding = rest.startBinding ?? null; element.endBinding = rest.endBinding ?? null; + element.startArrowhead = rest.startArrowhead ?? null; + element.endArrowhead = rest.endArrowhead ?? null; } if (id) { element.id = id;