Skip to content

Commit b22fe1a

Browse files
nickpeihlHeenawter
andauthored
[Controls] Do not ignore invalid selections (#174201)
Fixes #164633 ## Summary Improves dashboard performance by not ignoring invalid Controls selections. This improves Dashboard performance as panels do not wait until Controls selections are validated. Prior to this, the Controls would run validations to find selections that would result in No Data. These "invalid selections" would be ignored and not applied to the filters. With this PR, all selections whether valid or invalid are applied to the filters. This means all controls and panels can be loaded immediately which improves the performance of the Dashboard. Since this can be considered a breaking UI change, I've added a suppressible toast warning users about the change. The screenshots below show the same dashboard with invalid selections. In the "Before" screenshot, the "Agent version" control selection is validated before the rest of the panels are loaded. Once validated, the invalid selection is ignored and the panels load based only on valid selections. In the "After" screenshot the "Agent version" control selection is immediately applied to the filters and the rest of the dashboard is loaded. The control selections are checked for validity only after the filters are applied and highlighted. The warning popover notifies the user that invalid selections are no longer ignored. Clicking the "Do not show again" button updates the browser's localStorage so that future warnings are suppressed. Before: ![localhost_5601_wgf_app_home](https://github.com/elastic/kibana/assets/1638483/56f37376-7685-4225-b49a-65aa21f90f14) After: ![localhost_5701_vbw_app_dashboards (2)](https://github.com/elastic/kibana/assets/1638483/d0000b7e-8591-40ab-9302-6d1d5387b073) @amyjtechwriter Can you please review the text in the warnings? We want to make users aware of the change as it could abruptly change the data in their dashboards. ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: Hannah Mudge <[email protected]> Co-authored-by: Hannah Mudge <[email protected]>
1 parent 37a1920 commit b22fe1a

34 files changed

+779
-376
lines changed

src/plugins/controls/kibana.jsonc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
"unifiedSearch",
1818
"uiActions"
1919
],
20-
"extraPublicDirs": ["common"]
20+
"extraPublicDirs": ["common"],
21+
"requiredBundles": ["kibanaUtils"]
2122
}
2223
}

src/plugins/controls/public/control_group/component/control_group_component.tsx

Lines changed: 112 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,34 +8,44 @@
88

99
import '../control_group.scss';
1010

11-
import {
12-
arrayMove,
13-
SortableContext,
14-
rectSortingStrategy,
15-
sortableKeyboardCoordinates,
16-
} from '@dnd-kit/sortable';
11+
import classNames from 'classnames';
12+
import React, { useEffect, useMemo, useState } from 'react';
13+
import { TypedUseSelectorHook, useSelector } from 'react-redux';
14+
1715
import {
1816
closestCenter,
1917
DndContext,
2018
DragEndEvent,
2119
DragOverlay,
2220
KeyboardSensor,
21+
LayoutMeasuringStrategy,
2322
PointerSensor,
2423
useSensor,
2524
useSensors,
26-
LayoutMeasuringStrategy,
2725
} from '@dnd-kit/core';
28-
import classNames from 'classnames';
29-
import React, { useMemo, useState } from 'react';
30-
import { TypedUseSelectorHook, useSelector } from 'react-redux';
31-
import { EuiButtonIcon, EuiFlexGroup, EuiFlexItem, EuiPanel } from '@elastic/eui';
32-
26+
import {
27+
arrayMove,
28+
rectSortingStrategy,
29+
SortableContext,
30+
sortableKeyboardCoordinates,
31+
} from '@dnd-kit/sortable';
32+
import {
33+
EuiButtonEmpty,
34+
EuiButtonIcon,
35+
EuiCheckbox,
36+
EuiFlexGroup,
37+
EuiFlexItem,
38+
EuiIcon,
39+
EuiPanel,
40+
EuiText,
41+
EuiTourStep,
42+
} from '@elastic/eui';
3343
import { ViewMode } from '@kbn/embeddable-plugin/public';
3444

35-
import { ControlGroupReduxState } from '../types';
3645
import { ControlGroupStrings } from '../control_group_strings';
37-
import { ControlClone, SortableControl } from './control_group_sortable_item';
3846
import { useControlGroupContainer } from '../embeddable/control_group_container';
47+
import { ControlGroupReduxState } from '../types';
48+
import { ControlClone, SortableControl } from './control_group_sortable_item';
3949

4050
const contextSelect = useSelector as TypedUseSelectorHook<ControlGroupReduxState>;
4151

@@ -47,6 +57,12 @@ export const ControlGroup = () => {
4757
const viewMode = contextSelect((state) => state.explicitInput.viewMode);
4858
const controlStyle = contextSelect((state) => state.explicitInput.controlStyle);
4959
const showAddButton = contextSelect((state) => state.componentState.showAddButton);
60+
const controlWithInvalidSelectionsId = contextSelect(
61+
(state) => state.componentState.controlWithInvalidSelectionsId
62+
);
63+
const [tourStepOpen, setTourStepOpen] = useState<boolean>(true);
64+
const [suppressTourChecked, setSuppressTourChecked] = useState<boolean>(false);
65+
const [renderTourStep, setRenderTourStep] = useState(false);
5066

5167
const isEditable = viewMode === ViewMode.EDIT;
5268

@@ -61,6 +77,87 @@ export const ControlGroup = () => {
6177
[panels]
6278
);
6379

80+
useEffect(() => {
81+
/**
82+
* This forces the tour step to get unmounted so that it can attach to the new invalid
83+
* control - otherwise, the anchor will remain attached to the old invalid control
84+
*/
85+
setRenderTourStep(false);
86+
setTimeout(() => setRenderTourStep(true), 100);
87+
}, [controlWithInvalidSelectionsId]);
88+
89+
const tourStep = useMemo(() => {
90+
if (
91+
!renderTourStep ||
92+
!controlGroup.canShowInvalidSelectionsWarning() ||
93+
!tourStepOpen ||
94+
!controlWithInvalidSelectionsId
95+
) {
96+
return null;
97+
}
98+
const invalidControlType = panels[controlWithInvalidSelectionsId].type;
99+
100+
return (
101+
<EuiTourStep
102+
step={1}
103+
stepsTotal={1}
104+
minWidth={300}
105+
maxWidth={300}
106+
display="block"
107+
isStepOpen={true}
108+
repositionOnScroll
109+
onFinish={() => {}}
110+
panelPaddingSize="m"
111+
anchorPosition="downCenter"
112+
panelClassName="controlGroup--invalidSelectionsTour"
113+
anchor={`#controlFrame--${controlWithInvalidSelectionsId}`}
114+
title={
115+
<EuiFlexGroup gutterSize="s" alignItems="center">
116+
<EuiFlexItem grow={false}>
117+
<EuiIcon type="warning" color="warning" />
118+
</EuiFlexItem>
119+
<EuiFlexItem>{ControlGroupStrings.invalidControlWarning.getTourTitle()}</EuiFlexItem>
120+
</EuiFlexGroup>
121+
}
122+
content={ControlGroupStrings.invalidControlWarning.getTourContent(invalidControlType)}
123+
footerAction={[
124+
<EuiCheckbox
125+
compressed
126+
checked={suppressTourChecked}
127+
id={'controlGroup--suppressTourCheckbox'}
128+
className="controlGroup--suppressTourCheckbox"
129+
onChange={(e) => setSuppressTourChecked(e.target.checked)}
130+
label={
131+
<EuiText size="xs" className="controlGroup--suppressTourCheckboxLabel">
132+
{ControlGroupStrings.invalidControlWarning.getSuppressTourLabel()}
133+
</EuiText>
134+
}
135+
/>,
136+
<EuiButtonEmpty
137+
size="xs"
138+
flush="right"
139+
color="text"
140+
onClick={() => {
141+
setTourStepOpen(false);
142+
if (suppressTourChecked) {
143+
controlGroup.suppressInvalidSelectionsWarning();
144+
}
145+
}}
146+
>
147+
{ControlGroupStrings.invalidControlWarning.getDismissButton()}
148+
</EuiButtonEmpty>,
149+
]}
150+
/>
151+
);
152+
}, [
153+
panels,
154+
controlGroup,
155+
tourStepOpen,
156+
renderTourStep,
157+
suppressTourChecked,
158+
controlWithInvalidSelectionsId,
159+
]);
160+
64161
const [draggingId, setDraggingId] = useState<string | null>(null);
65162
const draggingIndex = useMemo(
66163
() => (draggingId ? idsInOrder.indexOf(draggingId) : -1),
@@ -117,6 +214,7 @@ export const ControlGroup = () => {
117214
alignItems="center"
118215
data-test-subj="controls-group"
119216
>
217+
{tourStep}
120218
<EuiFlexItem>
121219
<DndContext
122220
onDragStart={({ active }) => setDraggingId(active.id)}

src/plugins/controls/public/control_group/control_group.scss

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,3 +199,12 @@ $controlMinWidth: $euiSize * 14;
199199
top: (-$euiSizeXS) !important;
200200
}
201201
}
202+
203+
.controlGroup--invalidSelectionsTour {
204+
.controlGroup--suppressTourCheckbox {
205+
height: 22px;
206+
&Label {
207+
font-weight: $euiFontWeightMedium;
208+
}
209+
}
210+
}

src/plugins/controls/public/control_group/control_group_strings.ts

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,42 @@ import { i18n } from '@kbn/i18n';
1010
import { RANGE_SLIDER_CONTROL } from '../range_slider';
1111

1212
export const ControlGroupStrings = {
13+
invalidControlWarning: {
14+
getTourTitle: () =>
15+
i18n.translate('controls.controlGroup.invalidControlWarning.tourStepTitle.default', {
16+
defaultMessage: 'Invalid selections are no longer ignored',
17+
}),
18+
getTourContent: (controlType: string) => {
19+
switch (controlType) {
20+
case RANGE_SLIDER_CONTROL: {
21+
return i18n.translate(
22+
'controls.controlGroup.invalidControlWarning.tourStepContent.rangeSlider',
23+
{
24+
defaultMessage: 'The selected range is returning no results. Try changing the range.',
25+
}
26+
);
27+
}
28+
default: {
29+
return i18n.translate(
30+
'controls.controlGroup.invalidControlWarning.tourStepContent.default',
31+
{
32+
defaultMessage:
33+
'Some selections are returning no results. Try changing the selections.',
34+
}
35+
);
36+
}
37+
}
38+
},
39+
40+
getDismissButton: () =>
41+
i18n.translate('controls.controlGroup.invalidControlWarning.dismissButtonLabel', {
42+
defaultMessage: 'Dismiss',
43+
}),
44+
getSuppressTourLabel: () =>
45+
i18n.translate('controls.controlGroup.invalidControlWarning.suppressTourLabel', {
46+
defaultMessage: "Don't show again",
47+
}),
48+
},
1349
manageControl: {
1450
getFlyoutCreateTitle: () =>
1551
i18n.translate('controls.controlGroup.manageControl.createFlyoutTitle', {
@@ -258,8 +294,7 @@ export const ControlGroupStrings = {
258294
}),
259295
getValidateSelectionsSubTitle: () =>
260296
i18n.translate('controls.controlGroup.management.validate.subtitle', {
261-
defaultMessage:
262-
'Automatically ignore any control selection that would result in no data.',
297+
defaultMessage: 'Highlight control selections that result in no data.',
263298
}),
264299
},
265300
controlChaining: {

src/plugins/controls/public/control_group/embeddable/control_group_container.tsx

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
* in compliance with, at your election, the Elastic License 2.0 or the Server
66
* Side Public License, v 1.
77
*/
8+
89
import { compareFilters, COMPARE_ALL_OPTIONS, Filter, uniqFilters } from '@kbn/es-query';
910
import { isEqual, pick } from 'lodash';
1011
import React, { createContext, useContext } from 'react';
@@ -24,6 +25,7 @@ import {
2425
persistableControlGroupInputKeys,
2526
} from '../../../common';
2627
import { pluginServices } from '../../services';
28+
import { ControlsStorageService } from '../../services/storage/types';
2729
import { ControlEmbeddable, ControlInput, ControlOutput } from '../../types';
2830
import { ControlGroup } from '../component/control_group_component';
2931
import { openAddDataControlFlyout } from '../editor/open_add_data_control_flyout';
@@ -86,11 +88,15 @@ export class ControlGroupContainer extends Container<
8688

8789
private initialized$ = new BehaviorSubject(false);
8890

91+
private storageService: ControlsStorageService;
92+
8993
private subscriptions: Subscription = new Subscription();
9094
private domNode?: HTMLElement;
9195
private recalculateFilters$: Subject<null>;
9296
private relevantDataViewId?: string;
9397
private lastUsedDataViewId?: string;
98+
private invalidSelectionsState: { [childId: string]: boolean };
99+
94100
public diffingSubscription: Subscription = new Subscription();
95101

96102
// state management
@@ -126,6 +132,8 @@ export class ControlGroupContainer extends Container<
126132
ControlGroupChainingSystems[initialInput.chainingSystem]?.getContainerSettings(initialInput)
127133
);
128134

135+
({ storage: this.storageService } = pluginServices.getServices());
136+
129137
this.recalculateFilters$ = new Subject();
130138
this.onFiltersPublished$ = new Subject<Filter[]>();
131139
this.onControlRemoved$ = new Subject<string>();
@@ -153,6 +161,10 @@ export class ControlGroupContainer extends Container<
153161

154162
this.store = reduxEmbeddableTools.store;
155163

164+
this.invalidSelectionsState = this.getChildIds().reduce((prev, id) => {
165+
return { ...prev, [id]: false };
166+
}, {});
167+
156168
// when all children are ready setup subscriptions
157169
this.untilAllChildrenReady().then(() => {
158170
this.recalculateDataViews();
@@ -164,6 +176,32 @@ export class ControlGroupContainer extends Container<
164176
this.fieldFilterPredicate = fieldFilterPredicate;
165177
}
166178

179+
public canShowInvalidSelectionsWarning = () =>
180+
this.storageService.getShowInvalidSelectionWarning() ?? true;
181+
182+
public suppressInvalidSelectionsWarning = () => {
183+
this.storageService.setShowInvalidSelectionWarning(false);
184+
};
185+
186+
public reportInvalidSelections = ({
187+
id,
188+
hasInvalidSelections,
189+
}: {
190+
id: string;
191+
hasInvalidSelections: boolean;
192+
}) => {
193+
this.invalidSelectionsState = { ...this.invalidSelectionsState, [id]: hasInvalidSelections };
194+
195+
const childrenWithInvalidSelections = cachedChildEmbeddableOrder(
196+
this.getInput().panels
197+
).idsInOrder.filter((childId) => {
198+
return this.invalidSelectionsState[childId];
199+
});
200+
this.dispatch.setControlWithInvalidSelectionsId(
201+
childrenWithInvalidSelections.length > 0 ? childrenWithInvalidSelections[0] : undefined
202+
);
203+
};
204+
167205
private setupSubscriptions = () => {
168206
/**
169207
* refresh control order cache and make all panels refreshInputFromParent whenever panel orders change
@@ -201,7 +239,9 @@ export class ControlGroupContainer extends Container<
201239
* debounce output recalculation
202240
*/
203241
this.subscriptions.add(
204-
this.recalculateFilters$.pipe(debounceTime(10)).subscribe(() => this.recalculateFilters())
242+
this.recalculateFilters$.pipe(debounceTime(10)).subscribe(() => {
243+
this.recalculateFilters();
244+
})
205245
);
206246
};
207247

@@ -211,9 +251,14 @@ export class ControlGroupContainer extends Container<
211251
} = this.getState();
212252
if (!persistableControlGroupInputIsEqual(this.getPersistableInput(), lastSavedInput)) {
213253
this.updateInput(lastSavedInput);
254+
this.reload(); // this forces the children to update their inputs + perform validation as necessary
214255
}
215256
}
216257

258+
public reload() {
259+
super.reload();
260+
}
261+
217262
public getPersistableInput: () => PersistableControlGroupInput & { id: string } = () => {
218263
const input = this.getInput();
219264
return pick(input, [...persistableControlGroupInputKeys, 'id']);
@@ -284,13 +329,14 @@ export class ControlGroupContainer extends Container<
284329
private recalculateFilters = () => {
285330
const allFilters: Filter[] = [];
286331
let timeslice;
287-
Object.values(this.children).map((child) => {
332+
Object.values(this.children).map((child: ControlEmbeddable) => {
288333
const childOutput = child.getOutput() as ControlOutput;
289334
allFilters.push(...(childOutput?.filters ?? []));
290335
if (childOutput.timeslice) {
291336
timeslice = childOutput.timeslice;
292337
}
293338
});
339+
294340
// if filters are different, publish them
295341
if (
296342
!compareFilters(this.output.filters ?? [], allFilters ?? [], COMPARE_ALL_OPTIONS) ||

src/plugins/controls/public/control_group/state/control_group_reducers.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,12 @@ export const controlGroupReducers = {
1919
) => {
2020
state.componentState.lastSavedInput = action.payload;
2121
},
22+
setControlWithInvalidSelectionsId: (
23+
state: WritableDraft<ControlGroupReduxState>,
24+
action: PayloadAction<ControlGroupComponentState['controlWithInvalidSelectionsId']>
25+
) => {
26+
state.componentState.controlWithInvalidSelectionsId = action.payload;
27+
},
2228
setControlStyle: (
2329
state: WritableDraft<ControlGroupReduxState>,
2430
action: PayloadAction<ControlGroupInput['controlStyle']>

src/plugins/controls/public/control_group/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ export interface ControlGroupSettings {
4242

4343
export type ControlGroupComponentState = ControlGroupSettings & {
4444
lastSavedInput: PersistableControlGroupInput;
45+
controlWithInvalidSelectionsId?: string;
4546
};
4647

4748
export {

0 commit comments

Comments
 (0)