Refine shortcuts binding

getredash · Dec 23, 2019 · 16057bf · 16057bf
1 parent 8cf3e39
commit 16057bf
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 58 deletions.
diff --git a/client/app/components/queries/QueryEditor/QueryEditorControls.jsx b/client/app/components/queries/QueryEditor/QueryEditorControls.jsx
@@ -1,13 +1,37 @@
-import { map } from "lodash";
-import React from "react";
+import { isFunction, map, filter, fromPairs } from "lodash";
+import React, { useEffect } from "react";
 import PropTypes from "prop-types";
 import Tooltip from "antd/lib/tooltip";
 import Button from "antd/lib/button";
 import Select from "antd/lib/select";
+import { KeyboardShortcuts, humanReadableShortcut } from "@/services/keyboard-shortcuts";
 
 import AutocompleteToggle from "./AutocompleteToggle";
 import "./QueryEditorControls.less";
 
+function ButtonTooltip({ title, shortcut, ...props }) {
+  shortcut = humanReadableShortcut(shortcut);
+  title =
+    title && shortcut ? (
+      <React.Fragment>
+        {title} (<i>{shortcut}</i>)
+      </React.Fragment>
+    ) : (
+      title || shortcut
+    );
+  return <Tooltip placement="top" title={title} {...props} />;
+}
+
+ButtonTooltip.propTypes = {
+  title: PropTypes.node,
+  shortcut: PropTypes.string,
+};
+
+ButtonTooltip.defaultProps = {
+  title: null,
+  shortcut: null,
+};
+
 export default function EditorControl({
   addParameterButtonProps,
   formatButtonProps,
@@ -16,28 +40,42 @@ export default function EditorControl({
   autocompleteToggleProps,
   dataSourceSelectorProps,
 }) {
+  useEffect(() => {
+    const buttons = filter(
+      [addParameterButtonProps, formatButtonProps, saveButtonProps, executeButtonProps],
+      b => b.shortcut && !b.disabled && isFunction(b.onClick)
+    );
+    if (buttons.length > 0) {
+      const shortcuts = fromPairs(map(buttons, b => [b.shortcut, b.onClick]));
+      KeyboardShortcuts.bind(shortcuts);
+      return () => {
+        KeyboardShortcuts.unbind(shortcuts);
+      };
+    }
+  }, [addParameterButtonProps, formatButtonProps, saveButtonProps, executeButtonProps]);
+
   return (
     <div className="query-editor-controls">
       {addParameterButtonProps !== false && (
-        <Tooltip placement="top" title={addParameterButtonProps.title}>
+        <ButtonTooltip title={addParameterButtonProps.title} shortcut={addParameterButtonProps.shortcut}>
           <Button
             className="query-editor-controls-button m-r-5"
             disabled={addParameterButtonProps.disabled}
             onClick={addParameterButtonProps.onClick}>
             {"{{"}&nbsp;{"}}"}
           </Button>
-        </Tooltip>
+        </ButtonTooltip>
       )}
       {formatButtonProps !== false && (
-        <Tooltip placement="top" title={formatButtonProps.title}>
+        <ButtonTooltip title={formatButtonProps.title} shortcut={formatButtonProps.shortcut}>
           <Button
             className="query-editor-controls-button m-r-5"
             disabled={formatButtonProps.disabled}
             onClick={formatButtonProps.onClick}>
             <span className="zmdi zmdi-format-indent-increase" />
             {formatButtonProps.text}
           </Button>
-        </Tooltip>
+        </ButtonTooltip>
       )}
       {autocompleteToggleProps !== false && (
         <AutocompleteToggle
@@ -61,7 +99,7 @@ export default function EditorControl({
         </Select>
       )}
       {saveButtonProps !== false && (
-        <Tooltip placement="top" title={saveButtonProps.title}>
+        <ButtonTooltip title={saveButtonProps.title} shortcut={saveButtonProps.shortcut}>
           <Button
             className="query-editor-controls-button m-l-5"
             disabled={saveButtonProps.disabled}
@@ -70,10 +108,10 @@ export default function EditorControl({
             <span className="fa fa-floppy-o" />
             {saveButtonProps.text}
           </Button>
-        </Tooltip>
+        </ButtonTooltip>
       )}
       {executeButtonProps !== false && (
-        <Tooltip placement="top" title={executeButtonProps.title}>
+        <ButtonTooltip title={executeButtonProps.title} shortcut={executeButtonProps.shortcut}>
           <Button
             className="query-editor-controls-button m-l-5"
             type="primary"
@@ -83,7 +121,7 @@ export default function EditorControl({
             <span className="zmdi zmdi-play" />
             {executeButtonProps.text}
           </Button>
-        </Tooltip>
+        </ButtonTooltip>
       )}
     </div>
   );
@@ -96,6 +134,7 @@ const ButtonPropsPropType = PropTypes.oneOfType([
     disabled: PropTypes.bool,
     onClick: PropTypes.func,
     text: PropTypes.node,
+    shortcut: PropTypes.string,
   }),
 ]);
 

diff --git a/client/app/pages/queries/QuerySource.jsx b/client/app/pages/queries/QuerySource.jsx
@@ -19,7 +19,6 @@ import { Query } from "@/services/query";
 import { DataSource, SCHEMA_NOT_SUPPORTED } from "@/services/data-source";
 import notification from "@/services/notification";
 import recordEvent from "@/services/recordEvent";
-import { KeyboardShortcuts } from "@/services/keyboard-shortcuts";
 import navigateTo from "@/services/navigateTo";
 import localOptions from "@/lib/localOptions";
 
@@ -254,49 +253,24 @@ function QuerySource(props) {
 
   const canExecuteQuery = useMemo(
     () =>
+      !isEmpty(query.query) &&
+      !isQueryExecuting &&
       !dirtyParameters &&
       (query.is_safe || (currentUser.hasPermission("execute_query") && dataSource && !dataSource.view_only)),
-    [dirtyParameters, query, dataSource]
+    [isQueryExecuting, dirtyParameters, query, dataSource]
   );
   const isDirty = query.query !== originalQuerySource;
 
   const doExecuteQuery = useCallback(() => {
+    if (!canExecuteQuery) {
+      return;
+    }
     if (isDirty || !isEmpty(selectedText)) {
       executeAdhocQuery(selectedText);
     } else {
       executeQuery();
     }
-  }, [isDirty, selectedText, executeQuery, executeAdhocQuery]);
-
-  useEffect(() => {
-    const shortcuts = {
-      "mod+s": saveQuery,
-      "mod+p": openAddNewParameterDialog,
-      "mod+shift+f": () => formatQuery,
-    };
-
-    if (!isEmpty(query.query) && canExecuteQuery && !isQueryExecuting) {
-      extend(shortcuts, {
-        "mod+enter": doExecuteQuery,
-        "alt+enter": doExecuteQuery,
-      });
-    }
-
-    KeyboardShortcuts.bind(shortcuts);
-    return () => {
-      KeyboardShortcuts.unbind(shortcuts);
-    };
-  }, [
-    doExecuteQuery,
-    saveQuery,
-    openAddNewParameterDialog,
-    formatQuery,
-    query.query,
-    canExecuteQuery,
-    isQueryExecuting,
-  ]);
-
-  const modKey = KeyboardShortcuts.modKey;
+  }, [canExecuteQuery, isDirty, selectedText, executeQuery, executeAdhocQuery]);
 
   return (
     <div className="query-page-wrapper">
@@ -348,36 +322,30 @@ function QuerySource(props) {
 
                   <QueryEditor.Controls
                     addParameterButtonProps={{
-                      title: (
-                        <React.Fragment>
-                          Add New Parameter (<i>{modKey} + P</i>)
-                        </React.Fragment>
-                      ),
+                      title: "Add New Parameter",
+                      shortcut: "mod+p",
                       onClick: openAddNewParameterDialog,
                     }}
                     formatButtonProps={{
-                      title: (
-                        <React.Fragment>
-                          Format Query (<i>{modKey} + Shift + F</i>)
-                        </React.Fragment>
-                      ),
+                      title: "Format Query",
+                      shortcut: "mod+shift+f",
                       onClick: formatQuery,
                     }}
                     saveButtonProps={
                       query.can_edit && {
-                        title: `${modKey} + S`,
                         text: (
                           <React.Fragment>
                             <span className="hidden-xs">Save</span>
                             {isDirty ? "*" : null}
                           </React.Fragment>
                         ),
+                        shortcut: "mod+s",
                         onClick: saveQuery,
                       }
                     }
                     executeButtonProps={{
-                      title: `${modKey} + Enter`,
-                      disabled: isEmpty(query.query) || !canExecuteQuery || isQueryExecuting,
+                      disabled: !canExecuteQuery,
+                      shortcut: "mod+enter, alt+enter",
                       onClick: doExecuteQuery,
                       text: <span className="hidden-xs">{selectedText === null ? "Execute" : "Execute Selected"}</span>,
                     }}

diff --git a/client/app/services/keyboard-shortcuts.js b/client/app/services/keyboard-shortcuts.js
@@ -1,9 +1,26 @@
-import { each, trim, without } from "lodash";
+import { each, filter, map, toLower, toString, trim, upperFirst, without } from "lodash";
 import Mousetrap from "mousetrap";
 import "mousetrap/plugins/global-bind/mousetrap-global-bind";
 
+const modKey = /Mac|iPod|iPhone|iPad/.test(navigator.platform) ? "Cmd" : "Ctrl";
+
 export let KeyboardShortcuts = null; // eslint-disable-line import/no-mutable-exports
 
+export function humanReadableShortcut(shortcut) {
+  const modifiers = {
+    mod: upperFirst(modKey),
+  };
+
+  shortcut = toLower(toString(shortcut));
+  shortcut = filter(map(shortcut.split(","), trim), s => s !== "");
+  shortcut = map(shortcut, sc => {
+    sc = filter(map(sc.split("+")), s => s !== "");
+    return map(sc, s => modifiers[s] || upperFirst(s)).join(" + ");
+  }).join(", ");
+
+  return shortcut !== "" ? shortcut : null;
+}
+
 const handlers = {};
 
 function onShortcut(event, shortcut) {
@@ -13,7 +30,7 @@ function onShortcut(event, shortcut) {
 }
 
 function KeyboardShortcutsService() {
-  this.modKey = /Mac|iPod|iPhone|iPad/.test(navigator.platform) ? "Cmd" : "Ctrl";
+  this.modKey = modKey;
 
   this.bind = function bind(keymap) {
     each(keymap, (fn, key) => {