Refactor: Drop isActionName and convert getCustomActions to

`filterActions`.
2023-01-28 21:27:25 -06:00 · 2023-01-28 21:27:25 -06:00 · 14c6ea938a
commit 14c6ea938a
parent 87aba3f619
8 changed files with 145 additions and 133 deletions
--- a/src/actions/manager.tsx
+++ b/src/actions/manager.tsx
@ -6,7 +6,6 @@ import {
  PanelComponentProps,
  ActionSource,
  ActionPredicateFn,
-  isActionName,
 } from "./types";
 import { ExcalidrawElement } from "../element/types";
 import { AppClassProperties, AppState } from "../types";
@ -76,29 +75,29 @@ export class ActionManager {
    }
  }

-  getCustomActions(opts?: {
-    elements?: readonly ExcalidrawElement[];
-    data?: Record<string, any>;
-    guardsOnly?: boolean;
-  }): Action[] {
+  filterActions(
+    filter: ActionPredicateFn,
+    opts?: {
+      elements?: readonly ExcalidrawElement[];
+      data?: Record<string, any>;
+    },
+  ): Action[] {
    // For testing
    if (this === undefined) {
      return [];
    }
-    const filter =
-      opts !== undefined &&
-      ("elements" in opts || "data" in opts || "guardsOnly" in opts);
-    const customActions: Action[] = [];
+    const elements = opts?.elements ?? this.getElementsIncludingDeleted();
+    const appState = this.getAppState();
+    const data = opts?.data;
+
+    const actions: Action[] = [];
    for (const key in this.actions) {
      const action = this.actions[key];
-      if (
-        !isActionName(action.name) &&
-        (!filter || this.isActionEnabled(action, opts))
-      ) {
-        customActions.push(action);
+      if (filter(action, elements, appState, data)) {
+        actions.push(action);
      }
    }
-    return customActions;
+    return actions;
  }

  registerAction(action: Action) {
@ -117,7 +116,7 @@ export class ActionManager {
        (action) =>
          (action.name in canvasActions
            ? canvasActions[action.name as keyof typeof canvasActions]
-            : this.isActionEnabled(action, { guardsOnly: true })) &&
+            : this.isActionEnabled(action, { noPredicates: true })) &&
          action.keyTest &&
          action.keyTest(
            event,
@ -172,7 +171,7 @@ export class ActionManager {
      "PanelComponent" in this.actions[name] &&
      (name in canvasActions
        ? canvasActions[name as keyof typeof canvasActions]
-        : this.isActionEnabled(this.actions[name], { guardsOnly: true }))
+        : this.isActionEnabled(this.actions[name], { noPredicates: true }))
    ) {
      const action = this.actions[name];
      const PanelComponent = action.PanelComponent!;
@ -212,7 +211,7 @@ export class ActionManager {
    opts?: {
      elements?: readonly ExcalidrawElement[];
      data?: Record<string, any>;
-      guardsOnly?: boolean;
+      noPredicates?: boolean;
    },
  ): boolean => {
    const elements = opts?.elements ?? this.getElementsIncludingDeleted();
@ -220,7 +219,7 @@ export class ActionManager {
    const data = opts?.data;

    if (
-      !opts?.guardsOnly &&
+      !opts?.noPredicates &&
      action.predicate &&
      !action.predicate(elements, appState, this.app.props, this.app, data)
    ) {
--- a/src/actions/types.ts
+++ b/src/actions/types.ts
@ -43,92 +43,86 @@ export type ActionPredicateFn = (
 export type UpdaterFn = (res: ActionResult) => void;
 export type ActionFilterFn = (action: Action) => void;

-const actionNames = [
-  "copy",
-  "cut",
-  "paste",
-  "copyAsPng",
-  "copyAsSvg",
-  "copyText",
-  "sendBackward",
-  "bringForward",
-  "sendToBack",
-  "bringToFront",
-  "copyStyles",
-  "selectAll",
-  "pasteStyles",
-  "gridMode",
-  "zenMode",
-  "stats",
-  "changeStrokeColor",
-  "changeBackgroundColor",
-  "changeFillStyle",
-  "changeStrokeWidth",
-  "changeStrokeShape",
-  "changeSloppiness",
-  "changeStrokeStyle",
-  "changeArrowhead",
-  "changeOpacity",
-  "changeFontSize",
-  "toggleCanvasMenu",
-  "toggleEditMenu",
-  "undo",
-  "redo",
-  "finalize",
-  "changeProjectName",
-  "changeExportBackground",
-  "changeExportEmbedScene",
-  "changeExportScale",
-  "saveToActiveFile",
-  "saveFileToDisk",
-  "loadScene",
-  "duplicateSelection",
-  "deleteSelectedElements",
-  "changeViewBackgroundColor",
-  "clearCanvas",
-  "zoomIn",
-  "zoomOut",
-  "resetZoom",
-  "zoomToFit",
-  "zoomToSelection",
-  "changeFontFamily",
-  "changeTextAlign",
-  "changeVerticalAlign",
-  "toggleFullScreen",
-  "toggleShortcuts",
-  "group",
-  "ungroup",
-  "goToCollaborator",
-  "addToLibrary",
-  "changeRoundness",
-  "alignTop",
-  "alignBottom",
-  "alignLeft",
-  "alignRight",
-  "alignVerticallyCentered",
-  "alignHorizontallyCentered",
-  "distributeHorizontally",
-  "distributeVertically",
-  "flipHorizontal",
-  "flipVertical",
-  "viewMode",
-  "exportWithDarkMode",
-  "toggleTheme",
-  "increaseFontSize",
-  "decreaseFontSize",
-  "unbindText",
-  "hyperlink",
-  "bindText",
-  "toggleLock",
-  "toggleLinearEditor",
-  "toggleEraserTool",
-  "toggleHandTool",
-] as const;
-
-// So we can have the `isActionName` type guard
-export type ActionName = typeof actionNames[number];
-export const isActionName = (n: any): n is ActionName =>
-  actionNames.includes(n);
+export type ActionName =
+  | "copy"
+  | "cut"
+  | "paste"
+  | "copyAsPng"
+  | "copyAsSvg"
+  | "copyText"
+  | "sendBackward"
+  | "bringForward"
+  | "sendToBack"
+  | "bringToFront"
+  | "copyStyles"
+  | "selectAll"
+  | "pasteStyles"
+  | "gridMode"
+  | "zenMode"
+  | "stats"
+  | "changeStrokeColor"
+  | "changeBackgroundColor"
+  | "changeFillStyle"
+  | "changeStrokeWidth"
+  | "changeStrokeShape"
+  | "changeSloppiness"
+  | "changeStrokeStyle"
+  | "changeArrowhead"
+  | "changeOpacity"
+  | "changeFontSize"
+  | "toggleCanvasMenu"
+  | "toggleEditMenu"
+  | "undo"
+  | "redo"
+  | "finalize"
+  | "changeProjectName"
+  | "changeExportBackground"
+  | "changeExportEmbedScene"
+  | "changeExportScale"
+  | "saveToActiveFile"
+  | "saveFileToDisk"
+  | "loadScene"
+  | "duplicateSelection"
+  | "deleteSelectedElements"
+  | "changeViewBackgroundColor"
+  | "clearCanvas"
+  | "zoomIn"
+  | "zoomOut"
+  | "resetZoom"
+  | "zoomToFit"
+  | "zoomToSelection"
+  | "changeFontFamily"
+  | "changeTextAlign"
+  | "changeVerticalAlign"
+  | "toggleFullScreen"
+  | "toggleShortcuts"
+  | "group"
+  | "ungroup"
+  | "goToCollaborator"
+  | "addToLibrary"
+  | "changeRoundness"
+  | "alignTop"
+  | "alignBottom"
+  | "alignLeft"
+  | "alignRight"
+  | "alignVerticallyCentered"
+  | "alignHorizontallyCentered"
+  | "distributeHorizontally"
+  | "distributeVertically"
+  | "flipHorizontal"
+  | "flipVertical"
+  | "viewMode"
+  | "exportWithDarkMode"
+  | "toggleTheme"
+  | "increaseFontSize"
+  | "decreaseFontSize"
+  | "unbindText"
+  | "hyperlink"
+  | "bindText"
+  | "toggleLock"
+  | "toggleLinearEditor"
+  | "toggleEraserTool"
+  | "toggleHandTool";

 export type PanelComponentProps = {
  elements: readonly ExcalidrawElement[];
--- a/src/components/Actions.tsx
+++ b/src/components/Actions.tsx
@ -3,7 +3,7 @@ import { ActionManager } from "../actions/manager";
 import { getNonDeletedElements } from "../element";
 import { ExcalidrawElement, PointerType } from "../element/types";
 import { t } from "../i18n";
-import { useDevice, useExcalidrawActionManager } from "../components/App";
+import { useDevice } from "../components/App";
 import {
  canChangeRoundness,
  canHaveArrowheads,
@ -23,7 +23,7 @@ import {
 } from "../utils";
 import Stack from "./Stack";
 import { ToolButton } from "./ToolButton";
-import { SubtypeToggles } from "./SubtypeButton";
+import { SubtypeShapeActions, SubtypeToggles } from "./Subtypes";
 import { hasStrokeColor } from "../scene/comparisons";
 import { trackEvent } from "../analytics";
 import { hasBoundTextElement } from "../element/typeChecks";
@ -93,9 +93,7 @@ export const SelectedShapeActions = ({
      {showChangeBackgroundIcons && (
        <div>{renderAction("changeBackgroundColor")}</div>
      )}
-      {useExcalidrawActionManager()
-        .getCustomActions({ elements: targetElements })
-        .map((action) => renderAction(action.name))}
+      <SubtypeShapeActions elements={targetElements} />
      {showFillIcons && renderAction("changeFillStyle")}

      {(hasStrokeWidth(appState.activeTool.type) ||
--- a/src/components/App.tsx
+++ b/src/components/App.tsx
@ -242,6 +242,7 @@ import {
  prepareSubtype,
  selectSubtype,
  subtypeActionPredicate,
+  isSubtypeAction,
 } from "../subtypes";
 import {
  dataURLToFile,
@ -6229,26 +6230,26 @@ class App extends React.Component<AppProps, AppState> {
  };

  private getContextMenuItems = (
-    type: "canvas" | "element" | "custom",
-    source?: string,
+    type: "canvas" | "element",
  ): ContextMenuItems => {
-    const custom: ContextMenuItems = [];
+    const subtype: ContextMenuItems = [];
    this.actionManager
-      .getCustomActions({ data: { source: source ?? "" } })
-      .forEach((action) => custom.push(action));
-    if (type === "custom") {
-      return custom;
-    }
-    if (custom.length > 0) {
-      custom.push(CONTEXT_MENU_SEPARATOR);
+      .filterActions(isSubtypeAction)
+      .forEach(
+        (action) =>
+          this.actionManager.isActionEnabled(action, { data: {} }) &&
+          subtype.push(action),
+      );
+    if (subtype.length > 0) {
+      subtype.push(CONTEXT_MENU_SEPARATOR);
    }
    const standard: ContextMenuItems = this._getContextMenuItems(type).filter(
      (item) =>
        !item ||
        item === CONTEXT_MENU_SEPARATOR ||
-        this.actionManager.isActionEnabled(item, { guardsOnly: true }),
+        this.actionManager.isActionEnabled(item, { noPredicates: true }),
    );
-    return [...custom, ...standard];
+    return [...subtype, ...standard];
  };

  private _getContextMenuItems = (
--- a/src/components/SubtypeButton.tsx
+++ b/src/components/SubtypeButton.tsx
@ -7,6 +7,7 @@ import {
  Subtype,
  getSubtypeNames,
  hasAlwaysEnabledActions,
+  isSubtypeAction,
  isValidSubtype,
  subtypeCollides,
 } from "../subtypes";
@ -30,7 +31,7 @@ export const SubtypeButton = (
  const subtypeAction: Action = {
    name: subtype,
    trackEvent: false,
-    predicate: (...rest) => rest[4]?.source === subtype,
+    predicate: (...rest) => rest[4]?.subtype === subtype,
    perform: (elements, appState) => {
      const inactive = !appState.activeSubtypes?.includes(subtype) ?? true;
      const activeSubtypes: Subtype[] = [];
@ -121,7 +122,7 @@ export const SubtypeToggles = () => {

  const onContextMenu = (
    event: React.MouseEvent<HTMLButtonElement>,
-    source: string,
+    subtype: string,
  ) => {
    event.preventDefault();

@ -131,8 +132,9 @@ export const SubtypeToggles = () => {
    const top = event.clientY - offsetTop;

    const items: ContextMenuItems = [];
-    am.getCustomActions({ data: { source } }).forEach((action) =>
-      items.push(action),
+    am.filterActions(isSubtypeAction).forEach(
+      (action) =>
+        am.isActionEnabled(action, { data: { subtype } }) && items.push(action),
    );
    setAppState({}, () => {
      setAppState({
@ -154,3 +156,18 @@ export const SubtypeToggles = () => {
 };

 SubtypeToggles.displayName = "SubtypeToggles";
+
+export const SubtypeShapeActions = (props: {
+  elements: readonly ExcalidrawElement[];
+}) => {
+  const am = useExcalidrawActionManager();
+  return (
+    <>
+      {am
+        .filterActions(isSubtypeAction, { elements: props.elements })
+        .map((action) => am.renderAction(action.name))}
+    </>
+  );
+};
+
+SubtypeShapeActions.displayName = "SubtypeShapeActions";
--- a/src/packages/extensions/ts/mathjax/implementation.tsx
+++ b/src/packages/extensions/ts/mathjax/implementation.tsx
@ -45,7 +45,7 @@ import {
 } from "../../../../subtypes";
 import { mathSubtypeIcon } from "./icon";
 import { getMathSubtypeRecord } from "./types";
-import { SubtypeButton } from "../../../../components/SubtypeButton";
+import { SubtypeButton } from "../../../../components/Subtypes";
 import { getMaxContainerWidth } from "../../../../element/newElement";

 const mathSubtype = getMathSubtypeRecord().subtype;
@ -1318,7 +1318,7 @@ const createMathActions = () => {
      getMathProps.getUseTex(appState)
        ? "labels.useTexTrueActive"
        : "labels.useTexTrueInactive",
-    predicate: (...rest) => rest.length < 5 || rest[4]?.source === mathSubtype,
+    predicate: (...rest) => rest.length < 5 || rest[4]?.subtype === mathSubtype,
    trackEvent: false,
  };
  const actionUseTexFalse: Action = {
@ -1337,7 +1337,7 @@ const createMathActions = () => {
      !getMathProps.getUseTex(appState)
        ? "labels.useTexFalseActive"
        : "labels.useTexFalseInactive",
-    predicate: (...rest) => rest.length < 5 || rest[4]?.source === mathSubtype,
+    predicate: (...rest) => rest.length < 5 || rest[4]?.subtype === mathSubtype,
    trackEvent: false,
  };
  const actionResetUseTex: Action = {
@ -1466,8 +1466,7 @@ const createMathActions = () => {
      </fieldset>
    ),
    predicate: (...rest) =>
-      rest[4]?.source === undefined &&
-      enableActionChangeMathProps(rest[0], rest[1]),
+      rest[4] === undefined && enableActionChangeMathProps(rest[0], rest[1]),
    trackEvent: false,
  };
  const actionMath = SubtypeButton(mathSubtype, "text", mathSubtypeIcon, "M");
--- a/src/subtypes.ts
+++ b/src/subtypes.ts
@ -99,6 +99,10 @@ const isForSubtype = (
  return false;
 };

+export const isSubtypeAction: ActionPredicateFn = function (action) {
+  return isSubtypeActionName(action.name) && !isSubtypeName(action.name);
+};
+
 export const subtypeActionPredicate: ActionPredicateFn = function (
  action,
  elements,
--- a/src/tests/subtypes.test.tsx
+++ b/src/tests/subtypes.test.tsx
@ -18,7 +18,7 @@ import ExcalidrawApp from "../excalidraw-app";

 import { ExcalidrawElement, FontString, Theme } from "../element/types";
 import { createIcon, iconFillColor } from "../components/icons";
-import { SubtypeButton } from "../components/SubtypeButton";
+import { SubtypeButton } from "../components/Subtypes";
 import { registerAuxLangData } from "../i18n";
 import { getFontString, getShortcutKey } from "../utils";
 import * as textElementUtils from "../element/textElement";