diff --git a/src/components/entries/FEEL/Feel.js b/src/components/entries/FEEL/Feel.js index 13b734cc..c10b98cc 100644 --- a/src/components/entries/FEEL/Feel.js +++ b/src/components/entries/FEEL/Feel.js @@ -5,6 +5,7 @@ import { useCallback, useContext, useEffect, + useLayoutEffect, useRef, useState } from 'preact/hooks'; @@ -84,6 +85,7 @@ function FeelTextfield(props) { } = props; const [ localValue, setLocalValue ] = useState(getInitialFeelLocalValue(feel, value)); + const localValueRef = useRef(localValue); const editorRef = useShowEntryEvent(id); const containerRef = useRef(); @@ -100,6 +102,12 @@ function FeelTextfield(props) { const feelOnlyValue = getFeelValue(localValue); const feelLanguageContext = useContext(FeelLanguageContext); + // keep state between rerenders + const isUnmountingRef = useRef(false); + const feelActiveRef = useRef(feelActive); + feelActiveRef.current = feelActive; + + const [ focus, _setFocus ] = useState(undefined); const { @@ -121,6 +129,12 @@ function FeelTextfield(props) { * @type { import('min-dash').DebouncedFunction } */ const handleInput = useDebounce(onInput, debounce); + const previousElementRef = useRef(element); + + if (previousElementRef.current !== element) { + handleInput.flush?.(); + previousElementRef.current = element; + } const handleFeelToggle = useStaticCallback(() => { if (feel === 'required') { @@ -129,22 +143,38 @@ function FeelTextfield(props) { if (!feelActive) { setLocalValue('=' + localValue); - handleInput('=' + localValue); + handleInput.cancel?.(); + onInput('=' + localValue); } else { setLocalValue(feelOnlyValue); - handleInput(feelOnlyValue); + handleInput.cancel?.(); + onInput(feelOnlyValue); } + + // Prevent the FeelEditor unmount cleanup from double-committing + editorRef.current?.clearDirty?.(); }); const handleLocalInput = (newValue, useDebounce = true) => { + + const currentLocalValue = localValueRef.current; + + if (!useDebounce) { + handleInput.cancel?.(); + } + if (feelActive) { newValue = '=' + newValue; } - if (newValue === localValue) { + if (newValue === currentLocalValue) { + if (!useDebounce) { + onInput(newValue); + } return; } + localValueRef.current = newValue; setLocalValue(newValue); if (useDebounce) { handleInput(newValue); @@ -164,7 +194,12 @@ function FeelTextfield(props) { if (e.target.type === 'checkbox') { onInput(e.target.checked); } else { - const trimmedValue = e.target.value.trim(); + let trimmedValue = trimValue(feel, e.target.value); + + if (!trimmedValue && isString(localValueRef.current) && localValueRef.current.startsWith('=')) { + trimmedValue = localValueRef.current; + } + handleLocalInput(trimmedValue, false); } @@ -237,10 +272,29 @@ function FeelTextfield(props) { }, [ value ]); useEffect(() => { + localValueRef.current = localValue; + }, [ localValue ]); + + // ensure values are committed during element switch + useEffect(() => { + return () => { isUnmountingRef.current = true; }; + }, []); + + useLayoutEffect(() => { return () => { + if (!isUnmountingRef.current && feelActiveRef.current) { + handleInput.flush?.(); + editorRef.current?.commit?.(); + } + }; + }, [ element, handleInput ]); + + useEffect(() => { + return () => { + handleInput.flush?.(); eventBus.fire('propertiesPanel.closePopup'); }; - }, []); + }, [ eventBus, handleInput ]); // copy-paste integration useEffect(() => { @@ -271,10 +325,9 @@ function FeelTextfield(props) { if (isFieldEmpty || isAllSelected) { const textData = event.clipboardData.getData('text'); - const trimmedValue = textData.trim(); + const trimmedValue = trimValue(feel, textData); - setLocalValue(trimmedValue); - handleInput(trimmedValue); + handleLocalInput(trimmedValue, false); if (!feelActive && isString(trimmedValue) && trimmedValue.startsWith('=')) { setFocus(trimmedValue.length - 1); @@ -906,3 +959,16 @@ function getInitialFeelLocalValue(feelType, value) { return value; } + +/* +* trim value and also feel expression after the `=` + * ' = test ' -> '=test' + * ' hello ' -> 'hello' +*/ +function trimValue(feelType, value) { + const trimmedValue = value.trim(); + if (isFeelActive(feelType, trimmedValue)) { + return '=' + getFeelValue(trimmedValue).trim(); + } + return value.trim(); +} diff --git a/src/components/entries/FEEL/FeelEditor.js b/src/components/entries/FEEL/FeelEditor.js index 0e158efe..2b2a8f7d 100644 --- a/src/components/entries/FEEL/FeelEditor.js +++ b/src/components/entries/FEEL/FeelEditor.js @@ -22,18 +22,23 @@ const useBufferedFocus = function(editor, ref) { const [ buffer, setBuffer ] = useState(undefined); - ref.current = useMemo(() => ({ - focus: (offset) => { - if (editor) { - editor.focus(offset); - } else { - if (typeof offset === 'undefined') { - offset = Infinity; + ref.current = useMemo(() => { + const current = ref.current || {}; + + return { + ...current, + focus: (offset) => { + if (editor) { + editor.focus(offset); + } else { + if (typeof offset === 'undefined') { + offset = Infinity; + } + setBuffer(offset); } - setBuffer(offset); } - } - }), [ editor ]); + }; + }, [ editor ]); useEffect(() => { if (typeof buffer !== 'undefined' && editor) { @@ -49,7 +54,7 @@ const FeelEditor = forwardRef((props, ref) => { contentAttributes, enableGutters, value, - onInput, + onInput = noop, onKeyDown: onKeyDownProp = noop, onFeelToggle = noop, onLint = noop, @@ -64,7 +69,13 @@ const FeelEditor = forwardRef((props, ref) => { const inputRef = useRef(); const [ editor, setEditor ] = useState(); + const [ localValue, setLocalValue ] = useState(value || ''); + const isDirtyRef = useRef(false); + + const ignoreProgrammaticChangeRef = useRef(false); + const editorRef = useRef(null); + const blurHasCommittedRef = useRef(false); const { builtins, @@ -74,11 +85,58 @@ const FeelEditor = forwardRef((props, ref) => { useBufferedFocus(editor, ref); - const handleInput = useStaticCallback(newValue => { - onInput(newValue); + useEffect(() => { + if (!ref.current) { + return; + } + + ref.current.commit = () => { + if (blurHasCommittedRef.current) { + blurHasCommittedRef.current = false; + return; + } + + const stateValue = editor?._cmEditor?.state?.doc?.toString?.(); + const domValue = editor?._cmEditor?.contentDOM?.textContent; + const currentValue = stateValue === '' && typeof domValue === 'string' + ? domValue + : (stateValue || ''); + isDirtyRef.current = false; + handleInput(currentValue, false); + }; + + ref.current.clearDirty = () => { + isDirtyRef.current = false; + }; + }, [ editor ]); + + const handleInput = useStaticCallback((newValue, useDebounce = true) => { + if (!ignoreProgrammaticChangeRef.current) { + isDirtyRef.current = useDebounce; + } + + onInput(newValue, useDebounce); setLocalValue(newValue); }); + // Commit immediately on blur + const handleBlur = useStaticCallback(() => { + if (!isDirtyRef.current) { + return; + } + + const currentEditor = editorRef.current; + const stateValue = currentEditor?._cmEditor?.state?.doc?.toString?.(); + const domValue = currentEditor?._cmEditor?.contentDOM?.textContent; + const currentValue = stateValue === '' && typeof domValue === 'string' + ? domValue + : (stateValue || ''); + + blurHasCommittedRef.current = true; + isDirtyRef.current = false; + handleInput(currentValue, false); + }); + useEffect(() => { let editor; @@ -119,16 +177,29 @@ const FeelEditor = forwardRef((props, ref) => { parserDialect, extensions: [ ...enableGutters ? [ lineNumbers() ] : [], - EditorView.lineWrapping + EditorView.lineWrapping, + EditorView.domEventHandlers({ blur: handleBlur }) ], contentAttributes }); + editorRef.current = editor; setEditor( editor ); return () => { + editorRef.current = null; + const stateValue = editor?._cmEditor?.state?.doc?.toString?.(); + const domValue = editor?._cmEditor?.contentDOM?.textContent; + + const currentValue = stateValue === '' && typeof domValue === 'string' + ? domValue + : (stateValue || ''); + + if (isDirtyRef.current) { + handleInput(currentValue, false); + } onLint([]); inputRef.current.innerHTML = ''; setEditor(null); @@ -144,7 +215,9 @@ const FeelEditor = forwardRef((props, ref) => { return; } + ignoreProgrammaticChangeRef.current = true; editor.setValue(value); + ignoreProgrammaticChangeRef.current = false; setLocalValue(value); }, [ value ]); diff --git a/test/spec/components/Feel.spec.js b/test/spec/components/Feel.spec.js index 5efdb4f5..8f48756d 100644 --- a/test/spec/components/Feel.spec.js +++ b/test/spec/components/Feel.spec.js @@ -916,7 +916,7 @@ describe('', function() { }); - it('should preserve FEEL toggle when pasting FEEL expression and then blurring', async function() { + it('should commit pasted FEEL expression immediately on paste from non-FEEL text state', function() { // given const setValueSpy = sinonSpy(); @@ -925,7 +925,8 @@ describe('', function() { container, feel: 'optional', getValue: () => '', - setValue: setValueSpy + setValue: setValueSpy, + debounce: fn => debounce(fn, 50) }); const input = domQuery('.bio-properties-panel-input', result.container); @@ -935,19 +936,96 @@ describe('', function() { // when input.dispatchEvent(createFeelPasteEvent('=test')); - await waitFor(() => { - expect(getEditor(result.container)).to.exist; + // then + expect(setValueSpy).to.have.been.calledOnceWith('=test'); + }); + + + it('should not overwrite pasted FEEL expression when blurring immediately', async function() { + + // given + const setValueSpy = sinonSpy(); + + const result = createFeelField({ + container, + feel: 'optional', + getValue: () => '', + setValue: setValueSpy, + debounce: fn => debounce(fn, 50) }); - input.blur(); + const input = domQuery('.bio-properties-panel-input', result.container); + + expect(getEditor(result.container)).to.not.exist; + + // when + await act(() => { + input.dispatchEvent(createFeelPasteEvent('=test')); + input.blur(); + }); // then + return waitFor(() => { + expect(getEditor(result.container)).to.exist; + expect(setValueSpy).to.have.been.calledWith('=test'); + expect(setValueSpy).not.to.have.been.calledWith(undefined); + }); + }); + + }); + + it('should trim whitespace after = when pasting FEEL expression', function() { + + // given + const updateSpy = sinonSpy(); + + const result = createFeelField({ + container, + feel: 'optional', + setValue: updateSpy + }); + + const input = domQuery('.bio-properties-panel-input', result.container); + + // when + input.dispatchEvent(createFeelPasteEvent(' = test ')); + + // then + expect(updateSpy).to.have.been.calledWith('=test'); + + // Should also toggle to FEEL mode + return waitFor(() => { expect(getEditor(result.container)).to.exist; - expect(setValueSpy).to.have.been.calledWith('=test'); + }); + }); + + + it('should trim whitespace after = when blurring with FEEL expression', async function() { + + // given + const updateSpy = sinonSpy(); + + const result = createFeelField({ + container, + feel: 'optional', + setValue: updateSpy + }); + + const input = domQuery('.bio-properties-panel-input', result.container); + + // when + await act(() => { + input.focus(); + changeInput(input, ' = test '); + input.blur(); }); + // then + expect(updateSpy).to.have.been.calledWith('=test'); }); + + }); @@ -2434,6 +2512,45 @@ describe('', function() { }); + it('should persist value when leaving FEEL editor immediately after input', async function() { + + // given + const updateSpy = sinonSpy(); + + const firstElement = { id: 'first' }; + const secondElement = { id: 'second' }; + + const result = createFeelField({ + container, + element: firstElement, + setValue: updateSpy, + feel: 'required', + debounce: fn => debounce(fn, 50) + }); + + const input = getEditor(result.container); + + // when + await act(() => { + input.textContent = 'foo'; + + createFeelField({ + container, + element: secondElement, + setValue: updateSpy, + feel: 'required', + debounce: fn => debounce(fn, 50) + }, result.rerender); + }); + + // then + await waitFor(() => { + expect(updateSpy).to.have.been.calledWith('=foo'); + expect(updateSpy).to.have.been.calledOnce; + }); + }); + + it('should render with missing = sign', async function() { // given