Skip to content

Commit f0d9c77

Browse files
awahab07awahab07
authored
[Lens] [ES|QL] Lens ES|QL flyout "Apply and close" button's UX improvement (#247625)
To address the UX quirks mentioned the the parent issues, the PR: - Integrates syntax validation in Lens ES|QL editor to disable "Apply and close" button on Lens ES|QL panel configuration flyout when syntax errors are present - Shows contextual tooltip messages explaining why the button is disabled ~Shows the error help text if updated query hasn't been run, while disabling "Apply and close" button.~ ### ES|QL flyout form validation behavior The "Apply and close" button is now disabled when: - ~ES|QL query has syntax errors (detected client-side)~ (Now, query needs to be run first in order to let flyout know if there are syntax errors). - ES|QL query has been changed but not yet run - ES|QL query has errors after execution
1 parent 73afad8 commit f0d9c77

7 files changed

Lines changed: 77 additions & 19 deletions

File tree

‎src/platform/packages/shared/kbn-unified-histogram/components/chart/chart_config_panel.tsx‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ export function ChartConfigPanel({
4949
onSuggestionContextEdit: (suggestion: UnifiedHistogramSuggestionContext | undefined) => void;
5050
}) {
5151
const [editLensConfigPanel, setEditLensConfigPanel] = useState<JSX.Element | null>(null);
52-
const previousSuggestion = useRef<Suggestion | undefined>(undefined);
5352
const previousAdapters = useRef<Record<string, Datatable> | undefined>(undefined);
5453
const previousQuery = useRef<Query | AggregateQuery | undefined>(undefined);
5554

@@ -91,7 +90,6 @@ export function ChartConfigPanel({
9190
[onSuggestionContextEdit, visContext]
9291
);
9392

94-
const currentSuggestion = currentSuggestionContext.suggestion;
9593
const currentSuggestionType = currentSuggestionContext.type;
9694

9795
useEffect(() => {
@@ -119,15 +117,15 @@ export function ChartConfigPanel({
119117
/>
120118
);
121119
setEditLensConfigPanel(panel);
122-
previousSuggestion.current = currentSuggestion;
123120
previousAdapters.current = tablesAdapters;
124121
if (dataHasChanged) {
125122
previousQuery.current = query;
126123
}
127124
}
128-
const suggestionHasChanged = currentSuggestion?.title !== previousSuggestion?.current?.title;
129-
// rerender the component if the data has changed
130-
if (isPlainRecord && (dataHasChanged || suggestionHasChanged || !isFlyoutVisible)) {
125+
// rerender the component if the data has changed or flyout becomes visible
126+
// Note: when suggestion/chart type changes while flyout is visible, it flows through
127+
// visContext.attributes props instead of recreating the component (which would reset state)
128+
if (isPlainRecord && (dataHasChanged || !isFlyoutVisible)) {
131129
fetchLensConfigComponent();
132130
}
133131
}, [
@@ -136,7 +134,6 @@ export function ChartConfigPanel({
136134
updatePanelState,
137135
updateSuggestion,
138136
isPlainRecord,
139-
currentSuggestion,
140137
query,
141138
isFlyoutVisible,
142139
setIsFlyoutVisible,

‎x-pack/platform/plugins/shared/lens/public/app_plugin/shared/edit_on_the_fly/flyout_wrapper.tsx‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,13 @@ export const FlyoutWrapper = ({
3939
isInlineFlyoutVisible,
4040
isScrollable,
4141
displayFlyoutHeader,
42-
isNewPanel,
4342
isSaveable,
4443
onCancel,
4544
navigateToLensEditor,
4645
onApply,
4746
isReadOnly,
4847
applyButtonLabel = applyAndCloseLabel,
48+
applyButtonDisabledTooltip,
4949
}: FlyoutWrapperProps) => {
5050
const { euiTheme } = useEuiTheme();
5151
return (
@@ -205,15 +205,20 @@ export const FlyoutWrapper = ({
205205
</EuiFlexItem>
206206
{isReadOnly ? null : (
207207
<EuiFlexItem grow={false}>
208-
<EuiButton
209-
onClick={onApply}
210-
fill
211-
disabled={Boolean(isNewPanel) ? false : !isSaveable}
212-
iconType="check"
213-
data-test-subj="applyFlyoutButton"
208+
<EuiToolTip
209+
content={applyButtonDisabledTooltip}
210+
display={applyButtonDisabledTooltip ? 'inlineBlock' : 'block'}
214211
>
215-
{applyButtonLabel}
216-
</EuiButton>
212+
<EuiButton
213+
onClick={onApply}
214+
fill
215+
disabled={!isSaveable}
216+
iconType="check"
217+
data-test-subj="applyFlyoutButton"
218+
>
219+
{applyButtonLabel}
220+
</EuiButton>
221+
</EuiToolTip>
217222
</EuiFlexItem>
218223
)}
219224
</EuiFlexGroup>

‎x-pack/platform/plugins/shared/lens/public/app_plugin/shared/edit_on_the_fly/layer_configuration_section.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ export function LayerConfiguration({
3333
panelId,
3434
closeFlyout,
3535
editorContainer,
36+
onTextBasedQueryStateChange,
3637
}: LayerConfigurationProps) {
3738
// Derive whether we're in text-based mode from the query type
3839
const isTextBasedMode = isOfAggregateQueryType(attributes.state.query);
@@ -75,6 +76,7 @@ export function LayerConfiguration({
7576
panelId,
7677
closeFlyout,
7778
editorContainer,
79+
onTextBasedQueryStateChange,
7880
};
7981
return (
8082
<div

‎x-pack/platform/plugins/shared/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,9 @@ import {
2323
} from '@elastic/eui';
2424
import { isOfAggregateQueryType } from '@kbn/es-query';
2525
import type { TypedLensSerializedState, SupportedDatasourceId } from '@kbn/lens-common';
26-
import { getLensFeatureFlags } from '../../../get_feature_flags';
2726
import { buildExpression } from '../../../editor_frame_service/editor_frame/expression_helpers';
27+
import type { TextBasedQueryState } from '../../../editor_frame_service/editor_frame/config_panel/types';
28+
import { getLensFeatureFlags } from '../../../get_feature_flags';
2829
import {
2930
useLensSelector,
3031
selectFramePublicAPI,
@@ -87,6 +88,7 @@ export function LensEditConfigurationFlyout({
8788
const [isLayerAccordionOpen, setIsLayerAccordionOpen] = useState(true);
8889
const [isSuggestionsAccordionOpen, setIsSuggestionsAccordionOpen] = useState(false);
8990
const [isESQLResultsAccordionOpen, setIsESQLResultsAccordionOpen] = useState(false);
91+
const [esqlQueryState, setESQLQueryState] = useState<TextBasedQueryState | null>(null);
9092

9193
const { datasourceStates, visualization, isLoading, annotationGroups, searchSessionId } =
9294
useLensSelector((state) => state.lens);
@@ -199,6 +201,10 @@ export function LensEditConfigurationFlyout({
199201
initialAttributes: attributes,
200202
});
201203

204+
const onTextBasedQueryStateChange = useCallback((state: TextBasedQueryState) => {
205+
setESQLQueryState(state);
206+
}, []);
207+
202208
const onApply = useCallback(() => {
203209
if (visualization.activeId == null || !currentAttributes) {
204210
return;
@@ -258,6 +264,12 @@ export function LensEditConfigurationFlyout({
258264
if (!visualization.state || !visualization.activeId) {
259265
return false;
260266
}
267+
// For text-based mode, check if query has been successfully concluded (no runtime errors, and not pending)
268+
if (textBasedMode && esqlQueryState) {
269+
if (esqlQueryState.hasErrors || esqlQueryState.isQueryPendingSubmit) {
270+
return false;
271+
}
272+
}
261273
const visualizationErrors = getUserMessages(['visualization'], {
262274
severity: 'error',
263275
});
@@ -291,8 +303,20 @@ export function LensEditConfigurationFlyout({
291303
visualization.activeId,
292304
visualization.state,
293305
getUserMessages,
306+
textBasedMode,
307+
esqlQueryState,
294308
]);
295309

310+
// Tooltip message when Apply button is disabled due to an unrun ES|QL query
311+
const applyButtonDisabledTooltip = useMemo(() => {
312+
if (textBasedMode && esqlQueryState?.isQueryPendingSubmit) {
313+
return i18n.translate('xpack.lens.config.applyFlyoutRunQueryTooltip', {
314+
defaultMessage: 'Run the ES|QL query to apply changes',
315+
});
316+
}
317+
return undefined;
318+
}, [textBasedMode, esqlQueryState?.isQueryPendingSubmit]);
319+
296320
const addLayerButton = useAddLayerButton(
297321
framePublicAPI,
298322
coreStart,
@@ -404,10 +428,10 @@ export function LensEditConfigurationFlyout({
404428
navigateToLensEditor={navigateToLensEditor}
405429
onApply={onApply}
406430
isScrollable
407-
isNewPanel={isNewPanel}
408431
isSaveable={isSaveable}
409432
isReadOnly={isReadOnly}
410433
applyButtonLabel={applyButtonLabel}
434+
applyButtonDisabledTooltip={applyButtonDisabledTooltip}
411435
toolbar={toolbar}
412436
layerTabs={layerTabs}
413437
>
@@ -426,6 +450,7 @@ export function LensEditConfigurationFlyout({
426450
closeFlyout={closeFlyout}
427451
parentApi={parentApi}
428452
panelId={panelId}
453+
onTextBasedQueryStateChange={onTextBasedQueryStateChange}
429454
/>
430455
</FlyoutWrapper>
431456
</>
@@ -443,9 +468,9 @@ export function LensEditConfigurationFlyout({
443468
onApply={onApply}
444469
isSaveable={isSaveable}
445470
isScrollable
446-
isNewPanel={isNewPanel}
447471
isReadOnly={isReadOnly}
448472
applyButtonLabel={applyButtonLabel}
473+
applyButtonDisabledTooltip={applyButtonDisabledTooltip}
449474
toolbar={toolbar}
450475
layerTabs={layerTabs}
451476
>
@@ -560,6 +585,7 @@ export function LensEditConfigurationFlyout({
560585
parentApi={parentApi}
561586
panelId={panelId}
562587
editorContainer={editorContainer.current || undefined}
588+
onTextBasedQueryStateChange={onTextBasedQueryStateChange}
563589
/>
564590
</>
565591
</EuiAccordion>

‎x-pack/platform/plugins/shared/lens/public/app_plugin/shared/edit_on_the_fly/types.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import type {
1313
LensDocument,
1414
LensInspector,
1515
} from '@kbn/lens-common';
16+
import type { TextBasedQueryState } from '../../../editor_frame_service/editor_frame/config_panel/types';
1617
import type { LensPluginStartDependencies } from '../../../plugin';
1718

1819
export interface FlyoutWrapperProps {
@@ -29,6 +30,8 @@ export interface FlyoutWrapperProps {
2930
navigateToLensEditor?: () => void;
3031
isReadOnly?: boolean;
3132
applyButtonLabel?: string;
33+
/** Tooltip to show when Apply button is disabled */
34+
applyButtonDisabledTooltip?: string;
3235
}
3336

3437
export interface EditConfigPanelProps {
@@ -109,6 +112,8 @@ export interface LayerConfigurationProps {
109112
panelId?: string;
110113
closeFlyout?: () => void;
111114
editorContainer?: HTMLElement;
115+
/** Callback to report text-based query state changes */
116+
onTextBasedQueryStateChange?: (state: TextBasedQueryState) => void;
112117
}
113118

114119
export interface LayerTabsProps {

‎x-pack/platform/plugins/shared/lens/public/editor_frame_service/editor_frame/config_panel/esql_editor.tsx‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ export type ESQLEditorProps = Simplify<
5858
| 'updateSuggestion'
5959
| 'dataLoading$'
6060
| 'parentApi'
61+
| 'onTextBasedQueryStateChange'
6162
>
6263
>;
6364

@@ -84,6 +85,7 @@ export function ESQLEditor({
8485
dataLoading$,
8586
setCurrentAttributes,
8687
updateSuggestion,
88+
onTextBasedQueryStateChange,
8789
}: ESQLEditorProps) {
8890
const prevQuery = useRef<AggregateQuery | Query>(attributes?.state.query || { esql: '' });
8991
const [query, setQuery] = useState<AggregateQuery | Query>(
@@ -95,6 +97,9 @@ export function ESQLEditor({
9597
const canEditTextBasedQuery = useLensSelector(selectCanEditTextBasedQuery);
9698

9799
const [errors, setErrors] = useState<Error[]>([]);
100+
const [submittedQuery, setSubmittedQuery] = useState<AggregateQuery | Query>(
101+
attributes?.state.query || { esql: '' }
102+
);
98103
const [isLayerAccordionOpen, setIsLayerAccordionOpen] = useState(true);
99104
const [suggestsLimitedColumns, setSuggestsLimitedColumns] = useState(false);
100105
const [isVisualizationLoading, setIsVisualizationLoading] = useState(false);
@@ -167,6 +172,7 @@ export function ESQLEditor({
167172
updateSuggestion?.(attrs);
168173
}
169174
prevQuery.current = q;
175+
setSubmittedQuery(q);
170176
setIsVisualizationLoading(false);
171177
},
172178
[
@@ -195,6 +201,14 @@ export function ESQLEditor({
195201
setIsInitialized,
196202
});
197203

204+
// Track and report query state to parent
205+
useEffect(() => {
206+
onTextBasedQueryStateChange?.({
207+
hasErrors: errors.length > 0,
208+
isQueryPendingSubmit: !isEqual(query, submittedQuery),
209+
});
210+
}, [query, submittedQuery, errors.length, onTextBasedQueryStateChange]);
211+
198212
// Early exit if it's not in TextBased mode or the editor should be hidden
199213
if (!isTextBasedLanguage || !canEditTextBasedQuery || !isOfAggregateQueryType(query)) {
200214
return null;

‎x-pack/platform/plugins/shared/lens/public/editor_frame_service/editor_frame/config_panel/types.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,13 @@ import type {
2525
} from '@kbn/lens-common';
2626
import type { IndexPatternServiceAPI } from '../../../data_views_service/service';
2727

28+
export interface TextBasedQueryState {
29+
/** Whether the query has errors from the last run attempt */
30+
hasErrors: boolean;
31+
/** Whether the query has been modified but not yet submitted */
32+
isQueryPendingSubmit: boolean;
33+
}
34+
2835
export interface LensConfigPanelBaseProps {
2936
framePublicAPI: FramePublicAPI;
3037
core: DatasourceDimensionEditorProps['core'];
@@ -45,6 +52,8 @@ export interface LensConfigPanelBaseProps {
4552
panelId?: string;
4653
closeFlyout?: () => void;
4754
editorContainer?: HTMLElement;
55+
/** Callback to report text-based query state changes */
56+
onTextBasedQueryStateChange?: (state: TextBasedQueryState) => void;
4857
}
4958

5059
export interface ConfigPanelWrapperProps extends LensConfigPanelBaseProps {

0 commit comments

Comments
 (0)