Commit 5cf5d89d by Andrej Ocenas Committed by GitHub

Fix: clicking outside of some query editors required 2 clicks (#19822)

parent 573e78fe
...@@ -4,17 +4,17 @@ import { QueryField } from './QueryField'; ...@@ -4,17 +4,17 @@ import { QueryField } from './QueryField';
describe('<QueryField />', () => { describe('<QueryField />', () => {
it('should render with null initial value', () => { it('should render with null initial value', () => {
const wrapper = shallow(<QueryField initialQuery={null} />); const wrapper = shallow(<QueryField query={null} />);
expect(wrapper.find('div').exists()).toBeTruthy(); expect(wrapper.find('div').exists()).toBeTruthy();
}); });
it('should render with empty initial value', () => { it('should render with empty initial value', () => {
const wrapper = shallow(<QueryField initialQuery="" />); const wrapper = shallow(<QueryField query="" />);
expect(wrapper.find('div').exists()).toBeTruthy(); expect(wrapper.find('div').exists()).toBeTruthy();
}); });
it('should render with initial value', () => { it('should render with initial value', () => {
const wrapper = shallow(<QueryField initialQuery="my query" />); const wrapper = shallow(<QueryField query="my query" />);
expect(wrapper.find('div').exists()).toBeTruthy(); expect(wrapper.find('div').exists()).toBeTruthy();
}); });
}); });
...@@ -15,18 +15,18 @@ import IndentationPlugin from './slate-plugins/indentation'; ...@@ -15,18 +15,18 @@ import IndentationPlugin from './slate-plugins/indentation';
import ClipboardPlugin from './slate-plugins/clipboard'; import ClipboardPlugin from './slate-plugins/clipboard';
import RunnerPlugin from './slate-plugins/runner'; import RunnerPlugin from './slate-plugins/runner';
import SuggestionsPlugin, { SuggestionsState } from './slate-plugins/suggestions'; import SuggestionsPlugin, { SuggestionsState } from './slate-plugins/suggestions';
import { Typeahead } from './Typeahead'; import { Typeahead } from './Typeahead';
import { makeValue, SCHEMA } from '@grafana/ui'; import { makeValue, SCHEMA } from '@grafana/ui';
export const HIGHLIGHT_WAIT = 500;
export interface QueryFieldProps { export interface QueryFieldProps {
additionalPlugins?: Plugin[]; additionalPlugins?: Plugin[];
cleanText?: (text: string) => string; cleanText?: (text: string) => string;
disabled?: boolean; disabled?: boolean;
initialQuery: string | null; // We have both value and local state. This is usually an antipattern but we need to keep local state
// for perf reasons and also have outside value in for example in Explore redux that is mutable from logs
// creating a two way binding.
query: string | null;
onRunQuery?: () => void; onRunQuery?: () => void;
onChange?: (value: string) => void; onChange?: (value: string) => void;
onTypeahead?: (typeahead: TypeaheadInput) => Promise<TypeaheadOutput>; onTypeahead?: (typeahead: TypeaheadInput) => Promise<TypeaheadOutput>;
...@@ -43,7 +43,6 @@ export interface QueryFieldState { ...@@ -43,7 +43,6 @@ export interface QueryFieldState {
typeaheadPrefix: string; typeaheadPrefix: string;
typeaheadText: string; typeaheadText: string;
value: Value; value: Value;
lastExecutedValue: Value;
} }
export interface TypeaheadInput { export interface TypeaheadInput {
...@@ -62,18 +61,19 @@ export interface TypeaheadInput { ...@@ -62,18 +61,19 @@ export interface TypeaheadInput {
* Implement props.onTypeahead to use suggestions, see PromQueryField.tsx as an example. * Implement props.onTypeahead to use suggestions, see PromQueryField.tsx as an example.
*/ */
export class QueryField extends React.PureComponent<QueryFieldProps, QueryFieldState> { export class QueryField extends React.PureComponent<QueryFieldProps, QueryFieldState> {
menuEl: HTMLElement | null;
plugins: Plugin[]; plugins: Plugin[];
resetTimer: NodeJS.Timer; resetTimer: NodeJS.Timer;
mounted: boolean; mounted: boolean;
updateHighlightsTimer: Function; runOnChangeDebounced: Function;
editor: Editor; editor: Editor;
// Is required by SuggestionsPlugin
typeaheadRef: Typeahead; typeaheadRef: Typeahead;
lastExecutedValue: Value | null = null;
constructor(props: QueryFieldProps, context: Context<any>) { constructor(props: QueryFieldProps, context: Context<any>) {
super(props, context); super(props, context);
this.updateHighlightsTimer = _.debounce(this.updateLogsHighlights, HIGHLIGHT_WAIT); this.runOnChangeDebounced = _.debounce(this.runOnChange, 500);
const { onTypeahead, cleanText, portalOrigin, onWillApplySuggestion } = props; const { onTypeahead, cleanText, portalOrigin, onWillApplySuggestion } = props;
...@@ -82,7 +82,7 @@ export class QueryField extends React.PureComponent<QueryFieldProps, QueryFieldS ...@@ -82,7 +82,7 @@ export class QueryField extends React.PureComponent<QueryFieldProps, QueryFieldS
NewlinePlugin(), NewlinePlugin(),
SuggestionsPlugin({ onTypeahead, cleanText, portalOrigin, onWillApplySuggestion, component: this }), SuggestionsPlugin({ onTypeahead, cleanText, portalOrigin, onWillApplySuggestion, component: this }),
ClearPlugin(), ClearPlugin(),
RunnerPlugin({ handler: this.executeOnChangeAndRunQueries }), RunnerPlugin({ handler: this.runOnChangeAndRunQuery }),
SelectionShortcutsPlugin(), SelectionShortcutsPlugin(),
IndentationPlugin(), IndentationPlugin(),
ClipboardPlugin(), ClipboardPlugin(),
...@@ -94,8 +94,7 @@ export class QueryField extends React.PureComponent<QueryFieldProps, QueryFieldS ...@@ -94,8 +94,7 @@ export class QueryField extends React.PureComponent<QueryFieldProps, QueryFieldS
typeaheadContext: null, typeaheadContext: null,
typeaheadPrefix: '', typeaheadPrefix: '',
typeaheadText: '', typeaheadText: '',
value: makeValue(props.initialQuery || '', props.syntax), value: makeValue(props.query || '', props.syntax),
lastExecutedValue: null,
}; };
} }
...@@ -109,14 +108,15 @@ export class QueryField extends React.PureComponent<QueryFieldProps, QueryFieldS ...@@ -109,14 +108,15 @@ export class QueryField extends React.PureComponent<QueryFieldProps, QueryFieldS
} }
componentDidUpdate(prevProps: QueryFieldProps, prevState: QueryFieldState) { componentDidUpdate(prevProps: QueryFieldProps, prevState: QueryFieldState) {
const { initialQuery, syntax } = this.props; const { query, syntax } = this.props;
const { value } = this.state; const { value } = this.state;
// Handle two way binging between local state and outside prop.
// if query changed from the outside // if query changed from the outside
if (initialQuery !== prevProps.initialQuery) { if (query !== prevProps.query) {
// and we have a version that differs // and we have a version that differs
if (initialQuery !== Plain.serialize(value)) { if (query !== Plain.serialize(value)) {
this.setState({ value: makeValue(initialQuery || '', syntax) }); this.setState({ value: makeValue(query || '', syntax) });
} }
} }
} }
...@@ -129,25 +129,31 @@ export class QueryField extends React.PureComponent<QueryFieldProps, QueryFieldS ...@@ -129,25 +129,31 @@ export class QueryField extends React.PureComponent<QueryFieldProps, QueryFieldS
} }
} }
onChange = (value: Value, invokeParentOnValueChanged?: boolean) => { /**
* Update local state, propagate change upstream and optionally run the query afterwards.
*/
onChange = (value: Value, runQuery?: boolean) => {
const documentChanged = value.document !== this.state.value.document; const documentChanged = value.document !== this.state.value.document;
const prevValue = this.state.value; const prevValue = this.state.value;
// Control editor loop, then pass text change up to parent // Update local state with new value and optionally change value upstream.
this.setState({ value }, () => { this.setState({ value }, () => {
// The diff is needed because the actual value of editor have much more metadata (for example text selection)
// that is not passed upstream so every change of editor value does not mean change of the query text.
if (documentChanged) { if (documentChanged) {
const textChanged = Plain.serialize(prevValue) !== Plain.serialize(value); const textChanged = Plain.serialize(prevValue) !== Plain.serialize(value);
if (textChanged && invokeParentOnValueChanged) { if (textChanged && runQuery) {
this.executeOnChangeAndRunQueries(); this.runOnChangeAndRunQuery();
} }
if (textChanged && !invokeParentOnValueChanged) { if (textChanged && !runQuery) {
this.updateHighlightsTimer(); // Debounce change propagation by default for perf reasons.
this.runOnChangeDebounced();
} }
} }
}); });
}; };
updateLogsHighlights = () => { runOnChange = () => {
const { onChange } = this.props; const { onChange } = this.props;
if (onChange) { if (onChange) {
...@@ -155,30 +161,32 @@ export class QueryField extends React.PureComponent<QueryFieldProps, QueryFieldS ...@@ -155,30 +161,32 @@ export class QueryField extends React.PureComponent<QueryFieldProps, QueryFieldS
} }
}; };
executeOnChangeAndRunQueries = () => { runOnRunQuery = () => {
// Send text change to parent const { onRunQuery } = this.props;
const { onChange, onRunQuery } = this.props;
if (onChange) {
onChange(Plain.serialize(this.state.value));
}
if (onRunQuery) { if (onRunQuery) {
onRunQuery(); onRunQuery();
this.setState({ lastExecutedValue: this.state.value }); this.lastExecutedValue = this.state.value;
} }
}; };
runOnChangeAndRunQuery = () => {
// onRunQuery executes query from Redux in Explore so it needs to be updated sync in case we want to run
// the query.
this.runOnChange();
this.runOnRunQuery();
};
/**
* We need to handle blur events here mainly because of dashboard panels which expect to have query executed on blur.
*/
handleBlur = (event: Event, editor: CoreEditor, next: Function) => { handleBlur = (event: Event, editor: CoreEditor, next: Function) => {
const { lastExecutedValue } = this.state; const previousValue = this.lastExecutedValue ? Plain.serialize(this.lastExecutedValue) : null;
const previousValue = lastExecutedValue ? Plain.serialize(this.state.lastExecutedValue) : null;
const currentValue = Plain.serialize(editor.value); const currentValue = Plain.serialize(editor.value);
if (previousValue !== currentValue) { if (previousValue !== currentValue) {
this.executeOnChangeAndRunQueries(); this.runOnChangeAndRunQuery();
} }
editor.blur();
return next(); return next();
}; };
......
...@@ -71,7 +71,7 @@ class ElasticsearchQueryField extends React.PureComponent<Props, State> { ...@@ -71,7 +71,7 @@ class ElasticsearchQueryField extends React.PureComponent<Props, State> {
<div className="gf-form gf-form--grow flex-shrink-1"> <div className="gf-form gf-form--grow flex-shrink-1">
<QueryField <QueryField
additionalPlugins={this.plugins} additionalPlugins={this.plugins}
initialQuery={query.query} query={query.query}
onChange={this.onChangeQuery} onChange={this.onChangeQuery}
onRunQuery={this.props.onRunQuery} onRunQuery={this.props.onRunQuery}
placeholder="Enter a Lucene query" placeholder="Enter a Lucene query"
......
...@@ -184,7 +184,7 @@ export class LokiQueryFieldForm extends React.PureComponent<LokiQueryFieldFormPr ...@@ -184,7 +184,7 @@ export class LokiQueryFieldForm extends React.PureComponent<LokiQueryFieldFormPr
<QueryField <QueryField
additionalPlugins={this.plugins} additionalPlugins={this.plugins}
cleanText={cleanText} cleanText={cleanText}
initialQuery={query.expr} query={query.expr}
onTypeahead={this.onTypeahead} onTypeahead={this.onTypeahead}
onWillApplySuggestion={willApplySuggestion} onWillApplySuggestion={willApplySuggestion}
onChange={this.onChangeQuery} onChange={this.onChangeQuery}
......
...@@ -312,7 +312,7 @@ class PromQueryField extends React.PureComponent<PromQueryFieldProps, PromQueryF ...@@ -312,7 +312,7 @@ class PromQueryField extends React.PureComponent<PromQueryFieldProps, PromQueryF
<QueryField <QueryField
additionalPlugins={this.plugins} additionalPlugins={this.plugins}
cleanText={cleanText} cleanText={cleanText}
initialQuery={query.expr} query={query.expr}
onTypeahead={this.onTypeahead} onTypeahead={this.onTypeahead}
onWillApplySuggestion={willApplySuggestion} onWillApplySuggestion={willApplySuggestion}
onChange={this.onChangeQuery} onChange={this.onChangeQuery}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment