From abbeed3d5f4ac9d2ff39250dd89b55ea5c9e4ea4 Mon Sep 17 00:00:00 2001 From: David Luzar <5153846+dwelle@users.noreply.github.com> Date: Fri, 28 Jun 2024 13:52:29 +0200 Subject: [PATCH] feat: support Stats bound text `fontSize` editing (#8187) --- .../excalidraw/components/Stats/DragInput.tsx | 58 ++++---- .../excalidraw/components/Stats/FontSize.tsx | 65 +++++---- .../components/Stats/MultiFontSize.tsx | 137 +++++++++++------- .../excalidraw/components/Stats/index.tsx | 30 ++-- .../components/Stats/stats.test.tsx | 75 +++++++--- 5 files changed, 215 insertions(+), 150 deletions(-) diff --git a/packages/excalidraw/components/Stats/DragInput.tsx b/packages/excalidraw/components/Stats/DragInput.tsx index 0218e83690..463aaa2812 100644 --- a/packages/excalidraw/components/Stats/DragInput.tsx +++ b/packages/excalidraw/components/Stats/DragInput.tsx @@ -15,33 +15,42 @@ import "./DragInput.scss"; import type { AppState } from "../../types"; import { cloneJSON } from "../../utils"; -export type DragInputCallbackType = (props: { +export type DragInputCallbackType< + P extends StatsInputProperty, + E = ExcalidrawElement, +> = (props: { accumulatedChange: number; instantChange: number; - originalElements: readonly ExcalidrawElement[]; + originalElements: readonly E[]; originalElementsMap: ElementsMap; shouldKeepAspectRatio: boolean; shouldChangeByStepSize: boolean; nextValue?: number; - property: T; + property: P; scene: Scene; originalAppState: AppState; }) => void; -interface StatsDragInputProps { +interface StatsDragInputProps< + T extends StatsInputProperty, + E = ExcalidrawElement, +> { label: string | React.ReactNode; icon?: React.ReactNode; value: number | "Mixed"; - elements: readonly ExcalidrawElement[]; + elements: readonly E[]; editable?: boolean; shouldKeepAspectRatio?: boolean; - dragInputCallback: DragInputCallbackType; + dragInputCallback: DragInputCallbackType; property: T; scene: Scene; appState: AppState; } -const StatsDragInput = ({ +const StatsDragInput = < + T extends StatsInputProperty, + E extends ExcalidrawElement = ExcalidrawElement, +>({ label, icon, dragInputCallback, @@ -52,7 +61,7 @@ const StatsDragInput = ({ property, scene, appState, -}: StatsDragInputProps) => { +}: StatsDragInputProps) => { const app = useApp(); const inputRef = useRef(null); const labelRef = useRef(null); @@ -61,7 +70,7 @@ const StatsDragInput = ({ const stateRef = useRef<{ originalAppState: AppState; - originalElements: readonly ExcalidrawElement[]; + originalElements: readonly E[]; lastUpdatedValue: string; updatePending: boolean; }>(null!); @@ -82,7 +91,7 @@ const StatsDragInput = ({ const handleInputValue = ( updatedValue: string, - elements: readonly ExcalidrawElement[], + elements: readonly E[], appState: AppState, ) => { if (!stateRef.current.updatePending) { @@ -173,9 +182,18 @@ const StatsDragInput = ({ y: number; } | null = null; - let originalElements: ExcalidrawElement[] | null = null; let originalElementsMap: Map | null = - null; + app.scene + .getNonDeletedElements() + .reduce((acc: ElementsMap, element) => { + acc.set(element.id, deepCopyElement(element)); + return acc; + }, new Map()); + + let originalElements: readonly E[] | null = elements.map( + (element) => originalElementsMap!.get(element.id) as E, + ); + const originalAppState: AppState = cloneJSON(appState); let accumulatedChange: number | null = null; @@ -183,21 +201,6 @@ const StatsDragInput = ({ document.body.classList.add("excalidraw-cursor-resize"); const onPointerMove = (event: PointerEvent) => { - if (!originalElementsMap) { - originalElementsMap = app.scene - .getNonDeletedElements() - .reduce((acc, element) => { - acc.set(element.id, deepCopyElement(element)); - return acc; - }, new Map() as ElementsMap); - } - - if (!originalElements) { - originalElements = elements.map( - (element) => originalElementsMap!.get(element.id)!, - ); - } - if (!accumulatedChange) { accumulatedChange = 0; } @@ -205,6 +208,7 @@ const StatsDragInput = ({ if ( lastPointer && originalElementsMap !== null && + originalElements !== null && accumulatedChange !== null ) { const instantChange = event.clientX - lastPointer.x; diff --git a/packages/excalidraw/components/Stats/FontSize.tsx b/packages/excalidraw/components/Stats/FontSize.tsx index 8ed136f4fa..13dc6dbee7 100644 --- a/packages/excalidraw/components/Stats/FontSize.tsx +++ b/packages/excalidraw/components/Stats/FontSize.tsx @@ -1,5 +1,7 @@ -import type { ExcalidrawTextElement } from "../../element/types"; -import { refreshTextDimensions } from "../../element/newElement"; +import type { + ExcalidrawElement, + ExcalidrawTextElement, +} from "../../element/types"; import StatsDragInput from "./DragInput"; import type { DragInputCallbackType } from "./DragInput"; import { mutateElement } from "../../element/mutateElement"; @@ -7,10 +9,12 @@ import { getStepSizedValue } from "./utils"; import { fontSizeIcon } from "../icons"; import type Scene from "../../scene/Scene"; import type { AppState } from "../../types"; -import { isTextElement } from "../../element"; +import { isTextElement, redrawTextBoundingBox } from "../../element"; +import { hasBoundTextElement } from "../../element/typeChecks"; +import { getBoundTextElement } from "../../element/textElement"; interface FontSizeProps { - element: ExcalidrawTextElement; + element: ExcalidrawElement; scene: Scene; appState: AppState; property: "fontSize"; @@ -20,7 +24,8 @@ const MIN_FONT_SIZE = 4; const STEP_SIZE = 4; const handleFontSizeChange: DragInputCallbackType< - FontSizeProps["property"] + FontSizeProps["property"], + ExcalidrawTextElement > = ({ accumulatedChange, originalElements, @@ -36,50 +41,52 @@ const handleFontSizeChange: DragInputCallbackType< if (!latestElement || !isTextElement(latestElement)) { return; } + + let nextFontSize; + if (nextValue !== undefined) { - const nextFontSize = Math.max(Math.round(nextValue), MIN_FONT_SIZE); - - const newElement = { - ...latestElement, - fontSize: nextFontSize, - }; - const updates = refreshTextDimensions(newElement, null, elementsMap); - mutateElement(latestElement, { - ...updates, - fontSize: nextFontSize, - }); - return; - } - - if (origElement.type === "text") { + nextFontSize = Math.max(Math.round(nextValue), MIN_FONT_SIZE); + } else if (origElement.type === "text") { const originalFontSize = Math.round(origElement.fontSize); const changeInFontSize = Math.round(accumulatedChange); - let nextFontSize = Math.max( + nextFontSize = Math.max( originalFontSize + changeInFontSize, MIN_FONT_SIZE, ); if (shouldChangeByStepSize) { nextFontSize = getStepSizedValue(nextFontSize, STEP_SIZE); } - const newElement = { - ...latestElement, - fontSize: nextFontSize, - }; - const updates = refreshTextDimensions(newElement, null, elementsMap); + } + + if (nextFontSize) { mutateElement(latestElement, { - ...updates, fontSize: nextFontSize, }); + redrawTextBoundingBox( + latestElement, + scene.getContainerElement(latestElement), + scene.getNonDeletedElementsMap(), + ); } } }; const FontSize = ({ element, scene, appState, property }: FontSizeProps) => { + const _element = isTextElement(element) + ? element + : hasBoundTextElement(element) + ? getBoundTextElement(element, scene.getNonDeletedElementsMap()) + : null; + + if (!_element) { + return null; + } + return ( - elements.filter( - (el) => - el && !isInGroup(el) && isTextElement(el) && !isBoundToContainer(el), - ) as ExcalidrawTextElement[]; + elements.reduce( + (acc: ExcalidrawTextElement[], el) => { + if (!el || isInGroup(el)) { + return acc; + } + if (isTextElement(el)) { + acc.push(el); + return acc; + } + if (hasBoundTextElement(el)) { + const boundTextElement = getBoundTextElement(el, elementsMap); + if (boundTextElement) { + acc.push(boundTextElement); + return acc; + } + } + + return acc; + }, + + [], + ); const handleFontSizeChange: DragInputCallbackType< - MultiFontSizeProps["property"] + MultiFontSizeProps["property"], + ExcalidrawTextElement > = ({ accumulatedChange, originalElements, @@ -41,71 +64,67 @@ const handleFontSizeChange: DragInputCallbackType< scene, }) => { const elementsMap = scene.getNonDeletedElementsMap(); - const latestTextElements = getApplicableTextElements( - originalElements.map((el) => elementsMap.get(el.id)), - ); + const latestTextElements = originalElements.map((el) => + elementsMap.get(el.id), + ) as ExcalidrawTextElement[]; + + let nextFontSize; if (nextValue) { - const nextFontSize = Math.max(Math.round(nextValue), MIN_FONT_SIZE); + nextFontSize = Math.max(Math.round(nextValue), MIN_FONT_SIZE); - for (const textElement of latestTextElements.map((el) => - elementsMap.get(el.id), - )) { - if (!textElement || !isTextElement(textElement)) { - continue; - } - const newElement = { - ...textElement, - fontSize: nextFontSize, - }; - const updates = refreshTextDimensions(newElement, null, elementsMap); + for (const textElement of latestTextElements) { mutateElement( textElement, { - ...updates, fontSize: nextFontSize, }, false, ); + + redrawTextBoundingBox( + textElement, + scene.getContainerElement(textElement), + elementsMap, + false, + ); } scene.triggerUpdate(); - return; - } + } else { + const originalTextElements = originalElements as ExcalidrawTextElement[]; - const originalTextElements = originalElements.filter( - (el) => !isInGroup(el) && isTextElement(el) && !isBoundToContainer(el), - ) as ExcalidrawTextElement[]; + for (let i = 0; i < latestTextElements.length; i++) { + const latestElement = latestTextElements[i]; + const originalElement = originalTextElements[i]; - for (let i = 0; i < latestTextElements.length; i++) { - const latestElement = latestTextElements[i]; - const originalElement = originalTextElements[i]; + const originalFontSize = Math.round(originalElement.fontSize); + const changeInFontSize = Math.round(accumulatedChange); + let nextFontSize = Math.max( + originalFontSize + changeInFontSize, + MIN_FONT_SIZE, + ); + if (shouldChangeByStepSize) { + nextFontSize = getStepSizedValue(nextFontSize, STEP_SIZE); + } + mutateElement( + latestElement, + { + fontSize: nextFontSize, + }, + false, + ); - const originalFontSize = Math.round(originalElement.fontSize); - const changeInFontSize = Math.round(accumulatedChange); - let nextFontSize = Math.max( - originalFontSize + changeInFontSize, - MIN_FONT_SIZE, - ); - if (shouldChangeByStepSize) { - nextFontSize = getStepSizedValue(nextFontSize, STEP_SIZE); + redrawTextBoundingBox( + latestElement, + scene.getContainerElement(latestElement), + elementsMap, + false, + ); } - const newElement = { - ...latestElement, - fontSize: nextFontSize, - }; - const updates = refreshTextDimensions(newElement, null, elementsMap); - mutateElement( - latestElement, - { - ...updates, - fontSize: nextFontSize, - }, - false, - ); - } - scene.triggerUpdate(); + scene.triggerUpdate(); + } }; const MultiFontSize = ({ @@ -113,8 +132,14 @@ const MultiFontSize = ({ scene, appState, property, + elementsMap, }: MultiFontSizeProps) => { - const latestTextElements = getApplicableTextElements(elements); + const latestTextElements = getApplicableTextElements(elements, elementsMap); + + if (!latestTextElements.length) { + return null; + } + const fontSizes = latestTextElements.map( (textEl) => Math.round(textEl.fontSize * 10) / 10, ); @@ -125,7 +150,7 @@ const MultiFontSize = ({ - {singleElement.type === "text" && ( - - )} + )} @@ -278,14 +275,13 @@ export const StatsInner = memo( scene={scene} appState={appState} /> - {multipleElements.some((el) => isTextElement(el)) && ( - - )} + )} diff --git a/packages/excalidraw/components/Stats/stats.test.tsx b/packages/excalidraw/components/Stats/stats.test.tsx index 7fb014c5e9..16e8e733c0 100644 --- a/packages/excalidraw/components/Stats/stats.test.tsx +++ b/packages/excalidraw/components/Stats/stats.test.tsx @@ -11,7 +11,7 @@ import * as StaticScene from "../../renderer/staticScene"; import { vi } from "vitest"; import { reseed } from "../../random"; import { setDateTimeForTests } from "../../utils"; -import { Excalidraw } from "../.."; +import { Excalidraw, mutateElement } from "../.."; import { t } from "../../i18n"; import type { ExcalidrawElement, @@ -37,10 +37,14 @@ const editInput = (input: HTMLInputElement, value: string) => { }; const getStatsProperty = (label: string) => { + const elementStats = UI.queryStats()?.querySelector("#elementStats"); + if (elementStats) { const properties = elementStats?.querySelector(".statsItem"); - return properties?.querySelector?.( - `.drag-input-container[data-testid="${label}"]`, + return ( + properties?.querySelector?.( + `.drag-input-container[data-testid="${label}"]`, + ) || null ); } @@ -57,7 +61,7 @@ const testInputProperty = ( const input = getStatsProperty(label)?.querySelector( ".drag-input", ) as HTMLInputElement; - expect(input).not.toBeNull(); + expect(input).toBeDefined(); expect(input.value).toBe(initialValue.toString()); editInput(input, String(nextValue)); if (property === "angle") { @@ -131,8 +135,8 @@ describe("stats for a generic element", () => { }); it("should open stats", () => { - expect(stats).not.toBeNull(); - expect(elementStats).not.toBeNull(); + expect(stats).toBeDefined(); + expect(elementStats).toBeDefined(); // title const title = elementStats?.querySelector("h3"); @@ -140,18 +144,18 @@ describe("stats for a generic element", () => { // element type const elementType = elementStats?.querySelector(".elementType"); - expect(elementType).not.toBeNull(); + expect(elementType).toBeDefined(); expect(elementType?.lastChild?.nodeValue).toBe(t("element.rectangle")); // properties const properties = elementStats?.querySelector(".statsItem"); - expect(properties?.childNodes).not.toBeNull(); + expect(properties?.childNodes).toBeDefined(); ["X", "Y", "W", "H", "A"].forEach((label) => () => { expect( properties?.querySelector?.( `.drag-input-container[data-testid="${label}"]`, ), - ).not.toBeNull(); + ).toBeDefined(); }); }); @@ -174,7 +178,7 @@ describe("stats for a generic element", () => { const input = getStatsProperty("W")?.querySelector( ".drag-input", ) as HTMLInputElement; - expect(input).not.toBeNull(); + expect(input).toBeDefined(); expect(input.value).toBe(rectangle.width.toString()); editInput(input, "123.123"); expect(h.elements.length).toBe(1); @@ -333,7 +337,7 @@ describe("stats for a non-generic element", () => { const input = getStatsProperty("F")?.querySelector( ".drag-input", ) as HTMLInputElement; - expect(input).not.toBeNull(); + expect(input).toBeDefined(); expect(input.value).toBe(text.fontSize.toString()); editInput(input, "36"); expect(text.fontSize).toBe(36); @@ -366,7 +370,7 @@ describe("stats for a non-generic element", () => { elementStats = stats?.querySelector("#elementStats"); - expect(elementStats).not.toBeNull(); + expect(elementStats).toBeDefined(); // cannot change angle const angle = getStatsProperty("A")?.querySelector(".drag-input"); @@ -387,7 +391,7 @@ describe("stats for a non-generic element", () => { }, }); elementStats = stats?.querySelector("#elementStats"); - expect(elementStats).not.toBeNull(); + expect(elementStats).toBeDefined(); const widthToHeight = image.width / image.height; // when width or height is changed, the aspect ratio is preserved @@ -399,6 +403,35 @@ describe("stats for a non-generic element", () => { expect(image.height).toBe(80); expect(image.width / image.height).toBe(widthToHeight); }); + + it("should display fontSize for bound text", () => { + const container = API.createElement({ + type: "rectangle", + width: 200, + height: 100, + }); + const text = API.createElement({ + type: "text", + width: 200, + height: 100, + containerId: container.id, + fontSize: 20, + }); + mutateElement(container, { + boundElements: [{ type: "text", id: text.id }], + }); + h.elements = [container, text]; + + API.setSelectedElements([container]); + const fontSize = getStatsProperty("F")?.querySelector( + ".drag-input", + ) as HTMLInputElement; + expect(fontSize).toBeDefined(); + + editInput(fontSize, "40"); + + expect(text.fontSize).toBe(40); + }); }); // multiple elements @@ -515,25 +548,25 @@ describe("stats for multiple elements", () => { const width = getStatsProperty("W")?.querySelector( ".drag-input", ) as HTMLInputElement; - expect(width).not.toBeNull(); + expect(width).toBeDefined(); expect(width.value).toBe("Mixed"); const height = getStatsProperty("H")?.querySelector( ".drag-input", ) as HTMLInputElement; - expect(height).not.toBeNull(); + expect(height).toBeDefined(); expect(height.value).toBe("Mixed"); const angle = getStatsProperty("A")?.querySelector( ".drag-input", ) as HTMLInputElement; - expect(angle).not.toBeNull(); + expect(angle).toBeDefined(); expect(angle.value).toBe("0"); const fontSize = getStatsProperty("F")?.querySelector( ".drag-input", ) as HTMLInputElement; - expect(fontSize).not.toBeNull(); + expect(fontSize).toBeDefined(); // changing width does not affect text editInput(width, "200"); @@ -579,7 +612,7 @@ describe("stats for multiple elements", () => { ".drag-input", ) as HTMLInputElement; - expect(x).not.toBeNull(); + expect(x).toBeDefined(); expect(Number(x.value)).toBe(x1); editInput(x, "300"); @@ -592,7 +625,7 @@ describe("stats for multiple elements", () => { ".drag-input", ) as HTMLInputElement; - expect(y).not.toBeNull(); + expect(y).toBeDefined(); expect(Number(y.value)).toBe(y1); editInput(y, "200"); @@ -604,13 +637,13 @@ describe("stats for multiple elements", () => { const width = getStatsProperty("W")?.querySelector( ".drag-input", ) as HTMLInputElement; - expect(width).not.toBeNull(); + expect(width).toBeDefined(); expect(Number(width.value)).toBe(200); const height = getStatsProperty("H")?.querySelector( ".drag-input", ) as HTMLInputElement; - expect(height).not.toBeNull(); + expect(height).toBeDefined(); expect(Number(height.value)).toBe(200); editInput(width, "400");