fix: editor sometimes losing track on whether it has focus

Author: josdejong

src/lib/components/JSONEditor.svelte

@@ -312,7 +312,7 @@
     mode = newMode
 
     await tick()
-    focus()
+    await focus()
 
     onChangeMode(newMode)
   }

src/lib/components/controls/EditableDiv.svelte

@@ -6,15 +6,15 @@
   import { keyComboFromEvent } from '$lib/utils/keyBindings.js'
   import { createDebug } from '$lib/utils/debug.js'
   import { noop } from 'lodash-es'
-  import { UPDATE_SELECTION } from '$lib/constants.js'
   import type { OnFind, OnPaste } from '$lib/types'
+  import { UpdateSelectionAfterChange } from '$lib/types'
   import { classnames } from '$lib/utils/cssUtils.js'
 
   const debug = createDebug('jsoneditor:EditableDiv')
 
   export let value: string
   export let shortText = false
-  export let onChange: (newValue: string, updateSelection: string) => void
+  export let onChange: (newValue: string, updateSelection: UpdateSelectionAfterChange) => void
   export let onCancel: () => void
   export let onFind: OnFind
   export let onPaste: OnPaste = noop
@@ -30,16 +30,20 @@
     setDomValue(value)
 
     // focus
-    setTimeout(() => {
-      if (domValue) {
-        setCursorToEnd(domValue)
-
-        // The refresh method can be used to update the classnames for example
-        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-        // @ts-ignore
-        domValue.refresh = handleValueInput
-      }
-    })
+    if (domValue) {
+      setCursorToEnd(domValue)
+
+      // The refresh method can be used to update the classnames for example
+      // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+      // @ts-ignore
+      domValue.refresh = handleValueInput
+
+      // The cancel method can be used to cancel editing, without firing a change
+      // when the contents did change in the meantime. It is the same as pressing ESC
+      // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+      // @ts-ignore
+      domValue.cancel = handleCancel
+    }
   })
 
   onDestroy(() => {
@@ -48,7 +52,7 @@
     debug('onDestroy', { closed, value, newValue })
 
     if (!closed && newValue !== value) {
-      onChange(newValue, UPDATE_SELECTION.NO)
+      onChange(newValue, UpdateSelectionAfterChange.no)
     }
   })
 
@@ -78,24 +82,28 @@
     valueClass = onValueClass(newValue)
   }
 
+  function handleCancel() {
+    // cancel changes (needed to prevent triggering a change onDestroy)
+    closed = true
+
+    onCancel()
+  }
+
   function handleValueKeyDown(event: KeyboardEvent) {
     event.stopPropagation()
 
     const combo = keyComboFromEvent(event)
 
     if (combo === 'Escape') {
-      // cancel changes (needed to prevent triggering a change onDestroy)
-      closed = true
-
-      onCancel()
+      handleCancel()
     }
 
     if (combo === 'Enter' || combo === 'Tab') {
       // apply changes
       closed = true
 
       const newValue = getDomValue()
-      onChange(newValue, UPDATE_SELECTION.NEXT_INSIDE)
+      onChange(newValue, UpdateSelectionAfterChange.nextInside)
     }
 
     if (combo === 'Ctrl+F') {
@@ -133,9 +141,13 @@
     if (document.hasFocus() && !closed) {
       closed = true
       if (newValue !== value) {
-        onChange(newValue, UPDATE_SELECTION.SELF)
+        onChange(newValue, UpdateSelectionAfterChange.self)
       } else {
-        onCancel()
+        // Note that we do not fire an onCancel here: a blur action
+        // is caused by the user clicking somewhere else. If we apply
+        // onCancel now, we would override the selection that the user
+        // wants by clicking somewhere else in the editor (since `blur`
+        // is occurring *after* `mousedown`).
       }
     }
   }

src/lib/components/controls/contextmenu/ContextMenu.svelte

@@ -21,18 +21,16 @@
   export let items: ContextMenuItem[]
   export let tip: string | undefined
 
-  let refContextMenu
+  let refContextMenu: HTMLDivElement
 
   onMount(() => {
-    setTimeout(() => {
-      const firstEnabledButton = [...refContextMenu.querySelectorAll('button')].find(
-        (button) => !button.disabled
-      )
+    const firstEnabledButton = Array.from(refContextMenu.querySelectorAll('button')).find(
+      (button) => !button.disabled
+    )
 
-      if (firstEnabledButton) {
-        firstEnabledButton.focus()
-      }
-    })
+    if (firstEnabledButton) {
+      firstEnabledButton.focus()
+    }
   })
 
   const directionByCombo: Record<string, 'Up' | 'Down' | 'Left' | 'Right'> = {
@@ -44,9 +42,9 @@
 
   function handleKeyDown(event) {
     const combo = keyComboFromEvent(event)
-    const direction = directionByCombo[combo]
+    const direction: 'Up' | 'Down' | 'Left' | 'Right' | undefined = directionByCombo[combo]
 
-    if (typeof direction === 'string') {
+    if (direction && event.target) {
       event.preventDefault()
 
       const buttons: HTMLButtonElement[] = Array.from(

src/lib/components/controls/createFocusTracker.ts

@@ -42,9 +42,11 @@ export function createFocusTracker({
       // The focusIn handler will cancel any pending blur timer in those cases
       clearTimeout(blurTimeoutHandle)
       blurTimeoutHandle = setTimeout(() => {
-        debug('blur')
-        focus = false
-        onBlur()
+        if (!hasFocus()) {
+          debug('blur')
+          focus = false
+          onBlur()
+        }
       }) as unknown as number
     }
   }

src/lib/components/modals/repair/JSONRepairComponent.svelte

@@ -53,9 +53,7 @@
     if (domTextArea && error) {
       const position = error.position != null ? error.position : 0
       domTextArea.setSelectionRange(position, position)
-      setTimeout(() => {
-        domTextArea.focus()
-      })
+      domTextArea.focus()
     }
   }
 

src/lib/components/modes/tablemode/TableMode.svelte

@@ -73,7 +73,8 @@
     findParentWithNodeName,
     getDataPathFromTarget,
     getWindow,
-    isChildOfNodeName
+    isChildOfNodeName,
+    isEditableDivRef
   } from '$lib/utils/domUtils.js'
   import { createDebug } from '$lib/utils/debug.js'
   import {
@@ -697,6 +698,7 @@
   }
 
   export function focus() {
+    debug('focus')
     // with just .focus(), sometimes the input doesn't react on onpaste events
     // in Chrome when having a large document open and then doing cut/paste.
     // Calling both .focus() and .select() did solve this issue.
@@ -710,7 +712,7 @@
     scrollTop = event.target['scrollTop']
   }
 
-  function handleMouseDown(event: MouseEvent) {
+  function handleMouseDown(event: MouseEvent & { target: HTMLDivElement }) {
     const path = event?.target ? getDataPathFromTarget(event.target as HTMLElement) : undefined
     if (path) {
       // when clicking inside the current selection, editing a value, do nothing
@@ -726,15 +728,10 @@
       event.preventDefault()
     }
 
-    // TODO: ugly to have two setTimeout here. Without it, hiddenInput will blur
-    setTimeout(() => {
-      setTimeout(() => {
-        // for example when clicking on the empty area in the main menu
-        if (!hasFocus && !isChildOfNodeName(event.target, 'BUTTON')) {
-          focus()
-        }
-      })
-    })
+    // for example when clicking on the empty area in the main menu
+    if (!isChildOfNodeName(event.target, 'BUTTON') && !event.target.isContentEditable) {
+      focus()
+    }
   }
 
   function createDefaultSelection(): JSONSelection | null {
@@ -1017,7 +1014,7 @@
     return false
   }
 
-  function handleContextMenuFromTableMenu(event: Event) {
+  function handleContextMenuFromTableMenu(event: Event & { target: HTMLDivElement }) {
     if (readOnly) {
       return
     }
@@ -1093,9 +1090,10 @@
     const { path, contents } = pastedJson
 
     // exit edit mode
-    updateSelection(createValueSelection(path, false))
-
-    await tick()
+    const refEditableDiv = refContents?.querySelector('.jse-editable-div') || null
+    if (isEditableDivRef(refEditableDiv)) {
+      refEditableDiv.cancel()
+    }
 
     // replace the value with the JSON object/array
     const operations: JSONPatchDocument = [
@@ -1107,6 +1105,9 @@
     ]
 
     handlePatch(operations)
+
+    // TODO: get rid of the setTimeout here
+    setTimeout(focus)
   }
 
   function handlePasteFromMenu() {
@@ -1128,6 +1129,7 @@
   function handleClearPastedJson() {
     debug('clear pasted json')
     pastedJson = undefined
+    focus()
   }
 
   function handleRequestRepair() {
@@ -1880,10 +1882,6 @@
               onClick: handleClearPastedJson
             }
           ]}
-          onClose={() => {
-            // TODO: the need for setTimeout is ugly
-            setTimeout(focus)
-          }}
         />
       {/if}
 

src/lib/components/modes/treemode/JSONKey.svelte

@@ -11,8 +11,8 @@
   import SearchResultHighlighter from './highlight/SearchResultHighlighter.svelte'
   import EditableDiv from '../../controls/EditableDiv.svelte'
   import { addNewLineSuffix } from '$lib/utils/domUtils.js'
-  import { UPDATE_SELECTION } from '$lib/constants.js'
   import type { ExtendedSearchResultItem, JSONSelection, TreeModeContext } from '$lib/types.js'
+  import { UpdateSelectionAfterChange } from '$lib/types.js'
   import type { JSONPath } from 'immutable-json-patch'
   import ContextMenuPointer from '../../../components/controls/contextmenu/ContextMenuPointer.svelte'
   import { classnames } from '$lib/utils/cssUtils.js'
@@ -28,7 +28,9 @@
   $: isKeySelected = selection ? isKeySelection(selection) && isEqual(selection.path, path) : false
   $: isEditingKey = isKeySelected && isEditingSelection(selection)
 
-  function handleKeyDoubleClick(event) {
+  function handleKeyDoubleClick(
+    event: MouseEvent & { currentTarget: EventTarget & HTMLDivElement }
+  ) {
     if (!isEditingKey && !context.readOnly) {
       event.preventDefault()
       context.onSelect(createKeySelection(path, true))
@@ -41,17 +43,17 @@
     })
   }
 
-  function handleChangeValue(newKey, updateSelection) {
+  function handleChangeValue(newKey: string, updateSelection: UpdateSelectionAfterChange) {
     const updatedKey = onUpdateKey(key, context.normalization.unescapeValue(newKey))
     const updatedPath = initial(path).concat(updatedKey)
 
     context.onSelect(
-      updateSelection === UPDATE_SELECTION.NEXT_INSIDE
+      updateSelection === UpdateSelectionAfterChange.nextInside
         ? createValueSelection(updatedPath, false)
         : createKeySelection(updatedPath, false)
     )
 
-    if (updateSelection !== UPDATE_SELECTION.SELF) {
+    if (updateSelection !== UpdateSelectionAfterChange.self) {
       context.focus()
     }
   }

src/lib/components/modes/treemode/TreeMode.svelte

@@ -83,7 +83,8 @@
     findParentWithNodeName,
     getWindow,
     isChildOf,
-    isChildOfNodeName
+    isChildOfNodeName,
+    isEditableDivRef
   } from '$lib/utils/domUtils.js'
   import {
     convertValue,
@@ -1334,7 +1335,7 @@
    */
   export async function scrollTo(path: JSONPath, scrollToWhenVisible = true): Promise<void> {
     documentState = expandPath(json, documentState, path)
-    await tick() // await rerender
+    await tick() // await rerender (else the element we want to scroll to does not yet exist)
 
     const elem = findElement(path)
 
@@ -1570,11 +1571,7 @@
     }
 
     // set focus to the hidden input, so we can capture quick keys like Ctrl+X, Ctrl+C, Ctrl+V
-    setTimeout(() => {
-      if (!activeElementIsChildOf(refJsonEditor)) {
-        focus()
-      }
-    })
+    focus()
   }
 
   function handleExpandAll() {
@@ -1781,59 +1778,26 @@
 
     if (combo === 'Ctrl+Z') {
       event.preventDefault()
-
-      // TODO: find a better way to restore focus
-      // TODO: implement a proper TypeScript solution to check whether this is an element with blur, focus, select
-      const activeElement = document.activeElement as HTMLInputElement
-      if (activeElement && activeElement.blur && activeElement.select) {
-        activeElement.blur()
-        setTimeout(() => {
-          handleUndo()
-          setTimeout(() => activeElement?.select())
-        })
-      } else {
-        handleUndo()
-      }
+      handleUndo()
     }
 
     if (combo === 'Ctrl+Shift+Z') {
       event.preventDefault()
-
-      // TODO: find a better way to restore focus
-      // TODO: implement a proper TypeScript solution to check whether this is an element with blur, focus, select
-      const activeElement = document.activeElement as HTMLInputElement
-      if (activeElement && activeElement.blur && activeElement.select) {
-        activeElement.blur()
-        setTimeout(() => {
-          handleRedo()
-          setTimeout(() => activeElement?.select())
-        })
-      } else {
-        handleRedo()
-      }
+      handleRedo()
     }
   }
 
-  function handleMouseDown(event) {
+  function handleMouseDown(event: MouseEvent & { target: HTMLDivElement }) {
     debug('handleMouseDown', event)
 
-    // TODO: ugly to have two setTimeout here. Without it, hiddenInput will blur
-    setTimeout(() => {
-      setTimeout(() => {
-        if (!hasFocus && !isChildOfNodeName(event.target, 'BUTTON')) {
-          // for example when clicking on the empty area in the main menu
-          focus()
-
-          if (
-            !documentState.selection &&
-            json === undefined &&
-            (text === '' || text === undefined)
-          ) {
-            createDefaultSelection()
-          }
-        }
-      })
-    })
+    if (!isChildOfNodeName(event.target, 'BUTTON') && !event.target.isContentEditable) {
+      // for example when clicking on the empty area in the main menu
+      focus()
+
+      if (!documentState.selection && json === undefined && (text === '' || text === undefined)) {
+        createDefaultSelection()
+      }
+    }
   }
 
   function openContextMenu({
@@ -1966,11 +1930,13 @@
     }
 
     const { path, contents } = pastedJson
+    pastedJson = undefined
 
     // exit edit mode
-    updateSelection(createValueSelection(path, false))
-
-    await tick()
+    const refEditableDiv = refContents?.querySelector('.jse-editable-div') || null
+    if (isEditableDivRef(refEditableDiv)) {
+      refEditableDiv.cancel()
+    }
 
     // replace the value with the JSON object/array
     const operations: JSONPatchDocument = [
@@ -1986,11 +1952,15 @@
         state: expandRecursive(patchedJson, patchedState, path)
       }
     })
+
+    // TODO: get rid of the setTimeout here
+    setTimeout(focus)
   }
 
   function handleClearPastedJson() {
     debug('clear pasted json')
     pastedJson = undefined
+    focus()
   }
 
   function handleRequestRepair() {
@@ -2035,17 +2005,10 @@
           refHiddenInput.blur()
         }
 
-        // This is ugly: we need to wait until the EditableDiv has triggered onSelect,
-        //  and have onSelect call refHiddenInput.focus(). After that we can call blur()
-        //  to remove the focus.
-        // TODO: find a better solution
-        tick().then(() => {
-          setTimeout(() => {
-            if (refHiddenInput) {
-              refHiddenInput.blur()
-            }
-          })
-        })
+        debug('blur (outside editor)')
+        if (refHiddenInput) {
+          refHiddenInput.blur()
+        }
       }
     }
   }
@@ -2240,10 +2203,6 @@
               onClick: handleClearPastedJson
             }
           ]}
-          onClose={() => {
-            // TODO: the need for setTimeout is ugly
-            setTimeout(focus)
-          }}
         />
       {/if}
 

src/lib/constants.ts

@@ -68,13 +68,6 @@ export const JSON_STATUS_INVALID = 'invalid'
 export const CONTEXT_MENU_HEIGHT = (40 + 2) * 8 // px
 export const CONTEXT_MENU_WIDTH = 260 // px
 
-// TODO: change UPDATE_SELECTION into an enum
-export const UPDATE_SELECTION = {
-  NO: 'NO',
-  SELF: 'SELF',
-  NEXT_INSIDE: 'NEXT_INSIDE'
-}
-
 export const SORT_DIRECTION_NAMES = {
   [SortDirection.asc]: 'ascending',
   [SortDirection.desc]: 'descending'

src/lib/plugins/value/components/BooleanToggle.svelte

@@ -28,7 +28,7 @@
       }
     ])
 
-    setTimeout(focus)
+    focus()
   }
 </script>
 

src/lib/plugins/value/components/EditableValue.svelte

@@ -7,16 +7,16 @@
   import { createValueSelection, getFocusPath } from '$lib/logic/selection.js'
   import { getValueClass } from '$lib/plugins/value/components/utils/getValueClass.js'
   import EditableDiv from '../../../components/controls/EditableDiv.svelte'
-  import { UPDATE_SELECTION } from '$lib/constants.js'
   import type {
     FindNextInside,
     JSONParser,
     OnFind,
+    OnJSONSelect,
     OnPasteJson,
     OnPatch,
-    OnJSONSelect,
     ValueNormalization
   } from '$lib/types.js'
+  import { UpdateSelectionAfterChange } from '$lib/types.js'
   import { isEqual } from 'lodash-es'
 
   export let path: JSONPath
@@ -35,7 +35,7 @@
     return enforceString ? value : stringConvert(value, parser)
   }
 
-  function handleChangeValue(newValue: string, updateSelection: string) {
+  function handleChangeValue(newValue: string, updateSelection: UpdateSelectionAfterChange) {
     onPatch(
       [
         {
@@ -53,7 +53,7 @@
         }
 
         const selection =
-          updateSelection === UPDATE_SELECTION.NEXT_INSIDE
+          updateSelection === UpdateSelectionAfterChange.nextInside
             ? findNextInside(path)
             : createValueSelection(path, false)
 
@@ -66,9 +66,7 @@
       }
     )
 
-    if (updateSelection !== UPDATE_SELECTION.SELF) {
-      focus()
-    }
+    focus()
   }
 
   function handleCancelChange() {
@@ -82,7 +80,7 @@
       if (isObjectOrArray(pastedJson)) {
         onPasteJson({
           path,
-          contents: pastedJson
+          contents: pastedJson as JSONValue
         })
       }
     } catch (err) {

src/lib/types.ts

@@ -574,6 +574,12 @@ export enum SortDirection {
   desc = 'desc'
 }
 
+export enum UpdateSelectionAfterChange {
+  no = 'no',
+  self = 'self',
+  nextInside = 'nextInside'
+}
+
 export interface TableCellIndex {
   rowIndex: number
   columnIndex: number

src/lib/utils/domUtils.ts

@@ -410,3 +410,17 @@ export function findNearestElement<T extends Element>({
 
   return undefined
 }
+
+export interface EditableDivElement extends HTMLDivElement {
+  refresh: () => void
+  cancel: () => void
+}
+
+export function isEditableDivRef(element: Element | null): element is EditableDivElement {
+  return (
+    !!element &&
+    element.nodeName === 'DIV' &&
+    typeof (element as unknown as Record<string, unknown>).refresh === 'function' &&
+    typeof (element as unknown as Record<string, unknown>).cancel === 'function'
+  )
+}