feat: add line height attribute to text element (#6360)

* feat: add line height attribute to text element

* lint

* update line height when redrawing text bounding box

* fix tests

* retain line height when pasting styles

* fix test

* create a util for calculating ling height using old algo

* update line height when resizing multiple text elements

* make line height backward compatible

* udpate line height for older element when font size updated

* remove logs

* Add specs

* lint

* review fixes

* simplify by changing `lineHeight` from px to unitless

* make param non-optional

* update comment

* fix: jumping text due to font size being calculated incorrectly

* update line height when font family is updated

* lint

* Add spec

* more specs

* rename to getDefaultLineHeight

* fix getting lineHeight for potentially undefined fontFamily

* reduce duplication

* fix fallback

* refactor and comment tweaks

* fix

---------

Co-authored-by: dwelle <luzar.david@gmail.com>
This commit is contained in:
Aakansha Doshi 2023-03-22 11:32:38 +05:30 committed by GitHub
parent ac4c8b3ca7
commit 83383977f5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
18 changed files with 326 additions and 143 deletions

View file

@ -29,6 +29,7 @@ import {
normalizeText,
wrapText,
getMaxContainerWidth,
getDefaultLineHeight,
} from "./textElement";
import { VERTICAL_ALIGN } from "../constants";
import { isArrowElement } from "./typeChecks";
@ -137,10 +138,12 @@ export const newTextElement = (
textAlign: TextAlign;
verticalAlign: VerticalAlign;
containerId?: ExcalidrawTextContainer["id"];
lineHeight?: ExcalidrawTextElement["lineHeight"];
} & ElementConstructorOpts,
): NonDeleted<ExcalidrawTextElement> => {
const lineHeight = opts.lineHeight || getDefaultLineHeight(opts.fontFamily);
const text = normalizeText(opts.text);
const metrics = measureText(text, getFontString(opts));
const metrics = measureText(text, getFontString(opts), lineHeight);
const offsets = getTextElementPositionOffsets(opts, metrics);
const textElement = newElementWith(
{
@ -156,6 +159,7 @@ export const newTextElement = (
height: metrics.height,
containerId: opts.containerId || null,
originalText: text,
lineHeight,
},
{},
);
@ -176,6 +180,7 @@ const getAdjustedDimensions = (
const { width: nextWidth, height: nextHeight } = measureText(
nextText,
getFontString(element),
element.lineHeight,
);
const { textAlign, verticalAlign } = element;
let x: number;
@ -185,7 +190,11 @@ const getAdjustedDimensions = (
verticalAlign === VERTICAL_ALIGN.MIDDLE &&
!element.containerId
) {
const prevMetrics = measureText(element.text, getFontString(element));
const prevMetrics = measureText(
element.text,
getFontString(element),
element.lineHeight,
);
const offsets = getTextElementPositionOffsets(element, {
width: nextWidth - prevMetrics.width,
height: nextHeight - prevMetrics.height,

View file

@ -39,13 +39,13 @@ import {
import { Point, PointerDownState } from "../types";
import Scene from "../scene/Scene";
import {
getApproxMinLineHeight,
getApproxMinLineWidth,
getBoundTextElement,
getBoundTextElementId,
getContainerElement,
handleBindTextResize,
getMaxContainerWidth,
getApproxMinLineHeight,
} from "./textElement";
export const normalizeAngle = (angle: number): number => {
@ -360,7 +360,7 @@ export const resizeSingleElement = (
let scaleX = atStartBoundsWidth / boundsCurrentWidth;
let scaleY = atStartBoundsHeight / boundsCurrentHeight;
let boundTextFont: { fontSize?: number } = {};
let boundTextFontSize: number | null = null;
const boundTextElement = getBoundTextElement(element);
if (transformHandleDirection.includes("e")) {
@ -410,9 +410,7 @@ export const resizeSingleElement = (
boundTextElement.id,
) as typeof boundTextElement | undefined;
if (stateOfBoundTextElementAtResize) {
boundTextFont = {
fontSize: stateOfBoundTextElementAtResize.fontSize,
};
boundTextFontSize = stateOfBoundTextElementAtResize.fontSize;
}
if (shouldMaintainAspectRatio) {
const updatedElement = {
@ -428,12 +426,16 @@ export const resizeSingleElement = (
if (nextFontSize === null) {
return;
}
boundTextFont = {
fontSize: nextFontSize,
};
boundTextFontSize = nextFontSize;
} else {
const minWidth = getApproxMinLineWidth(getFontString(boundTextElement));
const minHeight = getApproxMinLineHeight(getFontString(boundTextElement));
const minWidth = getApproxMinLineWidth(
getFontString(boundTextElement),
boundTextElement.lineHeight,
);
const minHeight = getApproxMinLineHeight(
boundTextElement.fontSize,
boundTextElement.lineHeight,
);
eleNewWidth = Math.ceil(Math.max(eleNewWidth, minWidth));
eleNewHeight = Math.ceil(Math.max(eleNewHeight, minHeight));
}
@ -566,8 +568,10 @@ export const resizeSingleElement = (
});
mutateElement(element, resizedElement);
if (boundTextElement && boundTextFont) {
mutateElement(boundTextElement, { fontSize: boundTextFont.fontSize });
if (boundTextElement && boundTextFontSize != null) {
mutateElement(boundTextElement, {
fontSize: boundTextFontSize,
});
}
handleBindTextResize(element, transformHandleDirection);
}

View file

@ -1,4 +1,4 @@
import { BOUND_TEXT_PADDING } from "../constants";
import { BOUND_TEXT_PADDING, FONT_FAMILY } from "../constants";
import { API } from "../tests/helpers/api";
import {
computeContainerDimensionForBoundText,
@ -6,6 +6,9 @@ import {
getMaxContainerWidth,
getMaxContainerHeight,
wrapText,
detectLineHeight,
getLineHeightInPx,
getDefaultLineHeight,
} from "./textElement";
import { FontString } from "./types";
@ -40,9 +43,7 @@ describe("Test wrapText", () => {
{
desc: "break all words when width of each word is less than container width",
width: 80,
res: `Hello
whats
up`,
res: `Hello \nwhats \nup`,
},
{
desc: "break all characters when width of each character is less than container width",
@ -64,8 +65,7 @@ p`,
desc: "break words as per the width",
width: 140,
res: `Hello whats
up`,
res: `Hello whats \nup`,
},
{
desc: "fit the container",
@ -95,9 +95,7 @@ whats up`;
{
desc: "break all words when width of each word is less than container width",
width: 80,
res: `Hello
whats
up`,
res: `Hello\nwhats \nup`,
},
{
desc: "break all characters when width of each character is less than container width",
@ -143,11 +141,7 @@ whats up`,
{
desc: "fit characters of long string as per container width",
width: 170,
res: `hellolongtextth
isiswhatsupwith
youIamtypingggg
gandtypinggg
break it now`,
res: `hellolongtextth\nisiswhatsupwith\nyouIamtypingggg\ngandtypinggg \nbreak it now`,
},
{
@ -166,8 +160,7 @@ now`,
desc: "fit the long text when container width is greater than text length and move the rest to next line",
width: 600,
res: `hellolongtextthisiswhatsupwithyouIamtypingggggandtypinggg
break it now`,
res: `hellolongtextthisiswhatsupwithyouIamtypingggggandtypinggg \nbreak it now`,
},
].forEach((data) => {
it(`should ${data.desc}`, () => {
@ -181,8 +174,7 @@ break it now`,
const text = "Hello Excalidraw";
// Length of "Excalidraw" is 100 and exacty equal to max width
const res = wrapText(text, font, 100);
expect(res).toEqual(`Hello
Excalidraw`);
expect(res).toEqual(`Hello \nExcalidraw`);
});
it("should return the text as is if max width is invalid", () => {
@ -312,3 +304,35 @@ describe("Test measureText", () => {
});
});
});
const textElement = API.createElement({
type: "text",
text: "Excalidraw is a\nvirtual \nopensource \nwhiteboard for \nsketching \nhand-drawn like\ndiagrams",
fontSize: 20,
fontFamily: 1,
height: 175,
});
describe("Test detectLineHeight", () => {
it("should return correct line height", () => {
expect(detectLineHeight(textElement)).toBe(1.25);
});
});
describe("Test getLineHeightInPx", () => {
it("should return correct line height", () => {
expect(
getLineHeightInPx(textElement.fontSize, textElement.lineHeight),
).toBe(25);
});
});
describe("Test getDefaultLineHeight", () => {
it("should return line height using default font family when not passed", () => {
//@ts-ignore
expect(getDefaultLineHeight()).toBe(1.25);
});
it("should return correct line height", () => {
expect(getDefaultLineHeight(FONT_FAMILY.Cascadia)).toBe(1.2);
});
});

View file

@ -4,6 +4,7 @@ import {
ExcalidrawTextContainer,
ExcalidrawTextElement,
ExcalidrawTextElementWithContainer,
FontFamilyValues,
FontString,
NonDeletedExcalidrawElement,
} from "./types";
@ -12,6 +13,7 @@ import {
BOUND_TEXT_PADDING,
DEFAULT_FONT_FAMILY,
DEFAULT_FONT_SIZE,
FONT_FAMILY,
TEXT_ALIGN,
VERTICAL_ALIGN,
} from "../constants";
@ -41,12 +43,15 @@ export const normalizeText = (text: string) => {
);
};
export const splitIntoLines = (text: string) => {
return normalizeText(text).split("\n");
};
export const redrawTextBoundingBox = (
textElement: ExcalidrawTextElement,
container: ExcalidrawElement | null,
) => {
let maxWidth = undefined;
const boundTextUpdates = {
x: textElement.x,
y: textElement.y,
@ -68,6 +73,7 @@ export const redrawTextBoundingBox = (
const metrics = measureText(
boundTextUpdates.text,
getFontString(textElement),
textElement.lineHeight,
);
boundTextUpdates.width = metrics.width;
@ -185,7 +191,11 @@ export const handleBindTextResize = (
maxWidth,
);
}
const dimensions = measureText(text, getFontString(textElement));
const dimensions = measureText(
text,
getFontString(textElement),
textElement.lineHeight,
);
nextHeight = dimensions.height;
nextWidth = dimensions.width;
}
@ -261,32 +271,52 @@ const computeBoundTextPosition = (
// https://github.com/grassator/canvas-text-editor/blob/master/lib/FontMetrics.js
export const measureText = (text: string, font: FontString) => {
export const measureText = (
text: string,
font: FontString,
lineHeight: ExcalidrawTextElement["lineHeight"],
) => {
text = text
.split("\n")
// replace empty lines with single space because leading/trailing empty
// lines would be stripped from computation
.map((x) => x || " ")
.join("\n");
const height = getTextHeight(text, font);
const fontSize = parseFloat(font);
const height = getTextHeight(text, fontSize, lineHeight);
const width = getTextWidth(text, font);
return { width, height };
};
const DUMMY_TEXT = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789".toLocaleUpperCase();
const cacheApproxLineHeight: { [key: FontString]: number } = {};
/**
* To get unitless line-height (if unknown) we can calculate it by dividing
* height-per-line by fontSize.
*/
export const detectLineHeight = (textElement: ExcalidrawTextElement) => {
const lineCount = splitIntoLines(textElement.text).length;
return (textElement.height /
lineCount /
textElement.fontSize) as ExcalidrawTextElement["lineHeight"];
};
export const getApproxLineHeight = (font: FontString) => {
if (cacheApproxLineHeight[font]) {
return cacheApproxLineHeight[font];
}
const fontSize = parseInt(font);
/**
* We calculate the line height from the font size and the unitless line height,
* aligning with the W3C spec.
*/
export const getLineHeightInPx = (
fontSize: ExcalidrawTextElement["fontSize"],
lineHeight: ExcalidrawTextElement["lineHeight"],
) => {
return fontSize * lineHeight;
};
// Calculate line height relative to font size
cacheApproxLineHeight[font] = fontSize * 1.2;
return cacheApproxLineHeight[font];
// FIXME rename to getApproxMinContainerHeight
export const getApproxMinLineHeight = (
fontSize: ExcalidrawTextElement["fontSize"],
lineHeight: ExcalidrawTextElement["lineHeight"],
) => {
return getLineHeightInPx(fontSize, lineHeight) + BOUND_TEXT_PADDING * 2;
};
let canvas: HTMLCanvasElement | undefined;
@ -309,7 +339,7 @@ const getLineWidth = (text: string, font: FontString) => {
};
export const getTextWidth = (text: string, font: FontString) => {
const lines = text.replace(/\r\n?/g, "\n").split("\n");
const lines = splitIntoLines(text);
let width = 0;
lines.forEach((line) => {
width = Math.max(width, getLineWidth(line, font));
@ -317,10 +347,13 @@ export const getTextWidth = (text: string, font: FontString) => {
return width;
};
export const getTextHeight = (text: string, font: FontString) => {
const lines = text.replace(/\r\n?/g, "\n").split("\n");
const lineHeight = getApproxLineHeight(font);
return lineHeight * lines.length;
export const getTextHeight = (
text: string,
fontSize: number,
lineHeight: ExcalidrawTextElement["lineHeight"],
) => {
const lineCount = splitIntoLines(text).length;
return getLineHeightInPx(fontSize, lineHeight) * lineCount;
};
export const wrapText = (text: string, font: FontString, maxWidth: number) => {
@ -468,21 +501,23 @@ export const charWidth = (() => {
};
})();
export const getApproxMinLineWidth = (font: FontString) => {
const DUMMY_TEXT = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789".toLocaleUpperCase();
// FIXME rename to getApproxMinContainerWidth
export const getApproxMinLineWidth = (
font: FontString,
lineHeight: ExcalidrawTextElement["lineHeight"],
) => {
const maxCharWidth = getMaxCharWidth(font);
if (maxCharWidth === 0) {
return (
measureText(DUMMY_TEXT.split("").join("\n"), font).width +
measureText(DUMMY_TEXT.split("").join("\n"), font, lineHeight).width +
BOUND_TEXT_PADDING * 2
);
}
return maxCharWidth + BOUND_TEXT_PADDING * 2;
};
export const getApproxMinLineHeight = (font: FontString) => {
return getApproxLineHeight(font) + BOUND_TEXT_PADDING * 2;
};
export const getMinCharWidth = (font: FontString) => {
const cache = charWidth.getCache(font);
if (!cache) {
@ -828,3 +863,32 @@ export const isMeasureTextSupported = () => {
);
return width > 0;
};
/**
* Unitless line height
*
* In previous versions we used `normal` line height, which browsers interpret
* differently, and based on font-family and font-size.
*
* To make line heights consistent across browsers we hardcode the values for
* each of our fonts based on most common average line-heights.
* See https://github.com/excalidraw/excalidraw/pull/6360#issuecomment-1477635971
* where the values come from.
*/
const DEFAULT_LINE_HEIGHT = {
// ~1.25 is the average for Virgil in WebKit and Blink.
// Gecko (FF) uses ~1.28.
[FONT_FAMILY.Virgil]: 1.25 as ExcalidrawTextElement["lineHeight"],
// ~1.15 is the average for Virgil in WebKit and Blink.
// Gecko if all over the place.
[FONT_FAMILY.Helvetica]: 1.15 as ExcalidrawTextElement["lineHeight"],
// ~1.2 is the average for Virgil in WebKit and Blink, and kinda Gecko too
[FONT_FAMILY.Cascadia]: 1.2 as ExcalidrawTextElement["lineHeight"],
};
export const getDefaultLineHeight = (fontFamily: FontFamilyValues) => {
if (fontFamily) {
return DEFAULT_LINE_HEIGHT[fontFamily];
}
return DEFAULT_LINE_HEIGHT[DEFAULT_FONT_FAMILY];
};

View file

@ -783,7 +783,7 @@ describe("textWysiwyg", () => {
rectangle.y + h.elements[0].height / 2 - text.height / 2,
);
expect(text.x).toBe(25);
expect(text.height).toBe(48);
expect(text.height).toBe(50);
expect(text.width).toBe(60);
// Edit and text by removing second line and it should
@ -810,7 +810,7 @@ describe("textWysiwyg", () => {
expect(text.text).toBe("Hello");
expect(text.originalText).toBe("Hello");
expect(text.height).toBe(24);
expect(text.height).toBe(25);
expect(text.width).toBe(50);
expect(text.y).toBe(
rectangle.y + h.elements[0].height / 2 - text.height / 2,
@ -903,7 +903,7 @@ describe("textWysiwyg", () => {
expect([h.elements[1].x, h.elements[1].y]).toMatchInlineSnapshot(`
Array [
85,
5,
4.5,
]
`);
@ -929,7 +929,7 @@ describe("textWysiwyg", () => {
expect([h.elements[1].x, h.elements[1].y]).toMatchInlineSnapshot(`
Array [
15,
66,
65,
]
`);
@ -1067,9 +1067,9 @@ describe("textWysiwyg", () => {
mouse.moveTo(rectangle.x + 100, rectangle.y + 50);
mouse.up(rectangle.x + 100, rectangle.y + 50);
expect(rectangle.x).toBe(80);
expect(rectangle.y).toBe(-35);
expect(rectangle.y).toBe(-40);
expect(text.x).toBe(85);
expect(text.y).toBe(-30);
expect(text.y).toBe(-35);
Keyboard.withModifierKeys({ ctrl: true }, () => {
Keyboard.keyPress(KEYS.Z);
@ -1112,7 +1112,7 @@ describe("textWysiwyg", () => {
target: { value: "Online whiteboard collaboration made easy" },
});
editor.blur();
expect(rectangle.height).toBe(178);
expect(rectangle.height).toBe(185);
mouse.select(rectangle);
fireEvent.contextMenu(GlobalTestState.canvas, {
button: 2,
@ -1186,6 +1186,41 @@ describe("textWysiwyg", () => {
);
});
it("should update line height when font family 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();
expect(
(h.elements[1] as ExcalidrawTextElementWithContainer).lineHeight,
).toEqual(1.25);
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(
(h.elements[1] as ExcalidrawTextElementWithContainer).lineHeight,
).toEqual(1.2);
fireEvent.click(screen.getByTitle(/normal/i));
expect(
(h.elements[1] as ExcalidrawTextElementWithContainer).fontFamily,
).toEqual(FONT_FAMILY.Helvetica);
expect(
(h.elements[1] as ExcalidrawTextElementWithContainer).lineHeight,
).toEqual(1.15);
});
describe("should align correctly", () => {
let editor: HTMLTextAreaElement;
@ -1245,7 +1280,7 @@ describe("textWysiwyg", () => {
expect([h.elements[1].x, h.elements[1].y]).toMatchInlineSnapshot(`
Array [
15,
45.5,
45,
]
`);
});
@ -1257,7 +1292,7 @@ describe("textWysiwyg", () => {
expect([h.elements[1].x, h.elements[1].y]).toMatchInlineSnapshot(`
Array [
30,
45.5,
45,
]
`);
});
@ -1269,7 +1304,7 @@ describe("textWysiwyg", () => {
expect([h.elements[1].x, h.elements[1].y]).toMatchInlineSnapshot(`
Array [
45,
45.5,
45,
]
`);
});
@ -1281,7 +1316,7 @@ describe("textWysiwyg", () => {
expect([h.elements[1].x, h.elements[1].y]).toMatchInlineSnapshot(`
Array [
15,
66,
65,
]
`);
});
@ -1292,7 +1327,7 @@ describe("textWysiwyg", () => {
expect([h.elements[1].x, h.elements[1].y]).toMatchInlineSnapshot(`
Array [
30,
66,
65,
]
`);
});
@ -1303,7 +1338,7 @@ describe("textWysiwyg", () => {
expect([h.elements[1].x, h.elements[1].y]).toMatchInlineSnapshot(`
Array [
45,
66,
65,
]
`);
});
@ -1333,7 +1368,7 @@ describe("textWysiwyg", () => {
const textElement = h.elements[1] as ExcalidrawTextElement;
expect(textElement.width).toBe(600);
expect(textElement.height).toBe(24);
expect(textElement.height).toBe(25);
expect(textElement.textAlign).toBe(TEXT_ALIGN.LEFT);
expect((textElement as ExcalidrawTextElement).text).toBe(
"Excalidraw is an opensource virtual collaborative whiteboard",
@ -1365,7 +1400,7 @@ describe("textWysiwyg", () => {
],
fillStyle: "hachure",
groupIds: [],
height: 34,
height: 35,
isDeleted: false,
link: null,
locked: false,

View file

@ -22,7 +22,6 @@ import {
import { AppState } from "../types";
import { mutateElement } from "./mutateElement";
import {
getApproxLineHeight,
getBoundTextElementId,
getContainerCoords,
getContainerDims,
@ -150,9 +149,7 @@ export const textWysiwyg = ({
return;
}
const { textAlign, verticalAlign } = updatedTextElement;
const approxLineHeight = getApproxLineHeight(
getFontString(updatedTextElement),
);
if (updatedTextElement && isTextElement(updatedTextElement)) {
let coordX = updatedTextElement.x;
let coordY = updatedTextElement.y;
@ -213,7 +210,7 @@ export const textWysiwyg = ({
if (!isArrowElement(container) && textElementHeight > maxHeight) {
const diff = Math.min(
textElementHeight - maxHeight,
approxLineHeight,
element.lineHeight,
);
mutateElement(container, { height: containerDims.height + diff });
return;
@ -226,7 +223,7 @@ export const textWysiwyg = ({
) {
const diff = Math.min(
maxHeight - textElementHeight,
approxLineHeight,
element.lineHeight,
);
mutateElement(container, { height: containerDims.height - diff });
}
@ -266,10 +263,6 @@ export const textWysiwyg = ({
editable.selectionEnd = editable.value.length - diff;
}
const lines = updatedTextElement.originalText.split("\n");
const lineHeight = updatedTextElement.containerId
? approxLineHeight
: updatedTextElement.height / lines.length;
if (!container) {
maxWidth = (appState.width - 8 - viewportX) / appState.zoom.value;
textElementWidth = Math.min(textElementWidth, maxWidth);
@ -282,7 +275,7 @@ export const textWysiwyg = ({
Object.assign(editable.style, {
font: getFontString(updatedTextElement),
// must be defined *after* font ¯\_(ツ)_/¯
lineHeight: `${lineHeight}px`,
lineHeight: element.lineHeight,
width: `${textElementWidth}px`,
height: `${textElementHeight}px`,
left: `${viewportX}px`,
@ -388,7 +381,11 @@ export const textWysiwyg = ({
font,
getMaxContainerWidth(container!),
);
const { width, height } = measureText(wrappedText, font);
const { width, height } = measureText(
wrappedText,
font,
updatedTextElement.lineHeight,
);
editable.style.width = `${width}px`;
editable.style.height = `${height}px`;
}

View file

@ -135,6 +135,11 @@ export type ExcalidrawTextElement = _ExcalidrawElementBase &
verticalAlign: VerticalAlign;
containerId: ExcalidrawGenericElement["id"] | null;
originalText: string;
/**
* Unitless line height (aligned to W3C). To get line height in px, multiply
* with font size (using `getLineHeightInPx` helper).
*/
lineHeight: number & { _brand: "unitlessLineHeight" };
}>;
export type ExcalidrawBindableElement =