From 0c3d34261ec5480bdbb647c501445d5e254cac1d Mon Sep 17 00:00:00 2001
From: Lipis <lipiridis@gmail.com>
Date: Tue, 7 Apr 2020 14:39:06 +0300
Subject: [PATCH] Fixes in Shortcuts dialog and minor refactor (#1297)

---
 src/actions/actionCanvas.tsx             |   4 +-
 src/actions/actionDuplicateSelection.tsx |   2 +-
 src/actions/actionZindex.tsx             |   8 +-
 src/components/Actions.tsx               |  11 ++-
 src/components/HelpIcon.tsx              |   6 +-
 src/components/LockIcon.tsx              |   3 +-
 src/components/ShortcutsDialog.tsx       | 121 +++++++++++++----------
 src/components/ToolButton.tsx            |   2 +
 src/locales/en.json                      |   1 +
 src/tests/queries/toolQueries.ts         |  14 +--
 src/utils.ts                             |   6 +-
 11 files changed, 96 insertions(+), 82 deletions(-)

diff --git a/src/actions/actionCanvas.tsx b/src/actions/actionCanvas.tsx
index a6dc57fa..a819a17b 100644
--- a/src/actions/actionCanvas.tsx
+++ b/src/actions/actionCanvas.tsx
@@ -90,7 +90,7 @@ export const actionZoomIn = register({
     <ToolButton
       type="button"
       icon={zoomIn}
-      title={`${t("buttons.zoomIn")} ${getShortcutKey("CtrlOrCmd++")}`}
+      title={`${t("buttons.zoomIn")} — ${getShortcutKey("CtrlOrCmd++")}`}
       aria-label={t("buttons.zoomIn")}
       onClick={() => {
         updateData(null);
@@ -117,7 +117,7 @@ export const actionZoomOut = register({
     <ToolButton
       type="button"
       icon={zoomOut}
-      title={`${t("buttons.zoomOut")} ${getShortcutKey("CtrlOrCmd+-")}`}
+      title={`${t("buttons.zoomOut")} — ${getShortcutKey("CtrlOrCmd+-")}`}
       aria-label={t("buttons.zoomOut")}
       onClick={() => {
         updateData(null);
diff --git a/src/actions/actionDuplicateSelection.tsx b/src/actions/actionDuplicateSelection.tsx
index 61b4d206..5ab54e5b 100644
--- a/src/actions/actionDuplicateSelection.tsx
+++ b/src/actions/actionDuplicateSelection.tsx
@@ -38,7 +38,7 @@ export const actionDuplicateSelection = register({
     <ToolButton
       type="button"
       icon={clone}
-      title={`${t("labels.duplicateSelection")} ${getShortcutKey(
+      title={`${t("labels.duplicateSelection")} — ${getShortcutKey(
         "CtrlOrCmd+D",
       )}`}
       aria-label={t("labels.duplicateSelection")}
diff --git a/src/actions/actionZindex.tsx b/src/actions/actionZindex.tsx
index c1408600..f42ef11c 100644
--- a/src/actions/actionZindex.tsx
+++ b/src/actions/actionZindex.tsx
@@ -91,7 +91,7 @@ export const actionSendBackward = register({
       type="button"
       className="zIndexButton"
       onClick={() => updateData(null)}
-      title={`${t("labels.sendBackward")} ${getShortcutKey("CtrlOrCmd+[")}`}
+      title={`${t("labels.sendBackward")} — ${getShortcutKey("CtrlOrCmd+[")}`}
     >
       {sendBackward}
     </button>
@@ -116,7 +116,7 @@ export const actionBringForward = register({
       type="button"
       className="zIndexButton"
       onClick={() => updateData(null)}
-      title={`${t("labels.bringForward")} ${getShortcutKey("CtrlOrCmd+]")}`}
+      title={`${t("labels.bringForward")} — ${getShortcutKey("CtrlOrCmd+]")}`}
     >
       {bringForward}
     </button>
@@ -145,7 +145,7 @@ export const actionSendToBack = register({
       type="button"
       className="zIndexButton"
       onClick={() => updateData(null)}
-      title={`${t("labels.sendToBack")} ${
+      title={`${t("labels.sendToBack")} — ${
         isDarwin
           ? getShortcutKey("CtrlOrCmd+Alt+[")
           : getShortcutKey("CtrlOrCmd+Shift+[")
@@ -178,7 +178,7 @@ export const actionBringToFront = register({
       type="button"
       className="zIndexButton"
       onClick={(event) => updateData(null)}
-      title={`${t("labels.bringToFront")} ${
+      title={`${t("labels.bringToFront")} — ${
         isDarwin
           ? getShortcutKey("CtrlOrCmd+Alt+]")
           : getShortcutKey("CtrlOrCmd+Shift+]")
diff --git a/src/components/Actions.tsx b/src/components/Actions.tsx
index 827989d5..44f74982 100644
--- a/src/components/Actions.tsx
+++ b/src/components/Actions.tsx
@@ -6,7 +6,7 @@ import { hasBackground, hasStroke, hasText, getTargetElement } from "../scene";
 import { t } from "../i18n";
 import { SHAPES } from "../shapes";
 import { ToolButton } from "./ToolButton";
-import { capitalizeString, getShortcutKey, setCursorForShape } from "../utils";
+import { capitalizeString, setCursorForShape } from "../utils";
 import Stack from "./Stack";
 import useIsMobile from "../is-mobile";
 
@@ -94,9 +94,9 @@ export function ShapesSwitcher({
     <>
       {SHAPES.map(({ value, icon }, index) => {
         const label = t(`toolBar.${value}`);
-        const shortcut = getShortcutKey(
-          `${capitalizeString(value)[0]}, ${index + 1}`,
-        );
+        const shortcut = `${capitalizeString(value)[0]} ${t(
+          "shortcutsDialog.or",
+        )} ${index + 1}`;
         return (
           <ToolButton
             key={value}
@@ -104,10 +104,11 @@ export function ShapesSwitcher({
             icon={icon}
             checked={elementType === value}
             name="editor-current-shape"
-            title={`${capitalizeString(label)} ${shortcut}`}
+            title={`${capitalizeString(label)} — ${shortcut}`}
             keyBindingLabel={`${index + 1}`}
             aria-label={capitalizeString(label)}
             aria-keyshortcuts={`${label[0]} ${index + 1}`}
+            data-testid={value}
             onChange={() => {
               setAppState({
                 elementType: value,
diff --git a/src/components/HelpIcon.tsx b/src/components/HelpIcon.tsx
index 5e700834..d5ed178b 100644
--- a/src/components/HelpIcon.tsx
+++ b/src/components/HelpIcon.tsx
@@ -1,5 +1,4 @@
 import React from "react";
-import { getShortcutKey } from "../utils";
 
 type HelpIconProps = {
   title?: string;
@@ -21,10 +20,7 @@ const ICON = (
 
 export function HelpIcon(props: HelpIconProps) {
   return (
-    <label
-      title={`${props.title} ${getShortcutKey("?")}`}
-      className="help-icon"
-    >
+    <label title={`${props.title} — ?`} className="help-icon">
       <div onClick={props.onClick}>{ICON}</div>
     </label>
   );
diff --git a/src/components/LockIcon.tsx b/src/components/LockIcon.tsx
index 9d576518..fcd330fd 100644
--- a/src/components/LockIcon.tsx
+++ b/src/components/LockIcon.tsx
@@ -1,7 +1,6 @@
 import "./ToolIcon.scss";
 
 import React from "react";
-import { getShortcutKey } from "../utils";
 
 type LockIconSize = "s" | "m";
 
@@ -46,7 +45,7 @@ export function LockIcon(props: LockIconProps) {
   return (
     <label
       className={`ToolIcon ToolIcon__lock ToolIcon_type_floating ${sizeCn}`}
-      title={`${props.title} ${getShortcutKey("Q")}`}
+      title={`${props.title} — Q`}
     >
       <input
         className="ToolIcon_type_checkbox"
diff --git a/src/components/ShortcutsDialog.tsx b/src/components/ShortcutsDialog.tsx
index e11730b5..d8cc6ab1 100644
--- a/src/components/ShortcutsDialog.tsx
+++ b/src/components/ShortcutsDialog.tsx
@@ -5,7 +5,7 @@ import { Dialog } from "./Dialog";
 import { getShortcutKey } from "../utils";
 
 const ShortcutIsland = (props: {
-  title: string;
+  caption: string;
   children: React.ReactNode;
 }) => (
   <div
@@ -24,13 +24,17 @@ const ShortcutIsland = (props: {
         textAlign: "center",
       }}
     >
-      {props.title}
+      {props.caption}
     </h3>
     {props.children}
   </div>
 );
 
-const Shortcut = (props: { title: string; shortcuts: string[] }) => (
+const Shortcut = (props: {
+  label: string;
+  shortcuts: string[];
+  isOr: boolean;
+}) => (
   <div
     style={{
       borderTop: "1px solid #ced4da",
@@ -53,7 +57,7 @@ const Shortcut = (props: { title: string; shortcuts: string[] }) => (
           lineHeight: 1.4,
         }}
       >
-        {props.title}
+        {props.label}
       </div>
       <div
         style={{
@@ -63,14 +67,23 @@ const Shortcut = (props: { title: string; shortcuts: string[] }) => (
           justifyContent: "center",
         }}
       >
-        {props.shortcuts.map((shortcut) => (
-          <ShortcutKey>{shortcut}</ShortcutKey>
+        {props.shortcuts.map((shortcut, index) => (
+          <>
+            <ShortcutKey>{shortcut}</ShortcutKey>
+            {props.isOr &&
+              index !== props.shortcuts.length - 1 &&
+              t("shortcutsDialog.or")}
+          </>
         ))}
       </div>
     </div>
   </div>
 );
 
+Shortcut.defaultProps = {
+  isOr: true,
+};
+
 const ShortcutKey = (props: { children: React.ReactNode }) => (
   <span
     style={{
@@ -90,7 +103,7 @@ const Footer = () => (
     style={{
       display: "flex",
       flexDirection: "row",
-      justifyContent: "space-between",
+      justifyContent: "space-evenly",
       borderTop: "1px solid #ced4da",
       marginTop: 8,
       paddingTop: 16,
@@ -142,96 +155,98 @@ export const ShortcutsDialog = ({ onClose }: { onClose?: () => void }) => {
             justifyContent: "space-between",
           }}
         >
-          <ShortcutIsland title={t("shortcutsDialog.shapes")}>
-            <Shortcut title={t("toolBar.selection")} shortcuts={["S", "1"]} />
-            <Shortcut title={t("toolBar.rectangle")} shortcuts={["R", "2"]} />
-            <Shortcut title={t("toolBar.diamond")} shortcuts={["D", "3"]} />
-            <Shortcut title={t("toolBar.ellipse")} shortcuts={["E", "4"]} />
-            <Shortcut title={t("toolBar.arrow")} shortcuts={["A", "5"]} />
-            <Shortcut title={t("toolBar.line")} shortcuts={["L", "6"]} />
-            <Shortcut title={t("toolBar.text")} shortcuts={["T", "7"]} />
+          <ShortcutIsland caption={t("shortcutsDialog.shapes")}>
+            <Shortcut label={t("toolBar.selection")} shortcuts={["S", "1"]} />
+            <Shortcut label={t("toolBar.rectangle")} shortcuts={["R", "2"]} />
+            <Shortcut label={t("toolBar.diamond")} shortcuts={["D", "3"]} />
+            <Shortcut label={t("toolBar.ellipse")} shortcuts={["E", "4"]} />
+            <Shortcut label={t("toolBar.arrow")} shortcuts={["A", "5"]} />
+            <Shortcut label={t("toolBar.line")} shortcuts={["L", "6"]} />
+            <Shortcut label={t("toolBar.text")} shortcuts={["T", "7"]} />
             <Shortcut
-              title={t("shortcutsDialog.curvedArrow")}
+              label={t("shortcutsDialog.curvedArrow")}
               shortcuts={["A", "click", "click", "click"]}
+              isOr={false}
             />
             <Shortcut
-              title={t("shortcutsDialog.curvedLine")}
+              label={t("shortcutsDialog.curvedLine")}
               shortcuts={["L", "click", "click", "click"]}
+              isOr={false}
             />
-            <Shortcut title={t("toolBar.lock")} shortcuts={["Q"]} />
+            <Shortcut label={t("toolBar.lock")} shortcuts={["Q"]} />
           </ShortcutIsland>
-          <ShortcutIsland title={t("shortcutsDialog.editor")}>
+          <ShortcutIsland caption={t("shortcutsDialog.editor")}>
             <Shortcut
-              title={t("labels.selectAll")}
-              shortcuts={[getShortcutKey("CtrlOrCmd+A", "")]}
+              label={t("labels.selectAll")}
+              shortcuts={[getShortcutKey("CtrlOrCmd+A")]}
             />
             <Shortcut
-              title={t("labels.copy")}
-              shortcuts={[getShortcutKey("CtrlOrCmd+C", "")]}
+              label={t("labels.copy")}
+              shortcuts={[getShortcutKey("CtrlOrCmd+C")]}
             />
             <Shortcut
-              title={t("labels.paste")}
-              shortcuts={[getShortcutKey("CtrlOrCmd+V", "")]}
+              label={t("labels.paste")}
+              shortcuts={[getShortcutKey("CtrlOrCmd+V")]}
             />
             <Shortcut
-              title={t("labels.copyAsPng")}
-              shortcuts={[getShortcutKey("Shift+Alt+C", "")]}
+              label={t("labels.copyAsPng")}
+              shortcuts={[getShortcutKey("Shift+Alt+C")]}
             />
             <Shortcut
-              title={t("labels.copyStyles")}
-              shortcuts={[getShortcutKey("CtrlOrCmd+Shift+C", "")]}
+              label={t("labels.copyStyles")}
+              shortcuts={[getShortcutKey("CtrlOrCmd+Shift+C")]}
             />
             <Shortcut
-              title={t("labels.pasteStyles")}
-              shortcuts={[getShortcutKey("CtrlOrCmd+Shift+V", "")]}
+              label={t("labels.pasteStyles")}
+              shortcuts={[getShortcutKey("CtrlOrCmd+Shift+V")]}
             />
             <Shortcut
-              title={t("labels.delete")}
-              shortcuts={[getShortcutKey("Del", "")]}
+              label={t("labels.delete")}
+              shortcuts={[getShortcutKey("Del")]}
             />
             <Shortcut
-              title={t("labels.sendToBack")}
+              label={t("labels.sendToBack")}
               shortcuts={[
                 isDarwin
-                  ? getShortcutKey("CtrlOrCmd+Alt+[", "")
-                  : getShortcutKey("CtrlOrCmd+Shift+[", ""),
+                  ? getShortcutKey("CtrlOrCmd+Alt+[")
+                  : getShortcutKey("CtrlOrCmd+Shift+["),
               ]}
             />
             <Shortcut
-              title={t("labels.bringToFront")}
+              label={t("labels.bringToFront")}
               shortcuts={[
                 isDarwin
-                  ? getShortcutKey("CtrlOrCmd+Alt+]", "")
-                  : getShortcutKey("CtrlOrCmd+Shift+]", ""),
+                  ? getShortcutKey("CtrlOrCmd+Alt+]")
+                  : getShortcutKey("CtrlOrCmd+Shift+]"),
               ]}
             />
             <Shortcut
-              title={t("labels.sendBackward")}
-              shortcuts={[getShortcutKey("CtrlOrCmd+[", "")]}
+              label={t("labels.sendBackward")}
+              shortcuts={[getShortcutKey("CtrlOrCmd+[")]}
             />
             <Shortcut
-              title={t("labels.bringForward")}
-              shortcuts={[getShortcutKey("CtrlOrCmd+]", "")]}
+              label={t("labels.bringForward")}
+              shortcuts={[getShortcutKey("CtrlOrCmd+]")]}
             />
             <Shortcut
-              title={t("labels.duplicateSelection")}
-              shortcuts={[getShortcutKey("CtrlOrCmd+D", "")]}
+              label={t("labels.duplicateSelection")}
+              shortcuts={[getShortcutKey("CtrlOrCmd+D")]}
             />
           </ShortcutIsland>
-          <ShortcutIsland title={t("shortcutsDialog.view")}>
+          <ShortcutIsland caption={t("shortcutsDialog.view")}>
             <Shortcut
-              title={t("buttons.zoomIn")}
-              shortcuts={[getShortcutKey("CtrlOrCmd++", "")]}
+              label={t("buttons.zoomIn")}
+              shortcuts={[getShortcutKey("CtrlOrCmd++")]}
             />
             <Shortcut
-              title={t("buttons.zoomOut")}
-              shortcuts={[getShortcutKey("CtrlOrCmd+-", "")]}
+              label={t("buttons.zoomOut")}
+              shortcuts={[getShortcutKey("CtrlOrCmd+-")]}
             />
             <Shortcut
-              title={t("buttons.resetZoom")}
-              shortcuts={[getShortcutKey("CtrlOrCmd+0", "")]}
+              label={t("buttons.resetZoom")}
+              shortcuts={[getShortcutKey("CtrlOrCmd+0")]}
             />
-            <Shortcut title={t("buttons.toggleFullScreen")} shortcuts={["F"]} />
+            <Shortcut label={t("buttons.toggleFullScreen")} shortcuts={["F"]} />
           </ShortcutIsland>
         </div>
         <Footer />
diff --git a/src/components/ToolButton.tsx b/src/components/ToolButton.tsx
index 3fb8ff79..0fe5450a 100644
--- a/src/components/ToolButton.tsx
+++ b/src/components/ToolButton.tsx
@@ -8,6 +8,7 @@ type ToolButtonBaseProps = {
   icon?: React.ReactNode;
   "aria-label": string;
   "aria-keyshortcuts"?: string;
+  "data-testid"?: string;
   label?: string;
   title?: string;
   name?: string;
@@ -78,6 +79,7 @@ export const ToolButton = React.forwardRef(function (
         name={props.name}
         aria-label={props["aria-label"]}
         aria-keyshortcuts={props["aria-keyshortcuts"]}
+        data-testid={props["data-testid"]}
         id={props.id}
         onChange={props.onChange}
         checked={props.checked}
diff --git a/src/locales/en.json b/src/locales/en.json
index ab11b74d..352c1c7d 100644
--- a/src/locales/en.json
+++ b/src/locales/en.json
@@ -134,6 +134,7 @@
   "shortcutsDialog": {
     "title": "Keyboard shortcuts",
     "shapes": "Shapes",
+    "or": "or",
     "curvedArrow": "Curved arrow",
     "curvedLine": "Curved line",
     "editor": "Editor",
diff --git a/src/tests/queries/toolQueries.ts b/src/tests/queries/toolQueries.ts
index cb111cfd..df1eb12b 100644
--- a/src/tests/queries/toolQueries.ts
+++ b/src/tests/queries/toolQueries.ts
@@ -1,19 +1,19 @@
 import { queries, buildQueries } from "@testing-library/react";
 
 const toolMap = {
-  selection: "Selection — S, 1",
-  rectangle: "Rectangle — R, 2",
-  diamond: "Diamond — D, 3",
-  ellipse: "Ellipse — E, 4",
-  arrow: "Arrow — A, 5",
-  line: "Line — L, 6",
+  selection: "selection",
+  rectangle: "rectangle",
+  diamond: "diamond",
+  ellipse: "ellipse",
+  arrow: "arrow",
+  line: "line",
 };
 
 export type ToolName = keyof typeof toolMap;
 
 const _getAllByToolName = (container: HTMLElement, tool: string) => {
   const toolTitle = toolMap[tool as ToolName];
-  return queries.getAllByTitle(container, toolTitle);
+  return queries.getAllByTestId(container, toolTitle);
 };
 
 const getMultipleError = (_container: any, tool: any) =>
diff --git a/src/utils.ts b/src/utils.ts
index eee06798..0cdd8a2c 100644
--- a/src/utils.ts
+++ b/src/utils.ts
@@ -161,17 +161,17 @@ export const allowFullScreen = () =>
 
 export const exitFullScreen = () => document.exitFullscreen();
 
-export const getShortcutKey = (shortcut: string, prefix = " — "): string => {
+export const getShortcutKey = (shortcut: string): string => {
   const isMac = /Mac|iPod|iPhone|iPad/.test(window.navigator.platform);
   if (isMac) {
-    return `${prefix}${shortcut
+    return `${shortcut
       .replace("CtrlOrCmd+", "⌘")
       .replace("Alt+", "⌥")
       .replace("Ctrl+", "⌃")
       .replace("Shift+", "⇧")
       .replace("Del", "⌫")}`;
   }
-  return `${prefix}${shortcut.replace("CtrlOrCmd", "Ctrl")}`;
+  return `${shortcut.replace("CtrlOrCmd", "Ctrl")}`;
 };
 export function viewportCoordsToSceneCoords(
   { clientX, clientY }: { clientX: number; clientY: number },