Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix object empty check and minor perf issue in query editor extensions #7077

Merged
merged 9 commits into from
Jun 28, 2024
2 changes: 2 additions & 0 deletions changelogs/fragments/7077.yml
Original file line number Diff line number Diff line change
@@ -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))
73 changes: 38 additions & 35 deletions src/plugins/data/public/ui/query_editor/query_editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -19,9 +19,11 @@ import { SuggestionsListSize } from '../typeahead/suggestions_component';
import { DataSettings } from '../types';
import { fetchIndexPatterns } from './fetch_index_patterns';
import { QueryLanguageSelector } from './language_selector';
import { QueryEditorExtensions } from './query_editor_extensions';

export interface QueryEditorProps {
indexPatterns: Array<IIndexPattern | string>;
dataSource?: DataSource;
query: Query;
containerRef?: React.RefCallback<HTMLDivElement>;
settings: Settings;
Expand All @@ -41,10 +43,9 @@ export interface QueryEditorProps {
size?: SuggestionsListSize;
className?: string;
isInvalid?: boolean;
queryEditorHeaderRef: React.RefObject<HTMLDivElement>;
queryEditorHeaderClassName?: string;
queryEditorBannerRef: React.RefObject<HTMLDivElement>;
queryEditorBannerClassName?: string;
queryLanguage?: string;
headerClassName?: string;
bannerClassName?: string;
}

interface Props extends QueryEditorProps {
Expand All @@ -57,7 +58,6 @@ interface State {
index: number | null;
suggestions: QuerySuggestion[];
indexPatterns: IIndexPattern[];
queryEditorRect: DOMRect | undefined;
}

const KEY_CODES = {
Expand All @@ -82,7 +82,6 @@ export default class QueryEditorUI extends Component<Props, State> {
index: null,
suggestions: [],
indexPatterns: [],
queryEditorRect: undefined,
};

public inputRef: HTMLElement | null = null;
Expand All @@ -91,7 +90,9 @@ export default class QueryEditorUI extends Component<Props, State> {
private abortController?: AbortController;
private services = this.props.opensearchDashboards.services;
private componentIsUnmounting = false;
private queryEditorDivRefInstance: RefObject<HTMLDivElement> = createRef();
private headerRef: RefObject<HTMLDivElement> = createRef();
private bannerRef: RefObject<HTMLDivElement> = createRef();
private extensionMap = this.props.settings?.getQueryEditorExtensionMap();

private getQueryString = () => {
if (!this.props.query.query) {
Expand Down Expand Up @@ -120,6 +121,30 @@ export default class QueryEditorUI extends Component<Props, State> {
});
};

private renderQueryEditorExtensions() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! nice clean up!

if (
!(
this.headerRef.current &&
this.bannerRef.current &&
this.props.queryLanguage &&
this.extensionMap &&
Object.keys(this.extensionMap).length > 0
)
) {
return null;
}
return (
<QueryEditorExtensions
language={this.props.queryLanguage}
configMap={this.extensionMap}
componentContainer={this.headerRef.current}
bannerContainer={this.bannerRef.current}
indexPatterns={this.props.indexPatterns}
dataSource={this.props.dataSource}
/>
);
}

private onSubmit = (query: Query, dateRange?: TimeRange) => {
if (this.props.onSubmit) {
if (this.persistedLog) {
Expand Down Expand Up @@ -221,12 +246,6 @@ export default class QueryEditorUI extends Component<Props, State> {
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) {
Expand All @@ -241,17 +260,8 @@ export default class QueryEditorUI extends Component<Props, State> {
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);
Expand All @@ -260,20 +270,12 @@ export default class QueryEditorUI extends Component<Props, State> {

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 (
<div className={className}>
<div ref={this.props.queryEditorBannerRef} className={queryEditorBannerClassName} />
<div ref={this.bannerRef} className={bannerClassName} />
<EuiFlexGroup gutterSize="xs" direction="column">
<EuiFlexItem grow={false}>
<EuiFlexGroup gutterSize="xs" alignItems="center" className={`${className}__wrapper`}>
Expand All @@ -294,7 +296,7 @@ export default class QueryEditorUI extends Component<Props, State> {
</EuiFlexGroup>
</EuiFlexItem>
<EuiFlexItem onClick={this.onClickInput} grow={true}>
<div ref={this.props.queryEditorHeaderRef} className={queryEditorHeaderClassName} />
<div ref={this.headerRef} className={headerClassName} />
<CodeEditor
height={70}
languageId="opensearchql"
Expand All @@ -315,6 +317,7 @@ export default class QueryEditorUI extends Component<Props, State> {
/>
</EuiFlexItem>
</EuiFlexGroup>
{this.renderQueryEditorExtensions()}
</div>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const QueryEditorExtensions: React.FC<QueryEditorExtensionsProps> = 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]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ 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';
Expand Down Expand Up @@ -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<HTMLDivElement | null>(null);
const queryEditorBannerRef = useRef<HTMLDivElement | null>(null);

const opensearchDashboards = useOpenSearchDashboards<IDataPluginServices>();
const { uiSettings, storage, appName } = opensearchDashboards.services;
Expand All @@ -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!
Expand Down Expand Up @@ -235,6 +231,7 @@ export default function QueryEditorTopRow(props: QueryEditorTopRowProps) {
<QueryEditor
disableAutoFocus={props.disableAutoFocus}
indexPatterns={props.indexPatterns!}
dataSource={props.dataSource}
prepend={props.prepend}
query={parsedQuery}
containerRef={props.containerRef}
Expand All @@ -247,33 +244,12 @@ export default function QueryEditorTopRow(props: QueryEditorTopRowProps) {
persistedLog={persistedLog}
className="osdQueryEditor"
dataTestSubj={props.dataTestSubj}
queryEditorHeaderRef={queryEditorHeaderRef}
queryEditorBannerRef={queryEditorBannerRef}
queryLanguage={queryLanguage}
/>
</EuiFlexItem>
);
}

function renderQueryEditorExtensions() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a good clean up. that i need to do after restructuring. I have a clean up task in #6957 but some of the render functions i added are in the wrong location and that's cause i tried to not touch query string input. so thanks for starting the clean up!

if (
!shouldRenderQueryEditorExtensions() ||
!queryEditorHeaderRef.current ||
!queryEditorBannerRef.current ||
!queryLanguage
)
return;
return (
<QueryEditorExtensions
language={queryLanguage}
configMap={queryEditorExtensionMap}
componentContainer={queryEditorHeaderRef.current}
bannerContainer={queryEditorBannerRef.current}
indexPatterns={props.indexPatterns}
dataSource={props.dataSource}
/>
);
}

function renderSharingMetaFields() {
const { from, to } = getDateRange();
const dateRangePretty = prettyDuration(
Expand Down Expand Up @@ -305,10 +281,6 @@ export default function QueryEditorTopRow(props: QueryEditorTopRowProps) {
);
}

function shouldRenderQueryEditorExtensions(): boolean {
return Boolean(queryEditorExtensionMap && Object.keys(queryEditorExtensionMap).length);
}

function renderUpdateButton() {
const button = props.customSubmitButton ? (
React.cloneElement(props.customSubmitButton, { onClick: onClickSubmitButton })
Expand Down Expand Up @@ -401,7 +373,6 @@ export default function QueryEditorTopRow(props: QueryEditorTopRowProps) {
direction="column"
justifyContent="flexEnd"
>
{renderQueryEditorExtensions()}
{renderQueryEditor()}
<EuiFlexItem>
<EuiFlexGroup responsive={false} gutterSize="none">
Expand Down
Loading