From ae7ff761268989131d4e5ff45759d838418dde59 Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Tue, 6 Jun 2023 14:36:18 +0530 Subject: [PATCH] fix: cleanup textWysiwyg and getAdjustedDimensions (#6520) * fix: cleanup textWysiwyg and getAdjustedDimensions * fix lint * fix test --- src/element/newElement.ts | 28 +--------------- src/element/textWysiwyg.test.tsx | 48 +++++++++++++++++---------- src/element/textWysiwyg.tsx | 57 +++++--------------------------- 3 files changed, 41 insertions(+), 92 deletions(-) diff --git a/src/element/newElement.ts b/src/element/newElement.ts index 4922a5b4..7a19f6b3 100644 --- a/src/element/newElement.ts +++ b/src/element/newElement.ts @@ -20,15 +20,13 @@ import { isTestEnv, } from "../utils"; import { randomInteger, randomId } from "../random"; -import { bumpVersion, mutateElement, newElementWith } from "./mutateElement"; +import { bumpVersion, newElementWith } from "./mutateElement"; import { getNewGroupIdsForDuplication } from "../groups"; import { AppState } from "../types"; import { getElementAbsoluteCoords } from "."; import { adjustXYWithRotation } from "../math"; import { getResizedElementAbsoluteCoords } from "./bounds"; import { - getBoundTextElementOffset, - getContainerDims, getContainerElement, measureText, normalizeText, @@ -44,7 +42,6 @@ import { DEFAULT_VERTICAL_ALIGN, VERTICAL_ALIGN, } from "../constants"; -import { isArrowElement } from "./typeChecks"; import { MarkOptional, Merge, Mutable } from "../utility-types"; type ElementConstructorOpts = MarkOptional< @@ -211,8 +208,6 @@ const getAdjustedDimensions = ( height: number; baseline: number; } => { - const container = getContainerElement(element); - const { width: nextWidth, height: nextHeight, @@ -268,27 +263,6 @@ const getAdjustedDimensions = ( ); } - // make sure container dimensions are set properly when - // text editor overflows beyond viewport dimensions - if (container) { - const boundTextElementPadding = getBoundTextElementOffset(element); - - const containerDims = getContainerDims(container); - let height = containerDims.height; - let width = containerDims.width; - if (nextHeight > height - boundTextElementPadding * 2) { - height = nextHeight + boundTextElementPadding * 2; - } - if (nextWidth > width - boundTextElementPadding * 2) { - width = nextWidth + boundTextElementPadding * 2; - } - if ( - !isArrowElement(container) && - (height !== containerDims.height || width !== containerDims.width) - ) { - mutateElement(container, { height, width }); - } - } return { width: nextWidth, height: nextHeight, diff --git a/src/element/textWysiwyg.test.tsx b/src/element/textWysiwyg.test.tsx index 294c1326..0a5bbf32 100644 --- a/src/element/textWysiwyg.test.tsx +++ b/src/element/textWysiwyg.test.tsx @@ -1213,32 +1213,46 @@ describe("textWysiwyg", () => { }); it("should restore original container height and clear cache once text is unbind", async () => { - const originalRectHeight = rectangle.height; - expect(rectangle.height).toBe(originalRectHeight); - - Keyboard.keyPress(KEYS.ENTER); - const editor = document.querySelector( - ".excalidraw-textEditorContainer > textarea", - ) as HTMLTextAreaElement; - await new Promise((r) => setTimeout(r, 0)); - - fireEvent.change(editor, { - target: { value: "Online whiteboard collaboration made easy" }, + const container = API.createElement({ + type: "rectangle", + height: 75, + width: 90, }); - editor.blur(); - expect(rectangle.height).toBe(185); - mouse.select(rectangle); + const originalRectHeight = container.height; + expect(container.height).toBe(originalRectHeight); + + const text = API.createElement({ + type: "text", + text: "Online whiteboard collaboration made easy", + }); + h.elements = [container, text]; + API.setSelectedElements([container, text]); + fireEvent.contextMenu(GlobalTestState.canvas, { button: 2, clientX: 20, clientY: 30, }); - const contextMenu = document.querySelector(".context-menu"); + let contextMenu = document.querySelector(".context-menu"); + + fireEvent.click( + queryByText(contextMenu as HTMLElement, "Bind text to the container")!, + ); + + expect((h.elements[1] as ExcalidrawTextElementWithContainer).text).toBe( + "Online \nwhitebo\nard \ncollabo\nration \nmade \neasy", + ); + fireEvent.contextMenu(GlobalTestState.canvas, { + button: 2, + clientX: 20, + clientY: 30, + }); + contextMenu = document.querySelector(".context-menu"); fireEvent.click(queryByText(contextMenu as HTMLElement, "Unbind text")!); expect(h.elements[0].boundElements).toEqual([]); - expect(getOriginalContainerHeightFromCache(rectangle.id)).toBe(null); + expect(getOriginalContainerHeightFromCache(container.id)).toBe(null); - expect(rectangle.height).toBe(originalRectHeight); + expect(container.height).toBe(originalRectHeight); }); it("should reset the container height cache when resizing", async () => { diff --git a/src/element/textWysiwyg.tsx b/src/element/textWysiwyg.tsx index 5b94ab3e..9105ba70 100644 --- a/src/element/textWysiwyg.tsx +++ b/src/element/textWysiwyg.tsx @@ -11,7 +11,7 @@ import { isBoundToContainer, isTextElement, } from "./typeChecks"; -import { CLASSES, isSafari, VERTICAL_ALIGN } from "../constants"; +import { CLASSES, isSafari } from "../constants"; import { ExcalidrawElement, ExcalidrawLinearElement, @@ -23,12 +23,10 @@ import { AppState } from "../types"; import { mutateElement } from "./mutateElement"; import { getBoundTextElementId, - getContainerCoords, getContainerDims, getContainerElement, getTextElementAngle, getTextWidth, - measureText, normalizeText, redrawTextBoundingBox, wrapText, @@ -36,6 +34,7 @@ import { getBoundTextMaxWidth, computeContainerDimensionForBoundText, detectLineHeight, + computeBoundTextPosition, } from "./textElement"; import { actionDecreaseFontSize, @@ -162,7 +161,7 @@ export const textWysiwyg = ({ let textElementWidth = updatedTextElement.width; // Set to element height by default since that's // what is going to be used for unbounded text - let textElementHeight = updatedTextElement.height; + const textElementHeight = updatedTextElement.height; if (container && updatedTextElement.containerId) { if (isArrowElement(container)) { @@ -179,15 +178,6 @@ export const textWysiwyg = ({ editable, ); const containerDims = getContainerDims(container); - // using editor.style.height to get the accurate height of text editor - const editorHeight = Number(editable.style.height.slice(0, -2)); - if (editorHeight > 0) { - textElementHeight = editorHeight; - } - if (propertiesUpdated) { - // update height of the editor after properties updated - textElementHeight = updatedTextElement.height; - } let originalContainerData; if (propertiesUpdated) { @@ -232,22 +222,12 @@ export const textWysiwyg = ({ container.type, ); mutateElement(container, { height: targetContainerHeight }); - } - // Start pushing text upward until a diff of 30px (padding) - // is reached - else { - const containerCoords = getContainerCoords(container); - - // vertically center align the text - if (verticalAlign === VERTICAL_ALIGN.MIDDLE) { - if (!isArrowElement(container)) { - coordY = - containerCoords.y + maxHeight / 2 - textElementHeight / 2; - } - } - if (verticalAlign === VERTICAL_ALIGN.BOTTOM) { - coordY = containerCoords.y + (maxHeight - textElementHeight); - } + } else { + const { y } = computeBoundTextPosition( + container, + updatedTextElement as ExcalidrawTextElementWithContainer, + ); + coordY = y; } } const [viewportX, viewportY] = getViewportCoords(coordX, coordY); @@ -388,25 +368,6 @@ export const textWysiwyg = ({ }; editable.oninput = () => { - const updatedTextElement = Scene.getScene(element)?.getElement( - id, - ) as ExcalidrawTextElement; - const font = getFontString(updatedTextElement); - if (isBoundToContainer(element)) { - const container = getContainerElement(element); - const wrappedText = wrapText( - normalizeText(editable.value), - font, - getBoundTextMaxWidth(container!), - ); - const { width, height } = measureText( - wrappedText, - font, - updatedTextElement.lineHeight, - ); - editable.style.width = `${width}px`; - editable.style.height = `${height}px`; - } onChange(normalizeText(editable.value)); }; }