From 0b8fc4f4b62a48e432b3fd6b1b97c5a4334f4792 Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Mon, 10 Apr 2023 16:31:58 +0530 Subject: [PATCH 1/3] fix: don't refresh dimensions for deleted text elements (#6438) --- src/element/newElement.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/element/newElement.ts b/src/element/newElement.ts index 6581097cd..b08bac92c 100644 --- a/src/element/newElement.ts +++ b/src/element/newElement.ts @@ -265,6 +265,9 @@ export const refreshTextDimensions = ( textElement: ExcalidrawTextElement, text = textElement.text, ) => { + if (textElement.isDeleted) { + return; + } const container = getContainerElement(textElement); if (container) { text = wrapText( From ec215362a1723ad9749e70aece84358add20ce94 Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Mon, 10 Apr 2023 18:52:46 +0530 Subject: [PATCH 2/3] 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 --- src/actions/actionBoundText.tsx | 3 +- src/data/restore.ts | 42 +++++++----- src/element/newElement.ts | 14 ++-- src/element/resizeElements.ts | 61 ++++++++++++----- src/element/textElement.ts | 65 +++++++++++++++++-- src/element/textWysiwyg.tsx | 16 ++++- src/element/types.ts | 1 + src/renderer/renderElement.ts | 7 +- .../data/__snapshots__/restore.test.ts.snap | 2 + 9 files changed, 161 insertions(+), 50 deletions(-) diff --git a/src/actions/actionBoundText.tsx b/src/actions/actionBoundText.tsx index 4cc72d612..9f9fbfc0f 100644 --- a/src/actions/actionBoundText.tsx +++ b/src/actions/actionBoundText.tsx @@ -43,7 +43,7 @@ export const actionUnbindText = register({ selectedElements.forEach((element) => { const boundTextElement = getBoundTextElement(element); if (boundTextElement) { - const { width, height } = measureText( + const { width, height, baseline } = measureText( boundTextElement.originalText, getFontString(boundTextElement), boundTextElement.lineHeight, @@ -57,6 +57,7 @@ export const actionUnbindText = register({ containerId: null, width, height, + baseline, text: boundTextElement.originalText, }); mutateElement(element, { diff --git a/src/data/restore.ts b/src/data/restore.ts index 57ea329c3..2735d91d2 100644 --- a/src/data/restore.ts +++ b/src/data/restore.ts @@ -31,11 +31,15 @@ import { import { getDefaultAppState } from "../appState"; import { LinearElementEditor } from "../element/linearElementEditor"; import { bumpVersion } from "../element/mutateElement"; -import { getUpdatedTimestamp, updateActiveTool } from "../utils"; +import { getFontString, getUpdatedTimestamp, updateActiveTool } from "../utils"; import { arrayToMap } from "../utils"; import oc from "open-color"; import { MarkOptional, Mutable } from "../utility-types"; -import { detectLineHeight, getDefaultLineHeight } from "../element/textElement"; +import { + detectLineHeight, + getDefaultLineHeight, + measureBaseline, +} from "../element/textElement"; type RestoredAppState = Omit< AppState, @@ -171,6 +175,24 @@ const restoreElement = ( } 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, { fontSize, fontFamily, @@ -179,19 +201,9 @@ const restoreElement = ( verticalAlign: element.verticalAlign || DEFAULT_VERTICAL_ALIGN, containerId: element.containerId ?? null, originalText: element.originalText || 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. - 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)), + + lineHeight, + baseline, }); if (refreshDimensions) { diff --git a/src/element/newElement.ts b/src/element/newElement.ts index b08bac92c..b6b6cad2d 100644 --- a/src/element/newElement.ts +++ b/src/element/newElement.ts @@ -145,6 +145,7 @@ export const newTextElement = ( const text = normalizeText(opts.text); const metrics = measureText(text, getFontString(opts), lineHeight); const offsets = getTextElementPositionOffsets(opts, metrics); + const textElement = newElementWith( { ..._newElementBase("text", opts), @@ -157,6 +158,7 @@ export const newTextElement = ( y: opts.y - offsets.y, width: metrics.width, height: metrics.height, + baseline: metrics.baseline, containerId: opts.containerId || null, originalText: text, lineHeight, @@ -174,14 +176,15 @@ const getAdjustedDimensions = ( y: number; width: number; height: number; + baseline: number; } => { const container = getContainerElement(element); - const { width: nextWidth, height: nextHeight } = measureText( - nextText, - getFontString(element), - element.lineHeight, - ); + const { + width: nextWidth, + height: nextHeight, + baseline: nextBaseline, + } = measureText(nextText, getFontString(element), element.lineHeight); const { textAlign, verticalAlign } = element; let x: number; let y: number; @@ -256,6 +259,7 @@ const getAdjustedDimensions = ( return { width: nextWidth, height: nextHeight, + baseline: nextBaseline, x: Number.isFinite(x) ? x : element.x, y: Number.isFinite(y) ? y : element.y, }; diff --git a/src/element/resizeElements.ts b/src/element/resizeElements.ts index a49559a7e..69b8afae7 100644 --- a/src/element/resizeElements.ts +++ b/src/element/resizeElements.ts @@ -46,6 +46,8 @@ import { handleBindTextResize, getMaxContainerWidth, getApproxMinLineHeight, + measureText, + getMaxContainerHeight, } from "./textElement"; export const normalizeAngle = (angle: number): number => { @@ -193,7 +195,8 @@ const MIN_FONT_SIZE = 1; const measureFontSizeFromWidth = ( element: NonDeleted, nextWidth: number, -): number | null => { + nextHeight: number, +): { size: number; baseline: number } | null => { // We only use width to scale font on resize let width = element.width; @@ -208,8 +211,15 @@ const measureFontSizeFromWidth = ( if (nextFontSize < MIN_FONT_SIZE) { return null; } - - return nextFontSize; + const metrics = measureText( + element.text, + getFontString({ fontSize: nextFontSize, fontFamily: element.fontFamily }), + element.lineHeight, + ); + return { + size: nextFontSize, + baseline: metrics.baseline + (nextHeight - metrics.height), + }; }; const getSidesForTransformHandle = ( @@ -280,8 +290,8 @@ const resizeSingleTextElement = ( if (scale > 0) { const nextWidth = element.width * scale; const nextHeight = element.height * scale; - const nextFontSize = measureFontSizeFromWidth(element, nextWidth); - if (nextFontSize === null) { + const metrics = measureFontSizeFromWidth(element, nextWidth, nextHeight); + if (metrics === null) { return; } const [nextX1, nextY1, nextX2, nextY2] = getResizedElementAbsoluteCoords( @@ -305,9 +315,10 @@ const resizeSingleTextElement = ( deltaY2, ); mutateElement(element, { - fontSize: nextFontSize, + fontSize: metrics.size, width: nextWidth, height: nextHeight, + baseline: metrics.baseline, x: nextElementX, y: nextElementY, }); @@ -360,7 +371,7 @@ export const resizeSingleElement = ( let scaleX = atStartBoundsWidth / boundsCurrentWidth; let scaleY = atStartBoundsHeight / boundsCurrentHeight; - let boundTextFontSize: number | null = null; + let boundTextFont: { fontSize?: number; baseline?: number } = {}; const boundTextElement = getBoundTextElement(element); if (transformHandleDirection.includes("e")) { @@ -410,7 +421,10 @@ export const resizeSingleElement = ( boundTextElement.id, ) as typeof boundTextElement | undefined; if (stateOfBoundTextElementAtResize) { - boundTextFontSize = stateOfBoundTextElementAtResize.fontSize; + boundTextFont = { + fontSize: stateOfBoundTextElementAtResize.fontSize, + baseline: stateOfBoundTextElementAtResize.baseline, + }; } if (shouldMaintainAspectRatio) { const updatedElement = { @@ -419,14 +433,18 @@ export const resizeSingleElement = ( height: eleNewHeight, }; - const nextFontSize = measureFontSizeFromWidth( + const nextFont = measureFontSizeFromWidth( boundTextElement, getMaxContainerWidth(updatedElement), + getMaxContainerHeight(updatedElement), ); - if (nextFontSize === null) { + if (nextFont === null) { return; } - boundTextFontSize = nextFontSize; + boundTextFont = { + fontSize: nextFont.size, + baseline: nextFont.baseline, + }; } else { const minWidth = getApproxMinLineWidth( getFontString(boundTextElement), @@ -568,9 +586,10 @@ export const resizeSingleElement = ( }); mutateElement(element, resizedElement); - if (boundTextElement && boundTextFontSize != null) { + if (boundTextElement && boundTextFont != null) { mutateElement(boundTextElement, { - fontSize: boundTextFontSize, + fontSize: boundTextFont.fontSize, + baseline: boundTextFont.baseline, }); } handleBindTextResize(element, transformHandleDirection); @@ -677,6 +696,7 @@ const resizeMultipleElements = ( y: number; points?: Point[]; fontSize?: number; + baseline?: number; } = { width, height, @@ -685,7 +705,7 @@ const resizeMultipleElements = ( ...rescaledPoints, }; - let boundTextUpdates: { fontSize: number } | null = null; + let boundTextUpdates: { fontSize: number; baseline: number } | null = null; const boundTextElement = getBoundTextElement(element.latest); @@ -695,24 +715,29 @@ const resizeMultipleElements = ( width, height, }; - const fontSize = measureFontSizeFromWidth( + const metrics = measureFontSizeFromWidth( boundTextElement ?? (element.orig as ExcalidrawTextElement), boundTextElement ? getMaxContainerWidth(updatedElement) : updatedElement.width, + boundTextElement + ? getMaxContainerHeight(updatedElement) + : updatedElement.height, ); - if (!fontSize) { + if (!metrics) { return; } if (isTextElement(element.orig)) { - update.fontSize = fontSize; + update.fontSize = metrics.size; + update.baseline = metrics.baseline; } if (boundTextElement) { boundTextUpdates = { - fontSize, + fontSize: metrics.size, + baseline: metrics.baseline, }; } } diff --git a/src/element/textElement.ts b/src/element/textElement.ts index 8b3979133..26cf91a89 100644 --- a/src/element/textElement.ts +++ b/src/element/textElement.ts @@ -14,6 +14,7 @@ import { DEFAULT_FONT_FAMILY, DEFAULT_FONT_SIZE, FONT_FAMILY, + isSafari, TEXT_ALIGN, VERTICAL_ALIGN, } from "../constants"; @@ -58,6 +59,7 @@ export const redrawTextBoundingBox = ( text: textElement.text, width: textElement.width, height: textElement.height, + baseline: textElement.baseline, }; boundTextUpdates.text = textElement.text; @@ -78,6 +80,7 @@ export const redrawTextBoundingBox = ( boundTextUpdates.width = metrics.width; boundTextUpdates.height = metrics.height; + boundTextUpdates.baseline = metrics.baseline; if (container) { if (isArrowElement(container)) { @@ -183,6 +186,7 @@ export const handleBindTextResize = ( const maxWidth = getMaxContainerWidth(container); const maxHeight = getMaxContainerHeight(container); let containerHeight = containerDims.height; + let nextBaseLine = textElement.baseline; if (transformHandleType !== "n" && transformHandleType !== "s") { if (text) { text = wrapText( @@ -191,13 +195,14 @@ export const handleBindTextResize = ( maxWidth, ); } - const dimensions = measureText( + const metrics = measureText( text, getFontString(textElement), textElement.lineHeight, ); - nextHeight = dimensions.height; - nextWidth = dimensions.width; + nextHeight = metrics.height; + nextWidth = metrics.width; + nextBaseLine = metrics.baseline; } // increase height in case text element height exceeds if (nextHeight > maxHeight) { @@ -225,6 +230,7 @@ export const handleBindTextResize = ( text, width: nextWidth, height: nextHeight, + baseline: nextBaseLine, }); if (!isArrowElement(container)) { @@ -285,8 +291,59 @@ export const measureText = ( const fontSize = parseFloat(font); const height = getTextHeight(text, fontSize, lineHeight); 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; }; /** diff --git a/src/element/textWysiwyg.tsx b/src/element/textWysiwyg.tsx index 4e8ef3a5c..ef4f7c926 100644 --- a/src/element/textWysiwyg.tsx +++ b/src/element/textWysiwyg.tsx @@ -11,7 +11,7 @@ import { isBoundToContainer, isTextElement, } from "./typeChecks"; -import { CLASSES, VERTICAL_ALIGN } from "../constants"; +import { CLASSES, isSafari, VERTICAL_ALIGN } from "../constants"; import { ExcalidrawElement, ExcalidrawLinearElement, @@ -35,6 +35,7 @@ import { getMaxContainerHeight, getMaxContainerWidth, computeContainerDimensionForBoundText, + detectLineHeight, } from "./textElement"; import { actionDecreaseFontSize, @@ -271,13 +272,24 @@ export const textWysiwyg = ({ } else { 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 const editorMaxHeight = (appState.height - viewportY) / appState.zoom.value; Object.assign(editable.style, { font: getFontString(updatedTextElement), // must be defined *after* font ¯\_(ツ)_/¯ - lineHeight: element.lineHeight, + lineHeight, width: `${textElementWidth}px`, height: `${textElementHeight}px`, left: `${viewportX}px`, diff --git a/src/element/types.ts b/src/element/types.ts index 4b4bad74e..0760d054d 100644 --- a/src/element/types.ts +++ b/src/element/types.ts @@ -131,6 +131,7 @@ export type ExcalidrawTextElement = _ExcalidrawElementBase & fontSize: number; fontFamily: FontFamilyValues; text: string; + baseline: number; textAlign: TextAlign; verticalAlign: VerticalAlign; containerId: ExcalidrawGenericElement["id"] | null; diff --git a/src/renderer/renderElement.ts b/src/renderer/renderElement.ts index 7adcc5840..486e52bdf 100644 --- a/src/renderer/renderElement.ts +++ b/src/renderer/renderElement.ts @@ -245,7 +245,6 @@ const drawImagePlaceholder = ( size, ); }; - const drawElementOnCanvas = ( element: NonDeletedExcalidrawElement, rc: RoughCanvas, @@ -331,18 +330,16 @@ const drawElementOnCanvas = ( : element.textAlign === "right" ? element.width : 0; - context.textBaseline = "bottom"; - const lineHeightPx = getLineHeightInPx( element.fontSize, element.lineHeight, ); - + const verticalOffset = element.height - element.baseline; for (let index = 0; index < lines.length; index++) { context.fillText( lines[index], horizontalOffset, - (index + 1) * lineHeightPx, + (index + 1) * lineHeightPx - verticalOffset, ); } context.restore(); diff --git a/src/tests/data/__snapshots__/restore.test.ts.snap b/src/tests/data/__snapshots__/restore.test.ts.snap index e9a0da005..7e30b9d86 100644 --- a/src/tests/data/__snapshots__/restore.test.ts.snap +++ b/src/tests/data/__snapshots__/restore.test.ts.snap @@ -282,6 +282,7 @@ exports[`restoreElements should restore text element correctly passing value for Object { "angle": 0, "backgroundColor": "transparent", + "baseline": 0, "boundElements": Array [], "containerId": null, "fillStyle": "hachure", @@ -321,6 +322,7 @@ exports[`restoreElements should restore text element correctly with unknown font Object { "angle": 0, "backgroundColor": "transparent", + "baseline": 0, "boundElements": Array [], "containerId": null, "fillStyle": "hachure", From e4d8ba226f15836b4572fc3bae6a623e809cf08c Mon Sep 17 00:00:00 2001 From: David Luzar Date: Mon, 10 Apr 2023 15:38:50 +0200 Subject: [PATCH 3/3] feat: zigzag fill easter egg (#6439) --- src/actions/actionProperties.tsx | 92 ++++++++++++++++++----------- src/components/ButtonIconSelect.tsx | 78 ++++++++++++++++-------- src/components/icons.tsx | 7 +++ src/css/styles.scss | 3 + src/element/types.ts | 2 +- 5 files changed, 120 insertions(+), 62 deletions(-) diff --git a/src/actions/actionProperties.tsx b/src/actions/actionProperties.tsx index 08420d9ef..ed714816b 100644 --- a/src/actions/actionProperties.tsx +++ b/src/actions/actionProperties.tsx @@ -1,4 +1,5 @@ import { AppState } from "../../src/types"; +import { trackEvent } from "../analytics"; import { ButtonIconSelect } from "../components/ButtonIconSelect"; import { ColorPicker } from "../components/ColorPicker"; import { IconPicker } from "../components/IconPicker"; @@ -37,6 +38,7 @@ import { TextAlignLeftIcon, TextAlignCenterIcon, TextAlignRightIcon, + FillZigZagIcon, } from "../components/icons"; import { DEFAULT_FONT_FAMILY, @@ -294,7 +296,12 @@ export const actionChangeBackgroundColor = register({ export const actionChangeFillStyle = register({ name: "changeFillStyle", trackEvent: false, - perform: (elements, appState, value) => { + perform: (elements, appState, value, app) => { + trackEvent( + "element", + "changeFillStyle", + `${value} (${app.device.isMobile ? "mobile" : "desktop"})`, + ); return { elements: changeProperty(elements, appState, (el) => newElementWith(el, { @@ -305,40 +312,55 @@ export const actionChangeFillStyle = register({ commitToHistory: true, }; }, - PanelComponent: ({ elements, appState, updateData }) => ( -
- {t("labels.fill")} - element.fillStyle, - appState.currentItemFillStyle, - )} - onChange={(value) => { - updateData(value); - }} - /> -
- ), + PanelComponent: ({ elements, appState, updateData }) => { + const selectedElements = getSelectedElements(elements, appState); + const allElementsZigZag = selectedElements.every( + (el) => el.fillStyle === "zigzag", + ); + + return ( +
+ {t("labels.fill")} + element.fillStyle, + appState.currentItemFillStyle, + )} + onClick={(value, event) => { + const nextValue = + event.altKey && + value === "hachure" && + selectedElements.every((el) => el.fillStyle === "hachure") + ? "zigzag" + : value; + + updateData(nextValue); + }} + /> +
+ ); + }, }); export const actionChangeStrokeWidth = register({ diff --git a/src/components/ButtonIconSelect.tsx b/src/components/ButtonIconSelect.tsx index 899ec150d..eec8870a9 100644 --- a/src/components/ButtonIconSelect.tsx +++ b/src/components/ButtonIconSelect.tsx @@ -1,33 +1,59 @@ import clsx from "clsx"; // TODO: It might be "clever" to add option.icon to the existing component -export const ButtonIconSelect = ({ - options, - value, - onChange, - group, -}: { - options: { value: T; text: string; icon: JSX.Element; testId?: string }[]; - value: T | null; - onChange: (value: T) => void; - group: string; -}) => ( +export const ButtonIconSelect = ( + props: { + options: { + value: T; + text: string; + icon: JSX.Element; + testId?: string; + /** if not supplied, defaults to value identity check */ + active?: boolean; + }[]; + value: T | null; + type?: "radio" | "button"; + } & ( + | { type?: "radio"; group: string; onChange: (value: T) => void } + | { + type: "button"; + onClick: ( + value: T, + event: React.MouseEvent, + ) => void; + } + ), +) => (
- {options.map((option) => ( -
); diff --git a/src/components/icons.tsx b/src/components/icons.tsx index 046ee490b..784e81024 100644 --- a/src/components/icons.tsx +++ b/src/components/icons.tsx @@ -1008,6 +1008,13 @@ export const UngroupIcon = React.memo(({ theme }: { theme: Theme }) => ), ); +export const FillZigZagIcon = createIcon( + + + , + modifiedTablerIconProps, +); + export const FillHachureIcon = createIcon( <>