diff --git a/src/actions/index.ts b/src/actions/index.ts index 768a1fcf9d3..d892f1840a4 100644 --- a/src/actions/index.ts +++ b/src/actions/index.ts @@ -73,6 +73,7 @@ export { default as setFirstSubthought } from './setFirstSubthought' export { default as setImportThoughtPath } from './setImportThoughtPath' export { default as setIsMulticursorExecuting } from './setIsMulticursorExecuting' export { default as setNoteFocus } from './setNoteFocus' +export { default as setNoteOffset } from './setNoteOffset' export { default as setRemoteSearch } from './setRemoteSearch' export { default as setResourceCache } from './setResourceCache' export { default as setSortPreference } from './setSortPreference' diff --git a/src/actions/setNoteOffset.ts b/src/actions/setNoteOffset.ts new file mode 100644 index 00000000000..3b94360e3cb --- /dev/null +++ b/src/actions/setNoteOffset.ts @@ -0,0 +1,21 @@ +import State from '../@types/State' +import Thunk from '../@types/Thunk' +import { registerActionMetadata } from '../util/actionMetadata.registry' + +/** Sets the note caret offset without affecting undo history. */ +const setNoteOffset = (state: State, { value }: { value: number | null }): State => ({ + ...state, + noteOffset: value, +}) + +/** Action-creator for setNoteOffset. */ +export const setNoteOffsetActionCreator = + (payload: Parameters[1]): Thunk => + dispatch => + dispatch({ type: 'setNoteOffset', ...payload }) + +export default setNoteOffset + +registerActionMetadata('setNoteOffset', { + undoable: false, +}) diff --git a/src/commands/__tests__/undo-redo.ts b/src/commands/__tests__/undo-redo.ts index af835bde3cc..a7d6d37ff55 100644 --- a/src/commands/__tests__/undo-redo.ts +++ b/src/commands/__tests__/undo-redo.ts @@ -7,6 +7,8 @@ import { indentActionCreator as indent } from '../../actions/indent' import { moveThoughtDownActionCreator as moveThoughtDown } from '../../actions/moveThoughtDown' import { newThoughtActionCreator as newThought } from '../../actions/newThought' import { redoActionCreator as redo } from '../../actions/redo' +import { setNoteOffsetActionCreator as setNoteOffset } from '../../actions/setNoteOffset' +import { toggleNoteActionCreator as toggleNote } from '../../actions/toggleNote' import { undoActionCreator as undo } from '../../actions/undo' import { executeCommandWithMulticursor } from '../../commands' import moveThoughtDownCommand from '../../commands/moveThoughtDown' @@ -562,6 +564,155 @@ describe('grouping', () => { expect(exported).toEqual(expectedOutput) }) + it('contiguous note additions should undo and redo in one step', () => { + store.dispatch([ + importText({ + text: ` + - note-add + - =note + - x`, + }), + editThought(['note-add', '=note', 'x'], 'xy'), + editThought(['note-add', '=note', 'xy'], 'xyz'), + undo(), + ]) + + expect(exportContext(store.getState(), ['note-add'], 'text/plain')).toEqual(`- note-add + - =note + - x`) + + store.dispatch(redo()) + + expect(exportContext(store.getState(), ['note-add'], 'text/plain')).toEqual(`- note-add + - =note + - xyz`) + }) + + it('contiguous note deletes should undo in one step', () => { + store.dispatch([ + importText({ + text: ` + - note-delete + - =note + - xyz`, + }), + editThought(['note-delete', '=note', 'xyz'], 'xy'), + editThought(['note-delete', '=note', 'xy'], 'x'), + undo(), + ]) + + expect(exportContext(store.getState(), ['note-delete'], 'text/plain')).toEqual(`- note-delete + - =note + - xyz`) + }) + + it('note replacements should stay as separate undo steps', () => { + store.dispatch([ + importText({ + text: ` + - note-replace + - =note + - cat`, + }), + editThought(['note-replace', '=note', 'cat'], 'bat'), + editThought(['note-replace', '=note', 'bat'], 'bit'), + undo(), + ]) + + expect(exportContext(store.getState(), ['note-replace'], 'text/plain')).toEqual(`- note-replace + - =note + - bat`) + + store.dispatch(undo()) + + expect(exportContext(store.getState(), ['note-replace'], 'text/plain')).toEqual(`- note-replace + - =note + - cat`) + }) + + it('typed replacements should undo in one step for thoughts and notes', () => { + const original = 'The world of beautiful birds' + + store.dispatch([ + importText({ + text: ` + - ${original}`, + }), + editThought([original], 'The world of beautiful p'), + editThought(['The world of beautiful p'], 'The world of beautiful pe'), + editThought(['The world of beautiful pe'], 'The world of beautiful peo'), + editThought(['The world of beautiful peo'], 'The world of beautiful peop'), + editThought(['The world of beautiful peop'], 'The world of beautiful peopl'), + editThought(['The world of beautiful peopl'], 'The world of beautiful people'), + undo(), + ]) + + expect(exportContext(store.getState(), [HOME_TOKEN], 'text/plain')).toEqual(`- ${HOME_TOKEN} + - ${original}`) + + store.dispatch([ + importText({ + text: ` + - note-replace-word + - =note + - ${original}`, + }), + editThought(['note-replace-word', '=note', original], 'The world of beautiful p'), + editThought(['note-replace-word', '=note', 'The world of beautiful p'], 'The world of beautiful pe'), + editThought(['note-replace-word', '=note', 'The world of beautiful pe'], 'The world of beautiful peo'), + editThought(['note-replace-word', '=note', 'The world of beautiful peo'], 'The world of beautiful peop'), + editThought(['note-replace-word', '=note', 'The world of beautiful peop'], 'The world of beautiful peopl'), + editThought(['note-replace-word', '=note', 'The world of beautiful peopl'], 'The world of beautiful people'), + undo(), + ]) + + expect(exportContext(store.getState(), ['note-replace-word'], 'text/plain')).toEqual(`- note-replace-word + - =note + - ${original}`) + }) + + it('creating an empty note should undo without reverting the previous edit', () => { + store.dispatch([newThought({ value: 'note-new' }), setCursor(['note-new'])]) + + const undoPatchesBefore = store.getState().undoPatches.length + + store.dispatch(toggleNote()) + + expect(store.getState().undoPatches.length).toBe(undoPatchesBefore + 1) + expect(exportContext(store.getState(), ['note-new'], 'text/plain')).toEqual(`- note-new + - =note + - `) + + store.dispatch(undo()) + + expect(exportContext(store.getState(), [HOME_TOKEN], 'text/plain')).toEqual(`- ${HOME_TOKEN} + - note-new`) + }) + + it('note offset should survive undoing a note edit', () => { + const original = 'The world of birds' + + store.dispatch([ + importText({ + text: ` + - note-offset + - =note + - ${original}`, + }), + setCursor(['note-offset']), + toggleNote(), + setNoteOffset({ value: 10 }), + editThought(['note-offset', '=note', original], 'The cage of birds'), + undo(), + ]) + + expect(store.getState().noteFocus).toBe(true) + expect(store.getState().noteOffset).toBe(10) + expect(exportContext(store.getState(), ['note-offset'], 'text/plain')).toEqual(`- note-offset + - =note + - ${original}`) + }) + it('contiguous edit additions should should not be grouped with deletions', () => { store.dispatch([ importText({ diff --git a/src/components/Note.tsx b/src/components/Note.tsx index b96f439d488..82209fc51d1 100644 --- a/src/components/Note.tsx +++ b/src/components/Note.tsx @@ -4,22 +4,27 @@ import { useDispatch, useSelector } from 'react-redux' import { css, cx } from '../../styled-system/css' import { textNoteRecipe } from '../../styled-system/recipes' import Path from '../@types/Path' +import SimplePath from '../@types/SimplePath' import { cursorDownActionCreator as cursorDown } from '../actions/cursorDown' import { deleteThoughtActionCreator as deleteThought } from '../actions/deleteThought' +import { editThoughtActionCreator as editThought } from '../actions/editThought' import { keyboardOpenActionCreator as keyboardOpen } from '../actions/keyboardOpen' import { setCursorActionCreator as setCursor } from '../actions/setCursor' import { setDescendantActionCreator as setDescendant } from '../actions/setDescendant' import { setNoteFocusActionCreator as setNoteFocus } from '../actions/setNoteFocus' +import { setNoteOffsetActionCreator as setNoteOffset } from '../actions/setNoteOffset' import { toggleNoteActionCreator as toggleNote } from '../actions/toggleNote' import { isTouch } from '../browser' import preventAutoscroll, { preventAutoscrollEnd } from '../device/preventAutoscroll' import * as selection from '../device/selection' import useFreshCallback from '../hooks/useFreshCallback' +import { firstVisibleChild } from '../selectors/getChildren' import getThoughtById from '../selectors/getThoughtById' import noteValue from '../selectors/noteValue' import resolveNotePath from '../selectors/resolveNotePath' import store from '../stores/app' import batchEditingStore from '../stores/batchEditing' +import appendToPath from '../util/appendToPath' import equalPathHead from '../util/equalPathHead' import head from '../util/head' import strip from '../util/strip' @@ -44,7 +49,7 @@ const Note = React.memo( /** Gets the value of the note. Returns null if no note exists or if the context view is active. */ const note = useSelector(state => noteValue(state, path)) - const noteOffset = useSelector(state => state.noteOffset) + const editableNonce = useSelector(state => state.editableNonce) /** Focus Handling with useFreshCallback. */ const onFocus = useFreshCallback(() => { @@ -62,11 +67,18 @@ const Note = React.memo( // set the caret on the note if editing this thought and noteFocus is true useEffect(() => { + const noteOffset = store.getState().noteOffset + // cursor must be true if note is focused if (hasFocus && noteOffset !== null) { selection.set(noteRef.current!, { offset: noteOffset }) } - }, [hasFocus, noteOffset]) + }, [hasFocus, editableNonce]) + + /** Saves the note caret offset without creating an undo patch. */ + const saveNoteOffset = useCallback(() => { + dispatch(setNoteOffset({ value: selection.offsetThought() ?? selection.offset() })) + }, [dispatch]) /** Handles note keyboard shortcuts. */ const onKeyDown = useCallback( @@ -80,6 +92,16 @@ const Note = React.memo( e.preventDefault() dispatch(toggleNote()) } + // prevent Backspace at the beginning of a non-empty note from bubbling up to the thought deletion command + else if ( + e.key === 'Backspace' && + note && + selection.isCollapsed() && + (selection.offsetThought() ?? selection.offset()) === 0 + ) { + e.stopPropagation() + e.preventDefault() + } // delete empty note // (delete non-empty note is handled by delete command, which allows mobile gesture to work) // note may be '' or null if the attribute child was deleted @@ -119,22 +141,36 @@ const Note = React.memo( // Strip
from beginning and end of text e.target.value.replace(/^
|
$/gi, '') + saveNoteOffset() + // update the referenced thought directly if it exists dispatch((dispatch, getState) => { const state = getState() const targetPath = resolveNotePath(state, path) ?? path + const noteThought = firstVisibleChild(state, head(targetPath)) - dispatch( - setDescendant({ - path: targetPath, - values: [value], - mergePrev: batchEditingStore.getState(), // If batch editing is in progress, merge this edit with the previous one in the undo stack. - }), - ) + if (noteThought) { + dispatch( + editThought({ + path: appendToPath(targetPath, noteThought.id) as SimplePath, + oldValue: noteThought.value, + newValue: value, + mergePrev: batchEditingStore.getState(), // If batch editing is in progress, merge this edit with the previous one in the undo stack. + }), + ) + } else { + dispatch( + setDescendant({ + path: targetPath, + values: [value], + mergePrev: batchEditingStore.getState(), // If batch editing is in progress, merge this edit with the previous one in the undo stack. + }), + ) + } }) }, - [dispatch, path, justPasted], + [dispatch, path, justPasted, saveNoteOffset], ) /** Set state.noteFocus if Note lost focus and did not move to another Note. Set state.keyboardOpen if keyboard is closed. */ @@ -217,6 +253,7 @@ const Note = React.memo( onDrop={isTouch ? (e: React.DragEvent) => e.preventDefault() : undefined} onKeyDown={onKeyDown} onChange={onChange} + onSelect={saveNoteOffset} // Text copied from a note and pasted on a thought should not bring along the note's default color and italicization. (#3779) onCopy={onCopy} onCut={onCut} diff --git a/src/components/__tests__/Note.ts b/src/components/__tests__/Note.ts index 5d9ca40f4c7..9a72b7724d5 100644 --- a/src/components/__tests__/Note.ts +++ b/src/components/__tests__/Note.ts @@ -2,6 +2,10 @@ import { fireEvent, screen } from '@testing-library/dom' import { act } from 'react' import { importTextActionCreator as importText } from '../../actions/importText' import { toggleNoteActionCreator as toggleNote } from '../../actions/toggleNote' +import { HOME_TOKEN } from '../../constants' +import * as selection from '../../device/selection' +import exportContext from '../../selectors/exportContext' +import store from '../../stores/app' import createTestApp, { cleanupTestApp } from '../../test-helpers/createTestApp' import dispatch from '../../test-helpers/dispatch' import { setCursorFirstMatchActionCreator as setCursor } from '../../test-helpers/setCursorFirstMatch' @@ -69,6 +73,76 @@ describe('=note', () => { const element = screen.getByText('foo') expect(element) }) + + test('does not merge note text into the thought when Backspace is pressed at the beginning of a note', async () => { + await dispatch([ + importText({ + text: ` + - One + - Two + - =note + - Hello World`, + }), + setCursor(['Two']), + toggleNote(), + ]) + + await act(vi.runOnlyPendingTimersAsync) + + const noteEditor = screen.getByLabelText('note-editable') + + await act(async () => { + fireEvent.focus(noteEditor) + selection.set(noteEditor, { offset: 0 }) + fireEvent.keyDown(noteEditor, { key: 'Backspace' }) + }) + + await act(vi.runAllTimersAsync) + + expect(exportContext(store.getState(), [HOME_TOKEN], 'text/plain')).toEqual(`- ${HOME_TOKEN} + - One + - Two + - =note + - Hello World`) + }) + + test('allows partial note text selection without collapsing the range', async () => { + await dispatch([ + importText({ + text: ` + - One + - =note + - Hello World`, + }), + setCursor(['One']), + toggleNote(), + ]) + + await act(vi.runOnlyPendingTimersAsync) + + const noteEditor = screen.getByLabelText('note-editable') + + await act(async () => { + fireEvent.focus(noteEditor) + + const textNode = noteEditor.firstChild! + const range = document.createRange() + const currentSelection = window.getSelection()! + + range.setStart(textNode, 0) + range.setEnd(textNode, 5) + currentSelection.removeAllRanges() + currentSelection.addRange(range) + + fireEvent.select(noteEditor) + }) + + await act(vi.runOnlyPendingTimersAsync) + + const currentSelection = window.getSelection()! + expect(currentSelection.isCollapsed).toBe(false) + expect(currentSelection.toString()).toBe('Hello') + }) }) describe('=note/=path', () => { diff --git a/src/device/selection.ts b/src/device/selection.ts index 9ba8a3adfb5..25cc8ef435d 100644 --- a/src/device/selection.ts +++ b/src/device/selection.ts @@ -78,6 +78,10 @@ const isContentEditable = (node: Element | null): boolean => node?.hasAttribute( /** Check the node, if it exists, for an ariaLabel set to note-editable. */ const isNoteEditable = (node: Element | null): boolean => node?.ariaLabel === 'note-editable' +/** Returns true if the node is an editable thought or note root. */ +const isEditableRoot = (node: Node | null): boolean => + node instanceof Element && (isContentEditable(node) || isNoteEditable(node)) + /** Returns true if the node is part of a note. Defaults to using the active selection. */ export const isNote = (node?: EventTarget | null): boolean => { const editable = node === undefined ? document.activeElement : getEditableCandidate(node) @@ -194,7 +198,7 @@ export const offsetThought = (): number | null => { : 0 : selection.focusOffset let curNode: Node | null = selection.focusNode.nodeType === Node.TEXT_NODE ? selection.focusNode : selection.focusNode - while (curNode && !(curNode as HTMLElement)?.hasAttribute?.('data-editable')) { + while (curNode && !isEditableRoot(curNode)) { if (curNode?.previousSibling) { total += curNode.previousSibling.textContent?.length || 0 curNode = curNode.previousSibling diff --git a/src/redux-enhancers/undoRedoEnhancer.ts b/src/redux-enhancers/undoRedoEnhancer.ts index 8438353fa6c..c79705513ad 100644 --- a/src/redux-enhancers/undoRedoEnhancer.ts +++ b/src/redux-enhancers/undoRedoEnhancer.ts @@ -24,6 +24,13 @@ enum EditThoughtDirection { Shorter = 'Shorter', } +interface TextDiff { + newMiddle: string + oldMiddle: string + prefix: string + suffix: string +} + /** Interface for the setIsMulticursorExecuting action. */ interface SetIsMulticursorExecutingAction extends Action<'setIsMulticursorExecuting'> { value: boolean @@ -40,24 +47,86 @@ function isEditThoughtAction(action: UnknownAction): action is UnknownAction & e return action.type === 'editThought' } +/** Gets plain text from html. */ +function getTextContent(value: string): string { + const element = document.createElement('div') + element.innerHTML = value + return element.textContent || '' +} + +/** Gets the changed text range between two values. */ +function getTextDiff(oldValue: string, newValue: string): TextDiff { + let start = 0 + + while (start < oldValue.length && start < newValue.length && oldValue[start] === newValue[start]) { + start += 1 + } + + let oldEnd = oldValue.length - 1 + let newEnd = newValue.length - 1 + + while (oldEnd >= start && newEnd >= start && oldValue[oldEnd] === newValue[newEnd]) { + oldEnd -= 1 + newEnd -= 1 + } + + return { + oldMiddle: oldValue.slice(start, oldEnd + 1), + newMiddle: newValue.slice(start, newEnd + 1), + prefix: oldValue.slice(0, start), + suffix: oldValue.slice(oldEnd + 1), + } +} + /** Compare the text contents of the old and new values to determine the direction of the edit. * Returns None if the action is not an editThought action or if the text content length is the same. */ function getEditThoughtDirection(action: UnknownAction): EditThoughtDirection { if (!isEditThoughtAction(action)) return EditThoughtDirection.None - const oldElement = document.createElement('div') - const newElement = document.createElement('div') - oldElement.innerHTML = action.oldValue - newElement.innerHTML = action.newValue + const oldText = getTextContent(action.oldValue) + const newText = getTextContent(action.newValue) - return newElement.textContent.length === oldElement.textContent.length + return newText.length === oldText.length ? EditThoughtDirection.None - : newElement.textContent.length > oldElement.textContent.length + : newText.length > oldText.length ? EditThoughtDirection.Longer : EditThoughtDirection.Shorter } +/** Returns true if the current edit extends a selected-text replacement that started with the previous edit. */ +function isReplacementContinuation(previousAction: UnknownAction | undefined, currentAction: UnknownAction): boolean { + if (!previousAction || !isEditThoughtAction(previousAction) || !isEditThoughtAction(currentAction)) return false + if (!_.isEqual(previousAction.path, currentAction.path)) return false + + const previousOldText = getTextContent(previousAction.oldValue) + const previousNewText = getTextContent(previousAction.newValue) + const currentOldText = getTextContent(currentAction.oldValue) + const currentNewText = getTextContent(currentAction.newValue) + + if (currentOldText !== previousNewText || currentNewText.length <= currentOldText.length) return false + + const previousDiff = getTextDiff(previousOldText, previousNewText) + const isReplacement = previousDiff.oldMiddle.length > 0 && previousDiff.newMiddle.length > 0 + + if ( + !isReplacement || + !currentNewText.startsWith(previousDiff.prefix) || + !currentNewText.endsWith(previousDiff.suffix) + ) { + return false + } + + const currentReplacement = currentNewText.slice( + previousDiff.prefix.length, + previousDiff.suffix.length ? -previousDiff.suffix.length : undefined, + ) + + return ( + currentReplacement.startsWith(previousDiff.newMiddle) && currentReplacement.length > previousDiff.newMiddle.length + ) +} + /** Properties that are ignored when generating state patches. */ const statePropertiesToOmit: (keyof State)[] = ['alert', 'cursorCleared', 'editableNonce', 'pushQueue'] @@ -125,6 +194,13 @@ const getPatchAction = (patch: Patch): ActionType => patch[0]?.actions[0] */ const isPatchUndoable = (patch: Patch | undefined): boolean => !!patch?.[0]?.actions.some(isUndoable) +/** + * Returns true if a patch changes thoughts. Some navigation actions, such as toggleNote, can create or delete hidden + * note thoughts. Those should be undone independently instead of being grouped with the previous edit. + */ +const patchChangesThoughts = (patch: Pick[] | undefined): boolean => + !!patch?.some(operation => operation.path.startsWith('/thoughts/')) + /** * Gets the nth item from the end of an array. */ @@ -178,7 +254,7 @@ const undoReducer = (state: State, undoPatches: Patch[]): State => { if (!undoPatches.length) return state const undoTwice = isNavigation(lastAction) - ? isPatchUndoable(penultimateUndoPatch) + ? !patchChangesThoughts(lastUndoPatch) && isPatchUndoable(penultimateUndoPatch) : penultimateAction === 'newThought' const poppedUndoPatches = undoTwice ? [penultimateUndoPatch, lastUndoPatch] : [lastUndoPatch] @@ -200,7 +276,8 @@ const redoReducer = (state: State, redoPatches: Patch[]): State => { if (!redoPatches.length) return state - const redoTwice = lastAction && (isNavigation(lastAction) || lastAction === 'newThought') + const redoTwice = + lastAction && ((isNavigation(lastAction) && !patchChangesThoughts(lastRedoPatch)) || lastAction === 'newThought') const poppedRedoPatches = redoTwice ? [nthLast(redoPatches, 2), lastRedoPatch] : [lastRedoPatch] @@ -282,6 +359,9 @@ const undoRedoReducerEnhancer: StoreEnhancer = const shouldMergeWithLastEditThought = editThoughtDirection !== EditThoughtDirection.None && editThoughtDirection === lastEditThoughtDirection + const shouldMergeWithReplacementContinuation = isReplacementContinuation(lastAction, action) + const undoPatch = diffState(newState as Index, state) + const actionChangesThoughts = patchChangesThoughts(undoPatch) // Some actions are merged together into a single undo/redo patch. // - Navigation actions are merged with the previous non-navigation action. This matches the behavior of most word processors where undo will revert the last destructive action, and the cursor will be restored to where it was before. For example, if the user edits 'a' to 'aa', moves the cursor to 'b', and then undoes, the cursor will be restored to 'aa' then the edit will be undone. @@ -290,14 +370,18 @@ const undoRedoReducerEnhancer: StoreEnhancer = // - All actions during the execution of a multicursor command will be merged together. The prevous action will always be setIsMulticursorExecuting. // - Chained commands will be merged into the previous command, e.g. Select All + Categorize if ( - (isNavigation(actionType) && isNavigation(lastAction?.type)) || + (isNavigation(actionType) && isNavigation(lastAction?.type) && !actionChangesThoughts) || shouldMergeWithLastEditThought || + shouldMergeWithReplacementContinuation || actionType === 'closeAlert' || state.isMulticursorExecuting || (lastAction as UnknownAction)?.mergeNext || (action as UnknownAction)?.mergePrev ) { lastAction = action + if (isEditThoughtAction(action)) { + lastEditThoughtDirection = editThoughtDirection + } const lastUndoPatch = nthLast(state.undoPatches, 1) let lastState = state if (lastUndoPatch && lastUndoPatch.length > 0) { @@ -339,7 +423,6 @@ const undoRedoReducerEnhancer: StoreEnhancer = lastEditThoughtDirection = editThoughtDirection // add a new undo patch - const undoPatch = diffState(newState as Index, state) return undoPatch.length ? { ...newState,