From 58accc9310637d0aeed983f0d3b3f245540b2c46 Mon Sep 17 00:00:00 2001
From: David Luzar <luzar.david@gmail.com>
Date: Sun, 6 Nov 2022 20:14:53 +0100
Subject: [PATCH] feat: tweak toolbar shortcuts & remove library shortcut
 (#5832)

---
 src/components/Actions.tsx                    |   9 +-
 src/components/App.tsx                        |  12 -
 src/components/HelpDialog.tsx                 |  46 +-
 src/components/LibraryButton.tsx              |   2 +-
 src/keys.ts                                   |  11 +
 src/shapes.tsx                                |  16 +-
 .../regressionTests.test.tsx.snap             | 456 +++++++++---------
 src/tests/regressionTests.test.tsx            |   2 +-
 8 files changed, 291 insertions(+), 263 deletions(-)

diff --git a/src/components/Actions.tsx b/src/components/Actions.tsx
index 972c7ba8..75233d18 100644
--- a/src/components/Actions.tsx
+++ b/src/components/Actions.tsx
@@ -218,13 +218,12 @@ export const ShapesSwitcher = ({
   appState: AppState;
 }) => (
   <>
-    {SHAPES.map(({ value, icon, key, fillable }, index) => {
-      const numberKey = value === "eraser" ? 0 : index + 1;
+    {SHAPES.map(({ value, icon, key, numericKey, fillable }, index) => {
       const label = t(`toolBar.${value}`);
       const letter = key && (typeof key === "string" ? key : key[0]);
       const shortcut = letter
-        ? `${capitalizeString(letter)} ${t("helpDialog.or")} ${numberKey}`
-        : `${numberKey}`;
+        ? `${capitalizeString(letter)} ${t("helpDialog.or")} ${numericKey}`
+        : `${numericKey}`;
       return (
         <ToolButton
           className={clsx("Shape", { fillable })}
@@ -234,7 +233,7 @@ export const ShapesSwitcher = ({
           checked={activeTool.type === value}
           name="editor-current-shape"
           title={`${capitalizeString(label)} — ${shortcut}`}
-          keyBindingLabel={`${numberKey}`}
+          keyBindingLabel={numericKey}
           aria-label={capitalizeString(label)}
           aria-keyshortcuts={shortcut}
           data-testid={`toolbar-${value}`}
diff --git a/src/components/App.tsx b/src/components/App.tsx
index 9df3bb67..97905d31 100644
--- a/src/components/App.tsx
+++ b/src/components/App.tsx
@@ -1912,18 +1912,6 @@ class App extends React.Component<AppProps, AppState> {
         this.setState({ isBindingEnabled: false });
       }
 
-      if (event.code === CODES.ZERO) {
-        const nextState = this.toggleMenu("library");
-        // track only openings
-        if (nextState) {
-          trackEvent(
-            "library",
-            "toggleLibrary (open)",
-            `keyboard (${this.device.isMobile ? "mobile" : "desktop"})`,
-          );
-        }
-      }
-
       if (isArrowKey(event.key)) {
         const step =
           (this.state.gridSize &&
diff --git a/src/components/HelpDialog.tsx b/src/components/HelpDialog.tsx
index 96edf05f..1228cb91 100644
--- a/src/components/HelpDialog.tsx
+++ b/src/components/HelpDialog.tsx
@@ -1,6 +1,6 @@
 import React from "react";
 import { t } from "../i18n";
-import { isDarwin, isWindows } from "../keys";
+import { isDarwin, isWindows, KEYS } from "../keys";
 import { Dialog } from "./Dialog";
 import { getShortcutKey } from "../utils";
 import "./HelpDialog.scss";
@@ -118,22 +118,42 @@ export const HelpDialog = ({ onClose }: { onClose?: () => void }) => {
             className="HelpDialog__island--tools"
             caption={t("helpDialog.tools")}
           >
-            <Shortcut label={t("toolBar.selection")} shortcuts={["V", "1"]} />
-            <Shortcut label={t("toolBar.rectangle")} shortcuts={["R", "2"]} />
-            <Shortcut label={t("toolBar.diamond")} shortcuts={["D", "3"]} />
-            <Shortcut label={t("toolBar.ellipse")} shortcuts={["O", "4"]} />
-            <Shortcut label={t("toolBar.arrow")} shortcuts={["A", "5"]} />
-            <Shortcut label={t("toolBar.line")} shortcuts={["P", "6"]} />
+            <Shortcut
+              label={t("toolBar.selection")}
+              shortcuts={[KEYS.V, KEYS["1"]]}
+            />
+            <Shortcut
+              label={t("toolBar.rectangle")}
+              shortcuts={[KEYS.R, KEYS["2"]]}
+            />
+            <Shortcut
+              label={t("toolBar.diamond")}
+              shortcuts={[KEYS.D, KEYS["3"]]}
+            />
+            <Shortcut
+              label={t("toolBar.ellipse")}
+              shortcuts={[KEYS.O, KEYS["4"]]}
+            />
+            <Shortcut
+              label={t("toolBar.arrow")}
+              shortcuts={[KEYS.A, KEYS["5"]]}
+            />
+            <Shortcut
+              label={t("toolBar.line")}
+              shortcuts={[KEYS.P, KEYS["6"]]}
+            />
             <Shortcut
               label={t("toolBar.freedraw")}
-              shortcuts={["Shift + P", "X", "7"]}
+              shortcuts={["Shift + P", KEYS["7"]]}
             />
-            <Shortcut label={t("toolBar.text")} shortcuts={["T", "8"]} />
-            <Shortcut label={t("toolBar.image")} shortcuts={["9"]} />
-            <Shortcut label={t("toolBar.library")} shortcuts={["0"]} />
+            <Shortcut
+              label={t("toolBar.text")}
+              shortcuts={[KEYS.T, KEYS["8"]]}
+            />
+            <Shortcut label={t("toolBar.image")} shortcuts={[KEYS["9"]]} />
             <Shortcut
               label={t("toolBar.eraser")}
-              shortcuts={[getShortcutKey("E")]}
+              shortcuts={[KEYS.E, KEYS["0"]]}
             />
             <Shortcut
               label={t("helpDialog.editSelectedShape")}
@@ -173,7 +193,7 @@ export const HelpDialog = ({ onClose }: { onClose?: () => void }) => {
               ]}
               isOr={false}
             />
-            <Shortcut label={t("toolBar.lock")} shortcuts={["Q"]} />
+            <Shortcut label={t("toolBar.lock")} shortcuts={[KEYS.Q]} />
             <Shortcut
               label={t("helpDialog.preventBinding")}
               shortcuts={[getShortcutKey("CtrlOrCmd")]}
diff --git a/src/components/LibraryButton.tsx b/src/components/LibraryButton.tsx
index ef18e337..517e802a 100644
--- a/src/components/LibraryButton.tsx
+++ b/src/components/LibraryButton.tsx
@@ -22,7 +22,7 @@ export const LibraryButton: React.FC<{
   }
 
   return (
-    <label title={`${capitalizeString(t("toolBar.library"))} — 0`}>
+    <label title={`${capitalizeString(t("toolBar.library"))}`}>
       <input
         className="ToolIcon_type_checkbox"
         type="checkbox"
diff --git a/src/keys.ts b/src/keys.ts
index 92ff1084..71f34e7a 100644
--- a/src/keys.ts
+++ b/src/keys.ts
@@ -63,6 +63,17 @@ export const KEYS = {
   Y: "y",
   Z: "z",
   K: "k",
+
+  0: "0",
+  1: "1",
+  2: "2",
+  3: "3",
+  4: "4",
+  5: "5",
+  6: "6",
+  7: "7",
+  8: "8",
+  9: "9",
 } as const;
 
 export type Key = keyof typeof KEYS;
diff --git a/src/shapes.tsx b/src/shapes.tsx
index 0eb74b9b..4c48a27b 100644
--- a/src/shapes.tsx
+++ b/src/shapes.tsx
@@ -17,60 +17,70 @@ export const SHAPES = [
     icon: SelectionIcon,
     value: "selection",
     key: KEYS.V,
+    numericKey: KEYS["1"],
     fillable: true,
   },
   {
     icon: RectangleIcon,
     value: "rectangle",
     key: KEYS.R,
+    numericKey: KEYS["2"],
     fillable: true,
   },
   {
     icon: DiamondIcon,
     value: "diamond",
     key: KEYS.D,
+    numericKey: KEYS["3"],
     fillable: true,
   },
   {
     icon: EllipseIcon,
     value: "ellipse",
     key: KEYS.O,
+    numericKey: KEYS["4"],
     fillable: true,
   },
   {
     icon: ArrowIcon,
     value: "arrow",
     key: KEYS.A,
+    numericKey: KEYS["5"],
     fillable: true,
   },
   {
     icon: LineIcon,
     value: "line",
-    key: [KEYS.P, KEYS.L],
+    key: KEYS.L,
+    numericKey: KEYS["6"],
     fillable: true,
   },
   {
     icon: FreedrawIcon,
     value: "freedraw",
-    key: [KEYS.X, KEYS.P.toUpperCase()],
+    key: KEYS.P,
+    numericKey: KEYS["7"],
     fillable: false,
   },
   {
     icon: TextIcon,
     value: "text",
     key: KEYS.T,
+    numericKey: KEYS["8"],
     fillable: false,
   },
   {
     icon: ImageIcon,
     value: "image",
     key: null,
+    numericKey: KEYS["9"],
     fillable: false,
   },
   {
     icon: EraserIcon,
     value: "eraser",
     key: KEYS.E,
+    numericKey: KEYS["0"],
     fillable: false,
   },
 ] as const;
@@ -78,7 +88,7 @@ export const SHAPES = [
 export const findShapeByKey = (key: string) => {
   const shape = SHAPES.find((shape, index) => {
     return (
-      key === (shape.value === "eraser" ? 0 : index + 1).toString() ||
+      (shape.numericKey != null && key === shape.numericKey.toString()) ||
       (shape.key &&
         (typeof shape.key === "string"
           ? shape.key === key
diff --git a/src/tests/__snapshots__/regressionTests.test.tsx.snap b/src/tests/__snapshots__/regressionTests.test.tsx.snap
index 7cc38fbc..53e72100 100644
--- a/src/tests/__snapshots__/regressionTests.test.tsx.snap
+++ b/src/tests/__snapshots__/regressionTests.test.tsx.snap
@@ -12410,6 +12410,234 @@ exports[`regression tests key o selects ellipse tool: [end of test] number of el
 
 exports[`regression tests key o selects ellipse tool: [end of test] number of renders 1`] = `9`;
 
+exports[`regression tests key p selects freedraw tool: [end of test] appState 1`] = `
+Object {
+  "activeTool": Object {
+    "customType": null,
+    "lastActiveToolBeforeEraser": null,
+    "locked": false,
+    "type": "freedraw",
+  },
+  "collaborators": Map {},
+  "currentChartType": "bar",
+  "currentItemBackgroundColor": "transparent",
+  "currentItemEndArrowhead": "arrow",
+  "currentItemFillStyle": "hachure",
+  "currentItemFontFamily": 1,
+  "currentItemFontSize": 20,
+  "currentItemLinearStrokeSharpness": "round",
+  "currentItemOpacity": 100,
+  "currentItemRoughness": 1,
+  "currentItemStartArrowhead": null,
+  "currentItemStrokeColor": "#000000",
+  "currentItemStrokeSharpness": "sharp",
+  "currentItemStrokeStyle": "solid",
+  "currentItemStrokeWidth": 1,
+  "currentItemTextAlign": "left",
+  "cursorButton": "up",
+  "draggingElement": null,
+  "editingElement": null,
+  "editingGroupId": null,
+  "editingLinearElement": null,
+  "errorMessage": null,
+  "exportBackground": true,
+  "exportEmbedScene": false,
+  "exportScale": 1,
+  "exportWithDarkMode": false,
+  "fileHandle": null,
+  "gridSize": null,
+  "height": 768,
+  "isBindingEnabled": true,
+  "isLoading": false,
+  "isResizing": false,
+  "isRotating": false,
+  "isSidebarDocked": false,
+  "lastPointerDownWith": "mouse",
+  "multiElement": null,
+  "name": "Untitled-201933152653",
+  "offsetLeft": 0,
+  "offsetTop": 0,
+  "openDialog": null,
+  "openMenu": null,
+  "openPopup": null,
+  "openSidebar": null,
+  "pasteDialog": Object {
+    "data": null,
+    "shown": false,
+  },
+  "penDetected": false,
+  "penMode": false,
+  "pendingImageElementId": null,
+  "previousSelectedElementIds": Object {},
+  "resizingElement": null,
+  "scrollX": 0,
+  "scrollY": 0,
+  "scrolledOutside": false,
+  "selectedElementIds": Object {
+    "id0": false,
+  },
+  "selectedGroupIds": Object {},
+  "selectedLinearElement": null,
+  "selectionElement": null,
+  "shouldCacheIgnoreZoom": false,
+  "showHyperlinkPopup": false,
+  "showStats": false,
+  "showWelcomeScreen": true,
+  "startBoundElement": null,
+  "suggestedBindings": Array [],
+  "theme": "light",
+  "toast": null,
+  "viewBackgroundColor": "#ffffff",
+  "viewModeEnabled": false,
+  "width": 1024,
+  "zenModeEnabled": false,
+  "zoom": Object {
+    "value": 1,
+  },
+}
+`;
+
+exports[`regression tests key p selects freedraw tool: [end of test] element 0 1`] = `
+Object {
+  "angle": 0,
+  "backgroundColor": "transparent",
+  "boundElements": null,
+  "fillStyle": "hachure",
+  "groupIds": Array [],
+  "height": 10,
+  "id": "id0",
+  "isDeleted": false,
+  "lastCommittedPoint": Array [
+    10,
+    10,
+  ],
+  "link": null,
+  "locked": false,
+  "opacity": 100,
+  "points": Array [
+    Array [
+      0,
+      0,
+    ],
+    Array [
+      10,
+      10,
+    ],
+    Array [
+      10,
+      10,
+    ],
+  ],
+  "pressures": Array [
+    0,
+    0,
+    0,
+  ],
+  "roughness": 1,
+  "seed": 337897,
+  "simulatePressure": false,
+  "strokeColor": "#000000",
+  "strokeSharpness": "round",
+  "strokeStyle": "solid",
+  "strokeWidth": 1,
+  "type": "freedraw",
+  "updated": 1,
+  "version": 4,
+  "versionNonce": 453191,
+  "width": 10,
+  "x": 10,
+  "y": 10,
+}
+`;
+
+exports[`regression tests key p selects freedraw tool: [end of test] history 1`] = `
+Object {
+  "recording": false,
+  "redoStack": Array [],
+  "stateHistory": Array [
+    Object {
+      "appState": Object {
+        "editingGroupId": null,
+        "editingLinearElement": null,
+        "name": "Untitled-201933152653",
+        "selectedElementIds": Object {},
+        "selectedGroupIds": Object {},
+        "viewBackgroundColor": "#ffffff",
+      },
+      "elements": Array [],
+    },
+    Object {
+      "appState": Object {
+        "editingGroupId": null,
+        "editingLinearElement": null,
+        "name": "Untitled-201933152653",
+        "selectedElementIds": Object {
+          "id0": false,
+        },
+        "selectedGroupIds": Object {},
+        "viewBackgroundColor": "#ffffff",
+      },
+      "elements": Array [
+        Object {
+          "angle": 0,
+          "backgroundColor": "transparent",
+          "boundElements": null,
+          "fillStyle": "hachure",
+          "groupIds": Array [],
+          "height": 10,
+          "id": "id0",
+          "isDeleted": false,
+          "lastCommittedPoint": Array [
+            10,
+            10,
+          ],
+          "link": null,
+          "locked": false,
+          "opacity": 100,
+          "points": Array [
+            Array [
+              0,
+              0,
+            ],
+            Array [
+              10,
+              10,
+            ],
+            Array [
+              10,
+              10,
+            ],
+          ],
+          "pressures": Array [
+            0,
+            0,
+            0,
+          ],
+          "roughness": 1,
+          "seed": 337897,
+          "simulatePressure": false,
+          "strokeColor": "#000000",
+          "strokeSharpness": "round",
+          "strokeStyle": "solid",
+          "strokeWidth": 1,
+          "type": "freedraw",
+          "updated": 1,
+          "version": 4,
+          "versionNonce": 453191,
+          "width": 10,
+          "x": 10,
+          "y": 10,
+        },
+      ],
+    },
+  ],
+}
+`;
+
+exports[`regression tests key p selects freedraw tool: [end of test] number of elements 1`] = `1`;
+
+exports[`regression tests key p selects freedraw tool: [end of test] number of renders 1`] = `9`;
+
 exports[`regression tests key r selects rectangle tool: [end of test] appState 1`] = `
 Object {
   "activeTool": Object {
@@ -12590,234 +12818,6 @@ exports[`regression tests key r selects rectangle tool: [end of test] number of
 
 exports[`regression tests key r selects rectangle tool: [end of test] number of renders 1`] = `9`;
 
-exports[`regression tests key x selects freedraw tool: [end of test] appState 1`] = `
-Object {
-  "activeTool": Object {
-    "customType": null,
-    "lastActiveToolBeforeEraser": null,
-    "locked": false,
-    "type": "freedraw",
-  },
-  "collaborators": Map {},
-  "currentChartType": "bar",
-  "currentItemBackgroundColor": "transparent",
-  "currentItemEndArrowhead": "arrow",
-  "currentItemFillStyle": "hachure",
-  "currentItemFontFamily": 1,
-  "currentItemFontSize": 20,
-  "currentItemLinearStrokeSharpness": "round",
-  "currentItemOpacity": 100,
-  "currentItemRoughness": 1,
-  "currentItemStartArrowhead": null,
-  "currentItemStrokeColor": "#000000",
-  "currentItemStrokeSharpness": "sharp",
-  "currentItemStrokeStyle": "solid",
-  "currentItemStrokeWidth": 1,
-  "currentItemTextAlign": "left",
-  "cursorButton": "up",
-  "draggingElement": null,
-  "editingElement": null,
-  "editingGroupId": null,
-  "editingLinearElement": null,
-  "errorMessage": null,
-  "exportBackground": true,
-  "exportEmbedScene": false,
-  "exportScale": 1,
-  "exportWithDarkMode": false,
-  "fileHandle": null,
-  "gridSize": null,
-  "height": 768,
-  "isBindingEnabled": true,
-  "isLoading": false,
-  "isResizing": false,
-  "isRotating": false,
-  "isSidebarDocked": false,
-  "lastPointerDownWith": "mouse",
-  "multiElement": null,
-  "name": "Untitled-201933152653",
-  "offsetLeft": 0,
-  "offsetTop": 0,
-  "openDialog": null,
-  "openMenu": null,
-  "openPopup": null,
-  "openSidebar": null,
-  "pasteDialog": Object {
-    "data": null,
-    "shown": false,
-  },
-  "penDetected": false,
-  "penMode": false,
-  "pendingImageElementId": null,
-  "previousSelectedElementIds": Object {},
-  "resizingElement": null,
-  "scrollX": 0,
-  "scrollY": 0,
-  "scrolledOutside": false,
-  "selectedElementIds": Object {
-    "id0": false,
-  },
-  "selectedGroupIds": Object {},
-  "selectedLinearElement": null,
-  "selectionElement": null,
-  "shouldCacheIgnoreZoom": false,
-  "showHyperlinkPopup": false,
-  "showStats": false,
-  "showWelcomeScreen": true,
-  "startBoundElement": null,
-  "suggestedBindings": Array [],
-  "theme": "light",
-  "toast": null,
-  "viewBackgroundColor": "#ffffff",
-  "viewModeEnabled": false,
-  "width": 1024,
-  "zenModeEnabled": false,
-  "zoom": Object {
-    "value": 1,
-  },
-}
-`;
-
-exports[`regression tests key x selects freedraw tool: [end of test] element 0 1`] = `
-Object {
-  "angle": 0,
-  "backgroundColor": "transparent",
-  "boundElements": null,
-  "fillStyle": "hachure",
-  "groupIds": Array [],
-  "height": 10,
-  "id": "id0",
-  "isDeleted": false,
-  "lastCommittedPoint": Array [
-    10,
-    10,
-  ],
-  "link": null,
-  "locked": false,
-  "opacity": 100,
-  "points": Array [
-    Array [
-      0,
-      0,
-    ],
-    Array [
-      10,
-      10,
-    ],
-    Array [
-      10,
-      10,
-    ],
-  ],
-  "pressures": Array [
-    0,
-    0,
-    0,
-  ],
-  "roughness": 1,
-  "seed": 337897,
-  "simulatePressure": false,
-  "strokeColor": "#000000",
-  "strokeSharpness": "round",
-  "strokeStyle": "solid",
-  "strokeWidth": 1,
-  "type": "freedraw",
-  "updated": 1,
-  "version": 4,
-  "versionNonce": 453191,
-  "width": 10,
-  "x": 10,
-  "y": 10,
-}
-`;
-
-exports[`regression tests key x selects freedraw tool: [end of test] history 1`] = `
-Object {
-  "recording": false,
-  "redoStack": Array [],
-  "stateHistory": Array [
-    Object {
-      "appState": Object {
-        "editingGroupId": null,
-        "editingLinearElement": null,
-        "name": "Untitled-201933152653",
-        "selectedElementIds": Object {},
-        "selectedGroupIds": Object {},
-        "viewBackgroundColor": "#ffffff",
-      },
-      "elements": Array [],
-    },
-    Object {
-      "appState": Object {
-        "editingGroupId": null,
-        "editingLinearElement": null,
-        "name": "Untitled-201933152653",
-        "selectedElementIds": Object {
-          "id0": false,
-        },
-        "selectedGroupIds": Object {},
-        "viewBackgroundColor": "#ffffff",
-      },
-      "elements": Array [
-        Object {
-          "angle": 0,
-          "backgroundColor": "transparent",
-          "boundElements": null,
-          "fillStyle": "hachure",
-          "groupIds": Array [],
-          "height": 10,
-          "id": "id0",
-          "isDeleted": false,
-          "lastCommittedPoint": Array [
-            10,
-            10,
-          ],
-          "link": null,
-          "locked": false,
-          "opacity": 100,
-          "points": Array [
-            Array [
-              0,
-              0,
-            ],
-            Array [
-              10,
-              10,
-            ],
-            Array [
-              10,
-              10,
-            ],
-          ],
-          "pressures": Array [
-            0,
-            0,
-            0,
-          ],
-          "roughness": 1,
-          "seed": 337897,
-          "simulatePressure": false,
-          "strokeColor": "#000000",
-          "strokeSharpness": "round",
-          "strokeStyle": "solid",
-          "strokeWidth": 1,
-          "type": "freedraw",
-          "updated": 1,
-          "version": 4,
-          "versionNonce": 453191,
-          "width": 10,
-          "x": 10,
-          "y": 10,
-        },
-      ],
-    },
-  ],
-}
-`;
-
-exports[`regression tests key x selects freedraw tool: [end of test] number of elements 1`] = `1`;
-
-exports[`regression tests key x selects freedraw tool: [end of test] number of renders 1`] = `9`;
-
 exports[`regression tests make a group and duplicate it: [end of test] appState 1`] = `
 Object {
   "activeTool": Object {
diff --git a/src/tests/regressionTests.test.tsx b/src/tests/regressionTests.test.tsx
index 6c1ec82c..d8e149d9 100644
--- a/src/tests/regressionTests.test.tsx
+++ b/src/tests/regressionTests.test.tsx
@@ -138,7 +138,7 @@ describe("regression tests", () => {
     [`4${KEYS.O}`, "ellipse", true],
     [`5${KEYS.A}`, "arrow", true],
     [`6${KEYS.L}`, "line", true],
-    [`7${KEYS.X}`, "freedraw", false],
+    [`7${KEYS.P}`, "freedraw", false],
   ] as [string, ExcalidrawElement["type"], boolean][]) {
     for (const key of keys) {
       it(`key ${key} selects ${shape} tool`, () => {