From 76b177f03b68097c7e521bf07ed753a5a1acf931 Mon Sep 17 00:00:00 2001
From: Jos de Jong <wjosdejong@gmail.com>
Date: Mon, 23 May 2022 10:03:43 +0200
Subject: [PATCH] fix: improve the behavior of the arrow quickkeys to navigate
 the context menu (#83)

---
 .../components/controls/DropdownButton.svelte |   8 +-
 .../modals/repair/JSONRepairComponent.svelte  |   2 +-
 .../components/modes/treemode/JSONKey.svelte  |   2 +-
 .../components/modes/treemode/JSONNode.svelte |   2 +-
 .../modes/treemode/JSONValue.svelte           |   2 +-
 .../treemode/contextmenu/ContextMenu.svelte   | 118 +++---------------
 .../highlight/SearchResultHighlighter.svelte  |   2 +-
 .../value/components/ReadonlyValue.svelte     |   2 +-
 src/lib/utils/{domUtils.js => domUtils.ts}    |  88 +++++++++++++
 9 files changed, 117 insertions(+), 109 deletions(-)
 rename src/lib/utils/{domUtils.js => domUtils.ts} (71%)

diff --git a/src/lib/components/controls/DropdownButton.svelte b/src/lib/components/controls/DropdownButton.svelte
index 47f5aea7..8be7faf4 100644
--- a/src/lib/components/controls/DropdownButton.svelte
+++ b/src/lib/components/controls/DropdownButton.svelte
@@ -46,7 +46,13 @@
 <div class="jse-dropdown-button" {title} on:click={handleClick}>
   <slot name="defaultItem" />
 
-  <button type="button" class="jse-open-dropdown" class:jse-visible={visible} on:click={toggleShow}>
+  <button
+    type="button"
+    class="jse-open-dropdown"
+    data-type="jse-open-dropdown"
+    class:jse-visible={visible}
+    on:click={toggleShow}
+  >
     <Icon data={faCaretDown} />
   </button>
 
diff --git a/src/lib/components/modals/repair/JSONRepairComponent.svelte b/src/lib/components/modals/repair/JSONRepairComponent.svelte
index d0367f73..aaa29f95 100644
--- a/src/lib/components/modals/repair/JSONRepairComponent.svelte
+++ b/src/lib/components/modals/repair/JSONRepairComponent.svelte
@@ -11,7 +11,7 @@
   import { createDebug } from '../../../utils/debug'
   import Message from '../../controls/Message.svelte'
   import { onDestroy, onMount } from 'svelte'
-  import { activeElementIsChildOf, getWindow } from '../../../utils/domUtils.js'
+  import { activeElementIsChildOf, getWindow } from '../../../utils/domUtils'
   import { normalizeJsonParseError } from '../../../utils/jsonUtils.ts'
   import { createFocusTracker } from '../../controls/createFocusTracker.js'
   import Menu from '../../controls/Menu.svelte'
diff --git a/src/lib/components/modes/treemode/JSONKey.svelte b/src/lib/components/modes/treemode/JSONKey.svelte
index bfe85815..8dd2b7a3 100644
--- a/src/lib/components/modes/treemode/JSONKey.svelte
+++ b/src/lib/components/modes/treemode/JSONKey.svelte
@@ -6,7 +6,7 @@
   import { SELECTION_TYPE } from '$lib/logic/selection'
   import SearchResultHighlighter from './highlight/SearchResultHighlighter.svelte'
   import EditableDiv from '../../controls/EditableDiv.svelte'
-  import { addNewLineSuffix } from '$lib/utils/domUtils'
+  import { addNewLineSuffix } from '../../../utils/domUtils'
   import { UPDATE_SELECTION } from '../../../constants.js'
 
   export let path
diff --git a/src/lib/components/modes/treemode/JSONNode.svelte b/src/lib/components/modes/treemode/JSONNode.svelte
index ea55cd5c..203169e2 100644
--- a/src/lib/components/modes/treemode/JSONNode.svelte
+++ b/src/lib/components/modes/treemode/JSONNode.svelte
@@ -42,7 +42,7 @@
   import { forEachKey } from '../../../logic/documentState.js'
   import { onMoveSelection } from '../../../logic/dragging.js'
   import { forEachIndex } from '../../../utils/arrayUtils.ts'
-  import { getDataPathFromTarget } from '../../../utils/domUtils.js'
+  import { getDataPathFromTarget } from '../../../utils/domUtils'
   import { createMemoizePath } from '../../../utils/pathUtils.js'
   import { keyIsSelected } from '../../../logic/selection.js'
 
diff --git a/src/lib/components/modes/treemode/JSONValue.svelte b/src/lib/components/modes/treemode/JSONValue.svelte
index a119008c..a4d2bee0 100644
--- a/src/lib/components/modes/treemode/JSONValue.svelte
+++ b/src/lib/components/modes/treemode/JSONValue.svelte
@@ -3,7 +3,7 @@
 <script lang="ts">
   import { SELECTION_TYPE } from '$lib/logic/selection'
   import { isEqual } from 'lodash-es'
-  import type { SearchResultItem, Selection, TreeModeContext, Path } from '../../../types'
+  import type { SearchResultItem, Selection, Path } from '../../../types'
 
   export let path: Path
   export let value: JSON
diff --git a/src/lib/components/modes/treemode/contextmenu/ContextMenu.svelte b/src/lib/components/modes/treemode/contextmenu/ContextMenu.svelte
index 5d15fd7a..4abdce6f 100644
--- a/src/lib/components/modes/treemode/contextmenu/ContextMenu.svelte
+++ b/src/lib/components/modes/treemode/contextmenu/ContextMenu.svelte
@@ -1,6 +1,6 @@
 <svelte:options immutable={true} />
 
-<script>
+<script lang="ts">
   import {
     faCaretSquareDown,
     faCaretSquareUp,
@@ -26,6 +26,7 @@
   import { STATE_ENFORCE_STRING } from '$lib/constants'
   import { isObject } from '../../../../utils/typeUtils.ts'
   import { canConvert, singleItemSelected } from '../../../../logic/selection.js'
+  import { findNearestElement } from '../../../../utils/domUtils'
 
   export let json
   export let state
@@ -200,50 +201,24 @@
   function handleKeyDown(event) {
     const combo = keyComboFromEvent(event).replace(/^Command\+/, 'Ctrl+')
 
-    /**
-     * Find first enabled sibling button.
-     * Uses hints from the button attributes itself: data-name, data-up,
-     * data-down, data-left, data-right.
-     * @param {Element} currentButton
-     * @param {'left' | 'right' | 'up' | 'down'} direction
-     */
-    function findNextButton(currentButton, direction) {
-      const optionsString = currentButton.getAttribute('data-' + direction)
-      if (optionsString) {
-        const options = optionsString.split(',')
-
-        // Step 1: find exact match
-        for (const option of options) {
-          const match = refContextMenu.querySelector(`button[data-name="${option}"]`)
-          if (match && !match.disabled) {
-            return match
-          }
-        }
+    if (combo === 'Up' || combo === 'Down' || combo === 'Left' || combo === 'Right') {
+      event.preventDefault()
 
-        // Step 2: recurse over multiple buttons to find an enabled one
-        for (const option of options) {
-          const match = refContextMenu.querySelector(`button[data-name="${option}"]`)
-          if (match && match.disabled) {
-            const match2 = findNextButton(match, direction)
-            if (match2) {
-              return match2
-            }
-          }
+      const buttons: HTMLButtonElement[] = Array.from(
+        refContextMenu.querySelectorAll('button:not([disabled])')
+      )
+      const nearest = findNearestElement({
+        allElements: buttons,
+        currentElement: event.target,
+        direction: combo,
+        hasPrio: (element: HTMLButtonElement) => {
+          return element.getAttribute('data-type') !== 'jse-open-dropdown'
         }
+      })
+      if (nearest) {
+        nearest.focus()
       }
     }
-
-    function focusNextButton(currentButton, direction) {
-      const next = findNextButton(currentButton, direction)
-      if (next) {
-        next.focus()
-      }
-    }
-
-    if (combo === 'Up' || combo === 'Down' || combo === 'Left' || combo === 'Right') {
-      event.preventDefault()
-      focusNextButton(event.target, combo.toLowerCase())
-    }
   }
 
   $: editValueDropdownItems = [
@@ -303,9 +278,6 @@
     <button
       type="button"
       title="Edit the key (Double-click on the key)"
-      data-name="edit-key"
-      data-down="cut,copy,paste"
-      data-right="edit-value"
       on:click={handleEditKey}
       disabled={!canEditKey}
     >
@@ -316,9 +288,6 @@
         type="button"
         slot="defaultItem"
         title="Edit the value (Double-click on the value)"
-        data-name="edit-value"
-        data-down="paste,copy,cut"
-        data-left="edit-key"
         on:click={handleEditValue}
         disabled={!canEditValue}
       >
@@ -333,10 +302,6 @@
         type="button"
         slot="defaultItem"
         title="Cut selected contents, formatted with indentation (Ctrl+X)"
-        data-name="cut"
-        data-up="edit-key,edit-value"
-        data-down="remove"
-        data-right="copy"
         on:click={handleCut}
         disabled={!hasSelectionContents}
       >
@@ -348,11 +313,6 @@
         type="button"
         slot="defaultItem"
         title="Copy selected contents, formatted with indentation (Ctrl+C)"
-        data-name="copy"
-        data-up="edit-key,edit-value"
-        data-down="insert-structure"
-        data-left="cut"
-        data-right="paste"
         on:click={handleCopy}
         disabled={!hasSelectionContents}
       >
@@ -362,10 +322,6 @@
     <button
       type="button"
       title="Paste clipboard contents (Ctrl+V)"
-      data-name="paste"
-      data-up="edit-value,edit-key"
-      data-down="insert-structure"
-      data-left="copy"
       on:click={handlePaste}
       disabled={!hasSelection}
     >
@@ -378,10 +334,6 @@
       <button
         type="button"
         title="Remove selected contents (Delete)"
-        data-name="remove"
-        data-up="cut,copy,paste"
-        data-down="duplicate"
-        data-right="insert-structure"
         on:click={handleRemove}
         disabled={!hasSelectionContents}
       >
@@ -390,10 +342,6 @@
       <button
         type="button"
         title="Duplicate selected contents (Ctrl+D)"
-        data-name="duplicate"
-        data-up="remove"
-        data-down="extract"
-        data-right="insert-structure"
         on:click={handleDuplicate}
         disabled={!canDuplicate}
       >
@@ -402,10 +350,6 @@
       <button
         type="button"
         title="Extract selected contents"
-        data-name="extract"
-        data-up="duplicate"
-        data-down="sort"
-        data-right="insert-object"
         on:click={handleExtract}
         disabled={!canExtract}
       >
@@ -414,10 +358,6 @@
       <button
         type="button"
         title="Sort array or object contents"
-        data-name="sort"
-        data-up="extract"
-        data-down="transform"
-        data-right="insert-array"
         on:click={handleSort}
         disabled={!hasSelectionContents}
       >
@@ -426,10 +366,6 @@
       <button
         type="button"
         title="Transform array or object contents (filter, sort, project)"
-        data-name="transform"
-        data-up="sort"
-        data-down="insert-before"
-        data-right="insert-value"
         on:click={handleTransform}
         disabled={!hasSelectionContents}
       >
@@ -444,10 +380,6 @@
         type="button"
         on:click={() => handleInsertOrConvert('structure')}
         title="{insertOrConvertText} structure"
-        data-name="insert-structure"
-        data-up="paste,copy,cut"
-        data-down="insert-object"
-        data-left="duplicate"
         disabled={!canInsertOrConvertStructure}
       >
         <span class="jse-insert"><span class="jse-plus">{'+'}</span></span> Structure
@@ -456,10 +388,6 @@
         type="button"
         on:click={() => handleInsertOrConvert('object')}
         title="{insertOrConvertText} object"
-        data-name="insert-object"
-        data-up="insert-structure"
-        data-down="insert-array"
-        data-left="extract"
         disabled={!canInsertOrConvertObject}
       >
         <span class="jse-insert">{'{}'}</span> Object
@@ -468,10 +396,6 @@
         type="button"
         on:click={() => handleInsertOrConvert('array')}
         title="{insertOrConvertText} array"
-        data-name="insert-array"
-        data-up="insert-object"
-        data-down="insert-value"
-        data-left="sort"
         disabled={!canInsertOrConvertArray}
       >
         <span class="jse-insert">[]</span> Array
@@ -480,10 +404,6 @@
         type="button"
         on:click={() => handleInsertOrConvert('value')}
         title="{insertOrConvertText} value"
-        data-name="insert-value"
-        data-up="insert-array"
-        data-down="insert-after"
-        data-left="transform"
         disabled={!canInsertOrConvertValue}
       >
         <span class="jse-insert"><span class="jse-quote">"</span></span> Value
@@ -495,9 +415,6 @@
     <button
       type="button"
       title="Select area before current entry to insert or paste contents"
-      data-name="insert-before"
-      data-up="transform"
-      data-right="insert-after"
       disabled={!hasSelectionContents || rootSelected}
       on:click={handleInsertBefore}
     >
@@ -506,9 +423,6 @@
     <button
       type="button"
       title="Select area after current entry to insert or paste contents"
-      data-name="insert-after"
-      data-up="insert-value"
-      data-left="insert-before"
       disabled={!hasSelectionContents || rootSelected}
       on:click={handleInsertAfter}
     >
diff --git a/src/lib/components/modes/treemode/highlight/SearchResultHighlighter.svelte b/src/lib/components/modes/treemode/highlight/SearchResultHighlighter.svelte
index cee57af7..0402b06c 100644
--- a/src/lib/components/modes/treemode/highlight/SearchResultHighlighter.svelte
+++ b/src/lib/components/modes/treemode/highlight/SearchResultHighlighter.svelte
@@ -1,6 +1,6 @@
 <script>
   import { splitValue } from '$lib/logic/search'
-  import { addNewLineSuffix } from '$lib/utils/domUtils.js'
+  import { addNewLineSuffix } from '$lib/utils/domUtils'
 
   /** @type {string} */
   export let text
diff --git a/src/lib/plugins/value/components/ReadonlyValue.svelte b/src/lib/plugins/value/components/ReadonlyValue.svelte
index 6aba02f6..9d097260 100644
--- a/src/lib/plugins/value/components/ReadonlyValue.svelte
+++ b/src/lib/plugins/value/components/ReadonlyValue.svelte
@@ -5,7 +5,7 @@
   import { SELECTION_TYPE } from '$lib/logic/selection.js'
   import SearchResultHighlighter from '../../../components/modes/treemode/highlight/SearchResultHighlighter.svelte'
   import { getValueClass } from './utils/getValueClass'
-  import { addNewLineSuffix } from '$lib/utils/domUtils.js'
+  import { addNewLineSuffix } from '$lib/utils/domUtils'
 
   export let path
   export let value
diff --git a/src/lib/utils/domUtils.js b/src/lib/utils/domUtils.ts
similarity index 71%
rename from src/lib/utils/domUtils.js
rename to src/lib/utils/domUtils.ts
index 8c7dcf17..9b300de5 100644
--- a/src/lib/utils/domUtils.js
+++ b/src/lib/utils/domUtils.ts
@@ -1,4 +1,5 @@
 import { SELECTION_TYPE } from '../logic/selection.js'
+import { map, minBy } from 'lodash-es'
 
 /**
  * Create serialization functions to escape and stringify text,
@@ -298,3 +299,90 @@ export function getDataPathFromTarget(target) {
 
   return parent ? decodeDataPath(parent.getAttribute('data-path')) : null
 }
+
+/**
+ * Find the nearest element in a given context menu with buttons or inputs
+ */
+// TODO: unit test
+export function findNearestElement<T extends Element>({
+  allElements,
+  currentElement,
+  direction,
+  hasPrio = () => true,
+  margin = 10
+}: {
+  allElements: T[]
+  currentElement: T
+  direction: 'Up' | 'Down' | 'Left' | 'Right'
+  margin?: number
+  hasPrio?: (element: T) => boolean
+}): T | undefined {
+  const all = map(allElements.filter(isVisible), calculateCenter)
+  const current = calculateCenter(currentElement)
+
+  interface CenterLocation {
+    x: number
+    y: number
+    rect: DOMRect
+    element: T
+  }
+
+  function isVisible(element: T): boolean {
+    const rect = element.getBoundingClientRect()
+    return rect.width > 0 && rect.height > 0
+  }
+
+  function calculateCenter(element: T): CenterLocation {
+    const rect = element.getBoundingClientRect()
+    return {
+      x: rect.left + rect.width / 2,
+      y: rect.top + rect.height / 2,
+      rect,
+      element
+    }
+  }
+
+  const isOnSameRow = (a: CenterLocation, b: CenterLocation) => Math.abs(a.y - b.y) < margin
+  const isLeft = (a: CenterLocation, b: CenterLocation) => a.rect.left + margin < b.rect.left
+  const isRight = (a: CenterLocation, b: CenterLocation) => a.rect.right > b.rect.right + margin
+  const isAbove = (a: CenterLocation, b: CenterLocation) => a.y + margin < b.y
+  const isBelow = (a: CenterLocation, b: CenterLocation) => a.y > b.y + margin
+
+  function distance(a: CenterLocation, b: CenterLocation, weightY = 1): number {
+    const diffX = a.x - b.x
+    const diffY = (a.y - b.y) * weightY
+    return Math.sqrt(diffX * diffX + diffY * diffY)
+  }
+  const distanceToCurrent = (candidate) => distance(candidate, current)
+  const distanceToCurrentWeighted = (candidate) => distance(candidate, current, 10)
+
+  if (direction === 'Left' || direction === 'Right') {
+    // First we find the first button left from the current button on the same row
+    // if not found, search the closest button left/right from current button
+    const candidatesLeft =
+      direction === 'Left'
+        ? all.filter((button) => isLeft(button, current))
+        : all.filter((button) => isRight(button, current))
+    const candidatesLeftOnRow = candidatesLeft.filter((button) => isOnSameRow(button, current))
+    const nearest =
+      minBy(candidatesLeftOnRow, distanceToCurrent) ||
+      minBy(candidatesLeft, distanceToCurrentWeighted)
+
+    return nearest?.element
+  }
+
+  if (direction === 'Up' || direction === 'Down') {
+    // first we only search through the prio buttons
+    // if there were no matching prio buttons, search all matching buttons
+    const candidates =
+      direction === 'Up'
+        ? all.filter((button) => isAbove(button, current))
+        : all.filter((button) => isBelow(button, current))
+    const prioCandidates = candidates.filter((button) => hasPrio(button.element))
+    const nearest = minBy(prioCandidates, distanceToCurrent) || minBy(candidates, distanceToCurrent)
+
+    return nearest?.element
+  }
+
+  return undefined
+}