From bac6cc463376f014fb8546f69b2b2f45055704f0 Mon Sep 17 00:00:00 2001 From: Joshua Li Date: Thu, 20 Jun 2024 20:27:16 +0000 Subject: [PATCH 1/8] Fix object empty check in query editor extensions Signed-off-by: Joshua Li --- .../query_editor_extensions.tsx | 2 +- .../ui/query_editor/query_editor_top_row.tsx | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/plugins/data/public/ui/query_editor/query_editor_extensions/query_editor_extensions.tsx b/src/plugins/data/public/ui/query_editor/query_editor_extensions/query_editor_extensions.tsx index 6b2d5011216..45b0d076877 100644 --- a/src/plugins/data/public/ui/query_editor/query_editor_extensions/query_editor_extensions.tsx +++ b/src/plugins/data/public/ui/query_editor/query_editor_extensions/query_editor_extensions.tsx @@ -20,7 +20,7 @@ const QueryEditorExtensions: React.FC = React.memo(( const { configMap, componentContainer, bannerContainer, ...dependencies } = props; const sortedConfigs = useMemo(() => { - if (!configMap || !Object.keys(configMap)) return []; + if (!configMap || Object.keys(configMap).length === 0) return []; return Object.values(configMap).sort((a, b) => a.order - b.order); }, [configMap]); diff --git a/src/plugins/data/public/ui/query_editor/query_editor_top_row.tsx b/src/plugins/data/public/ui/query_editor/query_editor_top_row.tsx index 713bd3345e9..f86d114dc1d 100644 --- a/src/plugins/data/public/ui/query_editor/query_editor_top_row.tsx +++ b/src/plugins/data/public/ui/query_editor/query_editor_top_row.tsx @@ -29,10 +29,10 @@ import { } from '../../../../opensearch_dashboards_react/public'; import { UI_SETTINGS } from '../../../common'; import { fromUser, getQueryLog, PersistedLog } from '../../query'; -import { QueryEditorExtensions } from './query_editor_extensions'; import { Settings } from '../types'; import { NoDataPopover } from './no_data_popover'; import QueryEditorUI from './query_editor'; +import { QueryEditorExtensions } from './query_editor_extensions'; const QueryEditor = withOpenSearchDashboards(QueryEditorUI); @@ -255,10 +255,12 @@ export default function QueryEditorTopRow(props: QueryEditorTopRowProps) { function renderQueryEditorExtensions() { if ( - !shouldRenderQueryEditorExtensions() || - !queryEditorHeaderRef.current || - !queryEditorBannerRef.current || - !queryLanguage + !( + queryEditorHeaderRef.current && + queryEditorBannerRef.current && + queryLanguage && + shouldRenderQueryEditorExtensions() + ) ) return; return ( @@ -305,7 +307,7 @@ export default function QueryEditorTopRow(props: QueryEditorTopRowProps) { } function shouldRenderQueryEditorExtensions(): boolean { - return Boolean(queryEditorExtensionMap && Object.keys(queryEditorExtensionMap).length); + return Boolean(queryEditorExtensionMap && Object.keys(queryEditorExtensionMap).length > 0); } function renderUpdateButton() { From f02e725a101d4caa9c2a3ceb01101dbfc7f34094 Mon Sep 17 00:00:00 2001 From: Joshua Li Date: Fri, 21 Jun 2024 19:37:20 +0000 Subject: [PATCH 2/8] fix render order QueryEditorExtensions requires the container divs to be mounted first, but in the previous implementation, extensions will be mounted first and it relied on rerendering of queryEditorTopRow. Moving them into query editor fixes the render order and ensures refs are set. Signed-off-by: Joshua Li --- .../public/ui/query_editor/query_editor.tsx | 55 +++++++++++++------ .../ui/query_editor/query_editor_top_row.tsx | 35 +----------- 2 files changed, 40 insertions(+), 50 deletions(-) diff --git a/src/plugins/data/public/ui/query_editor/query_editor.tsx b/src/plugins/data/public/ui/query_editor/query_editor.tsx index 2688ae6a830..54048c34525 100644 --- a/src/plugins/data/public/ui/query_editor/query_editor.tsx +++ b/src/plugins/data/public/ui/query_editor/query_editor.tsx @@ -8,7 +8,7 @@ import classNames from 'classnames'; import { isEqual } from 'lodash'; import React, { Component, createRef, RefObject } from 'react'; import { Settings } from '..'; -import { IDataPluginServices, IIndexPattern, Query, TimeRange } from '../..'; +import { DataSource, IDataPluginServices, IIndexPattern, Query, TimeRange } from '../..'; import { CodeEditor, OpenSearchDashboardsReactContextValue, @@ -19,9 +19,11 @@ import { SuggestionsListSize } from '../typeahead/suggestions_component'; import { DataSettings, QueryEnhancement } from '../types'; import { fetchIndexPatterns } from './fetch_index_patterns'; import { QueryLanguageSelector } from './language_selector'; +import { QueryEditorExtensions } from './query_editor_extensions'; export interface QueryEditorProps { indexPatterns: Array; + dataSource?: DataSource; query: Query; containerRef?: React.RefCallback; settings: Settings; @@ -41,10 +43,9 @@ export interface QueryEditorProps { size?: SuggestionsListSize; className?: string; isInvalid?: boolean; - queryEditorHeaderRef: React.RefObject; - queryEditorHeaderClassName?: string; - queryEditorBannerRef: React.RefObject; - queryEditorBannerClassName?: string; + queryLanguage?: string; + headerClassName?: string; + bannerClassName?: string; } interface Props extends QueryEditorProps { @@ -92,6 +93,9 @@ export default class QueryEditorUI extends Component { private services = this.props.opensearchDashboards.services; private componentIsUnmounting = false; private queryEditorDivRefInstance: RefObject = createRef(); + private headerRef: RefObject = createRef(); + private bannerRef: RefObject = createRef(); + private extensionMap = this.props.settings?.getQueryEditorExtensionMap(); private getQueryString = () => { if (!this.props.query.query) { @@ -120,6 +124,30 @@ export default class QueryEditorUI extends Component { }); }; + private renderQueryEditorExtensions() { + if ( + !( + this.headerRef.current && + this.bannerRef.current && + this.props.queryLanguage && + this.extensionMap && + Object.keys(this.extensionMap).length > 0 + ) + ) { + return null; + } + return ( + + ); + } + private onSubmit = (query: Query, dateRange?: TimeRange) => { if (this.props.onSubmit) { if (this.persistedLog) { @@ -249,20 +277,12 @@ export default class QueryEditorUI extends Component { public render() { const className = classNames(this.props.className); - - const queryEditorHeaderClassName = classNames( - 'osdQueryEditorHeader', - this.props.queryEditorHeaderClassName - ); - - const queryEditorBannerClassName = classNames( - 'osdQueryEditorBanner', - this.props.queryEditorBannerClassName - ); + const headerClassName = classNames('osdQueryEditorHeader', this.props.headerClassName); + const bannerClassName = classNames('osdQueryEditorBanner', this.props.bannerClassName); return (
-
+
@@ -285,7 +305,7 @@ export default class QueryEditorUI extends Component { -
+
{ /> + {this.renderQueryEditorExtensions()}
); } diff --git a/src/plugins/data/public/ui/query_editor/query_editor_top_row.tsx b/src/plugins/data/public/ui/query_editor/query_editor_top_row.tsx index f86d114dc1d..87f74ac8c8e 100644 --- a/src/plugins/data/public/ui/query_editor/query_editor_top_row.tsx +++ b/src/plugins/data/public/ui/query_editor/query_editor_top_row.tsx @@ -32,7 +32,6 @@ import { fromUser, getQueryLog, PersistedLog } from '../../query'; import { Settings } from '../types'; import { NoDataPopover } from './no_data_popover'; import QueryEditorUI from './query_editor'; -import { QueryEditorExtensions } from './query_editor_extensions'; const QueryEditor = withOpenSearchDashboards(QueryEditorUI); @@ -71,8 +70,6 @@ export interface QueryEditorTopRowProps { export default function QueryEditorTopRow(props: QueryEditorTopRowProps) { const [isDateRangeInvalid, setIsDateRangeInvalid] = useState(false); const [isQueryEditorFocused, setIsQueryEditorFocused] = useState(false); - const queryEditorHeaderRef = useRef(null); - const queryEditorBannerRef = useRef(null); const opensearchDashboards = useOpenSearchDashboards(); const { uiSettings, storage, appName } = opensearchDashboards.services; @@ -85,7 +82,6 @@ export default function QueryEditorTopRow(props: QueryEditorTopRowProps) { props.settings && props.settings.getQueryEnhancements(queryLanguage)?.searchBar) || null; - const queryEditorExtensionMap = props.settings?.getQueryEditorExtensionMap(); const parsedQuery = !queryUiEnhancement || isValidQuery(props.query) ? props.query! @@ -235,6 +231,7 @@ export default function QueryEditorTopRow(props: QueryEditorTopRowProps) { ); } - function renderQueryEditorExtensions() { - if ( - !( - queryEditorHeaderRef.current && - queryEditorBannerRef.current && - queryLanguage && - shouldRenderQueryEditorExtensions() - ) - ) - return; - return ( - - ); - } - function renderSharingMetaFields() { const { from, to } = getDateRange(); const dateRangePretty = prettyDuration( @@ -306,10 +280,6 @@ export default function QueryEditorTopRow(props: QueryEditorTopRowProps) { ); } - function shouldRenderQueryEditorExtensions(): boolean { - return Boolean(queryEditorExtensionMap && Object.keys(queryEditorExtensionMap).length > 0); - } - function renderUpdateButton() { const button = props.customSubmitButton ? ( React.cloneElement(props.customSubmitButton, { onClick: onClickSubmitButton }) @@ -402,7 +372,6 @@ export default function QueryEditorTopRow(props: QueryEditorTopRowProps) { direction="column" justifyContent="flexEnd" > - {renderQueryEditorExtensions()} {renderQueryEditor()} From fc5369e9e7ffa5f4f1d24ac19f85e236aff6425a Mon Sep 17 00:00:00 2001 From: Joshua Li Date: Mon, 24 Jun 2024 16:59:55 +0000 Subject: [PATCH 3/8] add changelog Signed-off-by: Joshua Li --- changelogs/fragments/7077.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/7077.yml diff --git a/changelogs/fragments/7077.yml b/changelogs/fragments/7077.yml new file mode 100644 index 00000000000..43f1052a0e1 --- /dev/null +++ b/changelogs/fragments/7077.yml @@ -0,0 +1,2 @@ +fix: +- Fix object empty check and minor perf issue in query editor extensions ([#7077](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7077)) From 5b3e3589cc3ab162b8d9c19850f9a59644dfcdef Mon Sep 17 00:00:00 2001 From: Joshua Li Date: Mon, 24 Jun 2024 17:02:57 +0000 Subject: [PATCH 4/8] remove unused handleListUpdate Signed-off-by: Joshua Li --- .../public/ui/query_editor/query_editor.tsx | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/src/plugins/data/public/ui/query_editor/query_editor.tsx b/src/plugins/data/public/ui/query_editor/query_editor.tsx index 54048c34525..f72daa0ba90 100644 --- a/src/plugins/data/public/ui/query_editor/query_editor.tsx +++ b/src/plugins/data/public/ui/query_editor/query_editor.tsx @@ -58,7 +58,6 @@ interface State { index: number | null; suggestions: QuerySuggestion[]; indexPatterns: IIndexPattern[]; - queryEditorRect: DOMRect | undefined; } const KEY_CODES = { @@ -83,7 +82,6 @@ export default class QueryEditorUI extends Component { index: null, suggestions: [], indexPatterns: [], - queryEditorRect: undefined, }; public inputRef: HTMLTextAreaElement | null = null; @@ -91,8 +89,6 @@ export default class QueryEditorUI extends Component { private persistedLog: PersistedLog | undefined; private abortController?: AbortController; private services = this.props.opensearchDashboards.services; - private componentIsUnmounting = false; - private queryEditorDivRefInstance: RefObject = createRef(); private headerRef: RefObject = createRef(); private bannerRef: RefObject = createRef(); private extensionMap = this.props.settings?.getQueryEditorExtensionMap(); @@ -238,12 +234,6 @@ export default class QueryEditorUI extends Component { this.initPersistedLog(); // this.fetchIndexPatterns().then(this.updateSuggestions); - this.handleListUpdate(); - - window.addEventListener('scroll', this.handleListUpdate, { - passive: true, // for better performance as we won't call preventDefault - capture: true, // scroll events don't bubble, they must be captured instead - }); } public componentDidUpdate(prevProps: Props) { @@ -257,18 +247,8 @@ export default class QueryEditorUI extends Component { public componentWillUnmount() { if (this.abortController) this.abortController.abort(); - this.componentIsUnmounting = true; - window.removeEventListener('scroll', this.handleListUpdate, { capture: true }); } - handleListUpdate = () => { - if (this.componentIsUnmounting) return; - - return this.setState({ - queryEditorRect: this.queryEditorDivRefInstance.current?.getBoundingClientRect(), - }); - }; - handleOnFocus = () => { if (this.props.onChangeQueryEditorFocus) { this.props.onChangeQueryEditorFocus(true); From 2cb217e7c0c579308392faa32b524e5ee86ea60e Mon Sep 17 00:00:00 2001 From: Joshua Li Date: Thu, 27 Jun 2024 16:55:45 +0000 Subject: [PATCH 5/8] Revert "remove unused handleListUpdate" This reverts commit 5b3e3589cc3ab162b8d9c19850f9a59644dfcdef. --- .../public/ui/query_editor/query_editor.tsx | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/plugins/data/public/ui/query_editor/query_editor.tsx b/src/plugins/data/public/ui/query_editor/query_editor.tsx index f72daa0ba90..54048c34525 100644 --- a/src/plugins/data/public/ui/query_editor/query_editor.tsx +++ b/src/plugins/data/public/ui/query_editor/query_editor.tsx @@ -58,6 +58,7 @@ interface State { index: number | null; suggestions: QuerySuggestion[]; indexPatterns: IIndexPattern[]; + queryEditorRect: DOMRect | undefined; } const KEY_CODES = { @@ -82,6 +83,7 @@ export default class QueryEditorUI extends Component { index: null, suggestions: [], indexPatterns: [], + queryEditorRect: undefined, }; public inputRef: HTMLTextAreaElement | null = null; @@ -89,6 +91,8 @@ export default class QueryEditorUI extends Component { private persistedLog: PersistedLog | undefined; private abortController?: AbortController; private services = this.props.opensearchDashboards.services; + private componentIsUnmounting = false; + private queryEditorDivRefInstance: RefObject = createRef(); private headerRef: RefObject = createRef(); private bannerRef: RefObject = createRef(); private extensionMap = this.props.settings?.getQueryEditorExtensionMap(); @@ -234,6 +238,12 @@ export default class QueryEditorUI extends Component { this.initPersistedLog(); // this.fetchIndexPatterns().then(this.updateSuggestions); + this.handleListUpdate(); + + window.addEventListener('scroll', this.handleListUpdate, { + passive: true, // for better performance as we won't call preventDefault + capture: true, // scroll events don't bubble, they must be captured instead + }); } public componentDidUpdate(prevProps: Props) { @@ -247,8 +257,18 @@ export default class QueryEditorUI extends Component { public componentWillUnmount() { if (this.abortController) this.abortController.abort(); + this.componentIsUnmounting = true; + window.removeEventListener('scroll', this.handleListUpdate, { capture: true }); } + handleListUpdate = () => { + if (this.componentIsUnmounting) return; + + return this.setState({ + queryEditorRect: this.queryEditorDivRefInstance.current?.getBoundingClientRect(), + }); + }; + handleOnFocus = () => { if (this.props.onChangeQueryEditorFocus) { this.props.onChangeQueryEditorFocus(true); From 63919865a7f238440345728ea87e4d9b6bc197fd Mon Sep 17 00:00:00 2001 From: Joshua Li Date: Thu, 27 Jun 2024 16:57:21 +0000 Subject: [PATCH 6/8] remove unused handleListUpdate Signed-off-by: Joshua Li --- .../public/ui/query_editor/query_editor.tsx | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/plugins/data/public/ui/query_editor/query_editor.tsx b/src/plugins/data/public/ui/query_editor/query_editor.tsx index f7297db89fc..0fe6aad3f8b 100644 --- a/src/plugins/data/public/ui/query_editor/query_editor.tsx +++ b/src/plugins/data/public/ui/query_editor/query_editor.tsx @@ -58,7 +58,6 @@ interface State { index: number | null; suggestions: QuerySuggestion[]; indexPatterns: IIndexPattern[]; - queryEditorRect: DOMRect | undefined; } const KEY_CODES = { @@ -83,7 +82,6 @@ export default class QueryEditorUI extends Component { index: null, suggestions: [], indexPatterns: [], - queryEditorRect: undefined, }; public inputRef: HTMLElement | null = null; @@ -92,7 +90,6 @@ export default class QueryEditorUI extends Component { private abortController?: AbortController; private services = this.props.opensearchDashboards.services; private componentIsUnmounting = false; - private queryEditorDivRefInstance: RefObject = createRef(); private headerRef: RefObject = createRef(); private bannerRef: RefObject = createRef(); private extensionMap = this.props.settings?.getQueryEditorExtensionMap(); @@ -249,12 +246,6 @@ export default class QueryEditorUI extends Component { this.initPersistedLog(); // this.fetchIndexPatterns().then(this.updateSuggestions); this.initDataSourcesVisibility(); - this.handleListUpdate(); - - window.addEventListener('scroll', this.handleListUpdate, { - passive: true, // for better performance as we won't call preventDefault - capture: true, // scroll events don't bubble, they must be captured instead - }); } public componentDidUpdate(prevProps: Props) { @@ -269,17 +260,8 @@ export default class QueryEditorUI extends Component { public componentWillUnmount() { if (this.abortController) this.abortController.abort(); this.componentIsUnmounting = true; - window.removeEventListener('scroll', this.handleListUpdate, { capture: true }); } - handleListUpdate = () => { - if (this.componentIsUnmounting) return; - - return this.setState({ - queryEditorRect: this.queryEditorDivRefInstance.current?.getBoundingClientRect(), - }); - }; - handleOnFocus = () => { if (this.props.onChangeQueryEditorFocus) { this.props.onChangeQueryEditorFocus(true); From 397b40220d2d402bbc912a1fe89a8ab644ae7055 Mon Sep 17 00:00:00 2001 From: "opensearch-changeset-bot[bot]" <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Date: Fri, 28 Jun 2024 17:20:10 +0000 Subject: [PATCH 7/8] Changeset file for PR #7077 deleted --- changelogs/fragments/7077.yml | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 changelogs/fragments/7077.yml diff --git a/changelogs/fragments/7077.yml b/changelogs/fragments/7077.yml deleted file mode 100644 index 43f1052a0e1..00000000000 --- a/changelogs/fragments/7077.yml +++ /dev/null @@ -1,2 +0,0 @@ -fix: -- Fix object empty check and minor perf issue in query editor extensions ([#7077](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7077)) From 1de91d92b4318049e289c3ac9521d71c0d071118 Mon Sep 17 00:00:00 2001 From: "opensearch-changeset-bot[bot]" <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Date: Fri, 28 Jun 2024 17:32:28 +0000 Subject: [PATCH 8/8] Changeset file for PR #7077 created/updated --- changelogs/fragments/7077.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/7077.yml diff --git a/changelogs/fragments/7077.yml b/changelogs/fragments/7077.yml new file mode 100644 index 00000000000..591c9f307d0 --- /dev/null +++ b/changelogs/fragments/7077.yml @@ -0,0 +1,2 @@ +fix: +- Fix object empty check and minor perf issue in query editor extensions ([#7077](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7077)) \ No newline at end of file