diff --git a/src/components/App.tsx b/src/components/App.tsx index d1f0d862..d873bfce 100644 --- a/src/components/App.tsx +++ b/src/components/App.tsx @@ -127,6 +127,7 @@ import { isInitializedImageElement, isLinearElement, isLinearElementType, + isTextBindableContainer, } from "../element/typeChecks"; import { ExcalidrawBindableElement, @@ -140,6 +141,7 @@ import { ExcalidrawImageElement, FileId, NonDeletedExcalidrawElement, + ExcalidrawTextContainer, } from "../element/types"; import { getCenter, getDistance } from "../gesture"; import { @@ -167,7 +169,7 @@ import { renderScene } from "../renderer"; import { invalidateShapeForElement } from "../renderer/renderElement"; import { calculateScrollCenter, - getElementContainingPosition, + getTextBindableContainerAtPosition, getElementsAtPosition, getElementsWithinSelection, getNormalizedZoom, @@ -238,7 +240,7 @@ import { bindTextToShapeAfterDuplication, getApproxMinLineHeight, getApproxMinLineWidth, - getBoundTextElementId, + getBoundTextElement, } from "../element/textElement"; import { isHittingElementNotConsideringBoundingBox } from "../element/collision"; import { @@ -2157,28 +2159,40 @@ class App extends React.Component { window.devicePixelRatio, ); - // bind to container when shouldBind is true or - // clicked on center of container - const container = - shouldBind || parentCenterPosition - ? getElementContainingPosition( - this.scene.getElements().filter((ele) => !isTextElement(ele)), - sceneX, - sceneY, - ) - : null; + let existingTextElement: NonDeleted | null = null; + let container: ExcalidrawTextContainer | null = null; - let existingTextElement = this.getTextElementAtPosition(sceneX, sceneY); + const selectedElements = getSelectedElements( + this.scene.getElements(), + this.state, + ); - // consider bounded text element if container present - if (container) { - const boundTextElementId = getBoundTextElementId(container); - if (boundTextElementId) { - existingTextElement = this.scene.getElement( - boundTextElementId, - ) as ExcalidrawTextElement; + if (selectedElements.length === 1) { + if (isTextElement(selectedElements[0])) { + existingTextElement = selectedElements[0]; + } else if (isTextBindableContainer(selectedElements[0])) { + container = selectedElements[0]; + existingTextElement = getBoundTextElement(container); } } + + existingTextElement = + existingTextElement ?? this.getTextElementAtPosition(sceneX, sceneY); + + // bind to container when shouldBind is true or + // clicked on center of container + if ( + !container && + !existingTextElement && + (shouldBind || parentCenterPosition) + ) { + container = getTextBindableContainerAtPosition( + this.scene.getElements().filter((ele) => !isTextElement(ele)), + sceneX, + sceneY, + ); + } + if (!existingTextElement && container) { const fontString = { fontSize: this.state.currentItemFontSize, @@ -5432,7 +5446,7 @@ class App extends React.Component { canvas: HTMLCanvasElement | null, scale: number, ) { - const elementClickedInside = getElementContainingPosition( + const elementClickedInside = getTextBindableContainerAtPosition( this.scene .getElementsIncludingDeleted() .filter((element) => !isTextElement(element)), diff --git a/src/element/dragElements.ts b/src/element/dragElements.ts index f2133b8d..b6c5383a 100644 --- a/src/element/dragElements.ts +++ b/src/element/dragElements.ts @@ -3,10 +3,9 @@ import { updateBoundElements } from "./binding"; import { getCommonBounds } from "./bounds"; import { mutateElement } from "./mutateElement"; import { getPerfectElementSize } from "./sizeHelpers"; -import Scene from "../scene/Scene"; import { NonDeletedExcalidrawElement } from "./types"; import { AppState, PointerDownState } from "../types"; -import { getBoundTextElementId } from "./textElement"; +import { getBoundTextElement } from "./textElement"; import { isSelectedViaGroup } from "../groups"; export const dragSelectedElements = ( @@ -39,16 +38,14 @@ export const dragSelectedElements = ( // container is part of a group, but we're dragging the container directly (appState.editingGroupId && !isSelectedViaGroup(appState, element)) ) { - const boundTextElementId = getBoundTextElementId(element); - if (boundTextElementId) { - const textElement = - Scene.getScene(element)!.getElement(boundTextElementId); + const textElement = getBoundTextElement(element); + if (textElement) { updateElementCoords( lockDirection, distanceX, distanceY, pointerDownState, - textElement!, + textElement, offset, ); } diff --git a/src/element/newElement.ts b/src/element/newElement.ts index 0567daff..11a0f23d 100644 --- a/src/element/newElement.ts +++ b/src/element/newElement.ts @@ -22,7 +22,6 @@ import { getElementAbsoluteCoords } from "."; import { adjustXYWithRotation } from "../math"; import { getResizedElementAbsoluteCoords } from "./bounds"; import { getContainerElement, measureText, wrapText } from "./textElement"; -import { isBoundToContainer } from "./typeChecks"; import { BOUND_TEXT_PADDING, VERTICAL_ALIGN } from "../constants"; type ElementConstructorOpts = MarkOptional< @@ -221,8 +220,7 @@ const getAdjustedDimensions = ( // make sure container dimensions are set properly when // text editor overflows beyond viewport dimensions - if (isBoundToContainer(element)) { - const container = getContainerElement(element)!; + if (container) { let height = container.height; let width = container.width; if (nextHeight > height - BOUND_TEXT_PADDING * 2) { diff --git a/src/element/textElement.ts b/src/element/textElement.ts index 3f86e0d3..5b65fce5 100644 --- a/src/element/textElement.ts +++ b/src/element/textElement.ts @@ -1,6 +1,5 @@ import { getFontString, arrayToMap, isTestEnv } from "../utils"; import { - ExcalidrawBindableElement, ExcalidrawElement, ExcalidrawTextElement, ExcalidrawTextElementWithContainer, @@ -12,6 +11,7 @@ import { BOUND_TEXT_PADDING, VERTICAL_ALIGN } from "../constants"; import { MaybeTransformHandleType } from "./transformHandles"; import Scene from "../scene/Scene"; import { AppState } from "../types"; +import { isTextElement } from "."; export const redrawTextBoundingBox = ( element: ExcalidrawTextElement, @@ -79,22 +79,24 @@ export const bindTextToShapeAfterDuplication = ( const boundTextElementId = getBoundTextElementId(element); if (boundTextElementId) { - const newTextElementId = oldIdToDuplicatedId.get(boundTextElementId)!; - mutateElement( - sceneElementMap.get(newElementId) as ExcalidrawBindableElement, - { - boundElements: element.boundElements?.concat({ - type: "text", - id: newTextElementId, - }), - }, - ); - mutateElement( - sceneElementMap.get(newTextElementId) as ExcalidrawTextElement, - { - containerId: newElementId, - }, - ); + const newTextElementId = oldIdToDuplicatedId.get(boundTextElementId); + if (newTextElementId) { + const newContainer = sceneElementMap.get(newElementId); + if (newContainer) { + mutateElement(newContainer, { + boundElements: element.boundElements?.concat({ + type: "text", + id: newTextElementId, + }), + }); + } + const newTextElement = sceneElementMap.get(newTextElementId); + if (newTextElement && isTextElement(newTextElement)) { + mutateElement(newTextElement, { + containerId: newContainer ? newElementId : null, + }); + } + } } }); }; @@ -440,7 +442,10 @@ export const getApproxCharsToFitInWidth = (font: FontString, width: number) => { }; export const getBoundTextElementId = (container: ExcalidrawElement | null) => { - return container?.boundElements?.filter((ele) => ele.type === "text")[0]?.id; + return container?.boundElements?.length + ? container?.boundElements?.filter((ele) => ele.type === "text")[0]?.id || + null + : null; }; export const getBoundTextElement = (element: ExcalidrawElement | null) => { diff --git a/src/element/textWysiwyg.test.tsx b/src/element/textWysiwyg.test.tsx index 0fe105ba..4d14cfb5 100644 --- a/src/element/textWysiwyg.test.tsx +++ b/src/element/textWysiwyg.test.tsx @@ -12,6 +12,8 @@ import { ExcalidrawTextElementWithContainer, } from "./types"; import * as textElementUtils from "./textElement"; +import { API } from "../tests/helpers/api"; +import { mutateElement } from "./mutateElement"; // Unmount ReactDOM from root ReactDOM.unmountComponentAtNode(document.getElementById("root")!); @@ -19,7 +21,201 @@ const tab = " "; const mouse = new Pointer("mouse"); describe("textWysiwyg", () => { - describe("Test unbounded text", () => { + describe("start text editing", () => { + const { h } = window; + beforeEach(async () => { + await render(); + h.elements = []; + }); + + it("should prefer editing selected text element (non-bindable container present)", async () => { + const line = API.createElement({ + type: "line", + width: 100, + height: 0, + points: [ + [0, 0], + [100, 0], + ], + }); + const textSize = 20; + const text = API.createElement({ + type: "text", + text: "ola", + x: line.width / 2 - textSize / 2, + y: -textSize / 2, + width: textSize, + height: textSize, + }); + h.elements = [text, line]; + + API.setSelectedElements([text]); + + Keyboard.keyPress(KEYS.ENTER); + + expect(h.state.editingElement?.id).toBe(text.id); + expect( + (h.state.editingElement as ExcalidrawTextElement).containerId, + ).toBe(null); + }); + + it("should prefer editing selected text element (bindable container present)", async () => { + const container = API.createElement({ + type: "rectangle", + width: 100, + boundElements: [], + }); + const textSize = 20; + + const boundText = API.createElement({ + type: "text", + text: "ola", + x: container.width / 2 - textSize / 2, + y: container.height / 2 - textSize / 2, + width: textSize, + height: textSize, + containerId: container.id, + }); + + const boundText2 = API.createElement({ + type: "text", + text: "ola", + x: container.width / 2 - textSize / 2, + y: container.height / 2 - textSize / 2, + width: textSize, + height: textSize, + containerId: container.id, + }); + + h.elements = [container, boundText, boundText2]; + + mutateElement(container, { + boundElements: [{ type: "text", id: boundText.id }], + }); + + API.setSelectedElements([boundText2]); + + Keyboard.keyPress(KEYS.ENTER); + + expect(h.state.editingElement?.id).toBe(boundText2.id); + }); + + it("should not create bound text on ENTER if text exists at container center", () => { + const container = API.createElement({ + type: "rectangle", + width: 100, + }); + const textSize = 20; + const text = API.createElement({ + type: "text", + text: "ola", + x: container.width / 2 - textSize / 2, + y: container.height / 2 - textSize / 2, + width: textSize, + height: textSize, + containerId: container.id, + }); + + h.elements = [container, text]; + + API.setSelectedElements([container]); + + Keyboard.keyPress(KEYS.ENTER); + + expect(h.state.editingElement?.id).toBe(text.id); + }); + + it("should edit existing bound text on ENTER even if higher z-index unbound text exists at container center", () => { + const container = API.createElement({ + type: "rectangle", + width: 100, + boundElements: [], + }); + const textSize = 20; + + const boundText = API.createElement({ + type: "text", + text: "ola", + x: container.width / 2 - textSize / 2, + y: container.height / 2 - textSize / 2, + width: textSize, + height: textSize, + containerId: container.id, + }); + + const boundText2 = API.createElement({ + type: "text", + text: "ola", + x: container.width / 2 - textSize / 2, + y: container.height / 2 - textSize / 2, + width: textSize, + height: textSize, + containerId: container.id, + }); + + h.elements = [container, boundText, boundText2]; + + mutateElement(container, { + boundElements: [{ type: "text", id: boundText.id }], + }); + + API.setSelectedElements([container]); + + Keyboard.keyPress(KEYS.ENTER); + + expect(h.state.editingElement?.id).toBe(boundText.id); + }); + + it("should edit text under cursor when clicked with text tool", () => { + const text = API.createElement({ + type: "text", + text: "ola", + x: 60, + y: 0, + width: 100, + height: 100, + }); + + h.elements = [text]; + UI.clickTool("text"); + + mouse.clickAt(text.x + 50, text.y + 50); + + const editor = document.querySelector( + ".excalidraw-textEditorContainer > textarea", + ) as HTMLTextAreaElement; + + expect(editor).not.toBe(null); + expect(h.state.editingElement?.id).toBe(text.id); + expect(h.elements.length).toBe(1); + }); + + it("should edit text under cursor when double-clicked with selection tool", () => { + const text = API.createElement({ + type: "text", + text: "ola", + x: 60, + y: 0, + width: 100, + height: 100, + }); + + h.elements = [text]; + UI.clickTool("selection"); + + mouse.doubleClickAt(text.x + 50, text.y + 50); + + const editor = document.querySelector( + ".excalidraw-textEditorContainer > textarea", + ) as HTMLTextAreaElement; + + expect(editor).not.toBe(null); + expect(h.state.editingElement?.id).toBe(text.id); + expect(h.elements.length).toBe(1); + }); + }); + + describe("Test container-unbound text", () => { const { h } = window; let textarea: HTMLTextAreaElement; @@ -235,7 +431,7 @@ describe("textWysiwyg", () => { }); }); - describe("Test bounded text", () => { + describe("Test container-bound text", () => { let rectangle: any; const { h } = window; @@ -315,6 +511,39 @@ describe("textWysiwyg", () => { ]); }); + it("shouldn't bind to non-text-bindable containers", async () => { + const line = API.createElement({ + type: "line", + width: 100, + height: 0, + points: [ + [0, 0], + [100, 0], + ], + }); + h.elements = [line]; + + UI.clickTool("text"); + + mouse.clickAt(line.x + line.width / 2, line.y + line.height / 2); + + const editor = document.querySelector( + ".excalidraw-textEditorContainer > textarea", + ) as HTMLTextAreaElement; + + fireEvent.change(editor, { + target: { + value: "Hello World!", + }, + }); + fireEvent.keyDown(editor, { key: KEYS.ESCAPE }); + editor.dispatchEvent(new Event("input")); + + expect(line.boundElements).toBe(null); + expect(h.elements[1].type).toBe("text"); + expect((h.elements[1] as ExcalidrawTextElement).containerId).toBe(null); + }); + it("should update font family correctly on undo/redo by selecting bounded text when font family was updated", async () => { expect(h.elements.length).toBe(1); diff --git a/src/element/typeChecks.ts b/src/element/typeChecks.ts index ea5b11ac..8f5627bf 100644 --- a/src/element/typeChecks.ts +++ b/src/element/typeChecks.ts @@ -8,6 +8,7 @@ import { InitializedExcalidrawImageElement, ExcalidrawImageElement, ExcalidrawTextElementWithContainer, + ExcalidrawTextContainer, } from "./types"; export const isGenericElement = ( @@ -91,7 +92,9 @@ export const isBindableElement = ( ); }; -export const isTextBindableContainer = (element: ExcalidrawElement | null) => { +export const isTextBindableContainer = ( + element: ExcalidrawElement | null, +): element is ExcalidrawTextContainer => { return ( element != null && (element.type === "rectangle" || diff --git a/src/element/types.ts b/src/element/types.ts index ccc2c614..f7177e44 100644 --- a/src/element/types.ts +++ b/src/element/types.ts @@ -135,8 +135,14 @@ export type ExcalidrawBindableElement = | ExcalidrawTextElement | ExcalidrawImageElement; +export type ExcalidrawTextContainer = + | ExcalidrawRectangleElement + | ExcalidrawDiamondElement + | ExcalidrawEllipseElement + | ExcalidrawImageElement; + export type ExcalidrawTextElementWithContainer = { - containerId: ExcalidrawGenericElement["id"]; + containerId: ExcalidrawTextContainer["id"]; } & ExcalidrawTextElement; export type PointBinding = { diff --git a/src/groups.ts b/src/groups.ts index 4bbf2bc4..e45a59fa 100644 --- a/src/groups.ts +++ b/src/groups.ts @@ -1,13 +1,7 @@ -import { - GroupId, - ExcalidrawElement, - NonDeleted, - ExcalidrawTextElementWithContainer, -} from "./element/types"; +import { GroupId, ExcalidrawElement, NonDeleted } from "./element/types"; import { AppState } from "./types"; import { getSelectedElements } from "./scene"; -import { getBoundTextElementId } from "./element/textElement"; -import Scene from "./scene/Scene"; +import { getBoundTextElement } from "./element/textElement"; export const selectGroup = ( groupId: GroupId, @@ -182,13 +176,10 @@ export const getMaximumGroups = ( const currentGroupMembers = groups.get(groupId) || []; - // Include bounded text if present when grouping - const boundTextElementId = getBoundTextElementId(element); - if (boundTextElementId) { - const textElement = Scene.getScene(element)!.getElement( - boundTextElementId, - ) as ExcalidrawTextElementWithContainer; - currentGroupMembers.push(textElement); + // Include bound text if present when grouping + const boundTextElement = getBoundTextElement(element); + if (boundTextElement) { + currentGroupMembers.push(boundTextElement); } groups.set(groupId, [...currentGroupMembers, element]); }); diff --git a/src/scene/comparisons.ts b/src/scene/comparisons.ts index 24b98d40..57b3672d 100644 --- a/src/scene/comparisons.ts +++ b/src/scene/comparisons.ts @@ -1,9 +1,11 @@ import { ExcalidrawElement, + ExcalidrawTextContainer, NonDeletedExcalidrawElement, } from "../element/types"; import { getElementAbsoluteCoords } from "../element"; +import { isTextBindableContainer } from "../element/typeChecks"; export const hasBackground = (type: string) => type === "rectangle" || @@ -72,11 +74,11 @@ export const getElementsAtPosition = ( ); }; -export const getElementContainingPosition = ( +export const getTextBindableContainerAtPosition = ( elements: readonly ExcalidrawElement[], x: number, y: number, -) => { +): ExcalidrawTextContainer | null => { let hitElement = null; // We need to to hit testing from front (end of the array) to back (beginning of the array) for (let index = elements.length - 1; index >= 0; --index) { @@ -89,5 +91,5 @@ export const getElementContainingPosition = ( break; } } - return hitElement; + return isTextBindableContainer(hitElement) ? hitElement : null; }; diff --git a/src/scene/index.ts b/src/scene/index.ts index 0b995664..3e1d50b2 100644 --- a/src/scene/index.ts +++ b/src/scene/index.ts @@ -14,7 +14,7 @@ export { canHaveArrowheads, canChangeSharpness, getElementAtPosition, - getElementContainingPosition, + getTextBindableContainerAtPosition, hasText, getElementsAtPosition, } from "./comparisons"; diff --git a/src/tests/binding.test.tsx b/src/tests/binding.test.tsx index 656d3bea..9986f762 100644 --- a/src/tests/binding.test.tsx +++ b/src/tests/binding.test.tsx @@ -152,6 +152,7 @@ describe("element binding", () => { UI.clickTool("text"); mouse.clickAt(text.x + 50, text.y + 50); + const editor = document.querySelector( ".excalidraw-textEditorContainer > textarea", ) as HTMLTextAreaElement; diff --git a/src/tests/data/__snapshots__/restore.test.ts.snap b/src/tests/data/__snapshots__/restore.test.ts.snap index a63c16ee..870b1216 100644 --- a/src/tests/data/__snapshots__/restore.test.ts.snap +++ b/src/tests/data/__snapshots__/restore.test.ts.snap @@ -9,7 +9,7 @@ Object { "endBinding": null, "fillStyle": "hachure", "groupIds": Array [], - "height": 0, + "height": 100, "id": "id-arrow01", "isDeleted": false, "lastCommittedPoint": null, @@ -21,8 +21,8 @@ Object { 0, ], Array [ - 0, - 0, + 100, + 100, ], ], "roughness": 1, @@ -37,7 +37,7 @@ Object { "updated": 1, "version": 1, "versionNonce": 0, - "width": 0, + "width": 100, "x": 0, "y": 0, } @@ -180,7 +180,7 @@ Object { "endBinding": null, "fillStyle": "hachure", "groupIds": Array [], - "height": 0, + "height": 100, "id": "id-line01", "isDeleted": false, "lastCommittedPoint": null, @@ -192,8 +192,8 @@ Object { 0, ], Array [ - 0, - 0, + 100, + 100, ], ], "roughness": 1, @@ -208,7 +208,7 @@ Object { "updated": 1, "version": 1, "versionNonce": 0, - "width": 0, + "width": 100, "x": 0, "y": 0, } @@ -223,7 +223,7 @@ Object { "endBinding": null, "fillStyle": "hachure", "groupIds": Array [], - "height": 0, + "height": 100, "id": "id-draw01", "isDeleted": false, "lastCommittedPoint": null, @@ -235,8 +235,8 @@ Object { 0, ], Array [ - 0, - 0, + 100, + 100, ], ], "roughness": 1, @@ -251,7 +251,7 @@ Object { "updated": 1, "version": 1, "versionNonce": 0, - "width": 0, + "width": 100, "x": 0, "y": 0, } diff --git a/src/tests/helpers/api.ts b/src/tests/helpers/api.ts index 25196e09..9c119574 100644 --- a/src/tests/helpers/api.ts +++ b/src/tests/helpers/api.ts @@ -14,6 +14,7 @@ import util from "util"; import path from "path"; import { getMimeType } from "../../data/blob"; import { newFreeDrawElement } from "../../element/newElement"; +import { Point } from "../../types"; const readFile = util.promisify(fs.readFile); @@ -98,6 +99,7 @@ export class API { containerId?: T extends "text" ? ExcalidrawTextElement["containerId"] : never; + points?: T extends "arrow" | "line" ? readonly Point[] : never; }): T extends "arrow" | "line" ? ExcalidrawLinearElement : T extends "freedraw" @@ -158,10 +160,13 @@ export class API { case "arrow": case "line": element = newLinearElement({ + ...base, + width, + height, type: type as "arrow" | "line", startArrowhead: null, endArrowhead: null, - ...base, + points: rest.points ?? [], }); break; }