fix(feel-popup): do not close popup on escape during auto suggest

Closes #279
bpmn-io · Sep 15, 2023 · dcbb1e6 · dcbb1e6
1 parent 6163c8f
commit dcbb1e6
Show file tree

Hide file tree

Showing 4 changed files with 225 additions and 20 deletions.
diff --git a/src/components/Popup.js b/src/components/Popup.js
@@ -23,35 +23,39 @@ const noop = () => {};
  * @param {Object} props
  * @param {HTMLElement} [props.container]
  * @param {string} [props.className]
+ * @param {boolean} [props.delayInitialFocus]
  * @param {{x: number, y: number}} [props.position]
  * @param {number} [props.width]
  * @param {number} [props.height]
  * @param {Function} props.onClose
  * @param {Function} [props.onPostActivate]
  * @param {Function} [props.onPostDeactivate]
  * @param {boolean} [props.returnFocus]
+ * @param {boolean} [props.closeOnEscape]
  * @param {string} props.title
  */
 export function Popup(props) {
 
   const {
     container,
     className,
+    delayInitialFocus,
     position,
     width,
     height,
     onClose,
     onPostActivate = noop,
     onPostDeactivate = noop,
     returnFocus = true,
+    closeOnEscape = true,
     title
   } = props;
 
   const focusTrapRef = useRef(null);
   const popupRef = useRef(null);
 
   const handleKeyPress = event => {
-    if (event.key === 'Escape') {
+    if (closeOnEscape && event.key === 'Escape') {
       onClose();
     }
   };
@@ -101,6 +105,7 @@ export function Popup(props) {
     if (popupRef.current) {
       focusTrapRef.current = focusTrap.createFocusTrap(popupRef.current, {
         clickOutsideDeactivates: true,
+        delayInitialFocus,
         fallbackFocus: popupRef.current,
         onPostActivate,
         onPostDeactivate,

diff --git a/src/components/entries/FEEL/FeelPopup.js b/src/components/entries/FEEL/FeelPopup.js
@@ -92,17 +92,34 @@ function FeelPopupComponent(props) {
 
   const editorRef = useRef();
 
+  const isAutoCompletionOpen = useRef(false);
+
   const handleSetReturnFocus = () => {
     sourceElement && sourceElement.focus();
   };
 
-  useEffect(() => {
-    const editor = editorRef.current;
+  const onKeyDownCapture = (event) => {
+
+    // we use capture here to make sure we handle the event before the editor does
+    if (event.key === 'Escape') {
+      isAutoCompletionOpen.current = autoCompletionOpen(event.target);
+    }
+  };
 
-    if (editor) {
-      editor.focus();
+  const onKeyDown = (event) => {
+
+    if (event.key === 'Escape') {
+
+      // close popup only if auto completion is not open
+      // we need to do check this because the editor is not
+      // stop propagating the keydown event
+      // cf. https://discuss.codemirror.net/t/how-can-i-replace-the-default-autocompletion-keymap-v6/3322/5
+      if (!isAutoCompletionOpen.current) {
+        onClose();
+        isAutoCompletionOpen.current = false;
+      }
     }
-  }, [ editorRef, id ]);
+  };
 
   return (
     <Popup
@@ -114,6 +131,8 @@ function FeelPopupComponent(props) {
 
       // handle focus manually on deactivate
       returnFocus={ false }
+      closeOnEscape={ false }
+      delayInitialFocus={ false }
       onPostDeactivate={ handleSetReturnFocus }
       height={ FEEL_POPUP_HEIGHT }
       width={ FEEL_POPUP_WIDTH }
@@ -122,7 +141,10 @@ function FeelPopupComponent(props) {
         title={ title }
         draggable />
       <Popup.Body>
-        <div class="bio-properties-panel-feel-popup__body">
+        <div
+          onKeyDownCapture={ onKeyDownCapture }
+          onKeyDown={ onKeyDown }
+          class="bio-properties-panel-feel-popup__body">
 
           {
             type === 'feel' && (
@@ -171,4 +193,8 @@ function FeelPopupComponent(props) {
 
 function prefixId(id) {
   return `bio-properties-panel-${id}`;
+}
+
+function autoCompletionOpen(element) {
+  return element.closest('.cm-editor').querySelector('.cm-tooltip-autocomplete');
 }
diff --git a/test/spec/components/FeelPopup.spec.js b/test/spec/components/FeelPopup.spec.js
@@ -1,5 +1,6 @@
 import {
   act,
+  fireEvent,
   render,
   waitFor
 } from '@testing-library/preact/pure';
@@ -29,7 +30,6 @@ const noopElement = {
 };
 
 
-
 describe('<FeelPopup>', function() {
 
   let container;
@@ -116,15 +116,15 @@ describe('<FeelPopup>', function() {
     });
 
     // assume
-    expect(domQuery('.bio-properties-panel-feel-editor-container', document.body)).to.exist;
+    expect(getFeelEditor(document.body)).to.exist;
 
     // and when
     await act(() => {
       result.rerender({ element: { id: 'bar', type: 'bar' } });
     });
 
     // then
-    expect(domQuery('.bio-properties-panel-feel-editor-container', document.body)).to.not.exist;
+    expect(getFeelEditor(document.body)).to.not.exist;
   });
 
 
@@ -139,19 +139,68 @@ describe('<FeelPopup>', function() {
       const btn = domQuery('button', childComponent);
 
       // assume
-      expect(domQuery('.bio-properties-panel-feel-editor-container', document.body)).to.not.exist;
+      expect(getFeelEditor(document.body)).to.not.exist;
+
+      // when
+      await act(() => {
+        btn.click();
+      });
+
+      // then
+      expect(getFeelEditor(document.body)).to.exist;
+    });
+
+
+    it('should focus <feel> editor', async function() {
+
+      // given
+      createFeelPopup({ type: 'feel' }, container);
+
+      const childComponent = domQuery('.child-component', container);
+      const btn = domQuery('button', childComponent);
 
       // when
       await act(() => {
         btn.click();
       });
 
+      const editor = getFeelEditor(document.body);
+
+      // then
+      expect(document.activeElement).to.eql(domQuery('.cm-content', editor));
+    });
+
+
+    it('should autosuggest in <feel> editor', async function() {
+
+      // given
+      createFeelPopup({ type: 'feel' }, container);
+
+      const childComponent = domQuery('.child-component', container);
+      const btn = domQuery('button', childComponent);
+
+      await act(() => {
+        btn.click();
+      });
+
+      const editor = getFeelEditor(document.body);
+
+      // assume
+      expect(editor).to.exist;
+
+      // when
+      fireEvent.keyDown(document.activeElement, { key: ' ', ctrlKey: true });
+
       // then
-      expect(domQuery('.bio-properties-panel-feel-editor-container', document.body)).to.exist;
+      let suggestions;
+      await waitFor(() => {
+        suggestions = domQuery('.cm-tooltip-autocomplete > ul', editor);
+        expect(suggestions).to.exist;
+      });
     });
 
 
-    it('should close <feel> editor', async function() {
+    it('should NOT close <feel> editor on ESC (autosuggest)', async function() {
 
       // given
       createFeelPopup({ type: 'feel' }, container);
@@ -163,8 +212,63 @@ describe('<FeelPopup>', function() {
         btn.click();
       });
 
+      const editor = getFeelEditor(document.body);
+
+      expect(editor).to.exist;
+
+      fireEvent.keyDown(domQuery('.cm-content', editor), { key: ' ', ctrlKey: true });
+
       // assume
-      expect(domQuery('.bio-properties-panel-feel-editor-container', document.body)).to.exist;
+      let suggestions;
+      await waitFor(() => {
+        suggestions = domQuery('.cm-tooltip-autocomplete > ul', editor);
+        expect(suggestions).to.exist;
+      });
+
+      // when
+      fireEvent.keyDown(domQuery('.cm-content', editor), { key: 'Escape' });
+
+      // then
+      expect(getFeelEditor(document.body)).to.exist;
+    });
+
+
+    it('should close <feel> editor on ESC (no autosuggest)', async function() {
+
+      // given
+      createFeelPopup({ type: 'feel' }, container);
+
+      const childComponent = domQuery('.child-component', container);
+      const btn = domQuery('button', childComponent);
+
+      await act(() => {
+        btn.click();
+      });
+
+      const editor = getFeelEditor(document.body);
+
+      // when
+      fireEvent.keyDown(domQuery('.cm-content', editor), { key: 'Escape' });
+
+      // then
+      expect(getFeelEditor(document.body)).to.not.exist;
+    });
+
+
+    it('should close <feel> editor on btn click', async function() {
+
+      // given
+      createFeelPopup({ type: 'feel' }, container);
+
+      const childComponent = domQuery('.child-component', container);
+      const btn = domQuery('button', childComponent);
+
+      await act(() => {
+        btn.click();
+      });
+
+      // assume
+      expect(getFeelEditor(document.body)).to.exist;
 
       const closeBtn = domQuery('.bio-properties-panel-feel-popup__close-btn', document.body);
 
@@ -174,7 +278,7 @@ describe('<FeelPopup>', function() {
       });
 
       // then
-      expect(domQuery('.bio-properties-panel-feel-editor-container', document.body)).to.not.exist;
+      expect(getFeelEditor(document.body)).to.not.exist;
     });
 
   });
@@ -191,19 +295,61 @@ describe('<FeelPopup>', function() {
       const btn = domQuery('button', childComponent);
 
       // assume
-      expect(domQuery('.bio-properties-panel-feelers-editor-container', document.body)).to.not.exist;
+      expect(getFeelersEditor(document.body)).to.not.exist;
 
       // when
       await act(() => {
         btn.click();
       });
 
       // then
-      expect(domQuery('.bio-properties-panel-feelers-editor-container', document.body)).to.exist;
+      expect(getFeelersEditor(document.body)).to.exist;
     });
 
 
-    it('should close <feelers> editor', async function() {
+    it('should focus <feelers> editor', async function() {
+
+      // given
+      createFeelPopup({ type: 'feelers' }, container);
+
+      const childComponent = domQuery('.child-component', container);
+      const btn = domQuery('button', childComponent);
+
+      // when
+      await act(() => {
+        btn.click();
+      });
+
+      const editor = getFeelersEditor(document.body);
+
+      // then
+      expect(document.activeElement).to.eql(domQuery('.cm-content', editor));
+    });
+
+
+    it('should close <feelers> editor on ESC (no autosuggest)', async function() {
+
+      // given
+      createFeelPopup({ type: 'feelers' }, container);
+
+      const childComponent = domQuery('.child-component', container);
+      const btn = domQuery('button', childComponent);
+
+      await act(() => {
+        btn.click();
+      });
+
+      const editor = getFeelersEditor(document.body);
+
+      // when
+      fireEvent.keyDown(domQuery('.cm-content', editor), { key: 'Escape' });
+
+      // then
+      expect(getFeelersEditor(document.body)).to.not.exist;
+    });
+
+
+    it('should close <feelers> editor on btn click', async function() {
 
       // given
       createFeelPopup({ type: 'feelers' }, container);
@@ -216,7 +362,7 @@ describe('<FeelPopup>', function() {
       });
 
       // assume
-      expect(domQuery('.bio-properties-panel-feelers-editor-container', document.body)).to.exist;
+      expect(getFeelersEditor(document.body)).to.exist;
 
       const closeBtn = domQuery('.bio-properties-panel-feel-popup__close-btn', document.body);
 
@@ -226,7 +372,7 @@ describe('<FeelPopup>', function() {
       });
 
       // then
-      expect(domQuery('.bio-properties-panel-feelers-editor-container', document.body)).to.not.exist;
+      expect(getFeelersEditor(document.body)).to.not.exist;
     });
 
   });
@@ -328,4 +474,12 @@ function ChildComponent(props) {
   return <div class="child-component">
     <button ref={ btnRef } onClick={ onClick }>Open popup</button>
   </div>;
+}
+
+function getFeelEditor(container) {
+  return domQuery('.bio-properties-panel-feel-editor-container', container);
+}
+
+function getFeelersEditor(container) {
+  return domQuery('.bio-properties-panel-feelers-editor-container', container);
 }