From eacee9a158cbe042e35df41435fba674718a33d6 Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Tue, 28 Feb 2023 13:31:55 +0530 Subject: [PATCH] cleanup getMaxContainerHeight and getMaxContainerWidth and add specs --- src/element/textElement.test.ts | 53 ++++++++++++++++++++++++-- src/element/textElement.ts | 45 ++++++++-------------- src/element/textWysiwyg.tsx | 5 ++- src/tests/linearElementEditor.test.tsx | 36 ++++++++--------- 4 files changed, 87 insertions(+), 52 deletions(-) diff --git a/src/element/textElement.test.ts b/src/element/textElement.test.ts index 87219b298..00f1d5a31 100644 --- a/src/element/textElement.test.ts +++ b/src/element/textElement.test.ts @@ -7,7 +7,7 @@ import { getMaxContainerHeight, wrapText, } from "./textElement"; -import { FontString } from "./types"; +import { ExcalidrawTextElementWithContainer, FontString } from "./types"; describe("Test wrapText", () => { const font = "20px Cascadia, width: Segoe UI Emoji" as FontString; @@ -265,27 +265,72 @@ describe("Test measureText", () => { const container = API.createElement({ type: "diamond", ...params }); expect(getMaxContainerWidth(container)).toBe(79); }); + + it("should return max width when container is arrow", () => { + const container = API.createElement({ + type: "arrow", + ...params, + }); + expect(getMaxContainerWidth(container)).toBe(220); + }); }); describe("Test getMaxContainerHeight", () => { const params = { width: 178, height: 194, + id: "container-id", }; + const boundTextElement = API.createElement({ + type: "text", + id: "text-id", + x: 560.51171875, + y: 202.033203125, + width: 154, + height: 175, + fontSize: 20, + fontFamily: 1, + text: "Excalidraw is a\nvirtual \nopensource \nwhiteboard for \nsketching \nhand-drawn like\ndiagrams", + textAlign: "center", + verticalAlign: "middle", + containerId: params.id, + }) as ExcalidrawTextElementWithContainer; + it("should return max height when container is rectangle", () => { const container = API.createElement({ type: "rectangle", ...params }); - expect(getMaxContainerHeight(container)).toBe(184); + expect(getMaxContainerHeight(container, boundTextElement)).toBe(184); }); it("should return max height when container is ellipse", () => { const container = API.createElement({ type: "ellipse", ...params }); - expect(getMaxContainerHeight(container)).toBe(127); + expect(getMaxContainerHeight(container, boundTextElement)).toBe(127); }); it("should return max height when container is diamond", () => { const container = API.createElement({ type: "diamond", ...params }); - expect(getMaxContainerHeight(container)).toBe(87); + expect(getMaxContainerHeight(container, boundTextElement)).toBe(87); + }); + + it("should return max height when container is arrow", () => { + const container = API.createElement({ + type: "arrow", + ...params, + }); + expect(getMaxContainerHeight(container, boundTextElement)).toBe(194); + }); + + it("should return max height when container is arrow and height is less than threshold", () => { + const container = API.createElement({ + type: "arrow", + ...params, + height: 70, + boundElements: [{ type: "text", id: "text-id" }], + }); + + expect(getMaxContainerHeight(container, boundTextElement)).toBe( + boundTextElement.height, + ); }); }); }); diff --git a/src/element/textElement.ts b/src/element/textElement.ts index 191f50114..71a3ce246 100644 --- a/src/element/textElement.ts +++ b/src/element/textElement.ts @@ -71,7 +71,10 @@ export const redrawTextBoundingBox = ( if (container) { const containerDims = getContainerDims(container); - const maxContainerHeight = getMaxContainerHeight(container); + const maxContainerHeight = getMaxContainerHeight( + container, + textElement as ExcalidrawTextElementWithContainer, + ); let nextHeight = containerDims.height; if (metrics.height > maxContainerHeight) { @@ -162,7 +165,10 @@ export const handleBindTextResize = ( let nextWidth = textElement.width; const containerDims = getContainerDims(container); const maxWidth = getMaxContainerWidth(container); - const maxHeight = getMaxContainerHeight(container); + const maxHeight = getMaxContainerHeight( + container, + textElement as ExcalidrawTextElementWithContainer, + ); let containerHeight = containerDims.height; if (transformHandleType !== "n" && transformHandleType !== "s") { if (text) { @@ -221,7 +227,7 @@ export const computeBoundTextPosition = ( boundTextElement: ExcalidrawTextElementWithContainer, ) => { const containerCoords = getContainerCoords(container); - const maxContainerHeight = getMaxContainerHeight(container); + const maxContainerHeight = getMaxContainerHeight(container, boundTextElement); const maxContainerWidth = getMaxContainerWidth(container); let x; @@ -638,18 +644,6 @@ export const getBoundTextElementOffset = ( return BOUND_TEXT_PADDING; }; -export const getBoundTextElementPosition = ( - container: ExcalidrawElement, - boundTextElement: ExcalidrawTextElementWithContainer, -) => { - if (isArrowElement(container)) { - return LinearElementEditor.getBoundTextElementPosition( - container, - boundTextElement, - ); - } -}; - export const shouldAllowVerticalAlign = ( selectedElements: NonDeletedExcalidrawElement[], ) => { @@ -741,15 +735,7 @@ export const computeContainerHeightForBoundText = ( export const getMaxContainerWidth = (container: ExcalidrawElement) => { const width = getContainerDims(container).width; if (isArrowElement(container)) { - const containerWidth = width - BOUND_TEXT_PADDING * 8 * 2; - if (containerWidth <= 0) { - const boundText = getBoundTextElement(container); - if (boundText) { - return boundText.width; - } - return BOUND_TEXT_PADDING * 8 * 2; - } - return containerWidth; + return width - BOUND_TEXT_PADDING * 8 * 2; } if (container.type === "ellipse") { @@ -766,16 +752,15 @@ export const getMaxContainerWidth = (container: ExcalidrawElement) => { return width - BOUND_TEXT_PADDING * 2; }; -export const getMaxContainerHeight = (container: ExcalidrawElement) => { +export const getMaxContainerHeight = ( + container: ExcalidrawElement, + boundTextElement: ExcalidrawTextElementWithContainer, +) => { const height = getContainerDims(container).height; if (isArrowElement(container)) { const containerHeight = height - BOUND_TEXT_PADDING * 8 * 2; if (containerHeight <= 0) { - const boundText = getBoundTextElement(container); - if (boundText) { - return boundText.height; - } - return BOUND_TEXT_PADDING * 8 * 2; + return boundTextElement.height; } return height; } diff --git a/src/element/textWysiwyg.tsx b/src/element/textWysiwyg.tsx index 4fd7633e8..0cd9ae674 100644 --- a/src/element/textWysiwyg.tsx +++ b/src/element/textWysiwyg.tsx @@ -204,7 +204,10 @@ export const textWysiwyg = ({ } maxWidth = getMaxContainerWidth(container); - maxHeight = getMaxContainerHeight(container); + maxHeight = getMaxContainerHeight( + container, + updatedTextElement as ExcalidrawTextElementWithContainer, + ); // autogrow container height if text exceeds if (!isArrowElement(container) && textElementHeight > maxHeight) { diff --git a/src/tests/linearElementEditor.test.tsx b/src/tests/linearElementEditor.test.tsx index a606fb384..f5660bbcb 100644 --- a/src/tests/linearElementEditor.test.tsx +++ b/src/tests/linearElementEditor.test.tsx @@ -17,11 +17,7 @@ import { KEYS } from "../keys"; import { LinearElementEditor } from "../element/linearElementEditor"; import { queryByTestId, queryByText } from "@testing-library/react"; import { resize, rotate } from "./utils"; -import { - getBoundTextElementPosition, - wrapText, - getMaxContainerWidth, -} from "../element/textElement"; +import { wrapText, getMaxContainerWidth } from "../element/textElement"; import * as textElementUtils from "../element/textElement"; import { ROUNDNESS } from "../constants"; @@ -937,8 +933,9 @@ describe("Test Linear Elements", () => { expect(container.angle).toBe(0); expect(textElement.angle).toBe(0); - expect(getBoundTextElementPosition(arrow, textElement)) - .toMatchInlineSnapshot(` + expect( + LinearElementEditor.getBoundTextElementPosition(arrow, textElement), + ).toMatchInlineSnapshot(` Object { "x": 75, "y": 60, @@ -964,8 +961,9 @@ describe("Test Linear Elements", () => { rotate(container, -35, 55); expect(container.angle).toMatchInlineSnapshot(`1.3988061968364685`); expect(textElement.angle).toBe(0); - expect(getBoundTextElementPosition(container, textElement)) - .toMatchInlineSnapshot(` + expect( + LinearElementEditor.getBoundTextElementPosition(container, textElement), + ).toMatchInlineSnapshot(` Object { "x": 21.73926141863671, "y": 73.31003398390868, @@ -1002,8 +1000,9 @@ describe("Test Linear Elements", () => { ); expect(container.width).toBe(70); expect(container.height).toBe(50); - expect(getBoundTextElementPosition(container, textElement)) - .toMatchInlineSnapshot(` + expect( + LinearElementEditor.getBoundTextElementPosition(container, textElement), + ).toMatchInlineSnapshot(` Object { "x": 75, "y": 60, @@ -1036,8 +1035,9 @@ describe("Test Linear Elements", () => { } `); - expect(getBoundTextElementPosition(container, textElement)) - .toMatchInlineSnapshot(` + expect( + LinearElementEditor.getBoundTextElementPosition(container, textElement), + ).toMatchInlineSnapshot(` Object { "x": 272, "y": 46, @@ -1070,8 +1070,9 @@ describe("Test Linear Elements", () => { arrow, ); expect(container.width).toBe(40); - expect(getBoundTextElementPosition(container, textElement)) - .toMatchInlineSnapshot(` + expect( + LinearElementEditor.getBoundTextElementPosition(container, textElement), + ).toMatchInlineSnapshot(` Object { "x": 25, "y": 10, @@ -1095,8 +1096,9 @@ describe("Test Linear Elements", () => { } `); - expect(getBoundTextElementPosition(container, textElement)) - .toMatchInlineSnapshot(` + expect( + LinearElementEditor.getBoundTextElementPosition(container, textElement), + ).toMatchInlineSnapshot(` Object { "x": 75, "y": -4,