treat cmd and ctrl key shortcuts separately on macOS (#15265)

* treat cmd and ctrl key shortcuts differently on macOS

* consolidate shortcut key string into Runtime helper

app/assets/javascripts/initializers/runtime.js

@@ -156,4 +156,14 @@ class Runtime {
       return event.ctrlKey;
     }
   }
+
+  /**
+   * Returns a string representation of the expected modifier key for the current OS.
+   * This allows us to display correct shortcut key hints to users in the UI, and set up correct shortcut key bindings.
+   *
+   * @returns {string} either 'cmd' if on macOS, or 'ctrl' otherwise
+   */
+  static getOSKeyboardModifierKeyString() {
+    return Runtime.currentOS() === 'macOS' ? 'cmd' : 'ctrl';
+  }
 }

app/javascript/.storybook/preview-head.html

@@ -1,7 +1,7 @@
 <script>
   var Runtime = {
-    currentOS: function () {
-      return 'macOS';
+    getOSKeyboardModifierKeyString: function () {
+      return 'cmd';
     },
   };
 </script>

app/javascript/article-form/articleForm.jsx

@@ -1,3 +1,4 @@
+/* global Runtime */
 import { h, Component } from 'preact';
 import PropTypes from 'prop-types';
 import linkState from 'linkstate';
@@ -476,7 +477,8 @@ export class ArticleForm extends Component {
 
         <KeyboardShortcuts
           shortcuts={{
-            'ctrl+shift+KeyP': this.fetchPreview,
+            [`${Runtime.getOSKeyboardModifierKeyString()}+shift+KeyP`]:
+              this.fetchPreview,
           }}
         />
       </form>

app/javascript/crayons/MarkdownToolbar/MarkdownToolbar.jsx

@@ -1,4 +1,3 @@
-/* global Runtime */
 import { h } from 'preact';
 import { useState, useLayoutEffect } from 'preact/hooks';
 import {
@@ -16,9 +15,6 @@ export const MarkdownToolbar = ({ textAreaId }) => {
   const [overflowMenuOpen, setOverflowMenuOpen] = useState(false);
   const smallScreen = useMediaQuery(`(max-width: ${BREAKPOINTS.Medium - 1}px)`);
 
-  const keyboardShortcutModifierText =
-    Runtime.currentOS() === 'macOS' ? 'CMD' : 'CTRL';
-
   const markdownSyntaxFormatters = {
     ...coreSyntaxFormatters,
     ...secondarySyntaxFormatters,
@@ -27,11 +23,13 @@ export const MarkdownToolbar = ({ textAreaId }) => {
   const keyboardShortcuts = Object.fromEntries(
     Object.keys(markdownSyntaxFormatters)
       .filter(
-        (syntaxName) => !!markdownSyntaxFormatters[syntaxName].keyboardShortcut,
+        (syntaxName) =>
+          !!markdownSyntaxFormatters[syntaxName].getKeyboardShortcut,
       )
       .map((syntaxName) => {
-        const { keyboardShortcut } = markdownSyntaxFormatters[syntaxName];
-        return [keyboardShortcut, () => insertSyntax(syntaxName)];
+        const { command } =
+          markdownSyntaxFormatters[syntaxName].getKeyboardShortcut?.();
+        return [command, () => insertSyntax(syntaxName)];
       }),
   );
 
@@ -189,8 +187,9 @@ export const MarkdownToolbar = ({ textAreaId }) => {
 
   const getSecondaryFormatterButtons = (isOverflow) =>
     Object.keys(secondarySyntaxFormatters).map((controlName, index) => {
-      const { icon, label, keyboardShortcutKeys } =
+      const { icon, label, getKeyboardShortcut } =
         secondarySyntaxFormatters[controlName];
+
       return (
         <Button
           key={`${controlName}-btn`}
@@ -216,9 +215,9 @@ export const MarkdownToolbar = ({ textAreaId }) => {
             smallScreen ? null : (
               <span aria-hidden="true">
                 {label}
-                {keyboardShortcutKeys ? (
+                {getKeyboardShortcut ? (
                   <span className="opacity-75">
-                    {` ${keyboardShortcutModifierText} + ${keyboardShortcutKeys}`}
+                    {` ${getKeyboardShortcut().tooltipHint}`}
                   </span>
                 ) : null}
               </span>
@@ -236,7 +235,7 @@ export const MarkdownToolbar = ({ textAreaId }) => {
       aria-controls={textAreaId}
     >
       {Object.keys(coreSyntaxFormatters).map((controlName, index) => {
-        const { icon, label, keyboardShortcutKeys } =
+        const { icon, label, getKeyboardShortcut } =
           coreSyntaxFormatters[controlName];
         return (
           <Button
@@ -253,9 +252,9 @@ export const MarkdownToolbar = ({ textAreaId }) => {
               smallScreen ? null : (
                 <span aria-hidden="true">
                   {label}
-                  {keyboardShortcutKeys ? (
+                  {getKeyboardShortcut ? (
                     <span className="opacity-75">
-                      {` ${keyboardShortcutModifierText} + ${keyboardShortcutKeys}`}
+                      {` ${getKeyboardShortcut().tooltipHint}`}
                     </span>
                   ) : null}
                 </span>

app/javascript/crayons/MarkdownToolbar/__tests__/MarkdownToolbar.test.jsx

@@ -7,7 +7,7 @@ import { MarkdownToolbar } from '../MarkdownToolbar';
 describe('<MarkdownToolbar />', () => {
   beforeEach(() => {
     global.Runtime = {
-      currentOS: jest.fn(() => 'macOS'),
+      getOSKeyboardModifierKeyString: jest.fn(() => 'cmd'),
     };
 
     global.window.matchMedia = jest.fn((query) => {

app/javascript/crayons/MarkdownToolbar/markdownSyntaxFormatters.js

@@ -1,3 +1,4 @@
+/* global Runtime */
 import {
   Bold,
   Italic,
@@ -22,8 +23,13 @@ export const coreSyntaxFormatters = {
   bold: {
     icon: Bold,
     label: 'Bold',
-    keyboardShortcut: 'ctrl+b',
-    keyboardShortcutKeys: `B`,
+    getKeyboardShortcut: () => {
+      const modifier = Runtime.getOSKeyboardModifierKeyString();
+      return {
+        command: `${modifier}+b`,
+        tooltipHint: `${modifier.toUpperCase()} + B`,
+      };
+    },
     getFormatting: (selection) => ({
       formattedText: `**${selection}**`,
       cursorOffsetStart: 2,
@@ -33,8 +39,13 @@ export const coreSyntaxFormatters = {
   italic: {
     icon: Italic,
     label: 'Italic',
-    keyboardShortcut: 'ctrl+i',
-    keyboardShortcutKeys: `I`,
+    getKeyboardShortcut: () => {
+      const modifier = Runtime.getOSKeyboardModifierKeyString();
+      return {
+        command: `${modifier}+i`,
+        tooltipHint: `${modifier.toUpperCase()} + I`,
+      };
+    },
     getFormatting: (selection) => ({
       formattedText: `_${selection}_`,
       cursorOffsetStart: 1,
@@ -44,8 +55,13 @@ export const coreSyntaxFormatters = {
   link: {
     icon: Link,
     label: 'Link',
-    keyboardShortcut: 'ctrl+k',
-    keyboardShortcutKeys: `K`,
+    getKeyboardShortcut: () => {
+      const modifier = Runtime.getOSKeyboardModifierKeyString();
+      return {
+        command: `${modifier}+k`,
+        tooltipHint: `${modifier.toUpperCase()} + K`,
+      };
+    },
     getFormatting: (selection) => {
       const isUrl = isStringStartAUrl(selection);
       const selectionLength = selection.length;
@@ -170,8 +186,13 @@ export const secondarySyntaxFormatters = {
   underline: {
     icon: Underline,
     label: 'Underline',
-    keyboardShortcut: 'ctrl+u',
-    keyboardShortcutKeys: `U`,
+    getKeyboardShortcut: () => {
+      const modifier = Runtime.getOSKeyboardModifierKeyString();
+      return {
+        command: `${modifier}+u`,
+        tooltipHint: `${modifier.toUpperCase()} + U`,
+      };
+    },
     getFormatting: (selection) => ({
       formattedText: `<u>${selection}</u>`,
       cursorOffsetStart: 3,
@@ -181,8 +202,13 @@ export const secondarySyntaxFormatters = {
   strikethrough: {
     icon: Strikethrough,
     label: 'Strikethrough',
-    keyboardShortcut: 'ctrl+shift+x',
-    keyboardShortcutKeys: `SHIFT + X`,
+    getKeyboardShortcut: () => {
+      const modifier = Runtime.getOSKeyboardModifierKeyString();
+      return {
+        command: `${modifier}+shift+x`,
+        tooltipHint: `${modifier.toUpperCase()} + SHIFT + X`,
+      };
+    },
     getFormatting: (selection) => ({
       formattedText: `~~${selection}~~`,
       cursorOffsetStart: 2,

app/javascript/shared/components/useKeyboardShortcuts.js

@@ -126,10 +126,13 @@ export function useKeyboardShortcuts(
     const keyEvent = (e) => {
       if (e.defaultPrevented) return;
 
-      // Get special keys
-      const keys = `${e.ctrlKey || e.metaKey ? 'ctrl+' : ''}${
-        e.altKey ? 'alt+' : ''
-      }${(e.ctrlKey || e.metaKey || e.altKey) && e.shiftKey ? 'shift+' : ''}`;
+      const ctrlKeyEntry = e.ctrlKey ? 'ctrl+' : '';
+      const cmdKeyEntry = e.metaKey ? 'cmd+' : '';
+      const altKeyEntry = e.altKey ? 'alt+' : '';
+      const shiftKeyEntry = e.shiftKey ? 'shift+' : '';
+
+      // We build the special keys string in an opinionated order to ensure consistency
+      const keys = `${ctrlKeyEntry}${cmdKeyEntry}${altKeyEntry}${shiftKeyEntry}`;
 
       // If no special keys, except shift, are pressed and focus is inside a field return
       if (e.target instanceof Node && isFormField(e.target) && !keys) return;