Skip to content

Commit

Permalink
fix: editor sometimes losing track on whether it has focus
Browse files Browse the repository at this point in the history
  • Loading branch information
josdejong committed Oct 31, 2023
1 parent 6588a01 commit 410f997
Show file tree
Hide file tree
Showing 13 changed files with 127 additions and 147 deletions.
2 changes: 1 addition & 1 deletion src/lib/components/JSONEditor.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@
mode = newMode
await tick()
focus()
await focus()
onChangeMode(newMode)
}
Expand Down
52 changes: 32 additions & 20 deletions src/lib/components/controls/EditableDiv.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(() => {
Expand All @@ -48,7 +52,7 @@
debug('onDestroy', { closed, value, newValue })
if (!closed && newValue !== value) {
onChange(newValue, UPDATE_SELECTION.NO)
onChange(newValue, UpdateSelectionAfterChange.no)
}
})
Expand Down Expand Up @@ -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') {
Expand Down Expand Up @@ -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`).
}
}
}
Expand Down
20 changes: 9 additions & 11 deletions src/lib/components/controls/contextmenu/ContextMenu.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -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'> = {
Expand All @@ -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(
Expand Down
8 changes: 5 additions & 3 deletions src/lib/components/controls/createFocusTracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
4 changes: 1 addition & 3 deletions src/lib/components/modals/repair/JSONRepairComponent.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,7 @@
if (domTextArea && error) {
const position = error.position != null ? error.position : 0
domTextArea.setSelectionRange(position, position)
setTimeout(() => {
domTextArea.focus()
})
domTextArea.focus()
}
}
Expand Down
36 changes: 17 additions & 19 deletions src/lib/components/modes/tablemode/TableMode.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@
findParentWithNodeName,
getDataPathFromTarget,
getWindow,
isChildOfNodeName
isChildOfNodeName,
isEditableDivRef
} from '$lib/utils/domUtils.js'
import { createDebug } from '$lib/utils/debug.js'
import {
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -1017,7 +1014,7 @@
return false
}
function handleContextMenuFromTableMenu(event: Event) {
function handleContextMenuFromTableMenu(event: Event & { target: HTMLDivElement }) {
if (readOnly) {
return
}
Expand Down Expand Up @@ -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 = [
Expand All @@ -1107,6 +1105,9 @@
]
handlePatch(operations)
// TODO: get rid of the setTimeout here
setTimeout(focus)
}
function handlePasteFromMenu() {
Expand All @@ -1128,6 +1129,7 @@
function handleClearPastedJson() {
debug('clear pasted json')
pastedJson = undefined
focus()
}
function handleRequestRepair() {
Expand Down Expand Up @@ -1880,10 +1882,6 @@
onClick: handleClearPastedJson
}
]}
onClose={() => {
// TODO: the need for setTimeout is ugly
setTimeout(focus)
}}
/>
{/if}

Expand Down
12 changes: 7 additions & 5 deletions src/lib/components/modes/treemode/JSONKey.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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))
Expand All @@ -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()
}
}
Expand Down
Loading

0 comments on commit 410f997

Please sign in to comment.