fix(ace-editor-popover): main AntD popover closes when clicking autocomplete suggestions in Ace Editor (#35986)

EnxDev · web-flow · commit 9ef87e75d506 · 2025-11-14T16:35:15.000+01:00
diff --git a/superset-frontend/packages/superset-ui-core/src/components/AsyncAceEditor/AsyncAceEditor.test.tsx b/superset-frontend/packages/superset-ui-core/src/components/AsyncAceEditor/AsyncAceEditor.test.tsx
@@ -16,7 +16,9 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+import { createRef } from 'react';
 import { render, screen, waitFor } from '@superset-ui/core/spec';
+import type AceEditor from 'react-ace';
 import {
   AsyncAceEditor,
   SQLEditor,
@@ -99,3 +101,253 @@ test('renders a custom placeholder', () => {
 
   expect(screen.getByRole('paragraph')).toBeInTheDocument();
 });
+
+test('registers afterExec event listener for command handling', async () => {
+  const ref = createRef<AceEditor>();
+  const { container } = render(<SQLEditor ref={ref as React.Ref<never>} />);
+
+  await waitFor(() => {
+    expect(container.querySelector(selector)).toBeInTheDocument();
+  });
+
+  const editorInstance = ref.current?.editor;
+  expect(editorInstance).toBeDefined();
+
+  if (!editorInstance) return;
+
+  // Verify the commands object has the 'on' method (confirms event listener capability)
+  expect(editorInstance.commands).toHaveProperty('on');
+  expect(typeof editorInstance.commands.on).toBe('function');
+});
+
+test('moves autocomplete popup to parent container when triggered', async () => {
+  const ref = createRef<AceEditor>();
+  const { container } = render(<SQLEditor ref={ref as React.Ref<never>} />);
+
+  await waitFor(() => {
+    expect(container.querySelector(selector)).toBeInTheDocument();
+  });
+
+  const editorInstance = ref.current?.editor;
+  expect(editorInstance).toBeDefined();
+
+  if (!editorInstance) return;
+
+  // Create a mock autocomplete popup in the editor container
+  const mockAutocompletePopup = document.createElement('div');
+  mockAutocompletePopup.className = 'ace_autocomplete';
+  editorInstance.container?.appendChild(mockAutocompletePopup);
+
+  const parentContainer =
+    editorInstance.container?.closest('#ace-editor') ??
+    editorInstance.container?.parentElement;
+
+  // Manually trigger the afterExec event with insertstring command using _emit
+  type CommandManagerWithEmit = typeof editorInstance.commands & {
+    _emit: (event: string, data: unknown) => void;
+  };
+  (editorInstance.commands as CommandManagerWithEmit)._emit('afterExec', {
+    command: { name: 'insertstring' },
+    args: ['SELECT'],
+  });
+
+  await waitFor(() => {
+    // Check that the popup has the data attribute set
+    expect(mockAutocompletePopup.dataset.aceAutocomplete).toBe('true');
+    // Check that the popup is in the parent container
+    expect(parentContainer?.contains(mockAutocompletePopup)).toBe(true);
+  });
+});
+
+test('moves autocomplete popup on startAutocomplete command event', async () => {
+  const ref = createRef<AceEditor>();
+  const { container } = render(<SQLEditor ref={ref as React.Ref<never>} />);
+
+  await waitFor(() => {
+    expect(container.querySelector(selector)).toBeInTheDocument();
+  });
+
+  const editorInstance = ref.current?.editor;
+  expect(editorInstance).toBeDefined();
+
+  if (!editorInstance) return;
+
+  // Create a mock autocomplete popup
+  const mockAutocompletePopup = document.createElement('div');
+  mockAutocompletePopup.className = 'ace_autocomplete';
+  editorInstance.container?.appendChild(mockAutocompletePopup);
+
+  const parentContainer =
+    editorInstance.container?.closest('#ace-editor') ??
+    editorInstance.container?.parentElement;
+
+  // Manually trigger the afterExec event with startAutocomplete command
+  type CommandManagerWithEmit = typeof editorInstance.commands & {
+    _emit: (event: string, data: unknown) => void;
+  };
+  (editorInstance.commands as CommandManagerWithEmit)._emit('afterExec', {
+    command: { name: 'startAutocomplete' },
+  });
+
+  await waitFor(() => {
+    // Check that the popup has the data attribute set
+    expect(mockAutocompletePopup.dataset.aceAutocomplete).toBe('true');
+    // Check that the popup is in the parent container
+    expect(parentContainer?.contains(mockAutocompletePopup)).toBe(true);
+  });
+});
+
+test('does not move autocomplete popup on unrelated commands', async () => {
+  const ref = createRef<AceEditor>();
+  const { container } = render(<SQLEditor ref={ref as React.Ref<never>} />);
+
+  await waitFor(() => {
+    expect(container.querySelector(selector)).toBeInTheDocument();
+  });
+
+  const editorInstance = ref.current?.editor;
+  expect(editorInstance).toBeDefined();
+
+  if (!editorInstance) return;
+
+  // Create a mock autocomplete popup in the body
+  const mockAutocompletePopup = document.createElement('div');
+  mockAutocompletePopup.className = 'ace_autocomplete';
+  document.body.appendChild(mockAutocompletePopup);
+
+  const originalParent = mockAutocompletePopup.parentElement;
+
+  // Simulate an unrelated command (e.g., 'selectall')
+  editorInstance.commands.exec('selectall', editorInstance, {});
+
+  // Wait a bit to ensure no movement happens
+  await new Promise(resolve => {
+    setTimeout(resolve, 100);
+  });
+
+  // The popup should remain in its original location
+  expect(mockAutocompletePopup.parentElement).toBe(originalParent);
+
+  // Cleanup
+  document.body.removeChild(mockAutocompletePopup);
+});
+
+test('revalidates cached autocomplete popup when detached from DOM', async () => {
+  const ref = createRef<AceEditor>();
+  const { container } = render(<SQLEditor ref={ref as React.Ref<never>} />);
+
+  await waitFor(() => {
+    expect(container.querySelector(selector)).toBeInTheDocument();
+  });
+
+  const editorInstance = ref.current?.editor;
+  expect(editorInstance).toBeDefined();
+
+  if (!editorInstance) return;
+
+  // Create first autocomplete popup
+  const firstPopup = document.createElement('div');
+  firstPopup.className = 'ace_autocomplete';
+  editorInstance.container?.appendChild(firstPopup);
+
+  // Trigger command to cache the first popup
+  editorInstance.commands.exec('insertstring', editorInstance, 'SELECT');
+
+  await waitFor(() => {
+    expect(firstPopup.dataset.aceAutocomplete).toBe('true');
+  });
+
+  // Remove the first popup from DOM (simulating ACE editor replacing it)
+  firstPopup.remove();
+
+  // Create a new autocomplete popup
+  const secondPopup = document.createElement('div');
+  secondPopup.className = 'ace_autocomplete';
+  editorInstance.container?.appendChild(secondPopup);
+
+  // Trigger command again - should find and move the new popup
+  editorInstance.commands.exec('insertstring', editorInstance, ' ');
+
+  await waitFor(() => {
+    expect(secondPopup.dataset.aceAutocomplete).toBe('true');
+    const parentContainer =
+      editorInstance.container?.closest('#ace-editor') ??
+      editorInstance.container?.parentElement;
+    expect(parentContainer?.contains(secondPopup)).toBe(true);
+  });
+});
+
+test('cleans up event listeners on unmount', async () => {
+  const ref = createRef<AceEditor>();
+  const { container, unmount } = render(
+    <SQLEditor ref={ref as React.Ref<never>} />,
+  );
+  await waitFor(() => {
+    expect(container.querySelector(selector)).toBeInTheDocument();
+  });
+
+  const editorInstance = ref.current?.editor;
+  expect(editorInstance).toBeDefined();
+
+  if (!editorInstance) return;
+
+  // Spy on the commands.off method
+  const offSpy = jest.spyOn(editorInstance.commands, 'off');
+
+  // Unmount the component
+  unmount();
+
+  // Verify that the event listener was removed
+  expect(offSpy).toHaveBeenCalledWith('afterExec', expect.any(Function));
+
+  offSpy.mockRestore();
+});
+
+test('does not move autocomplete popup if target container is document.body', async () => {
+  const ref = createRef<AceEditor>();
+  const { container } = render(<SQLEditor ref={ref as React.Ref<never>} />);
+
+  await waitFor(() => {
+    expect(container.querySelector(selector)).toBeInTheDocument();
+  });
+
+  const editorInstance = ref.current?.editor;
+  expect(editorInstance).toBeDefined();
+
+  if (!editorInstance) return;
+
+  // Create a mock autocomplete popup
+  const mockAutocompletePopup = document.createElement('div');
+  mockAutocompletePopup.className = 'ace_autocomplete';
+  document.body.appendChild(mockAutocompletePopup);
+
+  // Mock the closest method to return null (simulating no #ace-editor parent)
+  const originalClosest = editorInstance.container?.closest;
+  if (editorInstance.container) {
+    editorInstance.container.closest = jest.fn(() => null);
+  }
+
+  // Mock parentElement to be document.body
+  Object.defineProperty(editorInstance.container, 'parentElement', {
+    value: document.body,
+    configurable: true,
+  });
+
+  const initialParent = mockAutocompletePopup.parentElement;
+
+  // Trigger command
+  editorInstance.commands.exec('insertstring', editorInstance, 'SELECT');
+
+  await new Promise(resolve => {
+    setTimeout(resolve, 100);
+  });
+
+  // The popup should NOT be moved because target container is document.body
+  expect(mockAutocompletePopup.parentElement).toBe(initialParent);
+
+  // Cleanup
+  if (editorInstance.container && originalClosest) {
+    editorInstance.container.closest = originalClosest;
+  }
+  document.body.removeChild(mockAutocompletePopup);
+});
diff --git a/superset-frontend/packages/superset-ui-core/src/components/AsyncAceEditor/index.tsx b/superset-frontend/packages/superset-ui-core/src/components/AsyncAceEditor/index.tsx
@@ -26,6 +26,7 @@ import type {
 } from 'brace';
 import type AceEditor from 'react-ace';
 import type { IAceEditorProps } from 'react-ace';
+import type { Ace } from 'ace-builds';
 
 import {
   AsyncEsmComponent,
@@ -207,6 +208,68 @@ export function AsyncAceEditor(
           }
         }, [keywords, setCompleters]);
 
+        // Move autocomplete popup to the nearest parent container with data-ace-container
+        useEffect(() => {
+          const editorInstance = (ref as React.RefObject<AceEditor>)?.current
+            ?.editor;
+          if (!editorInstance) return;
+
+          const editorContainer = editorInstance.container;
+          if (!editorContainer) return;
+
+          // Cache DOM elements to avoid repeated queries on every command execution
+          let cachedAutocompletePopup: HTMLElement | null = null;
+          let cachedTargetContainer: Element | null = null;
+
+          const moveAutocompleteToContainer = () => {
+            // Revalidate cached popup if missing or detached from DOM
+            if (
+              !cachedAutocompletePopup ||
+              !document.body.contains(cachedAutocompletePopup)
+            ) {
+              cachedAutocompletePopup =
+                editorContainer.querySelector<HTMLElement>(
+                  '.ace_autocomplete',
+                ) ?? document.querySelector<HTMLElement>('.ace_autocomplete');
+            }
+
+            // Revalidate cached container if missing or detached
+            if (
+              !cachedTargetContainer ||
+              !document.body.contains(cachedTargetContainer)
+            ) {
+              cachedTargetContainer =
+                editorContainer.closest('#ace-editor') ??
+                editorContainer.parentElement;
+            }
+
+            if (
+              cachedAutocompletePopup &&
+              cachedTargetContainer &&
+              cachedTargetContainer !== document.body
+            ) {
+              cachedTargetContainer.appendChild(cachedAutocompletePopup);
+              cachedAutocompletePopup.dataset.aceAutocomplete = 'true';
+            }
+          };
+
+          const handleAfterExec = (e: Ace.Operation) => {
+            const name: string | undefined = e?.command?.name;
+            if (name === 'insertstring' || name === 'startAutocomplete') {
+              moveAutocompleteToContainer();
+            }
+          };
+
+          const { commands } = editorInstance;
+          commands.on('afterExec', handleAfterExec);
+
+          return () => {
+            commands.off('afterExec', handleAfterExec);
+            cachedAutocompletePopup = null;
+            cachedTargetContainer = null;
+          };
+        }, [ref]);
+
         return (
           <>
             <Global
@@ -288,14 +351,24 @@ export function AsyncAceEditor(
                   border: 1px solid ${token.colorBorderSecondary};
                   box-shadow: ${token.boxShadow};
                   border-radius: ${token.borderRadius}px;
+                  padding: ${token.paddingXS}px ${token.paddingXS}px;
+                }
+
+                .ace_tooltip.ace_doc-tooltip {
+                  display: flex !important;
                 }
 
-                & .tooltip-detail {
+                &&& .tooltip-detail {
+                  display: flex;
+                  justify-content: center;
+                  flex-direction: row;
+                  gap: ${token.paddingXXS}px;
+                  align-items: center;
                   background-color: ${token.colorBgContainer};
                   white-space: pre-wrap;
                   word-break: break-all;
-                  min-width: ${token.sizeXXL * 5}px;
                   max-width: ${token.sizeXXL * 10}px;
+                  font-size: ${token.fontSize}px;
 
                   & .tooltip-detail-head {
                     background-color: ${token.colorBgElevated};
@@ -318,7 +391,9 @@ export function AsyncAceEditor(
 
                   & .tooltip-detail-head,
                   & .tooltip-detail-body {
-                    padding: ${token.padding}px ${token.paddingLG}px;
+                    background-color: ${token.colorBgLayout};
+                    padding: 0px ${token.paddingXXS}px;
+                    border: 1px ${token.colorSplit} solid;
                   }
 
                   & .tooltip-detail-footer {
