diff --git a/src/components/App.tsx b/src/components/App.tsx index 1561b382..9b7dd422 100644 --- a/src/components/App.tsx +++ b/src/components/App.tsx @@ -133,7 +133,6 @@ import { isInitializedImageElement, isLinearElement, isLinearElementType, - isTextBindableContainer, } from "../element/typeChecks"; import { ExcalidrawBindableElement, @@ -175,7 +174,6 @@ import { renderScene } from "../renderer/renderScene"; import { invalidateShapeForElement } from "../renderer/renderElement"; import { calculateScrollCenter, - getTextBindableContainerAtPosition, getElementsAtPosition, getElementsWithinSelection, getNormalizedZoom, @@ -255,6 +253,7 @@ import { getApproxMinLineWidth, getBoundTextElement, getContainerDims, + getTextBindableContainerAtPosition, isValidTextContainer, } from "../element/textElement"; import { isHittingElementNotConsideringBoundingBox } from "../element/collision"; @@ -1990,10 +1989,14 @@ class App extends React.Component { isTextElement(selectedElement) || isValidTextContainer(selectedElement) ) { + let container; + if (!isTextElement(selectedElement)) { + container = selectedElement as ExcalidrawTextContainer; + } this.startTextEditing({ sceneX: selectedElement.x + selectedElement.width / 2, sceneY: selectedElement.y + selectedElement.height / 2, - shouldBind: true, + container, }); event.preventDefault(); return; @@ -2317,7 +2320,6 @@ class App extends React.Component { const element = this.getElementAtPosition(x, y, { includeBoundTextElement: true, }); - if (element && isTextElement(element) && !element.isDeleted) { return element; } @@ -2394,29 +2396,31 @@ class App extends React.Component { private startTextEditing = ({ sceneX, sceneY, - shouldBind, insertAtParentCenter = true, + container, }: { /** X position to insert text at */ sceneX: number; /** Y position to insert text at */ sceneY: number; - shouldBind: boolean; /** whether to attempt to insert at element center if applicable */ insertAtParentCenter?: boolean; + container?: ExcalidrawTextContainer | null; }) => { + let shouldBindToContainer = false; + let parentCenterPosition = insertAtParentCenter && this.getTextWysiwygSnappedToCenterPosition( sceneX, sceneY, this.state, - this.canvas, - window.devicePixelRatio, + container, ); - + if (container && parentCenterPosition) { + shouldBindToContainer = true; + } let existingTextElement: NonDeleted | null = null; - let container: ExcalidrawTextContainer | null = null; const selectedElements = getSelectedElements( this.scene.getNonDeletedElements(), @@ -2426,7 +2430,7 @@ class App extends React.Component { if (selectedElements.length === 1) { if (isTextElement(selectedElements[0])) { existingTextElement = selectedElements[0]; - } else if (isTextBindableContainer(selectedElements[0], false)) { + } else if (container) { existingTextElement = getBoundTextElement(selectedElements[0]); } else { existingTextElement = this.getTextElementAtPosition(sceneX, sceneY); @@ -2435,26 +2439,7 @@ class App extends React.Component { 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 - .getNonDeletedElements() - .filter( - (ele) => - isTextBindableContainer(ele, false) && !getBoundTextElement(ele), - ), - sceneX, - sceneY, - ); - } - - if (!existingTextElement && container) { + if (!existingTextElement && shouldBindToContainer && container) { const fontString = { fontSize: this.state.currentItemFontSize, fontFamily: this.state.currentItemFontFamily, @@ -2472,12 +2457,10 @@ class App extends React.Component { sceneX, sceneY, this.state, - this.canvas, - window.devicePixelRatio, + container, ); } } - const element = existingTextElement ? existingTextElement : newTextElement({ @@ -2504,7 +2487,7 @@ class App extends React.Component { verticalAlign: parentCenterPosition ? VERTICAL_ALIGN.MIDDLE : DEFAULT_VERTICAL_ALIGN, - containerId: container?.id ?? undefined, + containerId: shouldBindToContainer ? container?.id : undefined, groupIds: container?.groupIds ?? [], locked: false, }); @@ -2512,10 +2495,15 @@ class App extends React.Component { this.setState({ editingElement: element }); if (!existingTextElement) { - this.scene.replaceAllElements([ - ...this.scene.getElementsIncludingDeleted(), - element, - ]); + if (container && shouldBindToContainer) { + const containerIndex = this.scene.getElementIndex(container.id); + this.scene.insertElementAtIndex(element, containerIndex + 1); + } else { + this.scene.replaceAllElements([ + ...this.scene.getElementsIncludingDeleted(), + element, + ]); + } // case: creating new text not centered to parent element → offset Y // so that the text is centered to cursor position @@ -2603,22 +2591,23 @@ class App extends React.Component { resetCursor(this.canvas); if (!event[KEYS.CTRL_OR_CMD] && !this.state.viewModeEnabled) { - const selectedElements = getSelectedElements( + const container = getTextBindableContainerAtPosition( this.scene.getNonDeletedElements(), this.state, + sceneX, + sceneY, ); - if (selectedElements.length === 1) { - const selectedElement = selectedElements[0]; - if (hasBoundTextElement(selectedElement)) { - sceneX = selectedElement.x + selectedElement.width / 2; - sceneY = selectedElement.y + selectedElement.height / 2; + if (container) { + if (hasBoundTextElement(container)) { + sceneX = container.x + container.width / 2; + sceneY = container.y + container.height / 2; } } this.startTextEditing({ sceneX, sceneY, - shouldBind: false, insertAtParentCenter: !event.altKey, + container, }); } }; @@ -3911,15 +3900,23 @@ class App extends React.Component { includeBoundTextElement: true, }); + let container = getTextBindableContainerAtPosition( + this.scene.getNonDeletedElements(), + this.state, + sceneX, + sceneY, + ); + if (hasBoundTextElement(element)) { + container = element as ExcalidrawTextContainer; sceneX = element.x + element.width / 2; sceneY = element.y + element.height / 2; } this.startTextEditing({ sceneX, sceneY, - shouldBind: false, insertAtParentCenter: !event.altKey, + container, }); resetCursor(this.canvas); @@ -6171,21 +6168,11 @@ class App extends React.Component { x: number, y: number, appState: AppState, - canvas: HTMLCanvasElement | null, - scale: number, + container?: ExcalidrawTextContainer | null, ) { - const elementClickedInside = getTextBindableContainerAtPosition( - this.scene - .getElementsIncludingDeleted() - .filter((element) => !isTextElement(element)), - x, - y, - ); - if (elementClickedInside) { - const elementCenterX = - elementClickedInside.x + elementClickedInside.width / 2; - const elementCenterY = - elementClickedInside.y + elementClickedInside.height / 2; + if (container) { + const elementCenterX = container.x + container.width / 2; + const elementCenterY = container.y + container.height / 2; const distanceToCenter = Math.hypot( x - elementCenterX, y - elementCenterY, diff --git a/src/element/textElement.ts b/src/element/textElement.ts index 4c1dcb0f..b45127a3 100644 --- a/src/element/textElement.ts +++ b/src/element/textElement.ts @@ -1,6 +1,7 @@ import { getFontString, arrayToMap, isTestEnv } from "../utils"; import { ExcalidrawElement, + ExcalidrawTextContainer, ExcalidrawTextElement, ExcalidrawTextElementWithContainer, FontString, @@ -12,6 +13,10 @@ import { MaybeTransformHandleType } from "./transformHandles"; import Scene from "../scene/Scene"; import { isTextElement } from "."; import { getMaxContainerHeight, getMaxContainerWidth } from "./newElement"; +import { isTextBindableContainer } from "./typeChecks"; +import { getElementAbsoluteCoords } from "../element"; +import { AppState } from "../types"; +import { getSelectedElements } from "../scene"; import { isImageElement } from "./typeChecks"; export const redrawTextBoundingBox = ( @@ -492,6 +497,32 @@ export const getContainerDims = (element: ExcalidrawElement) => { return { width: element.width, height: element.height }; }; +export const getTextBindableContainerAtPosition = ( + elements: readonly ExcalidrawElement[], + appState: AppState, + x: number, + y: number, +): ExcalidrawTextContainer | null => { + const selectedElements = getSelectedElements(elements, appState); + if (selectedElements.length === 1) { + return selectedElements[0] as ExcalidrawTextContainer; + } + 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) { + if (elements[index].isDeleted) { + continue; + } + const [x1, y1, x2, y2] = getElementAbsoluteCoords(elements[index]); + if (x1 < x && x < x2 && y1 < y && y < y2) { + hitElement = elements[index]; + break; + } + } + + return isTextBindableContainer(hitElement, false) ? hitElement : null; +}; + export const isValidTextContainer = (element: ExcalidrawElement) => { return ( element.type === "rectangle" || diff --git a/src/element/textWysiwyg.test.tsx b/src/element/textWysiwyg.test.tsx index 033dff17..c900ba62 100644 --- a/src/element/textWysiwyg.test.tsx +++ b/src/element/textWysiwyg.test.tsx @@ -500,7 +500,7 @@ describe("textWysiwyg", () => { }); }); - it("should bind text to container when double clicked on center", async () => { + it("should bind text to container when double clicked on center of filled container", async () => { expect(h.elements.length).toBe(1); expect(h.elements[0].id).toBe(rectangle.id); @@ -527,6 +527,40 @@ describe("textWysiwyg", () => { ]); }); + it("should bind text to container when double clicked on center of transparent container", async () => { + const rectangle = API.createElement({ + type: "rectangle", + x: 10, + y: 20, + width: 90, + height: 75, + backgroundColor: "transparent", + }); + h.elements = [rectangle]; + + mouse.doubleClickAt( + rectangle.x + rectangle.width / 2, + rectangle.y + rectangle.height / 2, + ); + expect(h.elements.length).toBe(2); + + const text = h.elements[1] as ExcalidrawTextElementWithContainer; + expect(text.type).toBe("text"); + expect(text.containerId).toBe(rectangle.id); + mouse.down(); + const editor = document.querySelector( + ".excalidraw-textEditorContainer > textarea", + ) as HTMLTextAreaElement; + + fireEvent.change(editor, { target: { value: "Hello World!" } }); + + await new Promise((r) => setTimeout(r, 0)); + editor.blur(); + expect(rectangle.boundElements).toStrictEqual([ + { id: text.id, type: "text" }, + ]); + }); + it("should bind text to container when clicked on container and enter pressed", async () => { expect(h.elements.length).toBe(1); expect(h.elements[0].id).toBe(rectangle.id); @@ -825,9 +859,9 @@ describe("textWysiwyg", () => { expect(h.elements.length).toBe(2); // Bind first text - let text = h.elements[1] as ExcalidrawTextElementWithContainer; + const text = h.elements[1] as ExcalidrawTextElementWithContainer; expect(text.containerId).toBe(rectangle.id); - let editor = document.querySelector( + const editor = document.querySelector( ".excalidraw-textEditorContainer > textarea", ) as HTMLTextAreaElement; await new Promise((r) => setTimeout(r, 0)); @@ -837,25 +871,14 @@ describe("textWysiwyg", () => { { id: text.id, type: "text" }, ]); - // Attempt to bind another text - UI.clickTool("text"); - mouse.clickAt( - rectangle.x + rectangle.width / 2, - rectangle.y + rectangle.height / 2, - ); - mouse.down(); - expect(h.elements.length).toBe(3); - text = h.elements[2] as ExcalidrawTextElementWithContainer; - editor = document.querySelector( - ".excalidraw-textEditorContainer > textarea", - ) as HTMLTextAreaElement; - await new Promise((r) => setTimeout(r, 0)); - fireEvent.change(editor, { target: { value: "Whats up?" } }); - editor.blur(); + mouse.select(rectangle); + Keyboard.keyPress(KEYS.ENTER); + expect(h.elements.length).toBe(2); + expect(rectangle.boundElements).toStrictEqual([ { id: h.elements[1].id, type: "text" }, ]); - expect(text.containerId).toBe(null); + expect(text.containerId).toBe(rectangle.id); }); it("should respect text alignment when resizing", async () => { @@ -1045,5 +1068,36 @@ describe("textWysiwyg", () => { `"Wikipedia is hosted by the Wikimedia Foundation, a non-profit organization that also hosts a range of other projects.Hello this text should get merged with the existing one"`, ); }); + + it("should always bind to selected container and insert it in correct position", async () => { + const rectangle2 = UI.createElement("rectangle", { + x: 5, + y: 10, + width: 120, + height: 100, + }); + + API.setSelectedElements([rectangle]); + Keyboard.keyPress(KEYS.ENTER); + + expect(h.elements.length).toBe(3); + expect(h.elements[1].type).toBe("text"); + const text = h.elements[1] as ExcalidrawTextElementWithContainer; + expect(text.type).toBe("text"); + expect(text.containerId).toBe(rectangle.id); + mouse.down(); + const editor = document.querySelector( + ".excalidraw-textEditorContainer > textarea", + ) as HTMLTextAreaElement; + + fireEvent.change(editor, { target: { value: "Hello World!" } }); + + await new Promise((r) => setTimeout(r, 0)); + editor.blur(); + expect(rectangle2.boundElements).toBeNull(); + expect(rectangle.boundElements).toStrictEqual([ + { id: text.id, type: "text" }, + ]); + }); }); }); diff --git a/src/scene/Scene.ts b/src/scene/Scene.ts index 8b5b84eb..e9fc98f8 100644 --- a/src/scene/Scene.ts +++ b/src/scene/Scene.ts @@ -150,6 +150,24 @@ class Scene { // (I guess?) this.callbacks.clear(); } + + insertElementAtIndex(element: ExcalidrawElement, index: number) { + if (!Number.isFinite(index) || index < 0) { + throw new Error( + "insertElementAtIndex can only be called with index >= 0", + ); + } + const nextElements = [ + ...this.elements.slice(0, index), + element, + ...this.elements.slice(index), + ]; + this.replaceAllElements(nextElements); + } + + getElementIndex(elementId: string) { + return this.elements.findIndex((element) => element.id === elementId); + } } export default Scene; diff --git a/src/scene/comparisons.ts b/src/scene/comparisons.ts index d94be0a6..7b26ac4f 100644 --- a/src/scene/comparisons.ts +++ b/src/scene/comparisons.ts @@ -1,11 +1,4 @@ -import { - ExcalidrawElement, - ExcalidrawTextContainer, - NonDeletedExcalidrawElement, -} from "../element/types"; - -import { getElementAbsoluteCoords } from "../element"; -import { isTextBindableContainer } from "../element/typeChecks"; +import { NonDeletedExcalidrawElement } from "../element/types"; export const hasBackground = (type: string) => type === "rectangle" || @@ -73,23 +66,3 @@ export const getElementsAtPosition = ( (element) => !element.isDeleted && isAtPositionFn(element), ); }; - -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) { - if (elements[index].isDeleted) { - continue; - } - const [x1, y1, x2, y2] = getElementAbsoluteCoords(elements[index]); - if (x1 < x && x < x2 && y1 < y && y < y2) { - hitElement = elements[index]; - break; - } - } - return isTextBindableContainer(hitElement, false) ? hitElement : null; -}; diff --git a/src/scene/index.ts b/src/scene/index.ts index 3e1d50b2..62205077 100644 --- a/src/scene/index.ts +++ b/src/scene/index.ts @@ -14,7 +14,6 @@ export { canHaveArrowheads, canChangeSharpness, getElementAtPosition, - getTextBindableContainerAtPosition, hasText, getElementsAtPosition, } from "./comparisons"; diff --git a/src/tests/elementLocking.test.tsx b/src/tests/elementLocking.test.tsx index f23e2d21..6b98aa1e 100644 --- a/src/tests/elementLocking.test.tsx +++ b/src/tests/elementLocking.test.tsx @@ -232,7 +232,7 @@ describe("element locking", () => { API.setSelectedElements([container]); Keyboard.keyPress(KEYS.ENTER); expect(h.state.editingElement?.id).not.toBe(text.id); - expect(h.state.editingElement?.id).toBe(h.elements[2].id); + expect(h.state.editingElement?.id).toBe(h.elements[1].id); }); it("should ignore locked text under cursor when clicked with text tool", () => {