From 27e2888347cc1879d1353365aa7e4f974d300586 Mon Sep 17 00:00:00 2001 From: "Daniel J. Geiger" <1852529+DanielJGeiger@users.noreply.github.com> Date: Mon, 30 Jan 2023 09:05:08 -0600 Subject: [PATCH] Address remaining comments. --- src/actions/manager.tsx | 35 +---- src/actions/shortcuts.ts | 19 +-- src/actions/types.ts | 166 ++++++++++++------------ src/components/Actions.tsx | 5 +- src/components/App.tsx | 23 ---- src/components/ContextMenu.tsx | 5 +- src/packages/excalidraw/example/App.tsx | 41 ------ src/tests/customActions.test.tsx | 14 -- 8 files changed, 88 insertions(+), 220 deletions(-) diff --git a/src/actions/manager.tsx b/src/actions/manager.tsx index 6fc8ced71..a589c5e60 100644 --- 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,31 +75,6 @@ export class ActionManager { } } - getCustomActions(opts?: { - elements?: readonly ExcalidrawElement[]; - data?: Record; - 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) { this.actions[action.name] = action; } @@ -117,7 +91,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 +146,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!; @@ -194,7 +168,6 @@ export class ActionManager { return ( ; - guardsOnly?: boolean; }, ): boolean => { const elements = opts?.elements ?? this.getElementsIncludingDeleted(); @@ -220,7 +193,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) ) { diff --git a/src/actions/shortcuts.ts b/src/actions/shortcuts.ts index 8d5b164ea..4138ae085 100644 --- a/src/actions/shortcuts.ts +++ b/src/actions/shortcuts.ts @@ -80,23 +80,8 @@ const shortcutMap: Record = { toggleLock: [getShortcutKey("CtrlOrCmd+Shift+L")], }; -export type CustomShortcutName = string; - -let customShortcutMap: Record = {}; - -export const registerCustomShortcuts = ( - shortcuts: Record, -) => { - 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 return shortcuts && shortcuts.length > 0 ? shortcuts[0] : ""; }; diff --git a/src/actions/types.ts b/src/actions/types.ts index 81b0322a4..ad8f4138a 100644 --- 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[]; diff --git a/src/components/Actions.tsx b/src/components/Actions.tsx index e70e69f01..2ee9babfc 100644 --- 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, @@ -92,9 +92,6 @@ export const SelectedShapeActions = ({ {showChangeBackgroundIcons && (
{renderAction("changeBackgroundColor")}
)} - {useExcalidrawActionManager() - .getCustomActions({ elements: targetElements }) - .map((action) => renderAction(action.name))} {showFillIcons && renderAction("changeFillStyle")} {(hasStrokeWidth(appState.activeTool.type) || diff --git a/src/components/App.tsx b/src/components/App.tsx index b1f5c35a7..a29f83201 100644 --- a/src/components/App.tsx +++ b/src/components/App.tsx @@ -6163,29 +6163,6 @@ class App extends React.Component { }; 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", ): ContextMenuItems => { const options: ContextMenuItems = []; diff --git a/src/components/ContextMenu.tsx b/src/components/ContextMenu.tsx index 82ef667ec..1463f1033 100644 --- a/src/components/ContextMenu.tsx +++ b/src/components/ContextMenu.tsx @@ -5,7 +5,6 @@ import { t } from "../i18n"; import "./ContextMenu.scss"; import { getShortcutFromShortcutName, - CustomShortcutName, ShortcutName, } from "../actions/shortcuts"; import { Action } from "../actions/types"; @@ -111,9 +110,7 @@ export const ContextMenu = React.memo(
{label}
{actionName - ? getShortcutFromShortcutName( - actionName as ShortcutName | CustomShortcutName, - ) + ? getShortcutFromShortcutName(actionName as ShortcutName) : ""} diff --git a/src/packages/excalidraw/example/App.tsx b/src/packages/excalidraw/example/App.tsx index 71e3fbbad..21f91ecd4 100644 --- a/src/packages/excalidraw/example/App.tsx +++ b/src/packages/excalidraw/example/App.tsx @@ -30,22 +30,6 @@ import { NonDeletedExcalidrawElement } from "../../../element/types"; import { ImportedLibraryData } from "../../../data/types"; import CustomFooter from "./CustomFooter"; 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 { interface Window { @@ -128,8 +112,6 @@ export default function App() { if (!excalidrawAPI) { return; } - excalidrawAPI.actionManager.registerAction(exampleAction); - excalidrawAPI.actionManager.registerActionPredicate(examplePredicateFn); const fetchData = async () => { const res = await fetch("/rocket.jpeg"); const imageData = await res.blob(); @@ -166,29 +148,6 @@ export default function App() { }} /> )} -