fix: don't push whitespace to next line when exceeding max width during wrapping and make sure to use same width of text editor on DOM when measuring dimensions (#5996)

* fix: don't push whitespace to next line when exceeding max width during wrapping

* add a helper function and never push empty line

* use width same as in text area so dimensions are same

* add tests

* make sure dom element has exact same width as text editor
This commit is contained in:
Aakansha Doshi 2022-12-21 12:32:43 +05:30 committed by GitHub
parent 6ab3f0eb74
commit d2e371cdf0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 67 additions and 23 deletions

View File

@ -1,10 +1,17 @@
import { BOUND_TEXT_PADDING } from "../constants";
import { wrapText } from "./textElement";
import { measureText, wrapText } from "./textElement";
import { FontString } from "./types";
describe("Test wrapText", () => {
const font = "20px Cascadia, width: Segoe UI Emoji" as FontString;
it("shouldn't add new lines for trailing spaces", () => {
const text = "Hello whats up ";
const maxWidth = 200 - BOUND_TEXT_PADDING * 2;
const res = wrapText(text, font, maxWidth);
expect(res).toBe("Hello whats up ");
});
describe("When text doesn't contain new lines", () => {
const text = "Hello whats up";
[
@ -139,3 +146,37 @@ break it now`,
});
});
});
describe("Test measureText", () => {
const font = "20px Cascadia, width: Segoe UI Emoji" as FontString;
const text = "Hello World";
it("should add correct attributes when maxWidth is passed", () => {
const maxWidth = 200 - BOUND_TEXT_PADDING * 2;
const res = measureText(text, font, maxWidth);
expect(res.container).toMatchInlineSnapshot(`
<div
style="position: absolute; white-space: pre-wrap; font: Emoji 20px 20px; min-height: 1em; width: 191px; overflow: hidden; word-break: break-word; line-height: 0px;"
>
<span
style="display: inline-block; overflow: hidden; width: 1px; height: 1px;"
/>
</div>
`);
});
it("should add correct attributes when maxWidth is not passed", () => {
const res = measureText(text, font);
expect(res.container).toMatchInlineSnapshot(`
<div
style="position: absolute; white-space: pre; font: Emoji 20px 20px; min-height: 1em;"
>
<span
style="display: inline-block; overflow: hidden; width: 1px; height: 1px;"
/>
</div>
`);
});
});

View File

@ -49,11 +49,7 @@ export const redrawTextBoundingBox = (
maxWidth,
);
}
const metrics = measureText(
textElement.originalText,
getFontString(textElement),
maxWidth,
);
const metrics = measureText(text, getFontString(textElement), maxWidth);
let coordY = textElement.y;
let coordX = textElement.x;
// Resize container and vertically center align the text
@ -272,7 +268,7 @@ export const measureText = (
container.style.minHeight = "1em";
if (maxWidth) {
const lineHeight = getApproxLineHeight(font);
container.style.maxWidth = `${String(maxWidth)}px`;
container.style.width = `${String(maxWidth + 1)}px`;
container.style.overflow = "hidden";
container.style.wordBreak = "break-word";
container.style.lineHeight = `${String(lineHeight)}px`;
@ -290,10 +286,12 @@ export const measureText = (
// Baseline is important for positioning text on canvas
const baseline = span.offsetTop + span.offsetHeight;
// Since span adds 1px extra width to the container
const width = container.offsetWidth + 1;
const width = container.offsetWidth - 1;
const height = container.offsetHeight;
document.body.removeChild(container);
if (isTestEnv()) {
return { width, height, baseline, container };
}
return { width, height, baseline };
};
@ -331,6 +329,12 @@ export const wrapText = (text: string, font: FontString, maxWidth: number) => {
const lines: Array<string> = [];
const originalLines = text.split("\n");
const spaceWidth = getTextWidth(" ", font);
const push = (str: string) => {
if (str.trim()) {
lines.push(str);
}
};
originalLines.forEach((originalLine) => {
const words = originalLine.split(" ");
// This means its newline so push it
@ -348,9 +352,7 @@ export const wrapText = (text: string, font: FontString, maxWidth: number) => {
if (currentWordWidth >= maxWidth) {
// push current line since the current word exceeds the max width
// so will be appended in next line
if (currentLine) {
lines.push(currentLine);
}
push(currentLine);
currentLine = "";
currentLineWidthTillNow = 0;
while (words[index].length > 0) {
@ -364,7 +366,7 @@ export const wrapText = (text: string, font: FontString, maxWidth: number) => {
if (currentLine.slice(-1) === " ") {
currentLine = currentLine.slice(0, -1);
}
lines.push(currentLine);
push(currentLine);
currentLine = currentChar;
currentLineWidthTillNow = width;
if (currentLineWidthTillNow === maxWidth) {
@ -377,7 +379,7 @@ export const wrapText = (text: string, font: FontString, maxWidth: number) => {
}
// push current line if appending space exceeds max width
if (currentLineWidthTillNow + spaceWidth >= maxWidth) {
lines.push(currentLine);
push(currentLine);
currentLine = "";
currentLineWidthTillNow = 0;
} else {
@ -396,7 +398,7 @@ export const wrapText = (text: string, font: FontString, maxWidth: number) => {
currentLineWidthTillNow = getTextWidth(currentLine + word, font);
if (currentLineWidthTillNow >= maxWidth) {
lines.push(currentLine);
push(currentLine);
currentLineWidthTillNow = 0;
currentLine = "";
@ -407,7 +409,8 @@ export const wrapText = (text: string, font: FontString, maxWidth: number) => {
// Push the word if appending space exceeds max width
if (currentLineWidthTillNow + spaceWidth >= maxWidth) {
lines.push(currentLine.slice(0, -1));
const word = currentLine.slice(0, -1);
push(word);
currentLine = "";
currentLineWidthTillNow = 0;
break;
@ -424,7 +427,7 @@ export const wrapText = (text: string, font: FontString, maxWidth: number) => {
if (currentLine.slice(-1) === " ") {
currentLine = currentLine.slice(0, -1);
}
lines.push(currentLine);
push(currentLine);
}
}
});

View File

@ -861,7 +861,7 @@ describe("textWysiwyg", () => {
resize(rectangle, "ne", [rectangle.x + 100, rectangle.y - 100]);
expect([h.elements[1].x, h.elements[1].y]).toMatchInlineSnapshot(`
Array [
109.5,
110.5,
17,
]
`);
@ -909,7 +909,7 @@ describe("textWysiwyg", () => {
resize(rectangle, "ne", [rectangle.x + 100, rectangle.y - 100]);
expect([h.elements[1].x, h.elements[1].y]).toMatchInlineSnapshot(`
Array [
424,
426,
-539,
]
`);
@ -1026,7 +1026,7 @@ describe("textWysiwyg", () => {
mouse.up(rectangle.x + 100, rectangle.y + 50);
expect(rectangle.x).toBe(80);
expect(rectangle.y).toBe(85);
expect(text.x).toBe(89.5);
expect(text.x).toBe(90.5);
expect(text.y).toBe(90);
Keyboard.withModifierKeys({ ctrl: true }, () => {

View File

@ -312,7 +312,7 @@ Object {
"versionNonce": 0,
"verticalAlign": "middle",
"width": 100,
"x": -0.5,
"x": 0.5,
"y": 0,
}
`;

View File

@ -1027,7 +1027,7 @@ describe("Test Linear Elements", () => {
expect(getBoundTextElementPosition(container, textElement))
.toMatchInlineSnapshot(`
Object {
"x": 386.5,
"x": 387.5,
"y": 70,
}
`);
@ -1086,7 +1086,7 @@ describe("Test Linear Elements", () => {
expect(getBoundTextElementPosition(container, textElement))
.toMatchInlineSnapshot(`
Object {
"x": 189.5,
"x": 190.5,
"y": 20,
}
`);