From d668eaa060429e58d68aef3cf8a327ecbbd24502 Mon Sep 17 00:00:00 2001 From: zsviczian Date: Sat, 1 Nov 2025 11:48:26 +0000 Subject: [PATCH] Initial implementation of containerBehavior.margin --- packages/element/src/renderElement.ts | 16 +- packages/element/src/resizeElements.ts | 3 + packages/element/src/textElement.ts | 28 +-- packages/element/src/textMeasurements.ts | 11 +- packages/element/tests/textElement.test.ts | 32 ++-- .../excalidraw/actions/actionBoundText.tsx | 9 +- .../excalidraw/actions/actionProperties.tsx | 166 +++++++++++++++--- packages/excalidraw/components/Actions.tsx | 37 +--- packages/excalidraw/components/App.tsx | 8 +- packages/excalidraw/components/icons.tsx | 77 +++++++- packages/excalidraw/locales/en.json | 3 + packages/excalidraw/wysiwyg/textWysiwyg.tsx | 13 +- 12 files changed, 312 insertions(+), 91 deletions(-) diff --git a/packages/element/src/renderElement.ts b/packages/element/src/renderElement.ts index 8c17863ee0..209d022413 100644 --- a/packages/element/src/renderElement.ts +++ b/packages/element/src/renderElement.ts @@ -329,16 +329,24 @@ const generateElementCanvas = ( boundTextCanvasContext.translate(-shiftX, -shiftY); // Clear the bound text area boundTextCanvasContext.clearRect( - -(boundTextElement.width / 2 + BOUND_TEXT_PADDING) * + -( + boundTextElement.width / 2 + + (element.containerBehavior?.margin ?? BOUND_TEXT_PADDING) + ) * window.devicePixelRatio * scale, - -(boundTextElement.height / 2 + BOUND_TEXT_PADDING) * + -( + boundTextElement.height / 2 + + (element.containerBehavior?.margin ?? BOUND_TEXT_PADDING) + ) * window.devicePixelRatio * scale, - (boundTextElement.width + BOUND_TEXT_PADDING * 2) * + (boundTextElement.width + + (element.containerBehavior?.margin ?? BOUND_TEXT_PADDING) * 2) * window.devicePixelRatio * scale, - (boundTextElement.height + BOUND_TEXT_PADDING * 2) * + (boundTextElement.height + + (element.containerBehavior?.margin ?? BOUND_TEXT_PADDING) * 2) * window.devicePixelRatio * scale, ); diff --git a/packages/element/src/resizeElements.ts b/packages/element/src/resizeElements.ts index 8cfd807855..4952151c32 100644 --- a/packages/element/src/resizeElements.ts +++ b/packages/element/src/resizeElements.ts @@ -12,6 +12,7 @@ import { SHIFT_LOCKING_ANGLE, rescalePoints, getFontString, + BOUND_TEXT_PADDING, } from "@excalidraw/common"; import type { GlobalPoint } from "@excalidraw/math"; @@ -741,10 +742,12 @@ export const resizeSingleElement = ( const minWidth = getApproxMinLineWidth( getFontString(boundTextElement), boundTextElement.lineHeight, + latestElement.containerBehavior?.margin ?? BOUND_TEXT_PADDING, ); const minHeight = getApproxMinLineHeight( boundTextElement.fontSize, boundTextElement.lineHeight, + latestElement.containerBehavior?.margin ?? BOUND_TEXT_PADDING, ); nextWidth = Math.max(nextWidth, minWidth); nextHeight = Math.max(nextHeight, minHeight); diff --git a/packages/element/src/textElement.ts b/packages/element/src/textElement.ts index 523a8b8804..5f0fdee3fc 100644 --- a/packages/element/src/textElement.ts +++ b/packages/element/src/textElement.ts @@ -108,6 +108,7 @@ export const redrawTextBoundingBox = ( const nextHeight = computeContainerDimensionForBoundText( metrics.height, container.type, + container.containerBehavior?.margin ?? BOUND_TEXT_PADDING, ); scene.mutateElement(container, { height: nextHeight }); updateOriginalContainerCache(container.id, nextHeight); @@ -117,6 +118,7 @@ export const redrawTextBoundingBox = ( const nextWidth = computeContainerDimensionForBoundText( metrics.width, container.type, + container.containerBehavior?.margin ?? BOUND_TEXT_PADDING, ); scene.mutateElement(container, { width: nextWidth }); } @@ -187,6 +189,7 @@ export const handleBindTextResize = ( containerHeight = computeContainerDimensionForBoundText( nextHeight, container.type, + container.containerBehavior?.margin ?? BOUND_TEXT_PADDING, ); const diff = containerHeight - container.height; @@ -353,8 +356,8 @@ export const getContainerCenter = ( }; export const getContainerCoords = (container: NonDeletedExcalidrawElement) => { - let offsetX = BOUND_TEXT_PADDING; - let offsetY = BOUND_TEXT_PADDING; + let offsetX = container.containerBehavior?.margin ?? BOUND_TEXT_PADDING; + let offsetY = container.containerBehavior?.margin ?? BOUND_TEXT_PADDING; if (container.type === "ellipse") { // The derivation of coordinates is explained in https://github.com/excalidraw/excalidraw/pull/6172 @@ -446,9 +449,10 @@ export const isValidTextContainer = (element: { export const computeContainerDimensionForBoundText = ( dimension: number, containerType: ExtractSetType, + boundTextPadding: number, ) => { dimension = Math.ceil(dimension); - const padding = BOUND_TEXT_PADDING * 2; + const padding = boundTextPadding * 2; if (containerType === "ellipse") { return Math.round(((dimension + padding) / Math.sqrt(2)) * 2); @@ -467,6 +471,8 @@ export const getBoundTextMaxWidth = ( boundTextElement: ExcalidrawTextElement | null, ) => { const { width } = container; + const boundTextPadding = + container.containerBehavior?.margin ?? BOUND_TEXT_PADDING; if (isArrowElement(container)) { const minWidth = (boundTextElement?.fontSize ?? DEFAULT_FONT_SIZE) * @@ -477,14 +483,14 @@ export const getBoundTextMaxWidth = ( // The width of the largest rectangle inscribed inside an ellipse is // Math.round((ellipse.width / 2) * Math.sqrt(2)) which is derived from // equation of an ellipse -https://github.com/excalidraw/excalidraw/pull/6172 - return Math.round((width / 2) * Math.sqrt(2)) - BOUND_TEXT_PADDING * 2; + return Math.round((width / 2) * Math.sqrt(2)) - boundTextPadding * 2; } if (container.type === "diamond") { // The width of the largest rectangle inscribed inside a rhombus is // Math.round(width / 2) - https://github.com/excalidraw/excalidraw/pull/6265 - return Math.round(width / 2) - BOUND_TEXT_PADDING * 2; + return Math.round(width / 2) - boundTextPadding * 2; } - return width - BOUND_TEXT_PADDING * 2; + return width - boundTextPadding * 2; }; export const getBoundTextMaxHeight = ( @@ -492,8 +498,10 @@ export const getBoundTextMaxHeight = ( boundTextElement: ExcalidrawTextElementWithContainer, ) => { const { height } = container; + const boundTextPadding = + container.containerBehavior?.margin ?? BOUND_TEXT_PADDING; if (isArrowElement(container)) { - const containerHeight = height - BOUND_TEXT_PADDING * 8 * 2; + const containerHeight = height - boundTextPadding * 8 * 2; if (containerHeight <= 0) { return boundTextElement.height; } @@ -503,14 +511,14 @@ export const getBoundTextMaxHeight = ( // The height of the largest rectangle inscribed inside an ellipse is // Math.round((ellipse.height / 2) * Math.sqrt(2)) which is derived from // equation of an ellipse - https://github.com/excalidraw/excalidraw/pull/6172 - return Math.round((height / 2) * Math.sqrt(2)) - BOUND_TEXT_PADDING * 2; + return Math.round((height / 2) * Math.sqrt(2)) - boundTextPadding * 2; } if (container.type === "diamond") { // The height of the largest rectangle inscribed inside a rhombus is // Math.round(height / 2) - https://github.com/excalidraw/excalidraw/pull/6265 - return Math.round(height / 2) - BOUND_TEXT_PADDING * 2; + return Math.round(height / 2) - boundTextPadding * 2; } - return height - BOUND_TEXT_PADDING * 2; + return height - boundTextPadding * 2; }; /** retrieves text from text elements and concatenates to a single string */ diff --git a/packages/element/src/textMeasurements.ts b/packages/element/src/textMeasurements.ts index 5e790261d1..984b7b5d53 100644 --- a/packages/element/src/textMeasurements.ts +++ b/packages/element/src/textMeasurements.ts @@ -32,22 +32,24 @@ const DUMMY_TEXT = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789".toLocaleUpperCase(); export const getApproxMinLineWidth = ( font: FontString, lineHeight: ExcalidrawTextElement["lineHeight"], + boundTextPadding: number = BOUND_TEXT_PADDING, ) => { const maxCharWidth = getMaxCharWidth(font); if (maxCharWidth === 0) { return ( measureText(DUMMY_TEXT.split("").join("\n"), font, lineHeight).width + - BOUND_TEXT_PADDING * 2 + boundTextPadding * 2 ); } - return maxCharWidth + BOUND_TEXT_PADDING * 2; + return maxCharWidth + boundTextPadding * 2; }; export const getMinTextElementWidth = ( font: FontString, lineHeight: ExcalidrawTextElement["lineHeight"], + boundTextPadding: number = BOUND_TEXT_PADDING, ) => { - return measureText("", font, lineHeight).width + BOUND_TEXT_PADDING * 2; + return measureText("", font, lineHeight).width + boundTextPadding * 2; }; export const isMeasureTextSupported = () => { @@ -99,8 +101,9 @@ export const getLineHeightInPx = ( export const getApproxMinLineHeight = ( fontSize: ExcalidrawTextElement["fontSize"], lineHeight: ExcalidrawTextElement["lineHeight"], + boundTextPadding: number = BOUND_TEXT_PADDING, ) => { - return getLineHeightInPx(fontSize, lineHeight) + BOUND_TEXT_PADDING * 2; + return getLineHeightInPx(fontSize, lineHeight) + boundTextPadding * 2; }; let textMetricsProvider: TextMetricsProvider | undefined; diff --git a/packages/element/tests/textElement.test.ts b/packages/element/tests/textElement.test.ts index 986854b985..dc7828051c 100644 --- a/packages/element/tests/textElement.test.ts +++ b/packages/element/tests/textElement.test.ts @@ -1,4 +1,4 @@ -import { getLineHeight } from "@excalidraw/common"; +import { BOUND_TEXT_PADDING, getLineHeight } from "@excalidraw/common"; import { API } from "@excalidraw/excalidraw/tests/helpers/api"; import { FONT_FAMILY, TEXT_ALIGN, VERTICAL_ALIGN } from "@excalidraw/common"; @@ -63,9 +63,13 @@ describe("Test measureText", () => { type: "rectangle", ...params, }); - expect(computeContainerDimensionForBoundText(150, element.type)).toEqual( - 160, - ); + expect( + computeContainerDimensionForBoundText( + 150, + element.type, + BOUND_TEXT_PADDING, + ), + ).toEqual(160); }); it("should compute container height correctly for ellipse", () => { @@ -73,9 +77,13 @@ describe("Test measureText", () => { type: "ellipse", ...params, }); - expect(computeContainerDimensionForBoundText(150, element.type)).toEqual( - 226, - ); + expect( + computeContainerDimensionForBoundText( + 150, + element.type, + BOUND_TEXT_PADDING, + ), + ).toEqual(226); }); it("should compute container height correctly for diamond", () => { @@ -83,9 +91,13 @@ describe("Test measureText", () => { type: "diamond", ...params, }); - expect(computeContainerDimensionForBoundText(150, element.type)).toEqual( - 320, - ); + expect( + computeContainerDimensionForBoundText( + 150, + element.type, + BOUND_TEXT_PADDING, + ), + ).toEqual(320); }); }); diff --git a/packages/excalidraw/actions/actionBoundText.tsx b/packages/excalidraw/actions/actionBoundText.tsx index 8b47587fec..b0e307e903 100644 --- a/packages/excalidraw/actions/actionBoundText.tsx +++ b/packages/excalidraw/actions/actionBoundText.tsx @@ -236,6 +236,9 @@ export const actionWrapTextInContainer = register({ let updatedElements: readonly ExcalidrawElement[] = elements.slice(); const containerIds: Mutable = {}; + const boundTextPadding = + appState.currentItemContainerBehavior?.margin ?? BOUND_TEXT_PADDING; + for (const textElement of selectedElements) { if (isTextElement(textElement) && !isBoundToContainer(textElement)) { const container = newElement({ @@ -261,15 +264,17 @@ export const actionWrapTextInContainer = register({ : null, opacity: 100, locked: false, - x: textElement.x - BOUND_TEXT_PADDING, - y: textElement.y - BOUND_TEXT_PADDING, + x: textElement.x - boundTextPadding, + y: textElement.y - boundTextPadding, width: computeContainerDimensionForBoundText( textElement.width, "rectangle", + boundTextPadding, ), height: computeContainerDimensionForBoundText( textElement.height, "rectangle", + boundTextPadding, ), groupIds: textElement.groupIds, frameId: textElement.frameId, diff --git a/packages/excalidraw/actions/actionProperties.tsx b/packages/excalidraw/actions/actionProperties.tsx index 47d6dd90d3..1a806180b6 100644 --- a/packages/excalidraw/actions/actionProperties.tsx +++ b/packages/excalidraw/actions/actionProperties.tsx @@ -26,6 +26,7 @@ import { import { canBecomePolygon, getNonDeletedElements, + hasContainerBehavior, isFlowchartNodeElement, } from "@excalidraw/element"; @@ -132,6 +133,9 @@ import { ArrowheadCrowfootOneOrManyIcon, stickyNoteIcon, growingContainerIcon, + marginLargeIcon, + marginMediumIcon, + marginSmallIcon, } from "../components/icons"; import { Fonts } from "../fonts"; @@ -1518,6 +1522,35 @@ export const actionChangeRoundness = register({ }, }); +const getMargin = (value: "small" | "medium" | "large") => { + switch (value) { + case "small": + return BOUND_TEXT_PADDING; + case "medium": + return 15; + case "large": + return 25; + default: + return BOUND_TEXT_PADDING; + } +}; + +const getMarginValue = (margin: number | null) => { + if (margin === null) { + return null; + } + switch (margin) { + case BOUND_TEXT_PADDING: + return "small"; + case 15: + return "medium"; + case 25: + return "large"; + default: + return null; + } +}; + export const actionChangeContainerBehavior = register({ name: "changeContainerBehavior", label: "labels.container", @@ -1536,7 +1569,7 @@ export const actionChangeContainerBehavior = register({ // collect directly selected eligible containers for (const el of selected) { - if (isFlowchartNodeElement(el) && getBoundTextElement(el, elementsMap)) { + if (isFlowchartNodeElement(el)) { containerIdsToUpdate.add(el.id); } } @@ -1558,12 +1591,61 @@ export const actionChangeContainerBehavior = register({ } } - if (containerIdsToUpdate.size === 0) { - // nothing to update - return false; + if (value.hasOwnProperty("margin")) { + if (containerIdsToUpdate.size === 0) { + return { + appState: { + ...appState, + currentItemContainerBehavior: { + textFlow: + appState.currentItemContainerBehavior?.textFlow ?? "growing", + margin: getMargin(value.margin as "small" | "medium" | "large"), + }, + }, + captureUpdate: CaptureUpdateAction.IMMEDIATELY, + }; + } + + const nextElements = changeProperty(elements, appState, (el) => + containerIdsToUpdate.has(el.id) + ? newElementWith(el, { + containerBehavior: { + textFlow: el.containerBehavior?.textFlow ?? "growing", + margin: getMargin(value.margin as "small" | "medium" | "large"), + }, + }) + : el, + ); + + // Invalidate containers to trigger re-render + containerIdsToUpdate.forEach((id) => { + const container = nextElements.find((el) => el.id === id); + if (container) { + const boundText = getBoundTextElement( + container, + arrayToMap(nextElements), + ); + if (boundText) { + redrawTextBoundingBox(boundText, container, app.scene); + } + } + }); + + return { + elements: nextElements, + appState: { + ...appState, + currentItemContainerBehavior: { + textFlow: + appState.currentItemContainerBehavior?.textFlow ?? "growing", + margin: getMargin(value.margin as "small" | "medium" | "large"), + }, + }, + captureUpdate: CaptureUpdateAction.IMMEDIATELY, + }; } - const nextElements = elements.map((el) => + const nextElements = changeProperty(elements, appState, (el) => containerIdsToUpdate.has(el.id) ? newElementWith(el, { containerBehavior: { @@ -1615,29 +1697,39 @@ export const actionChangeContainerBehavior = register({ } } else { // case 2: any eligible containers directly selected - targetContainers = selected.filter( - (el) => - isFlowchartNodeElement(el) && getBoundTextElement(el, elementsMap), - ); + targetContainers = selected.filter((el) => isFlowchartNodeElement(el)); } - if (targetContainers.length === 0) { + if ( + targetContainers.length === 0 && + !hasContainerBehavior(appState.activeTool.type) + ) { return null; } - const value = - reduceToCommonValue( - targetContainers, - (el) => el.containerBehavior?.textFlow ?? "growing", - ) ?? - // mixed selection -> show null so nothing appears selected - null; + const textFlow = + targetContainers.length === 0 + ? appState.currentItemContainerBehavior?.textFlow ?? "growing" + : reduceToCommonValue( + targetContainers, + (el) => el.containerBehavior?.textFlow ?? "growing", + ) ?? + // mixed selection -> show null so nothing appears selected + null; + + const marginValue = + targetContainers.length === 0 + ? appState.currentItemContainerBehavior?.margin ?? BOUND_TEXT_PADDING + : reduceToCommonValue( + targetContainers, + (el) => el.containerBehavior?.margin ?? BOUND_TEXT_PADDING, + ) ?? + // mixed selection -> show null so nothing appears selected + null; return (
- {appState.stylesPanelMode === "full" && ( - {t("labels.container")} - )} + {t("labels.container")}
updateData(val)} + onChange={(val) => updateData({ textFlow: val })} + /> +
+
+ updateData({ margin: val })} />
diff --git a/packages/excalidraw/components/Actions.tsx b/packages/excalidraw/components/Actions.tsx index 196a1883ad..cf0c1501d8 100644 --- a/packages/excalidraw/components/Actions.tsx +++ b/packages/excalidraw/components/Actions.tsx @@ -223,11 +223,10 @@ export const SelectedShapeActions = ({ <>{renderAction("changeRoundness")} )} - {hasContainerBehavior(appState.activeTool.type) || - (targetElements.some( - (element) => - isFlowchartNodeElement(element) && hasBoundTextElement(element), - ) && <>{renderAction("changeContainerBehavior")})} + {(hasContainerBehavior(appState.activeTool.type) || + targetElements.some((element) => isFlowchartNodeElement(element))) && ( + <>{renderAction("changeContainerBehavior")} + )} {(toolIsArrow(appState.activeTool.type) || targetElements.some((element) => toolIsArrow(element.type))) && ( @@ -419,12 +418,10 @@ const CombinedShapeProperties = ({ canChangeRoundness(element.type), )) && renderAction("changeRoundness")} - {hasContainerBehavior(appState.activeTool.type) || - (targetElements.some( - (element) => - isFlowchartNodeElement(element) && - hasBoundTextElement(element), - ) && <>{renderAction("changeContainerBehavior")})} + {(hasContainerBehavior(appState.activeTool.type) || + targetElements.some((element) => + isFlowchartNodeElement(element), + )) && <>{renderAction("changeContainerBehavior")}} {renderAction("changeOpacity")} @@ -832,24 +829,6 @@ export const CompactShapeActions = ({ appState.editingTextElement || appState.newElement, ); - const textContainer = - targetElements.length === 1 && isTextElement(targetElements[0]) - ? getContainerElement(targetElements[0], elementsMap) - : null; - - const isStickyNoteContainer = - textContainer && isFlowchartNodeElement(textContainer); - - const showFillIcons = - (hasBackground(appState.activeTool.type) && - !isTransparent(appState.currentItemBackgroundColor)) || - targetElements.some( - (element) => - hasBackground(element.type) && !isTransparent(element.backgroundColor), - ); - - const showLinkIcon = targetElements.length === 1; - const showLineEditorAction = !appState.selectedLinearElement?.isEditing && targetElements.length === 1 && diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index 878960a738..c600b35399 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -104,6 +104,7 @@ import { MQ_MAX_TABLET, MQ_MAX_HEIGHT_LANDSCAPE, MQ_MAX_WIDTH_LANDSCAPE, + BOUND_TEXT_PADDING, } from "@excalidraw/common"; import { @@ -5413,8 +5414,13 @@ class App extends React.Component { const minWidth = getApproxMinLineWidth( getFontString(fontString), lineHeight, + container.containerBehavior?.margin ?? BOUND_TEXT_PADDING, + ); + const minHeight = getApproxMinLineHeight( + fontSize, + lineHeight, + container.containerBehavior?.margin ?? BOUND_TEXT_PADDING, ); - const minHeight = getApproxMinLineHeight(fontSize, lineHeight); const newHeight = Math.max(container.height, minHeight); const newWidth = Math.max(container.width, minWidth); this.scene.mutateElement(container, { diff --git a/packages/excalidraw/components/icons.tsx b/packages/excalidraw/components/icons.tsx index 8b39ffb8a5..5884201dcf 100644 --- a/packages/excalidraw/components/icons.tsx +++ b/packages/excalidraw/components/icons.tsx @@ -2328,19 +2328,82 @@ export const strokeIcon = createIcon( ); export const stickyNoteIcon = createIcon( - + , tablerIconProps, ); +export const marginSmallIcon = createIcon( + + + + , + tablerIconProps, +); + +export const marginMediumIcon = createIcon( + + + + , + tablerIconProps, +); + +export const marginLargeIcon = createIcon( + + + + , + tablerIconProps, +); + export const growingContainerIcon = createIcon( maxHeight) { const targetContainerHeight = - computeContainerDimensionForBoundText(height, container.type); + computeContainerDimensionForBoundText( + height, + container.type, + container.containerBehavior?.margin ?? BOUND_TEXT_PADDING, + ); app.scene.mutateElement(container, { height: targetContainerHeight, @@ -262,7 +267,11 @@ export const textWysiwyg = ({ height < maxHeight ) { const targetContainerHeight = - computeContainerDimensionForBoundText(height, container.type); + computeContainerDimensionForBoundText( + height, + container.type, + container.containerBehavior?.margin ?? BOUND_TEXT_PADDING, + ); app.scene.mutateElement(container, { height: targetContainerHeight, });