From 51dbd4831b5ae5bdd0efa30e51e345a7d2ee66df Mon Sep 17 00:00:00 2001 From: David Luzar <5153846+dwelle@users.noreply.github.com> Date: Sat, 10 May 2025 20:06:16 +0200 Subject: [PATCH] refactor: make element type conversion more generic (#9504) * feat: add `reduceToCommonValue()` & improve opacity slider * feat: generalize and simplify type conversion cache * refactor: change cache from atoms to Map * feat: always attempt to reuse original fontSize when converting generic types --- packages/common/src/utils.test.ts | 82 ++++ packages/common/src/utils.ts | 46 ++- packages/element/src/selection.ts | 19 - packages/element/src/typeChecks.ts | 16 + packages/element/src/types.ts | 6 +- .../excalidraw/actions/actionProperties.tsx | 79 ++-- .../components/ConvertElementTypePopup.tsx | 388 +++++++----------- packages/excalidraw/components/Range.tsx | 40 +- packages/excalidraw/scene/index.ts | 1 - packages/excalidraw/tests/utils.test.ts | 13 - 10 files changed, 346 insertions(+), 344 deletions(-) create mode 100644 packages/common/src/utils.test.ts delete mode 100644 packages/excalidraw/tests/utils.test.ts diff --git a/packages/common/src/utils.test.ts b/packages/common/src/utils.test.ts new file mode 100644 index 000000000..25929722f --- /dev/null +++ b/packages/common/src/utils.test.ts @@ -0,0 +1,82 @@ +import { + isTransparent, + mapFind, + reduceToCommonValue, +} from "@excalidraw/common"; + +describe("@excalidraw/common/utils", () => { + describe("isTransparent()", () => { + it("should return true when color is rgb transparent", () => { + expect(isTransparent("#ff00")).toEqual(true); + expect(isTransparent("#fff00000")).toEqual(true); + expect(isTransparent("transparent")).toEqual(true); + }); + + it("should return false when color is not transparent", () => { + expect(isTransparent("#ced4da")).toEqual(false); + }); + }); + + describe("reduceToCommonValue()", () => { + it("should return the common value when all values are the same", () => { + expect(reduceToCommonValue([1, 1])).toEqual(1); + expect(reduceToCommonValue([0, 0])).toEqual(0); + expect(reduceToCommonValue(["a", "a"])).toEqual("a"); + expect(reduceToCommonValue(new Set([1]))).toEqual(1); + expect(reduceToCommonValue([""])).toEqual(""); + expect(reduceToCommonValue([0])).toEqual(0); + + const o = {}; + expect(reduceToCommonValue([o, o])).toEqual(o); + + expect( + reduceToCommonValue([{ a: 1 }, { a: 1, b: 2 }], (o) => o.a), + ).toEqual(1); + expect( + reduceToCommonValue(new Set([{ a: 1 }, { a: 1, b: 2 }]), (o) => o.a), + ).toEqual(1); + }); + + it("should return `null` when values are different", () => { + expect(reduceToCommonValue([1, 2, 3])).toEqual(null); + expect(reduceToCommonValue(new Set([1, 2]))).toEqual(null); + expect(reduceToCommonValue([{ a: 1 }, { a: 2 }], (o) => o.a)).toEqual( + null, + ); + }); + + it("should return `null` when some values are nullable", () => { + expect(reduceToCommonValue([1, null, 1])).toEqual(null); + expect(reduceToCommonValue([null, 1])).toEqual(null); + expect(reduceToCommonValue([1, undefined])).toEqual(null); + expect(reduceToCommonValue([undefined, 1])).toEqual(null); + expect(reduceToCommonValue([null])).toEqual(null); + expect(reduceToCommonValue([undefined])).toEqual(null); + expect(reduceToCommonValue([])).toEqual(null); + }); + }); + + describe("mapFind()", () => { + it("should return the first mapped non-null element", () => { + { + let counter = 0; + + const result = mapFind(["a", "b", "c"], (value) => { + counter++; + return value === "b" ? 42 : null; + }); + expect(result).toEqual(42); + expect(counter).toBe(2); + } + + expect(mapFind([1, 2], (value) => value * 0)).toBe(0); + expect(mapFind([1, 2], () => false)).toBe(false); + expect(mapFind([1, 2], () => "")).toBe(""); + }); + + it("should return undefined if no mapped element is found", () => { + expect(mapFind([1, 2], () => undefined)).toBe(undefined); + expect(mapFind([1, 2], () => null)).toBe(undefined); + }); + }); +}); diff --git a/packages/common/src/utils.ts b/packages/common/src/utils.ts index 6bf309c62..707e7292e 100644 --- a/packages/common/src/utils.ts +++ b/packages/common/src/utils.ts @@ -544,6 +544,20 @@ export const findLastIndex = ( return -1; }; +/** returns the first non-null mapped value */ +export const mapFind = ( + collection: readonly T[], + iteratee: (value: T, index: number) => K | undefined | null, +): K | undefined => { + for (let idx = 0; idx < collection.length; idx++) { + const result = iteratee(collection[idx], idx); + if (result != null) { + return result; + } + } + return undefined; +}; + export const isTransparent = (color: string) => { const isRGBTransparent = color.length === 5 && color.substr(4, 1) === "0"; const isRRGGBBTransparent = color.length === 9 && color.substr(7, 2) === "00"; @@ -1244,11 +1258,39 @@ export const isReadonlyArray = (value?: any): value is readonly any[] => { }; export const sizeOf = ( - value: readonly number[] | Readonly> | Record, + value: + | readonly unknown[] + | Readonly> + | Readonly> + | ReadonlySet, ): number => { return isReadonlyArray(value) ? value.length - : value instanceof Map + : value instanceof Map || value instanceof Set ? value.size : Object.keys(value).length; }; + +export const reduceToCommonValue = ( + collection: readonly T[] | ReadonlySet, + getValue?: (item: T) => R, +): R | null => { + if (sizeOf(collection) === 0) { + return null; + } + + const valueExtractor = getValue || ((item: T) => item as unknown as R); + + let commonValue: R | null = null; + + for (const item of collection) { + const value = valueExtractor(item); + if ((commonValue === null || commonValue === value) && value != null) { + commonValue = value; + } else { + return null; + } + } + + return commonValue; +}; diff --git a/packages/element/src/selection.ts b/packages/element/src/selection.ts index ca8c28d40..bb94166db 100644 --- a/packages/element/src/selection.ts +++ b/packages/element/src/selection.ts @@ -169,25 +169,6 @@ export const isSomeElementSelected = (function () { return ret; })(); -/** - * Returns common attribute (picked by `getAttribute` callback) of selected - * elements. If elements don't share the same value, returns `null`. - */ -export const getCommonAttributeOfSelectedElements = ( - elements: readonly NonDeletedExcalidrawElement[], - appState: Pick, - getAttribute: (element: ExcalidrawElement) => T, -): T | null => { - const attributes = Array.from( - new Set( - getSelectedElements(elements, appState).map((element) => - getAttribute(element), - ), - ), - ); - return attributes.length === 1 ? attributes[0] : null; -}; - export const getSelectedElements = ( elements: ElementsMapOrArray, appState: Pick, diff --git a/packages/element/src/typeChecks.ts b/packages/element/src/typeChecks.ts index aed06c812..a69f4ffc9 100644 --- a/packages/element/src/typeChecks.ts +++ b/packages/element/src/typeChecks.ts @@ -28,6 +28,7 @@ import type { PointBinding, FixedPointBinding, ExcalidrawFlowchartNodeElement, + ExcalidrawLinearElementSubType, } from "./types"; export const isInitializedImageElement = ( @@ -356,3 +357,18 @@ export const isBounds = (box: unknown): box is Bounds => typeof box[1] === "number" && typeof box[2] === "number" && typeof box[3] === "number"; + +export const getLinearElementSubType = ( + element: ExcalidrawLinearElement, +): ExcalidrawLinearElementSubType => { + if (isSharpArrow(element)) { + return "sharpArrow"; + } + if (isCurvedArrow(element)) { + return "curvedArrow"; + } + if (isElbowArrow(element)) { + return "elbowArrow"; + } + return "line"; +}; diff --git a/packages/element/src/types.ts b/packages/element/src/types.ts index d307b03c6..d7a105900 100644 --- a/packages/element/src/types.ts +++ b/packages/element/src/types.ts @@ -418,10 +418,12 @@ export type ElementsMapOrArray = | readonly ExcalidrawElement[] | Readonly; -export type ConvertibleGenericTypes = "rectangle" | "diamond" | "ellipse"; -export type ConvertibleLinearTypes = +export type ExcalidrawLinearElementSubType = | "line" | "sharpArrow" | "curvedArrow" | "elbowArrow"; + +export type ConvertibleGenericTypes = "rectangle" | "diamond" | "ellipse"; +export type ConvertibleLinearTypes = ExcalidrawLinearElementSubType; export type ConvertibleTypes = ConvertibleGenericTypes | ConvertibleLinearTypes; diff --git a/packages/excalidraw/actions/actionProperties.tsx b/packages/excalidraw/actions/actionProperties.tsx index 741a558a5..0b7310629 100644 --- a/packages/excalidraw/actions/actionProperties.tsx +++ b/packages/excalidraw/actions/actionProperties.tsx @@ -20,6 +20,7 @@ import { getShortcutKey, tupleToCoors, getLineHeight, + reduceToCommonValue, } from "@excalidraw/common"; import { getNonDeletedElements } from "@excalidraw/element"; @@ -130,7 +131,6 @@ import { Fonts } from "../fonts"; import { getLanguage, t } from "../i18n"; import { canHaveArrowheads, - getCommonAttributeOfSelectedElements, getSelectedElements, getTargetElements, isSomeElementSelected, @@ -167,12 +167,12 @@ export const changeProperty = ( export const getFormValue = function ( elements: readonly ExcalidrawElement[], - appState: AppState, + app: AppClassProperties, getAttribute: (element: ExcalidrawElement) => T, isRelevantElement: true | ((element: ExcalidrawElement) => boolean), defaultValue: T | ((isSomeElementSelected: boolean) => T), ): T { - const editingTextElement = appState.editingTextElement; + const editingTextElement = app.state.editingTextElement; const nonDeletedElements = getNonDeletedElements(elements); let ret: T | null = null; @@ -182,17 +182,17 @@ export const getFormValue = function ( } if (!ret) { - const hasSelection = isSomeElementSelected(nonDeletedElements, appState); + const hasSelection = isSomeElementSelected(nonDeletedElements, app.state); if (hasSelection) { + const selectedElements = app.scene.getSelectedElements(app.state); + const targetElements = + isRelevantElement === true + ? selectedElements + : selectedElements.filter((el) => isRelevantElement(el)); + ret = - getCommonAttributeOfSelectedElements( - isRelevantElement === true - ? nonDeletedElements - : nonDeletedElements.filter((el) => isRelevantElement(el)), - appState, - getAttribute, - ) ?? + reduceToCommonValue(targetElements, getAttribute) ?? (typeof defaultValue === "function" ? defaultValue(true) : defaultValue); @@ -320,7 +320,7 @@ export const actionChangeStrokeColor = register({ : CaptureUpdateAction.EVENTUALLY, }; }, - PanelComponent: ({ elements, appState, updateData, appProps }) => ( + PanelComponent: ({ elements, appState, updateData, app }) => ( <> element.strokeColor, true, appState.currentItemStrokeColor, @@ -366,7 +366,7 @@ export const actionChangeBackgroundColor = register({ : CaptureUpdateAction.EVENTUALLY, }; }, - PanelComponent: ({ elements, appState, updateData, appProps }) => ( + PanelComponent: ({ elements, appState, updateData, app }) => ( <> element.backgroundColor, true, appState.currentItemBackgroundColor, @@ -410,7 +410,7 @@ export const actionChangeFillStyle = register({ captureUpdate: CaptureUpdateAction.IMMEDIATELY, }; }, - PanelComponent: ({ elements, appState, updateData }) => { + PanelComponent: ({ elements, appState, updateData, app }) => { const selectedElements = getSelectedElements(elements, appState); const allElementsZigZag = selectedElements.length > 0 && @@ -446,7 +446,7 @@ export const actionChangeFillStyle = register({ ]} value={getFormValue( elements, - appState, + app, (element) => element.fillStyle, (element) => element.hasOwnProperty("fillStyle"), (hasSelection) => @@ -483,7 +483,7 @@ export const actionChangeStrokeWidth = register({ captureUpdate: CaptureUpdateAction.IMMEDIATELY, }; }, - PanelComponent: ({ elements, appState, updateData }) => ( + PanelComponent: ({ elements, appState, updateData, app }) => (
{t("labels.strokeWidth")} element.strokeWidth, (element) => element.hasOwnProperty("strokeWidth"), (hasSelection) => @@ -538,7 +538,7 @@ export const actionChangeSloppiness = register({ captureUpdate: CaptureUpdateAction.IMMEDIATELY, }; }, - PanelComponent: ({ elements, appState, updateData }) => ( + PanelComponent: ({ elements, appState, updateData, app }) => (
{t("labels.sloppiness")} element.roughness, (element) => element.hasOwnProperty("roughness"), (hasSelection) => @@ -589,7 +589,7 @@ export const actionChangeStrokeStyle = register({ captureUpdate: CaptureUpdateAction.IMMEDIATELY, }; }, - PanelComponent: ({ elements, appState, updateData }) => ( + PanelComponent: ({ elements, appState, updateData, app }) => (
{t("labels.strokeStyle")} element.strokeStyle, (element) => element.hasOwnProperty("strokeStyle"), (hasSelection) => @@ -644,13 +644,8 @@ export const actionChangeOpacity = register({ captureUpdate: CaptureUpdateAction.IMMEDIATELY, }; }, - PanelComponent: ({ elements, appState, updateData }) => ( - + PanelComponent: ({ app, updateData }) => ( + ), }); @@ -694,7 +689,7 @@ export const actionChangeFontSize = register({ ]} value={getFormValue( elements, - appState, + app, (element) => { if (isTextElement(element)) { return element.fontSize; @@ -992,7 +987,7 @@ export const actionChangeFontFamily = register({ ) => getFormValue( elementsArray, - appState, + app, (element) => { if (isTextElement(element)) { return element.fontFamily; @@ -1030,7 +1025,7 @@ export const actionChangeFontFamily = register({ // popup props are not in sync, hence we are in the middle of an update, so keeping the previous value we've had return prevSelectedFontFamilyRef.current; - }, [batchedData.openPopup, appState, elements, app.scene]); + }, [batchedData.openPopup, appState, elements, app]); useEffect(() => { prevSelectedFontFamilyRef.current = selectedFontFamily; @@ -1216,7 +1211,7 @@ export const actionChangeTextAlign = register({ ]} value={getFormValue( elements, - appState, + app, (element) => { if (isTextElement(element)) { return element.textAlign; @@ -1304,7 +1299,7 @@ export const actionChangeVerticalAlign = register({ ]} value={getFormValue( elements, - appState, + app, (element) => { if (isTextElement(element) && element.containerId) { return element.verticalAlign; @@ -1362,7 +1357,7 @@ export const actionChangeRoundness = register({ captureUpdate: CaptureUpdateAction.IMMEDIATELY, }; }, - PanelComponent: ({ elements, appState, updateData }) => { + PanelComponent: ({ elements, appState, updateData, app }) => { const targetElements = getTargetElements( getNonDeletedElements(elements), appState, @@ -1391,7 +1386,7 @@ export const actionChangeRoundness = register({ ]} value={getFormValue( elements, - appState, + app, (element) => hasLegacyRoundness ? null : element.roundness ? "round" : "sharp", (element) => @@ -1521,7 +1516,7 @@ export const actionChangeArrowhead = register({ captureUpdate: CaptureUpdateAction.IMMEDIATELY, }; }, - PanelComponent: ({ elements, appState, updateData }) => { + PanelComponent: ({ elements, appState, updateData, app }) => { const isRTL = getLanguage().rtl; return ( @@ -1533,7 +1528,7 @@ export const actionChangeArrowhead = register({ options={getArrowheadOptions(!isRTL)} value={getFormValue( elements, - appState, + app, (element) => isLinearElement(element) && canHaveArrowheads(element.type) ? element.startArrowhead @@ -1550,7 +1545,7 @@ export const actionChangeArrowhead = register({ options={getArrowheadOptions(!!isRTL)} value={getFormValue( elements, - appState, + app, (element) => isLinearElement(element) && canHaveArrowheads(element.type) ? element.endArrowhead @@ -1759,7 +1754,7 @@ export const actionChangeArrowType = register({ captureUpdate: CaptureUpdateAction.IMMEDIATELY, }; }, - PanelComponent: ({ elements, appState, updateData }) => { + PanelComponent: ({ elements, appState, updateData, app }) => { return (
{t("labels.arrowtypes")} @@ -1787,7 +1782,7 @@ export const actionChangeArrowType = register({ ]} value={getFormValue( elements, - appState, + app, (element) => { if (isArrowElement(element)) { return element.elbowed diff --git a/packages/excalidraw/components/ConvertElementTypePopup.tsx b/packages/excalidraw/components/ConvertElementTypePopup.tsx index d64475d64..b1dd11cb4 100644 --- a/packages/excalidraw/components/ConvertElementTypePopup.tsx +++ b/packages/excalidraw/components/ConvertElementTypePopup.tsx @@ -1,6 +1,9 @@ import { type ReactNode, useEffect, useMemo, useRef, useState } from "react"; -import { updateElbowArrowPoints } from "@excalidraw/element"; +import { + getLinearElementSubType, + updateElbowArrowPoints, +} from "@excalidraw/element"; import { pointFrom, pointRotateRads, type LocalPoint } from "@excalidraw/math"; @@ -8,10 +11,8 @@ import { hasBoundTextElement, isArrowBoundToElement, isArrowElement, - isCurvedArrow, isElbowArrow, isLinearElement, - isSharpArrow, isUsingAdaptiveRadius, } from "@excalidraw/element"; @@ -34,6 +35,8 @@ import { CLASSES, getFontString, isProdEnv, + mapFind, + reduceToCommonValue, updateActiveTool, } from "@excalidraw/common"; @@ -55,9 +58,7 @@ import type { ConvertibleGenericTypes, ConvertibleLinearTypes, ConvertibleTypes, - ExcalidrawArrowElement, ExcalidrawDiamondElement, - ExcalidrawElbowArrowElement, ExcalidrawElement, ExcalidrawEllipseElement, ExcalidrawLinearElement, @@ -77,7 +78,7 @@ import { sceneCoordsToViewportCoords, } from ".."; import { trackEvent } from "../analytics"; -import { atom, editorJotaiStore, useSetAtom } from "../editor-jotai"; +import { atom } from "../editor-jotai"; import "./ConvertElementTypePopup.scss"; import { ToolButton } from "./ToolButton"; @@ -98,6 +99,12 @@ import type { AppClassProperties } from "../types"; const GAP_HORIZONTAL = 8; const GAP_VERTICAL = 10; +type ExcalidrawConvertibleElement = + | ExcalidrawRectangleElement + | ExcalidrawDiamondElement + | ExcalidrawEllipseElement + | ExcalidrawLinearElement; + // indicates order of switching const GENERIC_TYPES = ["rectangle", "diamond", "ellipse"] as const; // indicates order of switching @@ -131,28 +138,21 @@ export const convertElementTypePopupAtom = atom<{ type: "panel"; } | null>(null); -// NOTE doesn't need to be an atom. Review once we integrate with properties panel. -export const fontSize_conversionCacheAtom = atom<{ - [id: string]: { - fontSize: number; - elementType: ConvertibleGenericTypes; - }; -} | null>(null); +type CacheKey = string & { _brand: "CacheKey" }; -// NOTE doesn't need to be an atom. Review once we integrate with properties panel. -export const linearElement_conversionCacheAtom = atom<{ - [id: string]: { - properties: - | Partial - | Partial; - initialType: ConvertibleLinearTypes; - }; -} | null>(null); +const FONT_SIZE_CONVERSION_CACHE = new Map< + ExcalidrawElement["id"], + { + fontSize: number; + } +>(); + +const LINEAR_ELEMENT_CONVERSION_CACHE = new Map< + CacheKey, + ExcalidrawLinearElement +>(); const ConvertElementTypePopup = ({ app }: { app: App }) => { - const setFontSizeCache = useSetAtom(fontSize_conversionCacheAtom); - const setLinearElementCache = useSetAtom(linearElement_conversionCacheAtom); - const selectedElements = app.scene.getSelectedElements(app.state); const elementsCategoryRef = useRef(null); @@ -179,10 +179,10 @@ const ConvertElementTypePopup = ({ app }: { app: App }) => { useEffect(() => { return () => { - setFontSizeCache(null); - setLinearElementCache(null); + FONT_SIZE_CONVERSION_CACHE.clear(); + LINEAR_ELEMENT_CONVERSION_CACHE.clear(); }; - }, [setFontSizeCache, setLinearElementCache]); + }, []); return ; }; @@ -215,7 +215,8 @@ const Panel = ({ : conversionType === "linear" ? linearElements.every( (element) => - getArrowType(element) === getArrowType(linearElements[0]), + getLinearElementSubType(element) === + getLinearElementSubType(linearElements[0]), ) : false; @@ -264,51 +265,29 @@ const Panel = ({ }, [genericElements, linearElements, app.scene, app.state]); useEffect(() => { - if (editorJotaiStore.get(linearElement_conversionCacheAtom)) { - return; - } - for (const linearElement of linearElements) { - const initialType = getArrowType(linearElement); - const cachedProperties = - initialType === "line" - ? getLineProperties(linearElement) - : initialType === "sharpArrow" - ? getSharpArrowProperties(linearElement) - : initialType === "curvedArrow" - ? getCurvedArrowProperties(linearElement) - : initialType === "elbowArrow" - ? getElbowArrowProperties(linearElement) - : {}; - - editorJotaiStore.set(linearElement_conversionCacheAtom, { - ...editorJotaiStore.get(linearElement_conversionCacheAtom), - [linearElement.id]: { - properties: cachedProperties, - initialType, - }, - }); + const cacheKey = toCacheKey( + linearElement.id, + getConvertibleType(linearElement), + ); + if (!LINEAR_ELEMENT_CONVERSION_CACHE.has(cacheKey)) { + LINEAR_ELEMENT_CONVERSION_CACHE.set(cacheKey, linearElement); + } } }, [linearElements]); useEffect(() => { - if (editorJotaiStore.get(fontSize_conversionCacheAtom)) { - return; - } - for (const element of genericElements) { - const boundText = getBoundTextElement( - element, - app.scene.getNonDeletedElementsMap(), - ); - if (boundText) { - editorJotaiStore.set(fontSize_conversionCacheAtom, { - ...editorJotaiStore.get(fontSize_conversionCacheAtom), - [element.id]: { + if (!FONT_SIZE_CONVERSION_CACHE.has(element.id)) { + const boundText = getBoundTextElement( + element, + app.scene.getNonDeletedElementsMap(), + ); + if (boundText) { + FONT_SIZE_CONVERSION_CACHE.set(element.id, { fontSize: boundText.fontSize, - elementType: element.type as ConvertibleGenericTypes, - }, - }); + }); + } } } }, [genericElements, app.scene]); @@ -350,7 +329,7 @@ const Panel = ({ sameType && ((conversionType === "generic" && genericElements[0].type === type) || (conversionType === "linear" && - getArrowType(linearElements[0]) === type)); + getLinearElementSubType(linearElements[0]) === type)); return ( getArrowType(element) === arrowType, - ); + if (!nextType) { + const commonSubType = reduceToCommonValue( + convertibleLinearElements, + getLinearElementSubType, + ); - const index = sameType ? LINEAR_TYPES.indexOf(arrowType) : -1; - nextType = - nextType ?? - LINEAR_TYPES[ - (index + LINEAR_TYPES.length + advancement) % LINEAR_TYPES.length - ]; + const index = commonSubType ? LINEAR_TYPES.indexOf(commonSubType) : -1; + nextType = + LINEAR_TYPES[ + (index + LINEAR_TYPES.length + advancement) % LINEAR_TYPES.length + ]; + } + + if (isConvertibleLinearType(nextType)) { + const convertedElements: ExcalidrawElement[] = []; + + const nextElementsMap: Map = + app.scene.getElementsMapIncludingDeleted(); - if (nextType && isConvertibleLinearType(nextType)) { - const convertedElements: Record = {}; for (const element of convertibleLinearElements) { - const { properties, initialType } = - editorJotaiStore.get(linearElement_conversionCacheAtom)?.[ - element.id - ] || {}; + const cachedElement = LINEAR_ELEMENT_CONVERSION_CACHE.get( + toCacheKey(element.id, nextType), + ); - // If the initial type is not elbow, and when we switch to elbow, - // the linear line might be "bent" and the points would likely be different. - // When we then switch to other non elbow types from this converted elbow, - // we still want to use the original points instead. + // if switching to the original subType or a subType we've already + // converted to, reuse the cached element to get the original properties + // (needed for simple->elbow->simple conversions or between line + // and arrows) if ( - initialType && - properties && - isElbowArrow(element) && - initialType !== "elbowArrow" && - nextType !== "elbowArrow" + cachedElement && + getLinearElementSubType(cachedElement) === nextType ) { - // first convert back to the original type - const originalType = convertElementType( - element, - initialType, - app, - ) as ExcalidrawLinearElement; - // then convert to the target type - const converted = convertElementType( - initialType === "line" - ? newLinearElement({ - ...originalType, - ...properties, - type: "line", - }) - : newArrowElement({ - ...originalType, - ...properties, - type: "arrow", - }), - nextType, - app, - ); - convertedElements[converted.id] = converted; + nextElementsMap.set(cachedElement.id, cachedElement); + convertedElements.push(cachedElement); } else { const converted = convertElementType(element, nextType, app); - convertedElements[converted.id] = converted; + nextElementsMap.set(converted.id, converted); + convertedElements.push(converted); } } - const nextElements = []; + app.scene.replaceAllElements(nextElementsMap); - for (const element of app.scene.getElementsIncludingDeleted()) { - if (convertedElements[element.id]) { - nextElements.push(convertedElements[element.id]); - } else { - nextElements.push(element); - } - } - - app.scene.replaceAllElements(nextElements); - - for (const element of Object.values(convertedElements)) { - const cachedLinear = editorJotaiStore.get( - linearElement_conversionCacheAtom, - )?.[element.id]; - - if (cachedLinear) { - const { properties, initialType } = cachedLinear; - - if (initialType === nextType) { - mutateElement( + // post normalization + for (const element of convertedElements) { + if (isLinearElement(element)) { + if (isElbowArrow(element)) { + const nextPoints = convertLineToElbow(element); + if (nextPoints.length < 2) { + // skip if not enough points to form valid segments + continue; + } + const fixedSegments: FixedSegment[] = []; + for (let i = 0; i < nextPoints.length - 1; i++) { + fixedSegments.push({ + start: nextPoints[i], + end: nextPoints[i + 1], + index: i + 1, + }); + } + const updates = updateElbowArrowPoints( element, app.scene.getNonDeletedElementsMap(), - properties, + { + points: nextPoints, + fixedSegments, + }, ); - continue; - } - } - - if (isElbowArrow(element)) { - const nextPoints = convertLineToElbow(element); - if (nextPoints.length < 2) { - // skip if not enough points to form valid segments - continue; - } - const fixedSegments: FixedSegment[] = []; - for (let i = 0; i < nextPoints.length - 1; i++) { - fixedSegments.push({ - start: nextPoints[i], - end: nextPoints[i + 1], - index: i + 1, + mutateElement(element, app.scene.getNonDeletedElementsMap(), { + ...updates, }); + } else { + // if we're converting to non-elbow linear element, check if + // we've already cached one of these linear elements so we can + // reuse the points (case: curved->elbow->line and similar) + + const similarCachedLinearElement = mapFind( + ["line", "sharpArrow", "curvedArrow"] as const, + (type) => + LINEAR_ELEMENT_CONVERSION_CACHE.get( + toCacheKey(element.id, type), + ), + ); + + if (similarCachedLinearElement) { + const points = similarCachedLinearElement.points; + app.scene.mutateElement(element, { + points, + }); + } } - const updates = updateElbowArrowPoints( - element, - app.scene.getNonDeletedElementsMap(), - { - points: nextPoints, - fixedSegments, - }, - ); - mutateElement(element, app.scene.getNonDeletedElementsMap(), { - ...updates, - }); } } } + const convertedSelectedLinearElements = filterLinearConvertibleElements( app.scene.getSelectedElements(app.state), ); @@ -708,83 +661,11 @@ const isEligibleLinearElement = (element: ExcalidrawElement) => { ); }; -const getArrowType = (element: ExcalidrawLinearElement) => { - if (isSharpArrow(element)) { - return "sharpArrow"; - } - if (isCurvedArrow(element)) { - return "curvedArrow"; - } - if (isElbowArrow(element)) { - return "elbowArrow"; - } - return "line"; -}; - -const getLineProperties = ( - element: ExcalidrawLinearElement, -): Partial => { - if (element.type === "line") { - return { - points: element.points, - roundness: element.roundness, - }; - } - return {}; -}; - -const getSharpArrowProperties = ( - element: ExcalidrawLinearElement, -): Partial => { - if (isSharpArrow(element)) { - return { - points: element.points, - startArrowhead: element.startArrowhead, - endArrowhead: element.endArrowhead, - startBinding: element.startBinding, - endBinding: element.endBinding, - roundness: null, - }; - } - - return {}; -}; - -const getCurvedArrowProperties = ( - element: ExcalidrawLinearElement, -): Partial => { - if (isCurvedArrow(element)) { - return { - points: element.points, - startArrowhead: element.startArrowhead, - endArrowhead: element.endArrowhead, - startBinding: element.startBinding, - endBinding: element.endBinding, - roundness: element.roundness, - }; - } - - return {}; -}; - -const getElbowArrowProperties = ( - element: ExcalidrawLinearElement, -): Partial => { - if (isElbowArrow(element)) { - return { - points: element.points, - startArrowhead: element.startArrowhead, - endArrowhead: element.endArrowhead, - startBinding: element.startBinding, - endBinding: element.endBinding, - roundness: null, - fixedSegments: element.fixedSegments, - startIsSpecial: element.startIsSpecial, - endIsSpecial: element.endIsSpecial, - }; - } - - return {}; +const toCacheKey = ( + elementId: ExcalidrawElement["id"], + convertitleType: ConvertibleTypes, +) => { + return `${elementId}:${convertitleType}` as CacheKey; }; const filterGenericConvetibleElements = (elements: ExcalidrawElement[]) => @@ -1045,4 +926,13 @@ const isValidConversion = ( return false; }; +const getConvertibleType = ( + element: ExcalidrawConvertibleElement, +): ConvertibleTypes => { + if (isLinearElement(element)) { + return getLinearElementSubType(element); + } + return element.type; +}; + export default ConvertElementTypePopup; diff --git a/packages/excalidraw/components/Range.tsx b/packages/excalidraw/components/Range.tsx index 3ab9ede15..501ad7d18 100644 --- a/packages/excalidraw/components/Range.tsx +++ b/packages/excalidraw/components/Range.tsx @@ -1,32 +1,35 @@ import React, { useEffect } from "react"; -import { getFormValue } from "../actions/actionProperties"; import { t } from "../i18n"; import "./Range.scss"; +import type { AppClassProperties } from "../types"; + export type RangeProps = { updateData: (value: number) => void; - appState: any; - elements: any; + app: AppClassProperties; testId?: string; }; -export const Range = ({ - updateData, - appState, - elements, - testId, -}: RangeProps) => { +export const Range = ({ updateData, app, testId }: RangeProps) => { const rangeRef = React.useRef(null); const valueRef = React.useRef(null); - const value = getFormValue( - elements, - appState, - (element) => element.opacity, - true, - appState.currentItemOpacity, - ); + const selectedElements = app.scene.getSelectedElements(app.state); + let hasCommonOpacity = true; + const firstElement = selectedElements.at(0); + const leastCommonOpacity = selectedElements.reduce((acc, element) => { + if (acc != null && acc !== element.opacity) { + hasCommonOpacity = false; + } + if (acc == null || acc > element.opacity) { + return element.opacity; + } + return acc; + }, firstElement?.opacity ?? null); + + const value = leastCommonOpacity ?? app.state.currentItemOpacity; + useEffect(() => { if (rangeRef.current && valueRef.current) { const rangeElement = rangeRef.current; @@ -45,6 +48,11 @@ export const Range = ({ {t("labels.opacity")}
{ - it("should return true when color is rgb transparent", () => { - expect(isTransparent("#ff00")).toEqual(true); - expect(isTransparent("#fff00000")).toEqual(true); - expect(isTransparent("transparent")).toEqual(true); - }); - - it("should return false when color is not transparent", () => { - expect(isTransparent("#ced4da")).toEqual(false); - }); -});