From 87c87a9fb1b276988316444b82d0f335a45f03cc Mon Sep 17 00:00:00 2001 From: zsviczian Date: Mon, 26 May 2025 11:14:55 +0200 Subject: [PATCH] feat: line polygons (#9477) * Loop Lock/Unlock * fixed condition. 4 line points are required for the action to be available * extracted updateLoopLock to improve readability. Removed unnecessary SVG attributes * lint + added loopLock to restore.ts * added loopLock to newElement, updated test snapshots * lint * dislocate enpoint when breaking the loop. * change icon & turn into a state style button * POC: auto-transform to polygon on bg set * keep polygon icon constant * do not split points on de-polygonizing & highlight overlapping points * rewrite color picker to support no (mixed) colors & fix focus handling * refactor * tweak point rendering inside line editor * do not disable polygon when creating new points via alt * auto-enable polygon when aligning start/end points * TBD: remove bg color when disabling polygon * TBD: only show polygon button for enabled polygons * fix polygon behavior when adding/removing/moving points within line editor * convert to polygon when creating line * labels tweak * add to command palette * loopLock -> polygon * restore `polygon` state on type conversions * update snapshots * naming * break polygon on restore/finalize if invalid & prevent creation * snapshots * fix: merge issue and forgotten debug * snaps * do not merge points for 3-point lines --------- Co-authored-by: dwelle <5153846+dwelle@users.noreply.github.com> --- packages/common/src/constants.ts | 7 + packages/element/src/linearElementEditor.ts | 205 ++++++++++++------ packages/element/src/newElement.ts | 17 +- packages/element/src/shapes.ts | 48 ++++ packages/element/src/typeChecks.ts | 32 +++ packages/element/src/types.ts | 12 +- .../actions/actionDeleteSelected.tsx | 6 +- .../excalidraw/actions/actionFinalize.tsx | 49 ++++- .../excalidraw/actions/actionLinearEditor.tsx | 129 ++++++++++- .../excalidraw/actions/actionProperties.tsx | 61 ++++-- packages/excalidraw/actions/manager.tsx | 1 + packages/excalidraw/actions/types.ts | 7 +- packages/excalidraw/components/ButtonIcon.tsx | 2 + .../CommandPalette/CommandPalette.tsx | 1 + packages/excalidraw/components/icons.tsx | 15 ++ .../data/__snapshots__/transform.test.ts.snap | 2 + packages/excalidraw/data/restore.ts | 16 +- packages/excalidraw/data/transform.ts | 2 +- packages/excalidraw/locales/en.json | 4 + packages/excalidraw/renderer/helpers.ts | 7 +- .../excalidraw/renderer/interactiveScene.ts | 44 +++- .../__snapshots__/dragCreate.test.tsx.snap | 1 + .../multiPointCreate.test.tsx.snap | 1 + .../regressionTests.test.tsx.snap | 4 + .../__snapshots__/selection.test.tsx.snap | 1 + .../data/__snapshots__/restore.test.ts.snap | 2 + 26 files changed, 555 insertions(+), 121 deletions(-) diff --git a/packages/common/src/constants.ts b/packages/common/src/constants.ts index 3850ed1c7..b9e5661a9 100644 --- a/packages/common/src/constants.ts +++ b/packages/common/src/constants.ts @@ -477,3 +477,10 @@ export enum UserIdleState { AWAY = "away", IDLE = "idle", } + +/** + * distance at which we merge points instead of adding a new merge-point + * when converting a line to a polygon (merge currently means overlaping + * the start and end points) + */ +export const LINE_POLYGON_POINT_MERGE_DISTANCE = 20; diff --git a/packages/element/src/linearElementEditor.ts b/packages/element/src/linearElementEditor.ts index eec3fc7a0..ae752eeae 100644 --- a/packages/element/src/linearElementEditor.ts +++ b/packages/element/src/linearElementEditor.ts @@ -63,10 +63,13 @@ import { getControlPointsForBezierCurve, mapIntervalToBezierT, getBezierXY, + toggleLinePolygonState, } from "./shapes"; import { getLockedLinearCursorAlignSize } from "./sizeHelpers"; +import { isLineElement } from "./typeChecks"; + import type { Scene } from "./Scene"; import type { Bounds } from "./bounds"; @@ -85,6 +88,35 @@ import type { PointsPositionUpdates, } from "./types"; +/** + * Normalizes line points so that the start point is at [0,0]. This is + * expected in various parts of the codebase. + * + * Also returns the offsets - [0,0] if no normalization needed. + * + * @private + */ +const getNormalizedPoints = ({ + points, +}: { + points: ExcalidrawLinearElement["points"]; +}): { + points: LocalPoint[]; + offsetX: number; + offsetY: number; +} => { + const offsetX = points[0][0]; + const offsetY = points[0][1]; + + return { + points: points.map((p) => { + return pointFrom(p[0] - offsetX, p[1] - offsetY); + }), + offsetX, + offsetY, + }; +}; + export class LinearElementEditor { public readonly elementId: ExcalidrawElement["id"] & { _brand: "excalidrawLinearElementId"; @@ -127,7 +159,11 @@ export class LinearElementEditor { }; if (!pointsEqual(element.points[0], pointFrom(0, 0))) { console.error("Linear element is not normalized", Error().stack); - LinearElementEditor.normalizePoints(element, elementsMap); + mutateElement( + element, + elementsMap, + LinearElementEditor.getNormalizeElementPointsAndCoords(element), + ); } this.selectedPointsIndices = null; this.lastUncommittedPoint = null; @@ -459,6 +495,18 @@ export class LinearElementEditor { selectedPoint === element.points.length - 1 ) { if (isPathALoop(element.points, appState.zoom.value)) { + if (isLineElement(element)) { + scene.mutateElement( + element, + { + ...toggleLinePolygonState(element, true), + }, + { + informMutation: false, + isDragging: false, + }, + ); + } LinearElementEditor.movePoints( element, scene, @@ -946,9 +994,7 @@ export class LinearElementEditor { if (!event.altKey) { if (lastPoint === lastUncommittedPoint) { - LinearElementEditor.deletePoints(element, app.scene, [ - points.length - 1, - ]); + LinearElementEditor.deletePoints(element, app, [points.length - 1]); } return { ...appState.editingLinearElement, @@ -999,7 +1045,7 @@ export class LinearElementEditor { ]), ); } else { - LinearElementEditor.addPoints(element, app.scene, [{ point: newPoint }]); + LinearElementEditor.addPoints(element, app.scene, [newPoint]); } return { ...appState.editingLinearElement, @@ -1142,40 +1188,23 @@ export class LinearElementEditor { /** * Normalizes line points so that the start point is at [0,0]. This is - * expected in various parts of the codebase. Also returns new x/y to account - * for the potential normalization. + * expected in various parts of the codebase. + * + * Also returns normalized x and y coords to account for the normalization + * of the points. */ - static getNormalizedPoints(element: ExcalidrawLinearElement): { - points: LocalPoint[]; - x: number; - y: number; - } { - const { points } = element; - - const offsetX = points[0][0]; - const offsetY = points[0][1]; + static getNormalizeElementPointsAndCoords(element: ExcalidrawLinearElement) { + const { points, offsetX, offsetY } = getNormalizedPoints(element); return { - points: points.map((p) => { - return pointFrom(p[0] - offsetX, p[1] - offsetY); - }), + points, x: element.x + offsetX, y: element.y + offsetY, }; } + // element-mutating methods // --------------------------------------------------------------------------- - static normalizePoints( - element: NonDeleted, - elementsMap: ElementsMap, - ) { - mutateElement( - element, - elementsMap, - LinearElementEditor.getNormalizedPoints(element), - ); - } - static duplicateSelectedPoints(appState: AppState, scene: Scene): AppState { invariant( appState.editingLinearElement, @@ -1254,41 +1283,47 @@ export class LinearElementEditor { static deletePoints( element: NonDeleted, - scene: Scene, + app: AppClassProperties, pointIndices: readonly number[], ) { - let offsetX = 0; - let offsetY = 0; + const isUncommittedPoint = + app.state.editingLinearElement?.lastUncommittedPoint === + element.points[element.points.length - 1]; - const isDeletingOriginPoint = pointIndices.includes(0); + const isPolygon = isLineElement(element) && element.polygon; - // if deleting first point, make the next to be [0,0] and recalculate - // positions of the rest with respect to it - if (isDeletingOriginPoint) { - const firstNonDeletedPoint = element.points.find((point, idx) => { - return !pointIndices.includes(idx); - }); - if (firstNonDeletedPoint) { - offsetX = firstNonDeletedPoint[0]; - offsetY = firstNonDeletedPoint[1]; - } + // break polygon if deleting start/end point + if ( + isPolygon && + // don't disable polygon if cleaning up uncommitted point + !isUncommittedPoint && + (pointIndices.includes(0) || + pointIndices.includes(element.points.length - 1)) + ) { + app.scene.mutateElement(element, { polygon: false }); } - const nextPoints = element.points.reduce((acc: LocalPoint[], p, idx) => { - if (!pointIndices.includes(idx)) { - acc.push( - !acc.length - ? pointFrom(0, 0) - : pointFrom(p[0] - offsetX, p[1] - offsetY), - ); - } - return acc; - }, []); + const nextPoints = element.points.filter((_, idx) => { + return !pointIndices.includes(idx); + }); + + if (isUncommittedPoint && isLineElement(element) && element.polygon) { + nextPoints[0] = pointFrom( + nextPoints[nextPoints.length - 1][0], + nextPoints[nextPoints.length - 1][1], + ); + } + + const { + points: normalizedPoints, + offsetX, + offsetY, + } = getNormalizedPoints({ points: nextPoints }); LinearElementEditor._updatePoints( element, - scene, - nextPoints, + app.scene, + normalizedPoints, offsetX, offsetY, ); @@ -1297,16 +1332,27 @@ export class LinearElementEditor { static addPoints( element: NonDeleted, scene: Scene, - targetPoints: { point: LocalPoint }[], + addedPoints: LocalPoint[], ) { - const offsetX = 0; - const offsetY = 0; + const nextPoints = [...element.points, ...addedPoints]; + + if (isLineElement(element) && element.polygon) { + nextPoints[0] = pointFrom( + nextPoints[nextPoints.length - 1][0], + nextPoints[nextPoints.length - 1][1], + ); + } + + const { + points: normalizedPoints, + offsetX, + offsetY, + } = getNormalizedPoints({ points: nextPoints }); - const nextPoints = [...element.points, ...targetPoints.map((x) => x.point)]; LinearElementEditor._updatePoints( element, scene, - nextPoints, + normalizedPoints, offsetX, offsetY, ); @@ -1323,17 +1369,37 @@ export class LinearElementEditor { ) { const { points } = element; + // if polygon, move start and end points together + if (isLineElement(element) && element.polygon) { + const firstPointUpdate = pointUpdates.get(0); + const lastPointUpdate = pointUpdates.get(points.length - 1); + + if (firstPointUpdate) { + pointUpdates.set(points.length - 1, { + point: pointFrom( + firstPointUpdate.point[0], + firstPointUpdate.point[1], + ), + isDragging: firstPointUpdate.isDragging, + }); + } else if (lastPointUpdate) { + pointUpdates.set(0, { + point: pointFrom(lastPointUpdate.point[0], lastPointUpdate.point[1]), + isDragging: lastPointUpdate.isDragging, + }); + } + } + // in case we're moving start point, instead of modifying its position // which would break the invariant of it being at [0,0], we move // all the other points in the opposite direction by delta to // offset it. We do the same with actual element.x/y position, so // this hacks are completely transparent to the user. - const [deltaX, deltaY] = + + const updatedOriginPoint = pointUpdates.get(0)?.point ?? pointFrom(0, 0); - const [offsetX, offsetY] = pointFrom( - deltaX - points[0][0], - deltaY - points[0][1], - ); + + const [offsetX, offsetY] = updatedOriginPoint; const nextPoints = isElbowArrow(element) ? [ @@ -1503,6 +1569,7 @@ export class LinearElementEditor { isDragging: options?.isDragging ?? false, }); } else { + // TODO do we need to get precise coords here just to calc centers? const nextCoords = getElementPointsCoords(element, nextPoints); const prevCoords = getElementPointsCoords(element, element.points); const nextCenterX = (nextCoords[0] + nextCoords[2]) / 2; @@ -1511,7 +1578,7 @@ export class LinearElementEditor { const prevCenterY = (prevCoords[1] + prevCoords[3]) / 2; const dX = prevCenterX - nextCenterX; const dY = prevCenterY - nextCenterY; - const rotated = pointRotateRads( + const rotatedOffset = pointRotateRads( pointFrom(offsetX, offsetY), pointFrom(dX, dY), element.angle, @@ -1519,8 +1586,8 @@ export class LinearElementEditor { scene.mutateElement(element, { ...otherUpdates, points: nextPoints, - x: element.x + rotated[0], - y: element.y + rotated[1], + x: element.x + rotatedOffset[0], + y: element.y + rotatedOffset[1], }); } } diff --git a/packages/element/src/newElement.ts b/packages/element/src/newElement.ts index 2ce9f306c..69ccaf595 100644 --- a/packages/element/src/newElement.ts +++ b/packages/element/src/newElement.ts @@ -25,6 +25,8 @@ import { getBoundTextMaxWidth } from "./textElement"; import { normalizeText, measureText } from "./textMeasurements"; import { wrapText } from "./textWrapping"; +import { isLineElement } from "./typeChecks"; + import type { ExcalidrawElement, ExcalidrawImageElement, @@ -45,6 +47,7 @@ import type { ElementsMap, ExcalidrawArrowElement, ExcalidrawElbowArrowElement, + ExcalidrawLineElement, } from "./types"; export type ElementConstructorOpts = MarkOptional< @@ -457,9 +460,10 @@ export const newLinearElement = ( opts: { type: ExcalidrawLinearElement["type"]; points?: ExcalidrawLinearElement["points"]; + polygon?: ExcalidrawLineElement["polygon"]; } & ElementConstructorOpts, ): NonDeleted => { - return { + const element = { ..._newElementBase(opts.type, opts), points: opts.points || [], lastCommittedPoint: null, @@ -468,6 +472,17 @@ export const newLinearElement = ( startArrowhead: null, endArrowhead: null, }; + + if (isLineElement(element)) { + const lineElement: NonDeleted = { + ...element, + polygon: opts.polygon ?? false, + }; + + return lineElement; + } + + return element; }; export const newArrowElement = ( diff --git a/packages/element/src/shapes.ts b/packages/element/src/shapes.ts index 96542c538..20041ce1b 100644 --- a/packages/element/src/shapes.ts +++ b/packages/element/src/shapes.ts @@ -5,6 +5,7 @@ import { ROUNDNESS, invariant, elementCenterPoint, + LINE_POLYGON_POINT_MERGE_DISTANCE, } from "@excalidraw/common"; import { isPoint, @@ -35,10 +36,13 @@ import { ShapeCache } from "./ShapeCache"; import { getElementAbsoluteCoords, type Bounds } from "./bounds"; +import { canBecomePolygon } from "./typeChecks"; + import type { ElementsMap, ExcalidrawElement, ExcalidrawLinearElement, + ExcalidrawLineElement, NonDeleted, } from "./types"; @@ -396,3 +400,47 @@ export const isPathALoop = ( } return false; }; + +export const toggleLinePolygonState = ( + element: ExcalidrawLineElement, + nextPolygonState: boolean, +): { + polygon: ExcalidrawLineElement["polygon"]; + points: ExcalidrawLineElement["points"]; +} | null => { + const updatedPoints = [...element.points]; + + if (nextPolygonState) { + if (!canBecomePolygon(element.points)) { + return null; + } + + const firstPoint = updatedPoints[0]; + const lastPoint = updatedPoints[updatedPoints.length - 1]; + + const distance = Math.hypot( + firstPoint[0] - lastPoint[0], + firstPoint[1] - lastPoint[1], + ); + + if ( + distance > LINE_POLYGON_POINT_MERGE_DISTANCE || + updatedPoints.length < 4 + ) { + updatedPoints.push(pointFrom(firstPoint[0], firstPoint[1])); + } else { + updatedPoints[updatedPoints.length - 1] = pointFrom( + firstPoint[0], + firstPoint[1], + ); + } + } + + // TODO: satisfies ElementUpdate + const ret = { + polygon: nextPolygonState, + points: updatedPoints, + }; + + return ret; +}; diff --git a/packages/element/src/typeChecks.ts b/packages/element/src/typeChecks.ts index a69f4ffc9..37974f1f5 100644 --- a/packages/element/src/typeChecks.ts +++ b/packages/element/src/typeChecks.ts @@ -1,5 +1,7 @@ import { ROUNDNESS, assertNever } from "@excalidraw/common"; +import { pointsEqual } from "@excalidraw/math"; + import type { ElementOrToolType } from "@excalidraw/excalidraw/types"; import type { MarkNonNullable } from "@excalidraw/common/utility-types"; @@ -25,6 +27,7 @@ import type { ExcalidrawMagicFrameElement, ExcalidrawArrowElement, ExcalidrawElbowArrowElement, + ExcalidrawLineElement, PointBinding, FixedPointBinding, ExcalidrawFlowchartNodeElement, @@ -108,6 +111,12 @@ export const isLinearElement = ( return element != null && isLinearElementType(element.type); }; +export const isLineElement = ( + element?: ExcalidrawElement | null, +): element is ExcalidrawLineElement => { + return element != null && element.type === "line"; +}; + export const isArrowElement = ( element?: ExcalidrawElement | null, ): element is ExcalidrawArrowElement => { @@ -372,3 +381,26 @@ export const getLinearElementSubType = ( } return "line"; }; + +/** + * Checks if current element points meet all the conditions for polygon=true + * (this isn't a element type check, for that use isLineElement). + * + * If you want to check if points *can* be turned into a polygon, use + * canBecomePolygon(points). + */ +export const isValidPolygon = ( + points: ExcalidrawLineElement["points"], +): boolean => { + return points.length > 3 && pointsEqual(points[0], points[points.length - 1]); +}; + +export const canBecomePolygon = ( + points: ExcalidrawLineElement["points"], +): boolean => { + return ( + points.length > 3 || + // 3-point polygons can't have all points in a single line + (points.length === 3 && !pointsEqual(points[0], points[points.length - 1])) + ); +}; diff --git a/packages/element/src/types.ts b/packages/element/src/types.ts index d7a105900..23e4f9929 100644 --- a/packages/element/src/types.ts +++ b/packages/element/src/types.ts @@ -296,8 +296,10 @@ export type FixedPointBinding = Merge< } >; +type Index = number; + export type PointsPositionUpdates = Map< - number, + Index, { point: LocalPoint; isDragging?: boolean } >; @@ -326,10 +328,16 @@ export type ExcalidrawLinearElement = _ExcalidrawElementBase & endArrowhead: Arrowhead | null; }>; +export type ExcalidrawLineElement = ExcalidrawLinearElement & + Readonly<{ + type: "line"; + polygon: boolean; + }>; + export type FixedSegment = { start: LocalPoint; end: LocalPoint; - index: number; + index: Index; }; export type ExcalidrawArrowElement = ExcalidrawLinearElement & diff --git a/packages/excalidraw/actions/actionDeleteSelected.tsx b/packages/excalidraw/actions/actionDeleteSelected.tsx index c594a6e61..20d7d129f 100644 --- a/packages/excalidraw/actions/actionDeleteSelected.tsx +++ b/packages/excalidraw/actions/actionDeleteSelected.tsx @@ -258,11 +258,7 @@ export const actionDeleteSelected = register({ : endBindingElement, }; - LinearElementEditor.deletePoints( - element, - app.scene, - selectedPointsIndices, - ); + LinearElementEditor.deletePoints(element, app, selectedPointsIndices); return { elements, diff --git a/packages/excalidraw/actions/actionFinalize.tsx b/packages/excalidraw/actions/actionFinalize.tsx index bba3f4501..cd3960a53 100644 --- a/packages/excalidraw/actions/actionFinalize.tsx +++ b/packages/excalidraw/actions/actionFinalize.tsx @@ -5,9 +5,14 @@ import { bindOrUnbindLinearElement, isBindingEnabled, } from "@excalidraw/element/binding"; -import { LinearElementEditor } from "@excalidraw/element/linearElementEditor"; +import { isValidPolygon, LinearElementEditor } from "@excalidraw/element"; -import { isBindingElement, isLinearElement } from "@excalidraw/element"; +import { + isBindingElement, + isFreeDrawElement, + isLinearElement, + isLineElement, +} from "@excalidraw/element"; import { KEYS, arrayToMap, updateActiveTool } from "@excalidraw/common"; import { isPathALoop } from "@excalidraw/element"; @@ -16,6 +21,7 @@ import { isInvisiblySmallElement } from "@excalidraw/element"; import { CaptureUpdateAction } from "@excalidraw/element"; +import type { LocalPoint } from "@excalidraw/math"; import type { ExcalidrawElement, ExcalidrawLinearElement, @@ -93,6 +99,12 @@ export const actionFinalize = register({ scene, ); } + if (isLineElement(element) && !isValidPolygon(element.points)) { + scene.mutateElement(element, { + polygon: false, + }); + } + return { elements: element.points.length < 2 || isInvisiblySmallElement(element) @@ -166,25 +178,38 @@ export const actionFinalize = register({ newElements = newElements.filter((el) => el.id !== element!.id); } - if (isLinearElement(element) || element.type === "freedraw") { + if (isLinearElement(element) || isFreeDrawElement(element)) { // If the multi point line closes the loop, // set the last point to first point. // This ensures that loop remains closed at different scales. const isLoop = isPathALoop(element.points, appState.zoom.value); - if (element.type === "line" || element.type === "freedraw") { - if (isLoop) { - const linePoints = element.points; - const firstPoint = linePoints[0]; + + if (isLoop && (isLineElement(element) || isFreeDrawElement(element))) { + const linePoints = element.points; + const firstPoint = linePoints[0]; + const points: LocalPoint[] = linePoints.map((p, index) => + index === linePoints.length - 1 + ? pointFrom(firstPoint[0], firstPoint[1]) + : p, + ); + if (isLineElement(element)) { scene.mutateElement(element, { - points: linePoints.map((p, index) => - index === linePoints.length - 1 - ? pointFrom(firstPoint[0], firstPoint[1]) - : p, - ), + points, + polygon: true, + }); + } else { + scene.mutateElement(element, { + points, }); } } + if (isLineElement(element) && !isValidPolygon(element.points)) { + scene.mutateElement(element, { + polygon: false, + }); + } + if ( isBindingElement(element) && !isLoop && diff --git a/packages/excalidraw/actions/actionLinearEditor.tsx b/packages/excalidraw/actions/actionLinearEditor.tsx index 0cb783322..32eba3339 100644 --- a/packages/excalidraw/actions/actionLinearEditor.tsx +++ b/packages/excalidraw/actions/actionLinearEditor.tsx @@ -1,19 +1,29 @@ import { LinearElementEditor } from "@excalidraw/element"; - -import { isElbowArrow, isLinearElement } from "@excalidraw/element"; - +import { + isElbowArrow, + isLinearElement, + isLineElement, +} from "@excalidraw/element"; import { arrayToMap } from "@excalidraw/common"; import { CaptureUpdateAction } from "@excalidraw/element"; -import type { ExcalidrawLinearElement } from "@excalidraw/element/types"; +import type { + ExcalidrawLinearElement, + ExcalidrawLineElement, +} from "@excalidraw/element/types"; import { DEFAULT_CATEGORIES } from "../components/CommandPalette/CommandPalette"; import { ToolButton } from "../components/ToolButton"; -import { lineEditorIcon } from "../components/icons"; - +import { lineEditorIcon, polygonIcon } from "../components/icons"; import { t } from "../i18n"; +import { ButtonIcon } from "../components/ButtonIcon"; + +import { newElementWith } from "../../element/src/mutateElement"; + +import { toggleLinePolygonState } from "../../element/src/shapes"; + import { register } from "./register"; export const actionToggleLinearEditor = register({ @@ -83,3 +93,110 @@ export const actionToggleLinearEditor = register({ ); }, }); + +export const actionTogglePolygon = register({ + name: "togglePolygon", + category: DEFAULT_CATEGORIES.elements, + icon: polygonIcon, + keywords: ["loop"], + label: (elements, appState, app) => { + const selectedElements = app.scene.getSelectedElements({ + selectedElementIds: appState.selectedElementIds, + }); + + const allPolygons = !selectedElements.some( + (element) => !isLineElement(element) || !element.polygon, + ); + + return allPolygons + ? "labels.polygon.breakPolygon" + : "labels.polygon.convertToPolygon"; + }, + trackEvent: { + category: "element", + }, + predicate: (elements, appState, _, app) => { + const selectedElements = app.scene.getSelectedElements({ + selectedElementIds: appState.selectedElementIds, + }); + + return ( + selectedElements.length > 0 && + selectedElements.every( + (element) => isLineElement(element) && element.points.length >= 4, + ) + ); + }, + perform(elements, appState, _, app) { + const selectedElements = app.scene.getSelectedElements(appState); + + if (selectedElements.some((element) => !isLineElement(element))) { + return false; + } + + const targetElements = selectedElements as ExcalidrawLineElement[]; + + // if one element not a polygon, convert all to polygon + const nextPolygonState = targetElements.some((element) => !element.polygon); + + const targetElementsMap = arrayToMap(targetElements); + + return { + elements: elements.map((element) => { + if (!targetElementsMap.has(element.id) || !isLineElement(element)) { + return element; + } + + return newElementWith(element, { + backgroundColor: nextPolygonState + ? element.backgroundColor + : "transparent", + ...toggleLinePolygonState(element, nextPolygonState), + }); + }), + appState, + captureUpdate: CaptureUpdateAction.IMMEDIATELY, + }; + }, + PanelComponent: ({ appState, updateData, app }) => { + const selectedElements = app.scene.getSelectedElements({ + selectedElementIds: appState.selectedElementIds, + }); + + if ( + selectedElements.length === 0 || + selectedElements.some( + (element) => + !isLineElement(element) || + // only show polygon button if every selected element is already + // a polygon, effectively showing this button only to allow for + // disabling the polygon state + !element.polygon || + element.points.length < 3, + ) + ) { + return null; + } + + const allPolygon = selectedElements.every( + (element) => isLineElement(element) && element.polygon, + ); + + const label = t( + allPolygon + ? "labels.polygon.breakPolygon" + : "labels.polygon.convertToPolygon", + ); + + return ( + updateData(null)} + style={{ marginLeft: "auto" }} + /> + ); + }, +}); diff --git a/packages/excalidraw/actions/actionProperties.tsx b/packages/excalidraw/actions/actionProperties.tsx index 24906d6d6..30c9e7434 100644 --- a/packages/excalidraw/actions/actionProperties.tsx +++ b/packages/excalidraw/actions/actionProperties.tsx @@ -20,10 +20,11 @@ import { getShortcutKey, tupleToCoors, getLineHeight, + isTransparent, reduceToCommonValue, } from "@excalidraw/common"; -import { getNonDeletedElements } from "@excalidraw/element"; +import { canBecomePolygon, getNonDeletedElements } from "@excalidraw/element"; import { bindLinearElement, @@ -47,6 +48,7 @@ import { isBoundToContainer, isElbowArrow, isLinearElement, + isLineElement, isTextElement, isUsingAdaptiveRadius, } from "@excalidraw/element"; @@ -136,6 +138,8 @@ import { isSomeElementSelected, } from "../scene"; +import { toggleLinePolygonState } from "../../element/src/shapes"; + import { register } from "./register"; import type { AppClassProperties, AppState, Primitive } from "../types"; @@ -349,22 +353,52 @@ export const actionChangeBackgroundColor = register({ name: "changeBackgroundColor", label: "labels.changeBackground", trackEvent: false, - perform: (elements, appState, value) => { - return { - ...(value.currentItemBackgroundColor && { - elements: changeProperty(elements, appState, (el) => - newElementWith(el, { + perform: (elements, appState, value, app) => { + if (!value.currentItemBackgroundColor) { + return { + appState: { + ...appState, + ...value, + }, + captureUpdate: CaptureUpdateAction.EVENTUALLY, + }; + } + + let nextElements; + + const selectedElements = app.scene.getSelectedElements(appState); + const shouldEnablePolygon = + !isTransparent(value.currentItemBackgroundColor) && + selectedElements.every( + (el) => isLineElement(el) && canBecomePolygon(el.points), + ); + + if (shouldEnablePolygon) { + const selectedElementsMap = arrayToMap(selectedElements); + nextElements = elements.map((el) => { + if (selectedElementsMap.has(el.id) && isLineElement(el)) { + return newElementWith(el, { backgroundColor: value.currentItemBackgroundColor, - }), - ), - }), + ...toggleLinePolygonState(el, true), + }); + } + return el; + }); + } else { + nextElements = changeProperty(elements, appState, (el) => + newElementWith(el, { + backgroundColor: value.currentItemBackgroundColor, + }), + ); + } + + return { + elements: nextElements, appState: { ...appState, ...value, }, - captureUpdate: !!value.currentItemBackgroundColor - ? CaptureUpdateAction.IMMEDIATELY - : CaptureUpdateAction.EVENTUALLY, + captureUpdate: CaptureUpdateAction.IMMEDIATELY, }; }, PanelComponent: ({ elements, appState, updateData, app }) => ( @@ -1373,7 +1407,7 @@ export const actionChangeRoundness = register({ captureUpdate: CaptureUpdateAction.IMMEDIATELY, }; }, - PanelComponent: ({ elements, appState, updateData, app }) => { + PanelComponent: ({ elements, appState, updateData, app, renderAction }) => { const targetElements = getTargetElements( getNonDeletedElements(elements), appState, @@ -1417,6 +1451,7 @@ export const actionChangeRoundness = register({ )} onChange={(value) => updateData(value)} /> + {renderAction("togglePolygon")} ); diff --git a/packages/excalidraw/actions/manager.tsx b/packages/excalidraw/actions/manager.tsx index 171bb5df7..f3314bf35 100644 --- a/packages/excalidraw/actions/manager.tsx +++ b/packages/excalidraw/actions/manager.tsx @@ -179,6 +179,7 @@ export class ActionManager { appProps={this.app.props} app={this.app} data={data} + renderAction={this.renderAction} /> ); } diff --git a/packages/excalidraw/actions/types.ts b/packages/excalidraw/actions/types.ts index d5fbed4c0..e6f363126 100644 --- a/packages/excalidraw/actions/types.ts +++ b/packages/excalidraw/actions/types.ts @@ -142,7 +142,8 @@ export type ActionName = | "cropEditor" | "wrapSelectionInFrame" | "toggleLassoTool" - | "toggleShapeSwitch"; + | "toggleShapeSwitch" + | "togglePolygon"; export type PanelComponentProps = { elements: readonly ExcalidrawElement[]; @@ -151,6 +152,10 @@ export type PanelComponentProps = { appProps: ExcalidrawProps; data?: Record; app: AppClassProperties; + renderAction: ( + name: ActionName, + data?: PanelComponentProps["data"], + ) => React.JSX.Element | null; }; export interface Action { diff --git a/packages/excalidraw/components/ButtonIcon.tsx b/packages/excalidraw/components/ButtonIcon.tsx index 27b4dc3d8..77a918b3a 100644 --- a/packages/excalidraw/components/ButtonIcon.tsx +++ b/packages/excalidraw/components/ButtonIcon.tsx @@ -15,6 +15,7 @@ interface ButtonIconProps { /** include standalone style (could interfere with parent styles) */ standalone?: boolean; onClick: (event: React.MouseEvent) => void; + style?: React.CSSProperties; } export const ButtonIcon = forwardRef( @@ -30,6 +31,7 @@ export const ButtonIcon = forwardRef( data-testid={testId} className={clsx(className, { standalone, active })} onClick={onClick} + style={props.style} > {icon} diff --git a/packages/excalidraw/components/CommandPalette/CommandPalette.tsx b/packages/excalidraw/components/CommandPalette/CommandPalette.tsx index 60289cfa1..b0fc5e7ee 100644 --- a/packages/excalidraw/components/CommandPalette/CommandPalette.tsx +++ b/packages/excalidraw/components/CommandPalette/CommandPalette.tsx @@ -293,6 +293,7 @@ function CommandPaletteInner({ actionManager.actions.decreaseFontSize, actionManager.actions.toggleLinearEditor, actionManager.actions.cropEditor, + actionManager.actions.togglePolygon, actionLink, actionCopyElementLink, actionLinkToElement, diff --git a/packages/excalidraw/components/icons.tsx b/packages/excalidraw/components/icons.tsx index b4137053f..29bdc6d3c 100644 --- a/packages/excalidraw/components/icons.tsx +++ b/packages/excalidraw/components/icons.tsx @@ -129,6 +129,21 @@ export const PinIcon = createIcon( tablerIconProps, ); +export const polygonIcon = createIcon( + + + + + + + + + + + , + tablerIconProps, +); + // tabler-icons: lock-open (via Figma) export const UnlockedIcon = createIcon( diff --git a/packages/excalidraw/data/__snapshots__/transform.test.ts.snap b/packages/excalidraw/data/__snapshots__/transform.test.ts.snap index 70f8daa31..48337288a 100644 --- a/packages/excalidraw/data/__snapshots__/transform.test.ts.snap +++ b/packages/excalidraw/data/__snapshots__/transform.test.ts.snap @@ -948,6 +948,7 @@ exports[`Test Transform > should transform linear elements 3`] = ` 0, ], ], + "polygon": false, "roughness": 1, "roundness": null, "seed": Any, @@ -995,6 +996,7 @@ exports[`Test Transform > should transform linear elements 4`] = ` 0, ], ], + "polygon": false, "roughness": 1, "roundness": null, "seed": Any, diff --git a/packages/excalidraw/data/restore.ts b/packages/excalidraw/data/restore.ts index a1bcc4602..a609c0a0e 100644 --- a/packages/excalidraw/data/restore.ts +++ b/packages/excalidraw/data/restore.ts @@ -18,7 +18,7 @@ import { normalizeLink, getLineHeight, } from "@excalidraw/common"; -import { getNonDeletedElements } from "@excalidraw/element"; +import { getNonDeletedElements, isValidPolygon } from "@excalidraw/element"; import { normalizeFixedPoint } from "@excalidraw/element"; import { updateElbowArrowPoints, @@ -34,6 +34,7 @@ import { isElbowArrow, isFixedPointBinding, isLinearElement, + isLineElement, isTextElement, isUsingAdaptiveRadius, } from "@excalidraw/element"; @@ -323,7 +324,8 @@ const restoreElement = ( : element.points; if (points[0][0] !== 0 || points[0][1] !== 0) { - ({ points, x, y } = LinearElementEditor.getNormalizedPoints(element)); + ({ points, x, y } = + LinearElementEditor.getNormalizeElementPointsAndCoords(element)); } return restoreElementWithProperties(element, { @@ -339,6 +341,13 @@ const restoreElement = ( points, x, y, + ...(isLineElement(element) + ? { + polygon: isValidPolygon(element.points) + ? element.polygon ?? false + : false, + } + : {}), ...getSizeFromPoints(points), }); case "arrow": { @@ -351,7 +360,8 @@ const restoreElement = ( : element.points; if (points[0][0] !== 0 || points[0][1] !== 0) { - ({ points, x, y } = LinearElementEditor.getNormalizedPoints(element)); + ({ points, x, y } = + LinearElementEditor.getNormalizeElementPointsAndCoords(element)); } const base = { diff --git a/packages/excalidraw/data/transform.ts b/packages/excalidraw/data/transform.ts index b94406f25..fd0d3388f 100644 --- a/packages/excalidraw/data/transform.ts +++ b/packages/excalidraw/data/transform.ts @@ -466,7 +466,7 @@ const bindLinearElementToElement = ( Object.assign( linearElement, - LinearElementEditor.getNormalizedPoints({ + LinearElementEditor.getNormalizeElementPointsAndCoords({ ...linearElement, points: newPoints, }), diff --git a/packages/excalidraw/locales/en.json b/packages/excalidraw/locales/en.json index fad13c705..736c41722 100644 --- a/packages/excalidraw/locales/en.json +++ b/packages/excalidraw/locales/en.json @@ -141,6 +141,10 @@ "edit": "Edit line", "editArrow": "Edit arrow" }, + "polygon": { + "breakPolygon": "Break polygon", + "convertToPolygon": "Convert to polygon" + }, "elementLock": { "lock": "Lock", "unlock": "Unlock", diff --git a/packages/excalidraw/renderer/helpers.ts b/packages/excalidraw/renderer/helpers.ts index e0397e95d..e42f4166b 100644 --- a/packages/excalidraw/renderer/helpers.ts +++ b/packages/excalidraw/renderer/helpers.ts @@ -31,11 +31,14 @@ export const fillCircle = ( cx: number, cy: number, radius: number, - stroke = true, + stroke: boolean, + fill = true, ) => { context.beginPath(); context.arc(cx, cy, radius, 0, Math.PI * 2); - context.fill(); + if (fill) { + context.fill(); + } if (stroke) { context.stroke(); } diff --git a/packages/excalidraw/renderer/interactiveScene.ts b/packages/excalidraw/renderer/interactiveScene.ts index 254f6b8aa..ac0cb5269 100644 --- a/packages/excalidraw/renderer/interactiveScene.ts +++ b/packages/excalidraw/renderer/interactiveScene.ts @@ -1,5 +1,6 @@ import { pointFrom, + pointsEqual, type GlobalPoint, type LocalPoint, type Radians, @@ -28,6 +29,7 @@ import { isFrameLikeElement, isImageElement, isLinearElement, + isLineElement, isTextElement, } from "@excalidraw/element"; @@ -161,7 +163,8 @@ const renderSingleLinearPoint = ( point: Point, radius: number, isSelected: boolean, - isPhantomPoint = false, + isPhantomPoint: boolean, + isOverlappingPoint: boolean, ) => { context.strokeStyle = "#5e5ad8"; context.setLineDash([]); @@ -176,8 +179,11 @@ const renderSingleLinearPoint = ( context, point[0], point[1], - radius / appState.zoom.value, + (isOverlappingPoint + ? radius * (appState.editingLinearElement ? 1.5 : 2) + : radius) / appState.zoom.value, !isPhantomPoint, + !isOverlappingPoint || isSelected, ); }; @@ -253,7 +259,7 @@ const renderBindingHighlightForSuggestedPointBinding = ( index, elementsMap, ); - fillCircle(context, x, y, threshold); + fillCircle(context, x, y, threshold, true); }); }; @@ -442,15 +448,39 @@ const renderLinearPointHandles = ( const radius = appState.editingLinearElement ? POINT_HANDLE_SIZE : POINT_HANDLE_SIZE / 2; + + const _isElbowArrow = isElbowArrow(element); + const _isLineElement = isLineElement(element); + points.forEach((point, idx) => { - if (isElbowArrow(element) && idx !== 0 && idx !== points.length - 1) { + if (_isElbowArrow && idx !== 0 && idx !== points.length - 1) { return; } + const isOverlappingPoint = + idx > 0 && + (idx !== points.length - 1 || + appState.editingLinearElement || + !_isLineElement || + !element.polygon) && + pointsEqual( + point, + idx === points.length - 1 ? points[0] : points[idx - 1], + 2 / appState.zoom.value, + ); + const isSelected = !!appState.editingLinearElement?.selectedPointsIndices?.includes(idx); - renderSingleLinearPoint(context, appState, point, radius, isSelected); + renderSingleLinearPoint( + context, + appState, + point, + radius, + isSelected, + false, + isOverlappingPoint, + ); }); // Rendering segment mid points @@ -477,6 +507,7 @@ const renderLinearPointHandles = ( POINT_HANDLE_SIZE / 2, false, !fixedSegments.includes(idx + 1), + false, ); } }); @@ -500,6 +531,7 @@ const renderLinearPointHandles = ( POINT_HANDLE_SIZE / 2, false, true, + false, ); } }); @@ -526,7 +558,7 @@ const renderTransformHandles = ( context.strokeStyle = renderConfig.selectionColor; } if (key === "rotation") { - fillCircle(context, x + width / 2, y + height / 2, width / 2); + fillCircle(context, x + width / 2, y + height / 2, width / 2, true); // prefer round corners if roundRect API is available } else if (context.roundRect) { context.beginPath(); diff --git a/packages/excalidraw/tests/__snapshots__/dragCreate.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/dragCreate.test.tsx.snap index acc9b79f5..62beb7f7c 100644 --- a/packages/excalidraw/tests/__snapshots__/dragCreate.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/dragCreate.test.tsx.snap @@ -153,6 +153,7 @@ exports[`Test dragCreate > add element to the scene when pointer dragging long e 50, ], ], + "polygon": false, "roughness": 1, "roundness": { "type": 2, diff --git a/packages/excalidraw/tests/__snapshots__/multiPointCreate.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/multiPointCreate.test.tsx.snap index 3a0f3a58a..5f2a82e4e 100644 --- a/packages/excalidraw/tests/__snapshots__/multiPointCreate.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/multiPointCreate.test.tsx.snap @@ -93,6 +93,7 @@ exports[`multi point mode in linear elements > line 3`] = ` 110, ], ], + "polygon": false, "roughness": 1, "roundness": { "type": 2, diff --git a/packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap index 11447c731..c9013ddcc 100644 --- a/packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap @@ -6492,6 +6492,7 @@ exports[`regression tests > draw every type of shape > [end of test] undo stack 10, ], ], + "polygon": false, "roughness": 1, "roundness": { "type": 2, @@ -6716,6 +6717,7 @@ exports[`regression tests > draw every type of shape > [end of test] undo stack 10, ], ], + "polygon": false, "roughness": 1, "roundness": { "type": 2, @@ -8954,6 +8956,7 @@ exports[`regression tests > key 6 selects line tool > [end of test] undo stack 1 10, ], ], + "polygon": false, "roughness": 1, "roundness": { "type": 2, @@ -9773,6 +9776,7 @@ exports[`regression tests > key l selects line tool > [end of test] undo stack 1 10, ], ], + "polygon": false, "roughness": 1, "roundness": { "type": 2, diff --git a/packages/excalidraw/tests/__snapshots__/selection.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/selection.test.tsx.snap index 2eea90842..c134a23ed 100644 --- a/packages/excalidraw/tests/__snapshots__/selection.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/selection.test.tsx.snap @@ -79,6 +79,7 @@ exports[`select single element on the scene > arrow escape 1`] = ` 50, ], ], + "polygon": false, "roughness": 1, "roundness": { "type": 2, diff --git a/packages/excalidraw/tests/data/__snapshots__/restore.test.ts.snap b/packages/excalidraw/tests/data/__snapshots__/restore.test.ts.snap index 7dd0c01c3..dfb0f8d2d 100644 --- a/packages/excalidraw/tests/data/__snapshots__/restore.test.ts.snap +++ b/packages/excalidraw/tests/data/__snapshots__/restore.test.ts.snap @@ -240,6 +240,7 @@ exports[`restoreElements > should restore line and draw elements correctly 1`] = 100, ], ], + "polygon": false, "roughness": 1, "roundness": { "type": 2, @@ -289,6 +290,7 @@ exports[`restoreElements > should restore line and draw elements correctly 2`] = 100, ], ], + "polygon": false, "roughness": 1, "roundness": { "type": 2,