From 46a89fc37e9dbbf4ea399dfd2672299d78c72c4a Mon Sep 17 00:00:00 2001 From: Enzo Martellucci Date: Tue, 4 Nov 2025 15:51:54 +0100 Subject: [PATCH 01/11] fix(ace-editor-popover): move autocomplete popover to parent elemnt --- .../src/components/AsyncAceEditor/index.tsx | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) 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 bdd9df810468..a7f1a169356d 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 @@ -207,6 +207,44 @@ export function AsyncAceEditor( } }, [keywords, setCompleters]); + // Move autocomplete popup to the nearest parent container with data-ace-container + useEffect(() => { + const editorInstance = (ref as any)?.current?.editor; + if (!editorInstance) return; + + const moveAutocompleteToContainer = () => { + const autocompletePopup = document.querySelector( + '.ace_autocomplete', + ) as HTMLElement; + + if (autocompletePopup) { + const editorContainer = editorInstance.container; + + const targetContainer = editorContainer?.parentElement; + + if (targetContainer && targetContainer !== document.body) { + targetContainer.appendChild(autocompletePopup); + autocompletePopup.setAttribute('data-ace-autocomplete', 'true'); + } + } + }; + + // Hook into Ace's command execution to detect when autocomplete starts + const handleAfterExec = (e: any) => { + if (e.command.name === 'insertstring') { + moveAutocompleteToContainer(); + } + }; + + editorInstance.commands.on('afterExec', handleAfterExec); + + return () => { + if (editorInstance.commands) { + editorInstance.commands.off('afterExec', handleAfterExec); + } + }; + }, [ref]); + return ( <> Date: Tue, 4 Nov 2025 18:01:46 +0100 Subject: [PATCH 02/11] style(ace-editor): adjust color and positioning for tooltip-detail --- .../src/components/AsyncAceEditor/Tooltip.tsx | 2 +- .../src/components/AsyncAceEditor/index.tsx | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/superset-frontend/packages/superset-ui-core/src/components/AsyncAceEditor/Tooltip.tsx b/superset-frontend/packages/superset-ui-core/src/components/AsyncAceEditor/Tooltip.tsx index ef8158c98994..671bd35d506b 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/AsyncAceEditor/Tooltip.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/AsyncAceEditor/Tooltip.tsx @@ -28,7 +28,7 @@ export function getTooltipHTML({ title, body, footer }: Props): string { const html = `
${title ? `
${title}
` : ''} - ${body ? `
${body}
` : ''} + ${body ? `
${body}
` : ''} ${footer ? `
${footer}
` : ''}
`; 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 a7f1a169356d..02380f41e090 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 @@ -329,6 +329,9 @@ export function AsyncAceEditor( } & .tooltip-detail { + display: flex; + flex-direction: row; + align-items: center; background-color: ${token.colorBgContainer}; white-space: pre-wrap; word-break: break-all; @@ -352,6 +355,11 @@ export function AsyncAceEditor( & .tooltip-detail-body { word-break: break-word; color: ${token.colorTextSecondary}; + + & .tooltip-detail-body-element { + background-color: ${token.colorBgLayout}; + padding: ${token.paddingXS}px; + } } & .tooltip-detail-head, From 018095d042e851130c451a1d50df89a1a72583ac Mon Sep 17 00:00:00 2001 From: Enzo Martellucci Date: Wed, 5 Nov 2025 12:54:46 +0100 Subject: [PATCH 03/11] chore(AsyncAceEditor): improve consistency and add better types --- .../src/components/AsyncAceEditor/index.tsx | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) 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 02380f41e090..ee38a3aa0e3d 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, @@ -209,19 +210,19 @@ export function AsyncAceEditor( // Move autocomplete popup to the nearest parent container with data-ace-container useEffect(() => { - const editorInstance = (ref as any)?.current?.editor; + const editorInstance = (ref as React.RefObject)?.current + ?.editor; if (!editorInstance) return; const moveAutocompleteToContainer = () => { - const autocompletePopup = document.querySelector( - '.ace_autocomplete', - ) as HTMLElement; - + const editorContainer = editorInstance.container; + const autocompletePopup = + (editorContainer?.querySelector( + '.ace_autocomplete', + ) as HTMLElement) || + (document.querySelector('.ace_autocomplete') as HTMLElement); if (autocompletePopup) { - const editorContainer = editorInstance.container; - - const targetContainer = editorContainer?.parentElement; - + const targetContainer = editorContainer?.closest('#ace-editor') || editorContainer?.parentElement; if (targetContainer && targetContainer !== document.body) { targetContainer.appendChild(autocompletePopup); autocompletePopup.setAttribute('data-ace-autocomplete', 'true'); @@ -230,8 +231,11 @@ export function AsyncAceEditor( }; // Hook into Ace's command execution to detect when autocomplete starts - const handleAfterExec = (e: any) => { - if (e.command.name === 'insertstring') { + const handleAfterExec = (e: Ace.Operation) => { + if ( + e.command.name && + ['insertstring', 'startAutocomplete'].includes(e.command.name) + ) { moveAutocompleteToContainer(); } }; From c0e2f48b4c945f5a224baf499edcb0a081c86b5b Mon Sep 17 00:00:00 2001 From: Enzo Martellucci Date: Wed, 5 Nov 2025 13:18:55 +0100 Subject: [PATCH 04/11] lint --- .../superset-ui-core/src/components/AsyncAceEditor/index.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 ee38a3aa0e3d..289a228bc8c8 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 @@ -222,7 +222,9 @@ export function AsyncAceEditor( ) as HTMLElement) || (document.querySelector('.ace_autocomplete') as HTMLElement); if (autocompletePopup) { - const targetContainer = editorContainer?.closest('#ace-editor') || editorContainer?.parentElement; + const targetContainer = + editorContainer?.closest('#ace-editor') || + editorContainer?.parentElement; if (targetContainer && targetContainer !== document.body) { targetContainer.appendChild(autocompletePopup); autocompletePopup.setAttribute('data-ace-autocomplete', 'true'); From c93bb487bbe6a0d76e0fc10d378d2585aba32e5e Mon Sep 17 00:00:00 2001 From: Enzo Martellucci Date: Mon, 10 Nov 2025 17:54:59 +0100 Subject: [PATCH 05/11] style(ace-tooltip): enhance spacing and colors --- .../src/components/AsyncAceEditor/Tooltip.tsx | 2 +- .../src/components/AsyncAceEditor/index.tsx | 20 +++++++++++-------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/superset-frontend/packages/superset-ui-core/src/components/AsyncAceEditor/Tooltip.tsx b/superset-frontend/packages/superset-ui-core/src/components/AsyncAceEditor/Tooltip.tsx index 671bd35d506b..ef8158c98994 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/AsyncAceEditor/Tooltip.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/AsyncAceEditor/Tooltip.tsx @@ -28,7 +28,7 @@ export function getTooltipHTML({ title, body, footer }: Props): string { const html = `
${title ? `
${title}
` : ''} - ${body ? `
${body}
` : ''} + ${body ? `
${body}
` : ''} ${footer ? `
${footer}
` : ''}
`; 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 e69135077acb..7cece8562d96 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 @@ -332,17 +332,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; } - & .tooltip-detail { + .ace_tooltip.ace_doc-tooltip { + display: flex !important; + } + + &&& .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}; @@ -361,16 +368,13 @@ export function AsyncAceEditor( & .tooltip-detail-body { word-break: break-word; color: ${token.colorTextSecondary}; - - & .tooltip-detail-body-element { - background-color: ${token.colorBgLayout}; - padding: ${token.paddingXS}px; - } } & .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 { From 773f6fe9184600bf36dd205390f34e3cbaefdc55 Mon Sep 17 00:00:00 2001 From: Enzo Martellucci Date: Mon, 10 Nov 2025 18:40:01 +0100 Subject: [PATCH 06/11] lint --- docker/pythonpath_dev/superset_config_docker_light.py | 1 + 1 file changed, 1 insertion(+) diff --git a/docker/pythonpath_dev/superset_config_docker_light.py b/docker/pythonpath_dev/superset_config_docker_light.py index 9a5ae0ae67ab..1f053c2ce363 100644 --- a/docker/pythonpath_dev/superset_config_docker_light.py +++ b/docker/pythonpath_dev/superset_config_docker_light.py @@ -19,6 +19,7 @@ # Import all settings from the main config first from flask_caching.backends.filesystemcache import FileSystemCache + from superset_config import * # noqa: F403 # Override caching to use simple in-memory cache instead of Redis From 0975d5d667ff721a90edd08e33560db842ce152e Mon Sep 17 00:00:00 2001 From: Enzo Martellucci Date: Mon, 10 Nov 2025 21:41:27 +0100 Subject: [PATCH 07/11] perf(ace-autocomplete): cache DOM elements to reduce repeated queries during command execution --- .../src/components/AsyncAceEditor/index.tsx | 67 ++++++++++++------- 1 file changed, 42 insertions(+), 25 deletions(-) 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 7cece8562d96..1e542aa522e3 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 @@ -210,44 +210,61 @@ export function AsyncAceEditor( // Move autocomplete popup to the nearest parent container with data-ace-container useEffect(() => { - const editorInstance = (ref as React.RefObject)?.current - ?.editor; + const editorInstance = (ref as React.RefObject)?.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 = () => { - const editorContainer = editorInstance.container; - const autocompletePopup = - (editorContainer?.querySelector( - '.ace_autocomplete', - ) as HTMLElement) || - (document.querySelector('.ace_autocomplete') as HTMLElement); - if (autocompletePopup) { - const targetContainer = - editorContainer?.closest('#ace-editor') || - editorContainer?.parentElement; - if (targetContainer && targetContainer !== document.body) { - targetContainer.appendChild(autocompletePopup); - autocompletePopup.setAttribute('data-ace-autocomplete', 'true'); - } + // Revalidate cached popup if missing or detached from DOM + if ( + !cachedAutocompletePopup || + !document.body.contains(cachedAutocompletePopup) + ) { + cachedAutocompletePopup = + editorContainer.querySelector('.ace_autocomplete') ?? + document.querySelector('.ace_autocomplete'); } - }; - // Hook into Ace's command execution to detect when autocomplete starts - const handleAfterExec = (e: Ace.Operation) => { + // Revalidate cached container if missing or detached if ( - e.command.name && - ['insertstring', 'startAutocomplete'].includes(e.command.name) + !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 = e?.command?.name; + if (name === 'insertstring' || name === 'startAutocomplete') { moveAutocompleteToContainer(); } }; - editorInstance.commands.on('afterExec', handleAfterExec); + const { commands } = editorInstance; + commands.on('afterExec', handleAfterExec); return () => { - if (editorInstance.commands) { - editorInstance.commands.off('afterExec', handleAfterExec); - } + commands.off('afterExec', handleAfterExec); + cachedAutocompletePopup = null; + cachedTargetContainer = null; }; }, [ref]); From cf93c377b25c81ff8cbffe0b223e7fedcb7dddf0 Mon Sep 17 00:00:00 2001 From: Enzo Martellucci Date: Mon, 10 Nov 2025 22:39:06 +0100 Subject: [PATCH 08/11] lint --- .../src/components/AsyncAceEditor/index.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) 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 1e542aa522e3..764db6fd301f 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 @@ -210,7 +210,8 @@ export function AsyncAceEditor( // Move autocomplete popup to the nearest parent container with data-ace-container useEffect(() => { - const editorInstance = (ref as React.RefObject)?.current?.editor; + const editorInstance = (ref as React.RefObject)?.current + ?.editor; if (!editorInstance) return; const editorContainer = editorInstance.container; @@ -227,8 +228,9 @@ export function AsyncAceEditor( !document.body.contains(cachedAutocompletePopup) ) { cachedAutocompletePopup = - editorContainer.querySelector('.ace_autocomplete') ?? - document.querySelector('.ace_autocomplete'); + editorContainer.querySelector( + '.ace_autocomplete', + ) ?? document.querySelector('.ace_autocomplete'); } // Revalidate cached container if missing or detached From 5fae824782ff638b421f5e080174e20a3e177656 Mon Sep 17 00:00:00 2001 From: Enzo Martellucci Date: Fri, 14 Nov 2025 10:49:38 +0100 Subject: [PATCH 09/11] lint --- docker/pythonpath_dev/superset_config_docker_light.py | 1 - .../superset-ui-core/src/components/AsyncAceEditor/index.tsx | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/docker/pythonpath_dev/superset_config_docker_light.py b/docker/pythonpath_dev/superset_config_docker_light.py index 1f053c2ce363..9a5ae0ae67ab 100644 --- a/docker/pythonpath_dev/superset_config_docker_light.py +++ b/docker/pythonpath_dev/superset_config_docker_light.py @@ -19,7 +19,6 @@ # Import all settings from the main config first from flask_caching.backends.filesystemcache import FileSystemCache - from superset_config import * # noqa: F403 # Override caching to use simple in-memory cache instead of Redis 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 764db6fd301f..34609a2a1c46 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 @@ -254,7 +254,7 @@ export function AsyncAceEditor( }; const handleAfterExec = (e: Ace.Operation) => { - const name = e?.command?.name; + const name: string | undefined = e?.command?.name; if (name === 'insertstring' || name === 'startAutocomplete') { moveAutocompleteToContainer(); } From 1c1e4ef538a7c20ea8c0b8a5d8592258b30d3655 Mon Sep 17 00:00:00 2001 From: Enzo Martellucci Date: Fri, 14 Nov 2025 12:42:38 +0100 Subject: [PATCH 10/11] chore: improve AsyncAceEditor tests --- .../AsyncAceEditor/AsyncAceEditor.test.tsx | 251 ++++++++++++++++++ 1 file changed, 251 insertions(+) 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 151b1db1a930..7e28dd2c71b9 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,252 @@ test('renders a custom placeholder', () => { expect(screen.getByRole('paragraph')).toBeInTheDocument(); }); + +test('registers afterExec event listener for command handling', async () => { + const ref = createRef(); + const { container } = render(} />); + + 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(); + const { container } = render(} />); + + 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(); + const { container } = render(} />); + + 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(); + const { container } = render(} />); + + 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(); + const { container } = render(} />); + + 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(); + const { container, unmount } = render(} />); + + 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(); + const { container } = render(} />); + + 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); +}); From 32ff3a3757e2b1a8666b17e99503c68bdeae9287 Mon Sep 17 00:00:00 2001 From: Enzo Martellucci Date: Fri, 14 Nov 2025 16:06:15 +0100 Subject: [PATCH 11/11] lint --- .../src/components/AsyncAceEditor/AsyncAceEditor.test.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 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 7e28dd2c71b9..51c78d0b9cf3 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 @@ -279,8 +279,9 @@ test('revalidates cached autocomplete popup when detached from DOM', async () => test('cleans up event listeners on unmount', async () => { const ref = createRef(); - const { container, unmount } = render(} />); - + const { container, unmount } = render( + } />, + ); await waitFor(() => { expect(container.querySelector(selector)).toBeInTheDocument(); });