fix: introduce baseline to fix the layout shift when switching to text editor (#6397)

* fix: introduce baseline to fix the layout shift when switching to text editor

* uncomment

* change offset to 8pixels

* [debug]

* introduce DOM baseline in canvas rendering instead

* introduce baseline in element making it backward compat

* fix

* lint

* fix

* update baseline when resizing text element

* fix safari backward compat

* fix for safari

* lint

* reduce safari LS

* floor line height and height when dom height increases than canvas height

* Revert "floor line height and height when dom height increases than canvas height"

This reverts commit 8de65168238b8fb9a638e0c75f596f371983c2c7.

* cleanup

* use DOM height only for safari to fix LS

* Revert "use DOM height only for safari to fix LS"

This reverts commit d75889238da59b7954ec3a6ac2c29dc0ba420635.

* fix lint and test

* fix

* calculate line height by rounding off instead of DOM

* cleanup

---------

Co-authored-by: dwelle <luzar.david@gmail.com>
This commit is contained in:
Aakansha Doshi 2023-04-10 18:52:46 +05:30 committed by GitHub
parent 0b8fc4f4b6
commit ec215362a1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 161 additions and 50 deletions

View File

@ -43,7 +43,7 @@ export const actionUnbindText = register({
selectedElements.forEach((element) => { selectedElements.forEach((element) => {
const boundTextElement = getBoundTextElement(element); const boundTextElement = getBoundTextElement(element);
if (boundTextElement) { if (boundTextElement) {
const { width, height } = measureText( const { width, height, baseline } = measureText(
boundTextElement.originalText, boundTextElement.originalText,
getFontString(boundTextElement), getFontString(boundTextElement),
boundTextElement.lineHeight, boundTextElement.lineHeight,
@ -57,6 +57,7 @@ export const actionUnbindText = register({
containerId: null, containerId: null,
width, width,
height, height,
baseline,
text: boundTextElement.originalText, text: boundTextElement.originalText,
}); });
mutateElement(element, { mutateElement(element, {

View File

@ -31,11 +31,15 @@ import {
import { getDefaultAppState } from "../appState"; import { getDefaultAppState } from "../appState";
import { LinearElementEditor } from "../element/linearElementEditor"; import { LinearElementEditor } from "../element/linearElementEditor";
import { bumpVersion } from "../element/mutateElement"; import { bumpVersion } from "../element/mutateElement";
import { getUpdatedTimestamp, updateActiveTool } from "../utils"; import { getFontString, getUpdatedTimestamp, updateActiveTool } from "../utils";
import { arrayToMap } from "../utils"; import { arrayToMap } from "../utils";
import oc from "open-color"; import oc from "open-color";
import { MarkOptional, Mutable } from "../utility-types"; import { MarkOptional, Mutable } from "../utility-types";
import { detectLineHeight, getDefaultLineHeight } from "../element/textElement"; import {
detectLineHeight,
getDefaultLineHeight,
measureBaseline,
} from "../element/textElement";
type RestoredAppState = Omit< type RestoredAppState = Omit<
AppState, AppState,
@ -171,6 +175,24 @@ const restoreElement = (
} }
const text = element.text ?? ""; const text = element.text ?? "";
// line-height might not be specified either when creating elements
// programmatically, or when importing old diagrams.
// For the latter we want to detect the original line height which
// will likely differ from our per-font fixed line height we now use,
// to maintain backward compatibility.
const lineHeight =
element.lineHeight ||
(element.height
? // detect line-height from current element height and font-size
detectLineHeight(element)
: // no element height likely means programmatic use, so default
// to a fixed line height
getDefaultLineHeight(element.fontFamily));
const baseline = measureBaseline(
element.text,
getFontString(element),
lineHeight,
);
element = restoreElementWithProperties(element, { element = restoreElementWithProperties(element, {
fontSize, fontSize,
fontFamily, fontFamily,
@ -179,19 +201,9 @@ const restoreElement = (
verticalAlign: element.verticalAlign || DEFAULT_VERTICAL_ALIGN, verticalAlign: element.verticalAlign || DEFAULT_VERTICAL_ALIGN,
containerId: element.containerId ?? null, containerId: element.containerId ?? null,
originalText: element.originalText || text, originalText: element.originalText || text,
// line-height might not be specified either when creating elements
// programmatically, or when importing old diagrams. lineHeight,
// For the latter we want to detect the original line height which baseline,
// will likely differ from our per-font fixed line height we now use,
// to maintain backward compatibility.
lineHeight:
element.lineHeight ||
(element.height
? // detect line-height from current element height and font-size
detectLineHeight(element)
: // no element height likely means programmatic use, so default
// to a fixed line height
getDefaultLineHeight(element.fontFamily)),
}); });
if (refreshDimensions) { if (refreshDimensions) {

View File

@ -145,6 +145,7 @@ export const newTextElement = (
const text = normalizeText(opts.text); const text = normalizeText(opts.text);
const metrics = measureText(text, getFontString(opts), lineHeight); const metrics = measureText(text, getFontString(opts), lineHeight);
const offsets = getTextElementPositionOffsets(opts, metrics); const offsets = getTextElementPositionOffsets(opts, metrics);
const textElement = newElementWith( const textElement = newElementWith(
{ {
..._newElementBase<ExcalidrawTextElement>("text", opts), ..._newElementBase<ExcalidrawTextElement>("text", opts),
@ -157,6 +158,7 @@ export const newTextElement = (
y: opts.y - offsets.y, y: opts.y - offsets.y,
width: metrics.width, width: metrics.width,
height: metrics.height, height: metrics.height,
baseline: metrics.baseline,
containerId: opts.containerId || null, containerId: opts.containerId || null,
originalText: text, originalText: text,
lineHeight, lineHeight,
@ -174,14 +176,15 @@ const getAdjustedDimensions = (
y: number; y: number;
width: number; width: number;
height: number; height: number;
baseline: number;
} => { } => {
const container = getContainerElement(element); const container = getContainerElement(element);
const { width: nextWidth, height: nextHeight } = measureText( const {
nextText, width: nextWidth,
getFontString(element), height: nextHeight,
element.lineHeight, baseline: nextBaseline,
); } = measureText(nextText, getFontString(element), element.lineHeight);
const { textAlign, verticalAlign } = element; const { textAlign, verticalAlign } = element;
let x: number; let x: number;
let y: number; let y: number;
@ -256,6 +259,7 @@ const getAdjustedDimensions = (
return { return {
width: nextWidth, width: nextWidth,
height: nextHeight, height: nextHeight,
baseline: nextBaseline,
x: Number.isFinite(x) ? x : element.x, x: Number.isFinite(x) ? x : element.x,
y: Number.isFinite(y) ? y : element.y, y: Number.isFinite(y) ? y : element.y,
}; };

View File

@ -46,6 +46,8 @@ import {
handleBindTextResize, handleBindTextResize,
getMaxContainerWidth, getMaxContainerWidth,
getApproxMinLineHeight, getApproxMinLineHeight,
measureText,
getMaxContainerHeight,
} from "./textElement"; } from "./textElement";
export const normalizeAngle = (angle: number): number => { export const normalizeAngle = (angle: number): number => {
@ -193,7 +195,8 @@ const MIN_FONT_SIZE = 1;
const measureFontSizeFromWidth = ( const measureFontSizeFromWidth = (
element: NonDeleted<ExcalidrawTextElement>, element: NonDeleted<ExcalidrawTextElement>,
nextWidth: number, nextWidth: number,
): number | null => { nextHeight: number,
): { size: number; baseline: number } | null => {
// We only use width to scale font on resize // We only use width to scale font on resize
let width = element.width; let width = element.width;
@ -208,8 +211,15 @@ const measureFontSizeFromWidth = (
if (nextFontSize < MIN_FONT_SIZE) { if (nextFontSize < MIN_FONT_SIZE) {
return null; return null;
} }
const metrics = measureText(
return nextFontSize; element.text,
getFontString({ fontSize: nextFontSize, fontFamily: element.fontFamily }),
element.lineHeight,
);
return {
size: nextFontSize,
baseline: metrics.baseline + (nextHeight - metrics.height),
};
}; };
const getSidesForTransformHandle = ( const getSidesForTransformHandle = (
@ -280,8 +290,8 @@ const resizeSingleTextElement = (
if (scale > 0) { if (scale > 0) {
const nextWidth = element.width * scale; const nextWidth = element.width * scale;
const nextHeight = element.height * scale; const nextHeight = element.height * scale;
const nextFontSize = measureFontSizeFromWidth(element, nextWidth); const metrics = measureFontSizeFromWidth(element, nextWidth, nextHeight);
if (nextFontSize === null) { if (metrics === null) {
return; return;
} }
const [nextX1, nextY1, nextX2, nextY2] = getResizedElementAbsoluteCoords( const [nextX1, nextY1, nextX2, nextY2] = getResizedElementAbsoluteCoords(
@ -305,9 +315,10 @@ const resizeSingleTextElement = (
deltaY2, deltaY2,
); );
mutateElement(element, { mutateElement(element, {
fontSize: nextFontSize, fontSize: metrics.size,
width: nextWidth, width: nextWidth,
height: nextHeight, height: nextHeight,
baseline: metrics.baseline,
x: nextElementX, x: nextElementX,
y: nextElementY, y: nextElementY,
}); });
@ -360,7 +371,7 @@ export const resizeSingleElement = (
let scaleX = atStartBoundsWidth / boundsCurrentWidth; let scaleX = atStartBoundsWidth / boundsCurrentWidth;
let scaleY = atStartBoundsHeight / boundsCurrentHeight; let scaleY = atStartBoundsHeight / boundsCurrentHeight;
let boundTextFontSize: number | null = null; let boundTextFont: { fontSize?: number; baseline?: number } = {};
const boundTextElement = getBoundTextElement(element); const boundTextElement = getBoundTextElement(element);
if (transformHandleDirection.includes("e")) { if (transformHandleDirection.includes("e")) {
@ -410,7 +421,10 @@ export const resizeSingleElement = (
boundTextElement.id, boundTextElement.id,
) as typeof boundTextElement | undefined; ) as typeof boundTextElement | undefined;
if (stateOfBoundTextElementAtResize) { if (stateOfBoundTextElementAtResize) {
boundTextFontSize = stateOfBoundTextElementAtResize.fontSize; boundTextFont = {
fontSize: stateOfBoundTextElementAtResize.fontSize,
baseline: stateOfBoundTextElementAtResize.baseline,
};
} }
if (shouldMaintainAspectRatio) { if (shouldMaintainAspectRatio) {
const updatedElement = { const updatedElement = {
@ -419,14 +433,18 @@ export const resizeSingleElement = (
height: eleNewHeight, height: eleNewHeight,
}; };
const nextFontSize = measureFontSizeFromWidth( const nextFont = measureFontSizeFromWidth(
boundTextElement, boundTextElement,
getMaxContainerWidth(updatedElement), getMaxContainerWidth(updatedElement),
getMaxContainerHeight(updatedElement),
); );
if (nextFontSize === null) { if (nextFont === null) {
return; return;
} }
boundTextFontSize = nextFontSize; boundTextFont = {
fontSize: nextFont.size,
baseline: nextFont.baseline,
};
} else { } else {
const minWidth = getApproxMinLineWidth( const minWidth = getApproxMinLineWidth(
getFontString(boundTextElement), getFontString(boundTextElement),
@ -568,9 +586,10 @@ export const resizeSingleElement = (
}); });
mutateElement(element, resizedElement); mutateElement(element, resizedElement);
if (boundTextElement && boundTextFontSize != null) { if (boundTextElement && boundTextFont != null) {
mutateElement(boundTextElement, { mutateElement(boundTextElement, {
fontSize: boundTextFontSize, fontSize: boundTextFont.fontSize,
baseline: boundTextFont.baseline,
}); });
} }
handleBindTextResize(element, transformHandleDirection); handleBindTextResize(element, transformHandleDirection);
@ -677,6 +696,7 @@ const resizeMultipleElements = (
y: number; y: number;
points?: Point[]; points?: Point[];
fontSize?: number; fontSize?: number;
baseline?: number;
} = { } = {
width, width,
height, height,
@ -685,7 +705,7 @@ const resizeMultipleElements = (
...rescaledPoints, ...rescaledPoints,
}; };
let boundTextUpdates: { fontSize: number } | null = null; let boundTextUpdates: { fontSize: number; baseline: number } | null = null;
const boundTextElement = getBoundTextElement(element.latest); const boundTextElement = getBoundTextElement(element.latest);
@ -695,24 +715,29 @@ const resizeMultipleElements = (
width, width,
height, height,
}; };
const fontSize = measureFontSizeFromWidth( const metrics = measureFontSizeFromWidth(
boundTextElement ?? (element.orig as ExcalidrawTextElement), boundTextElement ?? (element.orig as ExcalidrawTextElement),
boundTextElement boundTextElement
? getMaxContainerWidth(updatedElement) ? getMaxContainerWidth(updatedElement)
: updatedElement.width, : updatedElement.width,
boundTextElement
? getMaxContainerHeight(updatedElement)
: updatedElement.height,
); );
if (!fontSize) { if (!metrics) {
return; return;
} }
if (isTextElement(element.orig)) { if (isTextElement(element.orig)) {
update.fontSize = fontSize; update.fontSize = metrics.size;
update.baseline = metrics.baseline;
} }
if (boundTextElement) { if (boundTextElement) {
boundTextUpdates = { boundTextUpdates = {
fontSize, fontSize: metrics.size,
baseline: metrics.baseline,
}; };
} }
} }

View File

@ -14,6 +14,7 @@ import {
DEFAULT_FONT_FAMILY, DEFAULT_FONT_FAMILY,
DEFAULT_FONT_SIZE, DEFAULT_FONT_SIZE,
FONT_FAMILY, FONT_FAMILY,
isSafari,
TEXT_ALIGN, TEXT_ALIGN,
VERTICAL_ALIGN, VERTICAL_ALIGN,
} from "../constants"; } from "../constants";
@ -58,6 +59,7 @@ export const redrawTextBoundingBox = (
text: textElement.text, text: textElement.text,
width: textElement.width, width: textElement.width,
height: textElement.height, height: textElement.height,
baseline: textElement.baseline,
}; };
boundTextUpdates.text = textElement.text; boundTextUpdates.text = textElement.text;
@ -78,6 +80,7 @@ export const redrawTextBoundingBox = (
boundTextUpdates.width = metrics.width; boundTextUpdates.width = metrics.width;
boundTextUpdates.height = metrics.height; boundTextUpdates.height = metrics.height;
boundTextUpdates.baseline = metrics.baseline;
if (container) { if (container) {
if (isArrowElement(container)) { if (isArrowElement(container)) {
@ -183,6 +186,7 @@ export const handleBindTextResize = (
const maxWidth = getMaxContainerWidth(container); const maxWidth = getMaxContainerWidth(container);
const maxHeight = getMaxContainerHeight(container); const maxHeight = getMaxContainerHeight(container);
let containerHeight = containerDims.height; let containerHeight = containerDims.height;
let nextBaseLine = textElement.baseline;
if (transformHandleType !== "n" && transformHandleType !== "s") { if (transformHandleType !== "n" && transformHandleType !== "s") {
if (text) { if (text) {
text = wrapText( text = wrapText(
@ -191,13 +195,14 @@ export const handleBindTextResize = (
maxWidth, maxWidth,
); );
} }
const dimensions = measureText( const metrics = measureText(
text, text,
getFontString(textElement), getFontString(textElement),
textElement.lineHeight, textElement.lineHeight,
); );
nextHeight = dimensions.height; nextHeight = metrics.height;
nextWidth = dimensions.width; nextWidth = metrics.width;
nextBaseLine = metrics.baseline;
} }
// increase height in case text element height exceeds // increase height in case text element height exceeds
if (nextHeight > maxHeight) { if (nextHeight > maxHeight) {
@ -225,6 +230,7 @@ export const handleBindTextResize = (
text, text,
width: nextWidth, width: nextWidth,
height: nextHeight, height: nextHeight,
baseline: nextBaseLine,
}); });
if (!isArrowElement(container)) { if (!isArrowElement(container)) {
@ -285,8 +291,59 @@ export const measureText = (
const fontSize = parseFloat(font); const fontSize = parseFloat(font);
const height = getTextHeight(text, fontSize, lineHeight); const height = getTextHeight(text, fontSize, lineHeight);
const width = getTextWidth(text, font); const width = getTextWidth(text, font);
const baseline = measureBaseline(text, font, lineHeight);
return { width, height, baseline };
};
return { width, height }; export const measureBaseline = (
text: string,
font: FontString,
lineHeight: ExcalidrawTextElement["lineHeight"],
wrapInContainer?: boolean,
) => {
const container = document.createElement("div");
container.style.position = "absolute";
container.style.whiteSpace = "pre";
container.style.font = font;
container.style.minHeight = "1em";
if (wrapInContainer) {
container.style.overflow = "hidden";
container.style.wordBreak = "break-word";
container.style.whiteSpace = "pre-wrap";
}
container.style.lineHeight = String(lineHeight);
container.innerText = text;
// Baseline is important for positioning text on canvas
document.body.appendChild(container);
const span = document.createElement("span");
span.style.display = "inline-block";
span.style.overflow = "hidden";
span.style.width = "1px";
span.style.height = "1px";
container.appendChild(span);
let baseline = span.offsetTop + span.offsetHeight;
const height = container.offsetHeight;
if (isSafari) {
const canvasHeight = getTextHeight(text, parseFloat(font), lineHeight);
const fontSize = parseFloat(font);
// In Safari the font size gets rounded off when rendering hence calculating the safari height and shifting the baseline if it differs
// from the actual canvas height
const domHeight = getTextHeight(text, Math.round(fontSize), lineHeight);
if (canvasHeight > height) {
baseline += canvasHeight - domHeight;
}
if (height > canvasHeight) {
baseline -= domHeight - canvasHeight;
}
}
document.body.removeChild(container);
return baseline;
}; };
/** /**

View File

@ -11,7 +11,7 @@ import {
isBoundToContainer, isBoundToContainer,
isTextElement, isTextElement,
} from "./typeChecks"; } from "./typeChecks";
import { CLASSES, VERTICAL_ALIGN } from "../constants"; import { CLASSES, isSafari, VERTICAL_ALIGN } from "../constants";
import { import {
ExcalidrawElement, ExcalidrawElement,
ExcalidrawLinearElement, ExcalidrawLinearElement,
@ -35,6 +35,7 @@ import {
getMaxContainerHeight, getMaxContainerHeight,
getMaxContainerWidth, getMaxContainerWidth,
computeContainerDimensionForBoundText, computeContainerDimensionForBoundText,
detectLineHeight,
} from "./textElement"; } from "./textElement";
import { import {
actionDecreaseFontSize, actionDecreaseFontSize,
@ -271,13 +272,24 @@ export const textWysiwyg = ({
} else { } else {
textElementWidth += 0.5; textElementWidth += 0.5;
} }
let lineHeight = updatedTextElement.lineHeight;
// In Safari the font size gets rounded off when rendering hence calculating the line height by rounding off font size
if (isSafari) {
lineHeight = detectLineHeight({
...updatedTextElement,
fontSize: Math.round(updatedTextElement.fontSize),
});
}
// Make sure text editor height doesn't go beyond viewport // Make sure text editor height doesn't go beyond viewport
const editorMaxHeight = const editorMaxHeight =
(appState.height - viewportY) / appState.zoom.value; (appState.height - viewportY) / appState.zoom.value;
Object.assign(editable.style, { Object.assign(editable.style, {
font: getFontString(updatedTextElement), font: getFontString(updatedTextElement),
// must be defined *after* font ¯\_(ツ)_/¯ // must be defined *after* font ¯\_(ツ)_/¯
lineHeight: element.lineHeight, lineHeight,
width: `${textElementWidth}px`, width: `${textElementWidth}px`,
height: `${textElementHeight}px`, height: `${textElementHeight}px`,
left: `${viewportX}px`, left: `${viewportX}px`,

View File

@ -131,6 +131,7 @@ export type ExcalidrawTextElement = _ExcalidrawElementBase &
fontSize: number; fontSize: number;
fontFamily: FontFamilyValues; fontFamily: FontFamilyValues;
text: string; text: string;
baseline: number;
textAlign: TextAlign; textAlign: TextAlign;
verticalAlign: VerticalAlign; verticalAlign: VerticalAlign;
containerId: ExcalidrawGenericElement["id"] | null; containerId: ExcalidrawGenericElement["id"] | null;

View File

@ -245,7 +245,6 @@ const drawImagePlaceholder = (
size, size,
); );
}; };
const drawElementOnCanvas = ( const drawElementOnCanvas = (
element: NonDeletedExcalidrawElement, element: NonDeletedExcalidrawElement,
rc: RoughCanvas, rc: RoughCanvas,
@ -331,18 +330,16 @@ const drawElementOnCanvas = (
: element.textAlign === "right" : element.textAlign === "right"
? element.width ? element.width
: 0; : 0;
context.textBaseline = "bottom";
const lineHeightPx = getLineHeightInPx( const lineHeightPx = getLineHeightInPx(
element.fontSize, element.fontSize,
element.lineHeight, element.lineHeight,
); );
const verticalOffset = element.height - element.baseline;
for (let index = 0; index < lines.length; index++) { for (let index = 0; index < lines.length; index++) {
context.fillText( context.fillText(
lines[index], lines[index],
horizontalOffset, horizontalOffset,
(index + 1) * lineHeightPx, (index + 1) * lineHeightPx - verticalOffset,
); );
} }
context.restore(); context.restore();

View File

@ -282,6 +282,7 @@ exports[`restoreElements should restore text element correctly passing value for
Object { Object {
"angle": 0, "angle": 0,
"backgroundColor": "transparent", "backgroundColor": "transparent",
"baseline": 0,
"boundElements": Array [], "boundElements": Array [],
"containerId": null, "containerId": null,
"fillStyle": "hachure", "fillStyle": "hachure",
@ -321,6 +322,7 @@ exports[`restoreElements should restore text element correctly with unknown font
Object { Object {
"angle": 0, "angle": 0,
"backgroundColor": "transparent", "backgroundColor": "transparent",
"baseline": 0,
"boundElements": Array [], "boundElements": Array [],
"containerId": null, "containerId": null,
"fillStyle": "hachure", "fillStyle": "hachure",