From 8ec5f7b982f1842a19c32322fac73b9a5bef9cb5 Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Fri, 23 Dec 2022 11:57:48 +0530 Subject: [PATCH] feat: support shrinking text containers to original height when text removed (#6025) * fix:cache bind text containers height so that it could autoshrink to original height when text deleted * revert * rename * reset cache when resized * safe check * restore original containr height when text is unbind * update cache when redrawing bounding box * reset cache when unbind * make type-safe * add specs * skip one test * remoe mock * fix Co-authored-by: dwelle --- src/actions/actionBoundText.tsx | 12 ++++ src/element/textElement.ts | 7 +- src/element/textWysiwyg.test.tsx | 112 ++++++++++++++++++++++++++++++- src/element/textWysiwyg.tsx | 88 +++++++++++++++++++----- 4 files changed, 200 insertions(+), 19 deletions(-) diff --git a/src/actions/actionBoundText.tsx b/src/actions/actionBoundText.tsx index 07a56d87..812e10b3 100644 --- a/src/actions/actionBoundText.tsx +++ b/src/actions/actionBoundText.tsx @@ -6,6 +6,10 @@ import { measureText, redrawTextBoundingBox, } from "../element/textElement"; +import { + getOriginalContainerHeightFromCache, + resetOriginalContainerCache, +} from "../element/textWysiwyg"; import { hasBoundTextElement, isTextBindableContainer, @@ -38,6 +42,11 @@ export const actionUnbindText = register({ boundTextElement.originalText, getFontString(boundTextElement), ); + const originalContainerHeight = getOriginalContainerHeightFromCache( + element.id, + ); + resetOriginalContainerCache(element.id); + mutateElement(boundTextElement as ExcalidrawTextElement, { containerId: null, width, @@ -49,6 +58,9 @@ export const actionUnbindText = register({ boundElements: element.boundElements?.filter( (ele) => ele.id !== boundTextElement.id, ), + height: originalContainerHeight + ? originalContainerHeight + : element.height, }); } }); diff --git a/src/element/textElement.ts b/src/element/textElement.ts index 3a03f936..47632bb5 100644 --- a/src/element/textElement.ts +++ b/src/element/textElement.ts @@ -24,6 +24,10 @@ import { isTextBindableContainer } from "./typeChecks"; import { getElementAbsoluteCoords } from "../element"; import { getSelectedElements } from "../scene"; import { isHittingElementNotConsideringBoundingBox } from "./collision"; +import { + resetOriginalContainerCache, + updateOriginalContainerCache, +} from "./textWysiwyg"; export const normalizeText = (text: string) => { return ( @@ -84,7 +88,7 @@ export const redrawTextBoundingBox = ( } else { coordX = container.x + containerDims.width / 2 - metrics.width / 2; } - + updateOriginalContainerCache(container.id, nextHeight); mutateElement(container, { height: nextHeight }); } else { const centerX = textElement.x + textElement.width / 2; @@ -149,6 +153,7 @@ export const handleBindTextResize = ( if (!boundTextElementId) { return; } + resetOriginalContainerCache(container.id); let textElement = Scene.getScene(container)!.getElement( boundTextElementId, ) as ExcalidrawTextElement; diff --git a/src/element/textWysiwyg.test.tsx b/src/element/textWysiwyg.test.tsx index d06d9236..59ed2829 100644 --- a/src/element/textWysiwyg.test.tsx +++ b/src/element/textWysiwyg.test.tsx @@ -15,6 +15,7 @@ import * as textElementUtils from "./textElement"; import { API } from "../tests/helpers/api"; import { mutateElement } from "./mutateElement"; import { resize } from "../tests/utils"; +import { getOriginalContainerHeightFromCache } from "./textWysiwyg"; // Unmount ReactDOM from root ReactDOM.unmountComponentAtNode(document.getElementById("root")!); @@ -1019,7 +1020,6 @@ describe("textWysiwyg", () => { const originalRectY = rectangle.y; const originalTextX = text.x; const originalTextY = text.y; - mouse.select(rectangle); mouse.downAt(rectangle.x, rectangle.y); mouse.moveTo(rectangle.x + 100, rectangle.y + 50); @@ -1055,5 +1055,115 @@ describe("textWysiwyg", () => { expect(rectangle.boundElements).toStrictEqual([]); expect(h.elements[1].isDeleted).toBe(true); }); + + it("should restore original container height and clear cache once text is unbind", async () => { + jest + .spyOn(textElementUtils, "measureText") + .mockImplementation((text, font, maxWidth) => { + let width = INITIAL_WIDTH; + let height = APPROX_LINE_HEIGHT; + let baseline = 10; + if (!text) { + return { + width, + height, + baseline, + }; + } + baseline = 30; + width = DUMMY_WIDTH; + height = APPROX_LINE_HEIGHT * 5; + + return { + width, + height, + baseline, + }; + }); + 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" }, + }); + editor.blur(); + expect(rectangle.height).toBe(135); + mouse.select(rectangle); + fireEvent.contextMenu(GlobalTestState.canvas, { + button: 2, + clientX: 20, + clientY: 30, + }); + const 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(rectangle.height).toBe(originalRectHeight); + }); + + it("should reset the container height cache when resizing", async () => { + Keyboard.keyPress(KEYS.ENTER); + expect(getOriginalContainerHeightFromCache(rectangle.id)).toBe(75); + let editor = document.querySelector( + ".excalidraw-textEditorContainer > textarea", + ) as HTMLTextAreaElement; + await new Promise((r) => setTimeout(r, 0)); + fireEvent.change(editor, { target: { value: "Hello" } }); + editor.blur(); + + resize(rectangle, "ne", [rectangle.x + 100, rectangle.y - 100]); + expect(rectangle.height).toBe(215); + expect(getOriginalContainerHeightFromCache(rectangle.id)).toBe(null); + + mouse.select(rectangle); + Keyboard.keyPress(KEYS.ENTER); + + editor = document.querySelector( + ".excalidraw-textEditorContainer > textarea", + ) as HTMLTextAreaElement; + + await new Promise((r) => setTimeout(r, 0)); + editor.blur(); + expect(rectangle.height).toBe(215); + // cache updated again + expect(getOriginalContainerHeightFromCache(rectangle.id)).toBe(215); + }); + + //@todo fix this test later once measureText is mocked correctly + it.skip("should reset the container height cache when font properties updated", async () => { + Keyboard.keyPress(KEYS.ENTER); + expect(getOriginalContainerHeightFromCache(rectangle.id)).toBe(75); + + const editor = document.querySelector( + ".excalidraw-textEditorContainer > textarea", + ) as HTMLTextAreaElement; + + await new Promise((r) => setTimeout(r, 0)); + fireEvent.change(editor, { target: { value: "Hello World!" } }); + editor.blur(); + + mouse.select(rectangle); + Keyboard.keyPress(KEYS.ENTER); + + fireEvent.click(screen.getByTitle(/code/i)); + + expect( + (h.elements[1] as ExcalidrawTextElementWithContainer).fontFamily, + ).toEqual(FONT_FAMILY.Cascadia); + expect(getOriginalContainerHeightFromCache(rectangle.id)).toBe(75); + + fireEvent.click(screen.getByTitle(/Very large/i)); + expect( + (h.elements[1] as ExcalidrawTextElementWithContainer).fontSize, + ).toEqual(36); + expect(getOriginalContainerHeightFromCache(rectangle.id)).toBe(75); + }); }); }); diff --git a/src/element/textWysiwyg.tsx b/src/element/textWysiwyg.tsx index a9737b46..3f462325 100644 --- a/src/element/textWysiwyg.tsx +++ b/src/element/textWysiwyg.tsx @@ -17,6 +17,7 @@ import { ExcalidrawLinearElement, ExcalidrawTextElementWithContainer, ExcalidrawTextElement, + ExcalidrawTextContainer, } from "./types"; import { AppState } from "../types"; import { mutateElement } from "./mutateElement"; @@ -60,6 +61,38 @@ const getTransform = ( return `translate(${translateX}px, ${translateY}px) scale(${zoom.value}) rotate(${degree}deg)`; }; +const originalContainerCache: { + [id: ExcalidrawTextContainer["id"]]: + | { + height: ExcalidrawTextContainer["height"]; + } + | undefined; +} = {}; + +export const updateOriginalContainerCache = ( + id: ExcalidrawTextContainer["id"], + height: ExcalidrawTextContainer["height"], +) => { + const data = + originalContainerCache[id] || (originalContainerCache[id] = { height }); + data.height = height; + return data; +}; + +export const resetOriginalContainerCache = ( + id: ExcalidrawTextContainer["id"], +) => { + if (originalContainerCache[id]) { + delete originalContainerCache[id]; + } +}; + +export const getOriginalContainerHeightFromCache = ( + id: ExcalidrawTextContainer["id"], +) => { + return originalContainerCache[id]?.height ?? null; +}; + export const textWysiwyg = ({ id, onChange, @@ -87,6 +120,9 @@ export const textWysiwyg = ({ updatedTextElement: ExcalidrawTextElement, editable: HTMLTextAreaElement, ) => { + if (!editable.style.fontFamily || !editable.style.fontSize) { + return false; + } const currentFont = editable.style.fontFamily.replace(/"/g, ""); if ( getFontFamilyString({ fontFamily: updatedTextElement.fontFamily }) !== @@ -99,7 +135,6 @@ export const textWysiwyg = ({ } return false; }; - let originalContainerHeight: number; const updateWysiwygStyle = () => { const appState = app.state; @@ -123,7 +158,7 @@ export const textWysiwyg = ({ const width = updatedTextElement.width; // Set to element height by default since that's // what is going to be used for unbounded text - let height = updatedTextElement.height; + let textElementHeight = updatedTextElement.height; if (container && updatedTextElement.containerId) { if (isArrowElement(container)) { const boundTextCoords = @@ -142,34 +177,52 @@ export const textWysiwyg = ({ // using editor.style.height to get the accurate height of text editor const editorHeight = Number(editable.style.height.slice(0, -2)); if (editorHeight > 0) { - height = editorHeight; + textElementHeight = editorHeight; } if (propertiesUpdated) { - originalContainerHeight = containerDims.height; - // update height of the editor after properties updated - height = updatedTextElement.height; + textElementHeight = updatedTextElement.height; } - if (!originalContainerHeight) { - originalContainerHeight = containerDims.height; + + let originalContainerData; + if (propertiesUpdated) { + originalContainerData = updateOriginalContainerCache( + container.id, + containerDims.height, + ); + } else { + originalContainerData = originalContainerCache[container.id]; + if (!originalContainerData) { + originalContainerData = updateOriginalContainerCache( + container.id, + containerDims.height, + ); + } } + maxWidth = getMaxContainerWidth(container); maxHeight = getMaxContainerHeight(container); // autogrow container height if text exceeds - if (!isArrowElement(container) && height > maxHeight) { - const diff = Math.min(height - maxHeight, approxLineHeight); + if (!isArrowElement(container) && textElementHeight > maxHeight) { + const diff = Math.min( + textElementHeight - maxHeight, + approxLineHeight, + ); mutateElement(container, { height: containerDims.height + diff }); return; } else if ( // autoshrink container height until original container height // is reached when text is removed !isArrowElement(container) && - containerDims.height > originalContainerHeight && - height < maxHeight + containerDims.height > originalContainerData.height && + textElementHeight < maxHeight ) { - const diff = Math.min(maxHeight - height, approxLineHeight); + const diff = Math.min( + maxHeight - textElementHeight, + approxLineHeight, + ); mutateElement(container, { height: containerDims.height - diff }); } // Start pushing text upward until a diff of 30px (padding) @@ -178,14 +231,15 @@ export const textWysiwyg = ({ // vertically center align the text if (verticalAlign === VERTICAL_ALIGN.MIDDLE) { if (!isArrowElement(container)) { - coordY = container.y + containerDims.height / 2 - height / 2; + coordY = + container.y + containerDims.height / 2 - textElementHeight / 2; } } if (verticalAlign === VERTICAL_ALIGN.BOTTOM) { coordY = container.y + containerDims.height - - height - + textElementHeight - getBoundTextElementOffset(updatedTextElement); } } @@ -226,12 +280,12 @@ export const textWysiwyg = ({ // must be defined *after* font ¯\_(ツ)_/¯ lineHeight: `${lineHeight}px`, width: `${Math.min(width, maxWidth)}px`, - height: `${height}px`, + height: `${textElementHeight}px`, left: `${viewportX}px`, top: `${viewportY}px`, transform: getTransform( width, - height, + textElementHeight, getTextElementAngle(updatedTextElement), appState, maxWidth,