This is an automated email from the ASF dual-hosted git repository.
enzomartellucci pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 9ef87e75d5 fix(ace-editor-popover): main AntD popover closes when
clicking autocomplete suggestions in Ace Editor (#35986)
9ef87e75d5 is described below
commit 9ef87e75d506cb02f1d24a62daac8dfb20073193
Author: Enzo Martellucci <[email protected]>
AuthorDate: Fri Nov 14 16:35:15 2025 +0100
fix(ace-editor-popover): main AntD popover closes when clicking
autocomplete suggestions in Ace Editor (#35986)
---
.../AsyncAceEditor/AsyncAceEditor.test.tsx | 252 +++++++++++++++++++++
.../src/components/AsyncAceEditor/index.tsx | 81 ++++++-
2 files changed, 330 insertions(+), 3 deletions(-)
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
index 151b1db1a9..51c78d0b9c 100644
---
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
index 7609ab7527..34609a2a1c 100644
---
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 {
