Skip to content

Commit

Permalink
fix: call stack size exceeded when paste large text (excalidraw#6373) (
Browse files Browse the repository at this point in the history
…excalidraw#6396)

* fix: call stack size exceeded when paste large text (excalidraw#6373)

* fix: add test case for paste multi-line text

* fix

* tweak

* add missing assertion

* add comments

* lint

---------

Co-authored-by: Aakansha Doshi <[email protected]>
  • Loading branch information
YinDongFang and ad1992 authored Mar 29, 2023
1 parent 25bb673 commit 44453b7
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 11 deletions.
42 changes: 39 additions & 3 deletions src/element/textWysiwyg.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,44 @@ describe("textWysiwyg", () => {
]);
});

it("should compute the container height correctly and not throw error when height is updated while editing the text", async () => {
const diamond = API.createElement({
type: "diamond",
x: 10,
y: 20,
width: 90,
height: 75,
});
h.elements = [diamond];

expect(h.elements.length).toBe(1);
expect(h.elements[0].id).toBe(diamond.id);

API.setSelectedElements([diamond]);
Keyboard.keyPress(KEYS.ENTER);

const editor = document.querySelector(
".excalidraw-textEditorContainer > textarea",
) as HTMLTextAreaElement;

await new Promise((r) => setTimeout(r, 0));
const value = new Array(1000).fill("1").join("\n");

// Pasting large text to simulate height increase
expect(() =>
fireEvent.input(editor, { target: { value } }),
).not.toThrow();

expect(diamond.height).toBe(50020);

// Clearing text to simulate height decrease
expect(() =>
fireEvent.input(editor, { target: { value: "" } }),
).not.toThrow();

expect(diamond.height).toBe(70);
});

it("should bind text to container when double clicked on center of transparent container", async () => {
const rectangle = API.createElement({
type: "rectangle",
Expand Down Expand Up @@ -1181,9 +1219,7 @@ describe("textWysiwyg", () => {
expect(
(h.elements[1] as ExcalidrawTextElementWithContainer).fontSize,
).toEqual(36);
expect(getOriginalContainerHeightFromCache(rectangle.id)).toBe(
96.39999999999999,
);
expect(getOriginalContainerHeightFromCache(rectangle.id)).toBe(97);
});

it("should update line height when font family updated", async () => {
Expand Down
18 changes: 10 additions & 8 deletions src/element/textWysiwyg.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
wrapText,
getMaxContainerHeight,
getMaxContainerWidth,
computeContainerDimensionForBoundText,
} from "./textElement";
import {
actionDecreaseFontSize,
Expand Down Expand Up @@ -208,11 +209,12 @@ export const textWysiwyg = ({

// autogrow container height if text exceeds
if (!isArrowElement(container) && textElementHeight > maxHeight) {
const diff = Math.min(
textElementHeight - maxHeight,
element.lineHeight,
const targetContainerHeight = computeContainerDimensionForBoundText(
textElementHeight,
container.type,
);
mutateElement(container, { height: containerDims.height + diff });

mutateElement(container, { height: targetContainerHeight });
return;
} else if (
// autoshrink container height until original container height
Expand All @@ -221,11 +223,11 @@ export const textWysiwyg = ({
containerDims.height > originalContainerData.height &&
textElementHeight < maxHeight
) {
const diff = Math.min(
maxHeight - textElementHeight,
element.lineHeight,
const targetContainerHeight = computeContainerDimensionForBoundText(
textElementHeight,
container.type,
);
mutateElement(container, { height: containerDims.height - diff });
mutateElement(container, { height: targetContainerHeight });
}
// Start pushing text upward until a diff of 30px (padding)
// is reached
Expand Down

0 comments on commit 44453b7

Please sign in to comment.