Browse Source

Address remaining comments.

Daniel J. Geiger 2 years ago
parent
commit
27e2888347

+ 4 - 31
src/actions/manager.tsx

@@ -6,7 +6,6 @@ import {
   PanelComponentProps,
   PanelComponentProps,
   ActionSource,
   ActionSource,
   ActionPredicateFn,
   ActionPredicateFn,
-  isActionName,
 } from "./types";
 } from "./types";
 import { ExcalidrawElement } from "../element/types";
 import { ExcalidrawElement } from "../element/types";
 import { AppClassProperties, AppState } from "../types";
 import { AppClassProperties, AppState } from "../types";
@@ -76,31 +75,6 @@ export class ActionManager {
     }
     }
   }
   }
 
 
-  getCustomActions(opts?: {
-    elements?: readonly ExcalidrawElement[];
-    data?: Record<string, any>;
-    guardsOnly?: boolean;
-  }): Action[] {
-    // For testing
-    if (this === undefined) {
-      return [];
-    }
-    const filter =
-      opts !== undefined &&
-      ("elements" in opts || "data" in opts || "guardsOnly" in opts);
-    const customActions: Action[] = [];
-    for (const key in this.actions) {
-      const action = this.actions[key];
-      if (
-        !isActionName(action.name) &&
-        (!filter || this.isActionEnabled(action, opts))
-      ) {
-        customActions.push(action);
-      }
-    }
-    return customActions;
-  }
-
   registerAction(action: Action) {
   registerAction(action: Action) {
     this.actions[action.name] = action;
     this.actions[action.name] = action;
   }
   }
@@ -117,7 +91,7 @@ export class ActionManager {
         (action) =>
         (action) =>
           (action.name in canvasActions
           (action.name in canvasActions
             ? canvasActions[action.name as keyof typeof canvasActions]
             ? canvasActions[action.name as keyof typeof canvasActions]
-            : this.isActionEnabled(action, { guardsOnly: true })) &&
+            : this.isActionEnabled(action, { noPredicates: true })) &&
           action.keyTest &&
           action.keyTest &&
           action.keyTest(
           action.keyTest(
             event,
             event,
@@ -172,7 +146,7 @@ export class ActionManager {
       "PanelComponent" in this.actions[name] &&
       "PanelComponent" in this.actions[name] &&
       (name in canvasActions
       (name in canvasActions
         ? canvasActions[name as keyof typeof 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 action = this.actions[name];
       const PanelComponent = action.PanelComponent!;
       const PanelComponent = action.PanelComponent!;
@@ -194,7 +168,6 @@ export class ActionManager {
 
 
       return (
       return (
         <PanelComponent
         <PanelComponent
-          key={name}
           elements={this.getElementsIncludingDeleted()}
           elements={this.getElementsIncludingDeleted()}
           appState={this.getAppState()}
           appState={this.getAppState()}
           updateData={updateData}
           updateData={updateData}
@@ -211,8 +184,8 @@ export class ActionManager {
     action: Action,
     action: Action,
     opts?: {
     opts?: {
       elements?: readonly ExcalidrawElement[];
       elements?: readonly ExcalidrawElement[];
+      noPredicates?: boolean;
       data?: Record<string, any>;
       data?: Record<string, any>;
-      guardsOnly?: boolean;
     },
     },
   ): boolean => {
   ): boolean => {
     const elements = opts?.elements ?? this.getElementsIncludingDeleted();
     const elements = opts?.elements ?? this.getElementsIncludingDeleted();
@@ -220,7 +193,7 @@ export class ActionManager {
     const data = opts?.data;
     const data = opts?.data;
 
 
     if (
     if (
-      !opts?.guardsOnly &&
+      !opts?.noPredicates &&
       action.predicate &&
       action.predicate &&
       !action.predicate(elements, appState, this.app.props, this.app, data)
       !action.predicate(elements, appState, this.app.props, this.app, data)
     ) {
     ) {

+ 2 - 17
src/actions/shortcuts.ts

@@ -80,23 +80,8 @@ const shortcutMap: Record<ShortcutName, string[]> = {
   toggleLock: [getShortcutKey("CtrlOrCmd+Shift+L")],
   toggleLock: [getShortcutKey("CtrlOrCmd+Shift+L")],
 };
 };
 
 
-export type CustomShortcutName = string;
-
-let customShortcutMap: Record<CustomShortcutName, string[]> = {};
-
-export const registerCustomShortcuts = (
-  shortcuts: Record<CustomShortcutName, string[]>,
-) => {
-  customShortcutMap = { ...customShortcutMap, ...shortcuts };
-};
-
-export const getShortcutFromShortcutName = (
-  name: ShortcutName | CustomShortcutName,
-) => {
-  const shortcuts =
-    name in customShortcutMap
-      ? customShortcutMap[name as CustomShortcutName]
-      : shortcutMap[name as ShortcutName];
+export const getShortcutFromShortcutName = (name: ShortcutName) => {
+  const shortcuts = shortcutMap[name];
   // if multiple shortcuts available, take the first one
   // if multiple shortcuts available, take the first one
   return shortcuts && shortcuts.length > 0 ? shortcuts[0] : "";
   return shortcuts && shortcuts.length > 0 ? shortcuts[0] : "";
 };
 };

+ 80 - 86
src/actions/types.ts

@@ -43,92 +43,86 @@ export type ActionPredicateFn = (
 export type UpdaterFn = (res: ActionResult) => void;
 export type UpdaterFn = (res: ActionResult) => void;
 export type ActionFilterFn = (action: Action) => 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 = {
 export type PanelComponentProps = {
   elements: readonly ExcalidrawElement[];
   elements: readonly ExcalidrawElement[];

+ 1 - 4
src/components/Actions.tsx

@@ -3,7 +3,7 @@ import { ActionManager } from "../actions/manager";
 import { getNonDeletedElements } from "../element";
 import { getNonDeletedElements } from "../element";
 import { ExcalidrawElement, PointerType } from "../element/types";
 import { ExcalidrawElement, PointerType } from "../element/types";
 import { t } from "../i18n";
 import { t } from "../i18n";
-import { useDevice, useExcalidrawActionManager } from "../components/App";
+import { useDevice } from "../components/App";
 import {
 import {
   canChangeRoundness,
   canChangeRoundness,
   canHaveArrowheads,
   canHaveArrowheads,
@@ -92,9 +92,6 @@ export const SelectedShapeActions = ({
       {showChangeBackgroundIcons && (
       {showChangeBackgroundIcons && (
         <div>{renderAction("changeBackgroundColor")}</div>
         <div>{renderAction("changeBackgroundColor")}</div>
       )}
       )}
-      {useExcalidrawActionManager()
-        .getCustomActions({ elements: targetElements })
-        .map((action) => renderAction(action.name))}
       {showFillIcons && renderAction("changeFillStyle")}
       {showFillIcons && renderAction("changeFillStyle")}
 
 
       {(hasStrokeWidth(appState.activeTool.type) ||
       {(hasStrokeWidth(appState.activeTool.type) ||

+ 0 - 23
src/components/App.tsx

@@ -6163,29 +6163,6 @@ class App extends React.Component<AppProps, AppState> {
   };
   };
 
 
   private getContextMenuItems = (
   private getContextMenuItems = (
-    type: "canvas" | "element" | "custom",
-    source?: string,
-  ): ContextMenuItems => {
-    const custom: 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);
-    }
-    const standard: ContextMenuItems = this._getContextMenuItems(type).filter(
-      (item) =>
-        !item ||
-        item === CONTEXT_MENU_SEPARATOR ||
-        this.actionManager.isActionEnabled(item, { guardsOnly: true }),
-    );
-    return [...custom, ...standard];
-  };
-
-  private _getContextMenuItems = (
     type: "canvas" | "element",
     type: "canvas" | "element",
   ): ContextMenuItems => {
   ): ContextMenuItems => {
     const options: ContextMenuItems = [];
     const options: ContextMenuItems = [];

+ 1 - 4
src/components/ContextMenu.tsx

@@ -5,7 +5,6 @@ import { t } from "../i18n";
 import "./ContextMenu.scss";
 import "./ContextMenu.scss";
 import {
 import {
   getShortcutFromShortcutName,
   getShortcutFromShortcutName,
-  CustomShortcutName,
   ShortcutName,
   ShortcutName,
 } from "../actions/shortcuts";
 } from "../actions/shortcuts";
 import { Action } from "../actions/types";
 import { Action } from "../actions/types";
@@ -111,9 +110,7 @@ export const ContextMenu = React.memo(
                   <div className="context-menu-item__label">{label}</div>
                   <div className="context-menu-item__label">{label}</div>
                   <kbd className="context-menu-item__shortcut">
                   <kbd className="context-menu-item__shortcut">
                     {actionName
                     {actionName
-                      ? getShortcutFromShortcutName(
-                          actionName as ShortcutName | CustomShortcutName,
-                        )
+                      ? getShortcutFromShortcutName(actionName as ShortcutName)
                       : ""}
                       : ""}
                   </kbd>
                   </kbd>
                 </button>
                 </button>

+ 0 - 41
src/packages/excalidraw/example/App.tsx

@@ -30,22 +30,6 @@ import { NonDeletedExcalidrawElement } from "../../../element/types";
 import { ImportedLibraryData } from "../../../data/types";
 import { ImportedLibraryData } from "../../../data/types";
 import CustomFooter from "./CustomFooter";
 import CustomFooter from "./CustomFooter";
 import MobileFooter from "./MobileFooter";
 import MobileFooter from "./MobileFooter";
-import { Action, ActionPredicateFn } from "../../../actions/types";
-import { ContextMenuItems } from "../../../components/ContextMenu";
-
-const exampleAction: Action = {
-  name: "example",
-  trackEvent: false,
-  perform: (elements, appState) => {
-    return { elements, appState, commitToHistory: false };
-  },
-  predicate: (elements, appState, appProps, app, data) =>
-    data === undefined || data.source === "custom",
-  contextItemLabel: "labels.untitled",
-};
-const examplePredicateFn: ActionPredicateFn = (action, elements) =>
-  action.name !== "example" ||
-  !elements.some((el) => el.type === "text" && !el.isDeleted);
 
 
 declare global {
 declare global {
   interface Window {
   interface Window {
@@ -128,8 +112,6 @@ export default function App() {
     if (!excalidrawAPI) {
     if (!excalidrawAPI) {
       return;
       return;
     }
     }
-    excalidrawAPI.actionManager.registerAction(exampleAction);
-    excalidrawAPI.actionManager.registerActionPredicate(examplePredicateFn);
     const fetchData = async () => {
     const fetchData = async () => {
       const res = await fetch("/rocket.jpeg");
       const res = await fetch("/rocket.jpeg");
       const imageData = await res.blob();
       const imageData = await res.blob();
@@ -166,29 +148,6 @@ export default function App() {
             }}
             }}
           />
           />
         )}
         )}
-        <button
-          onContextMenu={(event: React.MouseEvent<HTMLButtonElement>) => {
-            event.preventDefault();
-            const offsetLeft = excalidrawAPI?.getAppState().offsetLeft ?? 0;
-            const offsetTop = excalidrawAPI?.getAppState().offsetTop ?? 0;
-            const left = event.clientX - offsetLeft;
-            const top = event.clientY - offsetTop;
-
-            const items: ContextMenuItems = [];
-            excalidrawAPI?.actionManager
-              .getCustomActions({ data: { source: "custom" } })
-              .forEach((action) => items.push(action));
-            items.length > 0 &&
-              excalidrawAPI?.updateScene({
-                appState: {
-                  contextMenu: { top, left, items },
-                },
-              });
-          }}
-        >
-          {" "}
-          Context Menu{" "}
-        </button>
         <button
         <button
           onClick={() => alert("This is dummy top right UI")}
           onClick={() => alert("This is dummy top right UI")}
           style={{ height: "2.5rem" }}
           style={{ height: "2.5rem" }}

+ 0 - 14
src/tests/customActions.test.tsx

@@ -1,13 +1,7 @@
 import { ExcalidrawElement } from "../element/types";
 import { ExcalidrawElement } from "../element/types";
-import { getShortcutKey } from "../utils";
 import { API } from "./helpers/api";
 import { API } from "./helpers/api";
 import { render } from "./test-utils";
 import { render } from "./test-utils";
 import ExcalidrawApp from "../excalidraw-app";
 import ExcalidrawApp from "../excalidraw-app";
-import {
-  CustomShortcutName,
-  getShortcutFromShortcutName,
-  registerCustomShortcuts,
-} from "../actions/shortcuts";
 import { Action, ActionPredicateFn, ActionResult } from "../actions/types";
 import { Action, ActionPredicateFn, ActionResult } from "../actions/types";
 import {
 import {
   actionChangeFontFamily,
   actionChangeFontFamily,
@@ -18,14 +12,6 @@ import { isTextElement } from "../element";
 const { h } = window;
 const { h } = window;
 
 
 describe("regression tests", () => {
 describe("regression tests", () => {
-  it("should retrieve custom shortcuts", () => {
-    const shortcuts: Record<CustomShortcutName, string[]> = {
-      test: [getShortcutKey("CtrlOrCmd+1"), getShortcutKey("CtrlOrCmd+2")],
-    };
-    registerCustomShortcuts(shortcuts);
-    expect(getShortcutFromShortcutName("test")).toBe("Ctrl+1");
-  });
-
   it("should apply universal action predicates", async () => {
   it("should apply universal action predicates", async () => {
     await render(<ExcalidrawApp />);
     await render(<ExcalidrawApp />);
     // Create the test elements
     // Create the test elements