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

@@ -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);
+    });
+  });
+});

packages/common/src/utils.ts

@@ -544,6 +544,20 @@ export const findLastIndex = <T>(
   return -1;
 };
 
+/** returns the first non-null mapped value */
+export const mapFind = <T, K>(
+  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<Map<any, any>> | Record<any, any>,
+  value:
+    | readonly unknown[]
+    | Readonly<Map<string, unknown>>
+    | Readonly<Record<string, unknown>>
+    | ReadonlySet<unknown>,
 ): number => {
   return isReadonlyArray(value)
     ? value.length
-    : value instanceof Map
+    : value instanceof Map || value instanceof Set
     ? value.size
     : Object.keys(value).length;
 };
+
+export const reduceToCommonValue = <T, R = T>(
+  collection: readonly T[] | ReadonlySet<T>,
+  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;
+};

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 = <T>(
-  elements: readonly NonDeletedExcalidrawElement[],
-  appState: Pick<AppState, "selectedElementIds">,
-  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<InteractiveCanvasAppState, "selectedElementIds">,

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";
+};

packages/element/src/types.ts

@@ -418,10 +418,12 @@ export type ElementsMapOrArray =
   | readonly ExcalidrawElement[]
   | Readonly<ElementsMap>;
 
-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;

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 <T extends Primitive>(
   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 <T extends Primitive>(
   }
 
   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 }) => (
     <>
       <h3 aria-hidden="true">{t("labels.stroke")}</h3>
       <ColorPicker
@@ -330,7 +330,7 @@ export const actionChangeStrokeColor = register({
         label={t("labels.stroke")}
         color={getFormValue(
           elements,
-          appState,
+          app,
           (element) => 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 }) => (
     <>
       <h3 aria-hidden="true">{t("labels.background")}</h3>
       <ColorPicker
@@ -376,7 +376,7 @@ export const actionChangeBackgroundColor = register({
         label={t("labels.background")}
         color={getFormValue(
           elements,
-          appState,
+          app,
           (element) => 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 }) => (
     <fieldset>
       <legend>{t("labels.strokeWidth")}</legend>
       <ButtonIconSelect
@@ -510,7 +510,7 @@ export const actionChangeStrokeWidth = register({
         ]}
         value={getFormValue(
           elements,
-          appState,
+          app,
           (element) => 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 }) => (
     <fieldset>
       <legend>{t("labels.sloppiness")}</legend>
       <ButtonIconSelect
@@ -562,7 +562,7 @@ export const actionChangeSloppiness = register({
         ]}
         value={getFormValue(
           elements,
-          appState,
+          app,
           (element) => 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 }) => (
     <fieldset>
       <legend>{t("labels.strokeStyle")}</legend>
       <ButtonIconSelect
@@ -613,7 +613,7 @@ export const actionChangeStrokeStyle = register({
         ]}
         value={getFormValue(
           elements,
-          appState,
+          app,
           (element) => element.strokeStyle,
           (element) => element.hasOwnProperty("strokeStyle"),
           (hasSelection) =>
@@ -644,13 +644,8 @@ export const actionChangeOpacity = register({
       captureUpdate: CaptureUpdateAction.IMMEDIATELY,
     };
   },
-  PanelComponent: ({ elements, appState, updateData }) => (
-    <Range
-      updateData={updateData}
-      elements={elements}
-      appState={appState}
-      testId="opacity"
-    />
+  PanelComponent: ({ app, updateData }) => (
+    <Range updateData={updateData} app={app} testId="opacity" />
   ),
 });
 
@@ -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<Arrowhead | null>(
               elements,
-              appState,
+              app,
               (element) =>
                 isLinearElement(element) && canHaveArrowheads(element.type)
                   ? element.startArrowhead
@@ -1550,7 +1545,7 @@ export const actionChangeArrowhead = register({
             options={getArrowheadOptions(!!isRTL)}
             value={getFormValue<Arrowhead | null>(
               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 (
       <fieldset>
         <legend>{t("labels.arrowtypes")}</legend>
@@ -1787,7 +1782,7 @@ export const actionChangeArrowType = register({
           ]}
           value={getFormValue(
             elements,
-            appState,
+            app,
             (element) => {
               if (isArrowElement(element)) {
                 return element.elbowed

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]: {
+type CacheKey = string & { _brand: "CacheKey" };
+
+const FONT_SIZE_CONVERSION_CACHE = new Map<
+  ExcalidrawElement["id"],
+  {
     fontSize: number;
-    elementType: ConvertibleGenericTypes;
-  };
-} | null>(null);
+  }
+>();
 
-// NOTE doesn't need to be an atom. Review once we integrate with properties panel.
-export const linearElement_conversionCacheAtom = atom<{
-  [id: string]: {
-    properties:
-      | Partial<ExcalidrawLinearElement>
-      | Partial<ExcalidrawElbowArrowElement>;
-    initialType: ConvertibleLinearTypes;
-  };
-} | null>(null);
+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<ConversionType>(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 <Panel app={app} elements={selectedElements} />;
 };
@@ -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 (
           <ToolButton
@@ -500,14 +479,11 @@ export const convertElementTypes = (
           app.scene.getNonDeletedElementsMap(),
         );
         if (boundText) {
-          if (
-            editorJotaiStore.get(fontSize_conversionCacheAtom)?.[element.id]
-              ?.elementType === nextType
-          ) {
+          if (FONT_SIZE_CONVERSION_CACHE.get(element.id)) {
             mutateElement(boundText, app.scene.getNonDeletedElementsMap(), {
               fontSize:
-                editorJotaiStore.get(fontSize_conversionCacheAtom)?.[element.id]
-                  ?.fontSize ?? boundText.fontSize,
+                FONT_SIZE_CONVERSION_CACHE.get(element.id)?.fontSize ??
+                boundText.fontSize,
             });
           }
 
@@ -535,124 +511,101 @@ export const convertElementTypes = (
       selectedElements,
     ) as ExcalidrawLinearElement[];
 
-    const arrowType = getArrowType(convertibleLinearElements[0]);
-    const sameType = convertibleLinearElements.every(
-      (element) => 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<ExcalidrawElement["id"], ExcalidrawElement> =
+        app.scene.getElementsMapIncludingDeleted();
 
-    if (nextType && isConvertibleLinearType(nextType)) {
-      const convertedElements: Record<string, ExcalidrawElement> = {};
       for (const element of convertibleLinearElements) {
-        const { properties, initialType } =
-          editorJotaiStore.get(linearElement_conversionCacheAtom)?.[
-            element.id
-          ] || {};
-
-        // 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.
+        const cachedElement = LINEAR_ELEMENT_CONVERSION_CACHE.get(
+          toCacheKey(element.id, nextType),
+        );
+
+        // 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 = [];
-
-      for (const element of app.scene.getElementsIncludingDeleted()) {
-        if (convertedElements[element.id]) {
-          nextElements.push(convertedElements[element.id]);
-        } else {
-          nextElements.push(element);
-        }
-      }
+      app.scene.replaceAllElements(nextElementsMap);
 
-      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<ExcalidrawLinearElement> => {
-  if (element.type === "line") {
-    return {
-      points: element.points,
-      roundness: element.roundness,
-    };
-  }
-  return {};
-};
-
-const getSharpArrowProperties = (
-  element: ExcalidrawLinearElement,
-): Partial<ExcalidrawArrowElement> => {
-  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<ExcalidrawArrowElement> => {
-  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<ExcalidrawElbowArrowElement> => {
-  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;

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<HTMLInputElement>(null);
   const valueRef = React.useRef<HTMLDivElement>(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")}
       <div className="range-wrapper">
         <input
+          style={{
+            ["--color-slider-track" as string]: hasCommonOpacity
+              ? undefined
+              : "var(--button-bg)",
+          }}
           ref={rangeRef}
           type="range"
           min="0"

packages/excalidraw/scene/index.ts

@@ -1,7 +1,6 @@
 export {
   isSomeElementSelected,
   getElementsWithinSelection,
-  getCommonAttributeOfSelectedElements,
   getSelectedElements,
   getTargetElements,
 } from "@excalidraw/element";

packages/excalidraw/tests/utils.test.ts

@@ -1,13 +0,0 @@
-import { isTransparent } from "@excalidraw/common";
-
-describe("Test 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);
-  });
-});