feat: Add separators on context menu (#2659)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dwelle <luzar.david@gmail.com> Co-authored-by: Lipis <lipiridis@gmail.com>
2021-01-28 00:41:17 +05:30
parent b5e26ba81f
commit 978e85a33b
18 changed files with 354 additions and 173 deletions
--- a/src/components/App.tsx
+++ b/src/components/App.tsx
@ -3,7 +3,28 @@ import React from "react";
 import { RoughCanvas } from "roughjs/bin/canvas";
 import rough from "roughjs/bin/rough";
 import "../actions";
-import { actionDeleteSelected, actionFinalize } from "../actions";
+import {
+  actionAddToLibrary,
+  actionBringForward,
+  actionBringToFront,
+  actionCopy,
+  actionCopyAsPng,
+  actionCopyAsSvg,
+  actionCopyStyles,
+  actionCut,
+  actionDeleteSelected,
+  actionDuplicateSelection,
+  actionFinalize,
+  actionGroup,
+  actionPasteStyles,
+  actionSelectAll,
+  actionSendBackward,
+  actionSendToBack,
+  actionToggleGridMode,
+  actionToggleStats,
+  actionToggleZenMode,
+  actionUngroup,
+} from "../actions";
 import { createRedoAction, createUndoAction } from "../actions/actionHistory";
 import { ActionManager } from "../actions/manager";
 import { actions } from "../actions/register";
@ -18,7 +39,6 @@ import {
 } from "../clipboard";
 import {
  APP_NAME,
-  CANVAS_ONLY_ACTIONS,
  CURSOR_TYPE,
  DEFAULT_VERTICAL_ALIGN,
  DRAGGING_THRESHOLD,
@ -26,7 +46,6 @@ import {
  ELEMENT_TRANSLATE_AMOUNT,
  ENV,
  EVENT,
-  GRID_SIZE,
  LINE_CONFIRM_THRESHOLD,
  MIME_TYPES,
  POINTER_BUTTON,
@ -314,6 +333,7 @@ class App extends React.Component<ExcalidrawProps, AppState> {
      this.syncActionResult,
      () => this.state,
      () => this.scene.getElementsIncludingDeleted(),
+      this,
    );
    this.actionManager.registerAll(actions);

@ -927,25 +947,6 @@ class App extends React.Component<ExcalidrawProps, AppState> {
    }
  };

-  private copyToClipboardAsSvg = async () => {
-    const selectedElements = getSelectedElements(
-      this.scene.getElements(),
-      this.state,
-    );
-    try {
-      await exportCanvas(
-        "clipboard-svg",
-        selectedElements.length ? selectedElements : this.scene.getElements(),
-        this.state,
-        this.canvas!,
-        this.state,
-      );
-    } catch (error) {
-      console.error(error);
-      this.setState({ errorMessage: error.message });
-    }
-  };
-
  private static resetTapTwice() {
    didTapTwice = false;
  }
@ -1148,15 +1149,11 @@ class App extends React.Component<ExcalidrawProps, AppState> {
  };

  toggleZenMode = () => {
-    this.setState({
-      zenModeEnabled: !this.state.zenModeEnabled,
-    });
+    this.actionManager.executeAction(actionToggleZenMode);
  };

  toggleGridMode = () => {
-    this.setState({
-      gridSize: this.state.gridSize ? null : GRID_SIZE,
-    });
+    this.actionManager.executeAction(actionToggleGridMode);
  };

  toggleStats = () => {
@ -3618,52 +3615,52 @@ class App extends React.Component<ExcalidrawProps, AppState> {
      this.state,
    );

+    const maybeGroupAction = actionGroup.contextItemPredicate!(
+      this.actionManager.getElementsIncludingDeleted(),
+      this.actionManager.getAppState(),
+    );
+
+    const maybeUngroupAction = actionUngroup.contextItemPredicate!(
+      this.actionManager.getElementsIncludingDeleted(),
+      this.actionManager.getAppState(),
+    );
+
+    const separator = "separator";
+
    const elements = this.scene.getElements();
    const element = this.getElementAtPosition(x, y);
    if (!element) {
      ContextMenu.push({
        options: [
          navigator.clipboard && {
-            shortcutName: "paste",
-            label: t("labels.paste"),
-            action: () => this.pasteFromClipboard(null),
+            name: "paste",
+            perform: (elements, appStates) => {
+              this.pasteFromClipboard(null);
+              return {
+                commitToHistory: false,
+              };
+            },
+            contextItemLabel: "labels.paste",
          },
+          separator,
          probablySupportsClipboardBlob &&
-            elements.length > 0 && {
-              shortcutName: "copyAsPng",
-              label: t("labels.copyAsPng"),
-              action: this.copyToClipboardAsPng,
-            },
+            elements.length > 0 &&
+            actionCopyAsPng,
          probablySupportsClipboardWriteText &&
-            elements.length > 0 && {
-              shortcutName: "copyAsSvg",
-              label: t("labels.copyAsSvg"),
-              action: this.copyToClipboardAsSvg,
-            },
-          ...this.actionManager.getContextMenuItems((action) =>
-            CANVAS_ONLY_ACTIONS.includes(action.name),
-          ),
-          {
-            checked: this.state.gridSize !== null,
-            shortcutName: "gridMode",
-            label: t("labels.gridMode"),
-            action: this.toggleGridMode,
-          },
-          {
-            checked: this.state.zenModeEnabled,
-            shortcutName: "zenMode",
-            label: t("buttons.zenMode"),
-            action: this.toggleZenMode,
-          },
-          {
-            checked: this.state.showStats,
-            shortcutName: "stats",
-            label: t("stats.title"),
-            action: this.toggleStats,
-          },
+            elements.length > 0 &&
+            actionCopyAsSvg,
+          ((probablySupportsClipboardBlob && elements.length > 0) ||
+            (probablySupportsClipboardWriteText && elements.length > 0)) &&
+            separator,
+          actionSelectAll,
+          separator,
+          actionToggleGridMode,
+          actionToggleZenMode,
+          actionToggleStats,
        ],
        top: clientY,
        left: clientX,
+        actionManager: this.actionManager,
      });
      return;
    }
@ -3674,37 +3671,41 @@ class App extends React.Component<ExcalidrawProps, AppState> {

    ContextMenu.push({
      options: [
-        {
-          shortcutName: "cut",
-          label: t("labels.cut"),
-          action: this.cutAll,
-        },
+        actionCut,
+        navigator.clipboard && actionCopy,
        navigator.clipboard && {
-          shortcutName: "copy",
-          label: t("labels.copy"),
-          action: this.copyAll,
+          name: "paste",
+          perform: (elements, appStates) => {
+            this.pasteFromClipboard(null);
+            return {
+              commitToHistory: false,
+            };
+          },
+          contextItemLabel: "labels.paste",
        },
-        navigator.clipboard && {
-          shortcutName: "paste",
-          label: t("labels.paste"),
-          action: () => this.pasteFromClipboard(null),
-        },
-        probablySupportsClipboardBlob && {
-          shortcutName: "copyAsPng",
-          label: t("labels.copyAsPng"),
-          action: this.copyToClipboardAsPng,
-        },
-        probablySupportsClipboardWriteText && {
-          shortcutName: "copyAsSvg",
-          label: t("labels.copyAsSvg"),
-          action: this.copyToClipboardAsSvg,
-        },
-        ...this.actionManager.getContextMenuItems(
-          (action) => !CANVAS_ONLY_ACTIONS.includes(action.name),
-        ),
+        separator,
+        probablySupportsClipboardBlob && actionCopyAsPng,
+        probablySupportsClipboardWriteText && actionCopyAsSvg,
+        separator,
+        actionCopyStyles,
+        actionPasteStyles,
+        separator,
+        maybeGroupAction && actionGroup,
+        maybeUngroupAction && actionUngroup,
+        (maybeGroupAction || maybeUngroupAction) && separator,
+        actionAddToLibrary,
+        separator,
+        actionSendBackward,
+        actionBringForward,
+        actionSendToBack,
+        actionBringToFront,
+        separator,
+        actionDuplicateSelection,
+        actionDeleteSelected,
      ],
      top: clientY,
      left: clientX,
+      actionManager: this.actionManager,
    });
  };

--- a/src/components/ContextMenu.scss
+++ b/src/components/ContextMenu.scss
@ -9,9 +9,10 @@
    list-style: none;
    user-select: none;
    margin: -0.25rem 0 0 0.125rem;
-    padding: 0.25rem 0;
+    padding: 0.5rem 0;
    background-color: var(--popup-secondary-background-color);
    border: 1px solid var(--button-gray-3);
+    cursor: default;
  }

  .context-menu button {
@ -88,4 +89,9 @@
      }
    }
  }
+
+  .context-menu-option-separator {
+    border: none;
+    border-top: 1px solid $oc-gray-5;
+  }
 }
--- a/src/components/ContextMenu.tsx
+++ b/src/components/ContextMenu.tsx
@ -2,31 +2,37 @@ import React from "react";
 import { render, unmountComponentAtNode } from "react-dom";
 import clsx from "clsx";
 import { Popover } from "./Popover";
+import { t } from "../i18n";

 import "./ContextMenu.scss";
 import {
  getShortcutFromShortcutName,
  ShortcutName,
 } from "../actions/shortcuts";
+import { Action } from "../actions/types";
+import { ActionManager } from "../actions/manager";

-type ContextMenuOption = {
-  checked?: boolean;
-  shortcutName: ShortcutName;
-  label: string;
-  action(): void;
-};
+type ContextMenuOption = "separator" | Action;

-type Props = {
+type ContextMenuProps = {
  options: ContextMenuOption[];
  onCloseRequest?(): void;
  top: number;
  left: number;
+  actionManager: ActionManager;
 };

-const ContextMenu = ({ options, onCloseRequest, top, left }: Props) => {
+const ContextMenu = ({
+  options,
+  onCloseRequest,
+  top,
+  left,
+  actionManager,
+}: ContextMenuProps) => {
  const isDarkTheme = !!document
    .querySelector(".excalidraw")
    ?.classList.contains("Appearance_dark");
+
  return (
    <div
      className={clsx("excalidraw", {
@ -43,23 +49,33 @@ const ContextMenu = ({ options, onCloseRequest, top, left }: Props) => {
          className="context-menu"
          onContextMenu={(event) => event.preventDefault()}
        >
-          {options.map(({ action, checked, shortcutName, label }, idx) => (
-            <li data-testid={shortcutName} key={idx} onClick={onCloseRequest}>
-              <button
-                className={`context-menu-option 
-                ${shortcutName === "delete" ? "dangerous" : ""}
-                ${checked ? "checkmark" : ""}`}
-                onClick={action}
-              >
-                <div className="context-menu-option__label">{label}</div>
-                <kbd className="context-menu-option__shortcut">
-                  {shortcutName
-                    ? getShortcutFromShortcutName(shortcutName)
-                    : ""}
-                </kbd>
-              </button>
-            </li>
-          ))}
+          {options.map((option, idx) => {
+            if (option === "separator") {
+              return <hr key={idx} className="context-menu-option-separator" />;
+            }
+
+            const actionName = option.name;
+            const label = option.contextItemLabel
+              ? t(option.contextItemLabel)
+              : "";
+            return (
+              <li key={idx} data-testid={actionName} onClick={onCloseRequest}>
+                <button
+                  className={`context-menu-option
+                  ${actionName === "deleteSelectedElements" ? "dangerous" : ""}
+                  ${option.checked ? "checkmark" : ""}`}
+                  onClick={() => actionManager.executeAction(option)}
+                >
+                  <div className="context-menu-option__label">{label}</div>
+                  <kbd className="context-menu-option__shortcut">
+                    {actionName
+                      ? getShortcutFromShortcutName(actionName as ShortcutName)
+                      : ""}
+                  </kbd>
+                </button>
+              </li>
+            );
+          })}
        </ul>
      </Popover>
    </div>
@ -78,8 +94,9 @@ const getContextMenuNode = (): HTMLDivElement => {

 type ContextMenuParams = {
  options: (ContextMenuOption | false | null | undefined)[];
-  top: number;
-  left: number;
+  top: ContextMenuProps["top"];
+  left: ContextMenuProps["left"];
+  actionManager: ContextMenuProps["actionManager"];
 };

 const handleClose = () => {
@ -101,6 +118,7 @@ export default {
          left={params.left}
          options={options}
          onCloseRequest={handleClose}
+          actionManager={params.actionManager}
        />,
        getContextMenuNode(),
      );