fix(bug): Error when adding a filter using custom sql (#38246)

This commit is contained in:
Alexandru Soare
2026-03-05 11:39:21 +02:00
committed by GitHub
parent 4dfb0e66cb
commit 880cab58c3
5 changed files with 99 additions and 30 deletions

View File

@@ -480,6 +480,18 @@ export interface EditorHandle {
* @returns A Disposable that removes the provider when disposed
*/
registerCompletionProvider(provider: CompletionProvider): Disposable;
/**
* Force the editor to recalculate its dimensions.
* Called when the container size changes or when the editor becomes
* visible after being hidden (e.g., in a tab).
*
* Each editor implementation maps this to their equivalent:
* - Ace: editor.resize()
* - Monaco: editor.layout()
* - CodeMirror: editor.requestMeasure()
*/
resize(): void;
}
/**

View File

@@ -182,6 +182,10 @@ const createAceEditorHandle = (
completionProviders.current.delete(provider.id);
});
},
resize: () => {
aceEditorRef.current?.editor?.resize();
},
});
/**

View File

@@ -22,30 +22,52 @@ import {
screen,
selectOption,
userEvent,
waitFor,
} from 'spec/helpers/testing-library';
import AdhocFilter from '../AdhocFilter';
import { Clauses, ExpressionTypes } from '../types';
import AdhocFilterEditPopoverSqlTabContent from '.';
// Track resize calls for testing
const mockResize = jest.fn();
// Mock EditorHost with ref support for resize
jest.mock('src/core/editors', () => {
const React = require('react');
return {
EditorHost: React.forwardRef(
(
{
value,
onChange,
}: {
value: string;
onChange: (v: string) => void;
},
ref: React.Ref<{ resize: () => void }>,
) => {
React.useImperativeHandle(ref, () => ({
resize: mockResize,
}));
return (
<textarea
defaultValue={value}
onChange={e => onChange?.(e.target.value)}
/>
);
},
),
};
});
// Add cleanup after each test
afterEach(async () => {
cleanup();
mockResize.mockClear();
// Wait for any pending effects to complete
await new Promise(resolve => setTimeout(resolve, 0));
});
jest.mock('src/core/editors', () => ({
EditorHost: ({
value,
onChange,
}: {
value: string;
onChange: (v: string) => void;
}) => (
<textarea defaultValue={value} onChange={e => onChange?.(e.target.value)} />
),
}));
const adhocFilter = new AdhocFilter({
expressionType: ExpressionTypes.Sql,
sqlExpression: 'value > 10',
@@ -89,3 +111,41 @@ test('calls onChange when the SQL expression changes', async () => {
expect.objectContaining({ sqlExpression: input }),
);
});
test('calls editor resize when adhocFilter changes', async () => {
const onChange = jest.fn();
const { rerender } = render(
<AdhocFilterEditPopoverSqlTabContent
adhocFilter={adhocFilter}
onChange={onChange}
options={[]}
height={100}
/>,
);
// Initial render should call resize
await waitFor(() => {
expect(mockResize).toHaveBeenCalled();
});
mockResize.mockClear();
// Create a new filter to trigger the useEffect
const newFilter = new AdhocFilter({
expressionType: ExpressionTypes.Sql,
sqlExpression: 'value > 20',
clause: Clauses.Where,
});
rerender(
<AdhocFilterEditPopoverSqlTabContent
adhocFilter={newFilter}
onChange={onChange}
options={[]}
height={100}
/>,
);
await waitFor(() => {
expect(mockResize).toHaveBeenCalled();
});
});

View File

@@ -17,6 +17,7 @@
* under the License.
*/
import { useEffect, useRef, useMemo } from 'react';
import type { editors } from '@apache-superset/core';
import { Select } from '@superset-ui/core/components';
import { t } from '@apache-superset/core';
import { css, styled, useTheme } from '@apache-superset/core/ui';
@@ -49,12 +50,11 @@ export default function AdhocFilterEditPopoverSqlTabContent({
height: number;
datasource?: any;
}) {
const aceEditorRef = useRef(null);
const editorRef = useRef<editors.EditorHandle>(null);
const theme = useTheme();
useEffect(() => {
// @ts-expect-error - AceEditor ref type doesn't expose editor.resize()
aceEditorRef?.current?.editor.resize();
editorRef.current?.resize();
}, [adhocFilter]);
const onSqlExpressionClauseChange = (clause: string) => {
@@ -125,7 +125,7 @@ export default function AdhocFilterEditPopoverSqlTabContent({
`}
>
<SQLEditorWithValidation
ref={aceEditorRef}
ref={editorRef}
keywords={keywords}
height={`${height - 130}px`}
onChange={onSqlExpressionChange}

View File

@@ -130,7 +130,7 @@ export default class AdhocMetricEditPopover extends PureComponent<
// "Saved" is a default tab unless there are no saved metrics for dataset
defaultActiveTabKey = this.getDefaultTab();
aceEditorRef: RefObject<editors.EditorHandle>;
editorRef: RefObject<editors.EditorHandle>;
dragStartX = 0;
@@ -152,8 +152,8 @@ export default class AdhocMetricEditPopover extends PureComponent<
this.onMouseMove = this.onMouseMove.bind(this);
this.onMouseUp = this.onMouseUp.bind(this);
this.onTabChange = this.onTabChange.bind(this);
this.aceEditorRef = createRef();
this.refreshAceEditor = this.refreshAceEditor.bind(this);
this.editorRef = createRef();
this.refreshEditor = this.refreshEditor.bind(this);
this.getDefaultTab = this.getDefaultTab.bind(this);
this.state = {
@@ -313,20 +313,13 @@ export default class AdhocMetricEditPopover extends PureComponent<
}
onTabChange(tab: string): void {
this.refreshAceEditor();
this.refreshEditor();
this.props.getCurrentTab?.(tab);
}
refreshAceEditor(): void {
refreshEditor(): void {
setTimeout(() => {
if (this.aceEditorRef.current) {
// Cast to access ace editor API
(
this.aceEditorRef.current as unknown as {
editor?: { resize?: () => void };
}
).editor?.resize?.();
}
this.editorRef.current?.resize();
}, 0);
}
@@ -549,7 +542,7 @@ export default class AdhocMetricEditPopover extends PureComponent<
children: (
<SQLEditorWithValidation
data-test="sql-editor"
ref={this.aceEditorRef}
ref={this.editorRef}
keywords={keywords}
height={`${this.state.height - 120}px`}
onChange={this.onSqlExpressionChange}