fix: #436 Ctrl+Click (Cmd+Click) not working on Mac

josdejong · Jun 5, 2024 · 7142783 · 7142783
1 parent f2b21c1
commit 7142783
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 29 deletions.
diff --git a/src/lib/components/modes/treemode/JSONNode.svelte b/src/lib/components/modes/treemode/JSONNode.svelte
@@ -72,6 +72,7 @@
   import ValidationErrorIcon from './ValidationErrorIcon.svelte'
   import { isObject } from '$lib/utils/typeUtils.js'
   import { classnames } from '$lib/utils/cssUtils.js'
+  import { isCtrlKeyDown } from 'svelte-jsoneditor/utils/keyBindings'
 
   export let value: unknown
   export let path: JSONPath
@@ -216,7 +217,7 @@
   function toggleExpand(event: MouseEvent) {
     event.stopPropagation()
 
-    const recursive = event.ctrlKey
+    const recursive = isCtrlKeyDown(event)
     context.onExpand(path, !expanded, recursive)
   }
 

diff --git a/src/lib/plugins/value/components/ReadonlyValue.svelte b/src/lib/plugins/value/components/ReadonlyValue.svelte
@@ -13,6 +13,7 @@
     ValueNormalization
   } from '$lib/types.js'
   import type { JSONPath } from 'immutable-json-patch'
+  import { isCtrlKeyDown } from 'svelte-jsoneditor/utils/keyBindings'
 
   export let path: JSONPath
   export let value: unknown
@@ -26,7 +27,7 @@
   $: valueIsUrl = isUrl(value)
 
   function handleValueClick(event: MouseEvent) {
-    if (typeof value === 'string' && valueIsUrl && event.ctrlKey) {
+    if (typeof value === 'string' && valueIsUrl && isCtrlKeyDown(event)) {
       event.preventDefault()
       event.stopPropagation()
 

diff --git a/src/lib/utils/keyBindings.test.ts b/src/lib/utils/keyBindings.test.ts
@@ -8,26 +8,34 @@ describe('keyBindings', () => {
   const altKey = true
   const metaKey = true
 
+  const defaults = { ctrlKey: false, shiftKey: false, altKey: false, metaKey: false }
+
   test('keyComboFromEvent', () => {
-    strictEqual(keyComboFromEvent({ key: 'a' }), 'A')
-    strictEqual(keyComboFromEvent({ key: 'A' }), 'A')
-    strictEqual(keyComboFromEvent({ key: '=' }), '=')
-    strictEqual(keyComboFromEvent({ shiftKey, key: '+' }), 'Shift++')
-    strictEqual(keyComboFromEvent({ altKey, key: '=' }), 'Alt+=')
-    strictEqual(keyComboFromEvent({ key: '-' }), '-')
-    strictEqual(keyComboFromEvent({ shiftKey, key: '-' }), 'Shift+-')
-    strictEqual(keyComboFromEvent({ ctrlKey, key: 'a' }), 'Ctrl+A')
-    strictEqual(keyComboFromEvent({ metaKey, key: 'a' }), 'Ctrl+A')
-    strictEqual(keyComboFromEvent({ shiftKey, key: 'a' }), 'Shift+A')
-    strictEqual(keyComboFromEvent({ ctrlKey, shiftKey, key: 'a' }), 'Ctrl+Shift+A')
-    strictEqual(keyComboFromEvent({ key: 'Control' }), '') // does not happen in practice
-    strictEqual(keyComboFromEvent({ ctrlKey, key: 'Control' }), 'Ctrl')
+    strictEqual(keyComboFromEvent({ ...defaults, key: 'a' }), 'A')
+    strictEqual(keyComboFromEvent({ ...defaults, key: 'A' }), 'A')
+    strictEqual(keyComboFromEvent({ ...defaults, key: '=' }), '=')
+    strictEqual(keyComboFromEvent({ ...defaults, shiftKey, key: '+' }), 'Shift++')
+    strictEqual(keyComboFromEvent({ ...defaults, altKey, key: '=' }), 'Alt+=')
+    strictEqual(keyComboFromEvent({ ...defaults, key: '-' }), '-')
+    strictEqual(keyComboFromEvent({ ...defaults, shiftKey, key: '-' }), 'Shift+-')
+    strictEqual(keyComboFromEvent({ ...defaults, ctrlKey, key: 'a' }), 'Ctrl+A')
+    strictEqual(
+      keyComboFromEvent({ ...defaults, metaKey, key: 'a' }, '+', () => true),
+      'Ctrl+A'
+    )
+    strictEqual(keyComboFromEvent({ ...defaults, shiftKey, key: 'a' }), 'Shift+A')
+    strictEqual(keyComboFromEvent({ ...defaults, ctrlKey, shiftKey, key: 'a' }), 'Ctrl+Shift+A')
+    strictEqual(keyComboFromEvent({ ...defaults, key: 'Control' }), '') // does not happen in practice
+    strictEqual(keyComboFromEvent({ ...defaults, ctrlKey, key: 'Control' }), 'Ctrl')
   })
 
   test('keyComboFromEvent with custom separator', () => {
     const separator = '///'
-    strictEqual(keyComboFromEvent({ key: 'a' }, separator), 'A')
-    strictEqual(keyComboFromEvent({ ctrlKey, key: 'a' }, separator), 'Ctrl///A')
-    strictEqual(keyComboFromEvent({ ctrlKey, shiftKey, key: 'A' }, separator), 'Ctrl///Shift///A')
+    strictEqual(keyComboFromEvent({ ...defaults, key: 'a' }, separator), 'A')
+    strictEqual(keyComboFromEvent({ ...defaults, ctrlKey, key: 'a' }, separator), 'Ctrl///A')
+    strictEqual(
+      keyComboFromEvent({ ...defaults, ctrlKey, shiftKey, key: 'A' }, separator),
+      'Ctrl///Shift///A'
+    )
   })
 })
diff --git a/src/lib/utils/keyBindings.ts b/src/lib/utils/keyBindings.ts
@@ -1,11 +1,13 @@
 // inspiration: https://github.com/andrepolischuk/keycomb
 
+import { isMac as _isMac } from './navigatorUtils.js'
+
 // KeyComboEvent is a subset of KeyboardEvent
 export interface KeyComboEvent {
-  ctrlKey?: boolean
-  metaKey?: boolean
-  altKey?: boolean
-  shiftKey?: boolean
+  ctrlKey: boolean
+  metaKey: boolean
+  altKey: boolean
+  shiftKey: boolean
   key: string
 }
 
@@ -17,15 +19,10 @@ export interface KeyComboEvent {
  * meta keys "Ctrl" ("Command" on Mac), and "Alt" ("Alt" or "Option" on Mac)
  * So pressing "Command" and "A"on Mac will return "Ctrl+A"
  */
-export function keyComboFromEvent(event: KeyComboEvent, separator = '+'): string {
+export function keyComboFromEvent(event: KeyComboEvent, separator = '+', isMac = _isMac): string {
   const combi = []
 
-  if (event.ctrlKey) {
-    // Control on Windows
-    combi.push('Ctrl')
-  }
-  if (event.metaKey) {
-    // Command on Mac
+  if (isCtrlKeyDown(event, isMac)) {
     combi.push('Ctrl')
   }
   if (event.altKey) {
@@ -45,6 +42,17 @@ export function keyComboFromEvent(event: KeyComboEvent, separator = '+'): string
   return combi.join(separator)
 }
 
+/**
+ * Test whether the Ctrl key (windows, linux) or Command key (mac) is down
+ */
+export function isCtrlKeyDown(
+  event: { ctrlKey: boolean; metaKey: boolean },
+  isMac = _isMac
+): boolean {
+  // metaKey is the Command key ⌘ on a Mac (but the Windows Key ⊞ on Windows)
+  return event.ctrlKey || (event.metaKey && isMac())
+}
+
 const metaKeys = {
   Ctrl: true,
   Command: true,