From cefbec63b3e6985a36add3f1684b13baf36b488d Mon Sep 17 00:00:00 2001 From: Nick Peihl Date: Fri, 23 Feb 2024 15:07:23 -0500 Subject: [PATCH] [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 Co-authored-by: Hannah Mudge --- src/plugins/controls/kibana.jsonc | 3 +- .../component/control_group_component.tsx | 126 ++++++++-- .../public/control_group/control_group.scss | 9 + .../control_group/control_group_strings.ts | 39 +++- .../embeddable/control_group_container.tsx | 50 +++- .../state/control_group_reducers.ts | 6 + .../controls/public/control_group/types.ts | 1 + .../options_list/components/options_list.scss | 15 +- .../components/options_list_control.test.tsx | 14 +- .../components/options_list_control.tsx | 171 +++++++++----- .../components/options_list_popover.test.tsx | 218 +++++++++-------- ...ptions_list_popover_invalid_selections.tsx | 37 ++- .../components/options_list_strings.ts | 16 +- .../embeddable/options_list_embeddable.tsx | 69 +++--- .../options_list/options_list_reducers.ts | 6 + .../controls/public/options_list/types.ts | 1 + .../range_slider/components/range_slider.scss | 24 +- .../components/range_slider_control.tsx | 24 +- .../components/range_slider_strings.ts | 6 + .../range_slider_embeddable.test.tsx | 2 +- .../embeddable/range_slider_embeddable.tsx | 219 +++++++++--------- .../public/services/core/core.story.ts | 1 + .../public/services/core/core_service.ts | 3 +- .../controls/public/services/core/types.ts | 1 + .../public/services/plugin_services.story.ts | 3 +- .../public/services/plugin_services.stub.ts | 2 + .../public/services/plugin_services.ts | 2 + .../services/storage/storage_service.stub.ts | 19 ++ .../services/storage/storage_service.ts | 30 +++ .../controls/public/services/storage/types.ts | 12 + src/plugins/controls/public/services/types.ts | 2 + .../controls/common/control_group_chaining.ts | 11 +- .../controls/common/range_slider.ts | 7 +- .../options_list/options_list_validation.ts | 6 +- 34 files changed, 779 insertions(+), 376 deletions(-) create mode 100644 src/plugins/controls/public/services/storage/storage_service.stub.ts create mode 100644 src/plugins/controls/public/services/storage/storage_service.ts create mode 100644 src/plugins/controls/public/services/storage/types.ts diff --git a/src/plugins/controls/kibana.jsonc b/src/plugins/controls/kibana.jsonc index 14718f533a8f6..bd65ecc2d0b6f 100644 --- a/src/plugins/controls/kibana.jsonc +++ b/src/plugins/controls/kibana.jsonc @@ -17,6 +17,7 @@ "unifiedSearch", "uiActions" ], - "extraPublicDirs": ["common"] + "extraPublicDirs": ["common"], + "requiredBundles": ["kibanaUtils"] } } diff --git a/src/plugins/controls/public/control_group/component/control_group_component.tsx b/src/plugins/controls/public/control_group/component/control_group_component.tsx index e77e88f6e900e..20d599aed0eb4 100644 --- a/src/plugins/controls/public/control_group/component/control_group_component.tsx +++ b/src/plugins/controls/public/control_group/component/control_group_component.tsx @@ -8,34 +8,44 @@ import '../control_group.scss'; -import { - arrayMove, - SortableContext, - rectSortingStrategy, - sortableKeyboardCoordinates, -} from '@dnd-kit/sortable'; +import classNames from 'classnames'; +import React, { useEffect, useMemo, useState } from 'react'; +import { TypedUseSelectorHook, useSelector } from 'react-redux'; + import { closestCenter, DndContext, DragEndEvent, DragOverlay, KeyboardSensor, + LayoutMeasuringStrategy, PointerSensor, useSensor, useSensors, - LayoutMeasuringStrategy, } from '@dnd-kit/core'; -import classNames from 'classnames'; -import React, { useMemo, useState } from 'react'; -import { TypedUseSelectorHook, useSelector } from 'react-redux'; -import { EuiButtonIcon, EuiFlexGroup, EuiFlexItem, EuiPanel } from '@elastic/eui'; - +import { + arrayMove, + rectSortingStrategy, + SortableContext, + sortableKeyboardCoordinates, +} from '@dnd-kit/sortable'; +import { + EuiButtonEmpty, + EuiButtonIcon, + EuiCheckbox, + EuiFlexGroup, + EuiFlexItem, + EuiIcon, + EuiPanel, + EuiText, + EuiTourStep, +} from '@elastic/eui'; import { ViewMode } from '@kbn/embeddable-plugin/public'; -import { ControlGroupReduxState } from '../types'; import { ControlGroupStrings } from '../control_group_strings'; -import { ControlClone, SortableControl } from './control_group_sortable_item'; import { useControlGroupContainer } from '../embeddable/control_group_container'; +import { ControlGroupReduxState } from '../types'; +import { ControlClone, SortableControl } from './control_group_sortable_item'; const contextSelect = useSelector as TypedUseSelectorHook; @@ -47,6 +57,12 @@ export const ControlGroup = () => { const viewMode = contextSelect((state) => state.explicitInput.viewMode); const controlStyle = contextSelect((state) => state.explicitInput.controlStyle); const showAddButton = contextSelect((state) => state.componentState.showAddButton); + const controlWithInvalidSelectionsId = contextSelect( + (state) => state.componentState.controlWithInvalidSelectionsId + ); + const [tourStepOpen, setTourStepOpen] = useState(true); + const [suppressTourChecked, setSuppressTourChecked] = useState(false); + const [renderTourStep, setRenderTourStep] = useState(false); const isEditable = viewMode === ViewMode.EDIT; @@ -61,6 +77,87 @@ export const ControlGroup = () => { [panels] ); + useEffect(() => { + /** + * This forces the tour step to get unmounted so that it can attach to the new invalid + * control - otherwise, the anchor will remain attached to the old invalid control + */ + setRenderTourStep(false); + setTimeout(() => setRenderTourStep(true), 100); + }, [controlWithInvalidSelectionsId]); + + const tourStep = useMemo(() => { + if ( + !renderTourStep || + !controlGroup.canShowInvalidSelectionsWarning() || + !tourStepOpen || + !controlWithInvalidSelectionsId + ) { + return null; + } + const invalidControlType = panels[controlWithInvalidSelectionsId].type; + + return ( + {}} + panelPaddingSize="m" + anchorPosition="downCenter" + panelClassName="controlGroup--invalidSelectionsTour" + anchor={`#controlFrame--${controlWithInvalidSelectionsId}`} + title={ + + + + + {ControlGroupStrings.invalidControlWarning.getTourTitle()} + + } + content={ControlGroupStrings.invalidControlWarning.getTourContent(invalidControlType)} + footerAction={[ + setSuppressTourChecked(e.target.checked)} + label={ + + {ControlGroupStrings.invalidControlWarning.getSuppressTourLabel()} + + } + />, + { + setTourStepOpen(false); + if (suppressTourChecked) { + controlGroup.suppressInvalidSelectionsWarning(); + } + }} + > + {ControlGroupStrings.invalidControlWarning.getDismissButton()} + , + ]} + /> + ); + }, [ + panels, + controlGroup, + tourStepOpen, + renderTourStep, + suppressTourChecked, + controlWithInvalidSelectionsId, + ]); + const [draggingId, setDraggingId] = useState(null); const draggingIndex = useMemo( () => (draggingId ? idsInOrder.indexOf(draggingId) : -1), @@ -117,6 +214,7 @@ export const ControlGroup = () => { alignItems="center" data-test-subj="controls-group" > + {tourStep} setDraggingId(active.id)} diff --git a/src/plugins/controls/public/control_group/control_group.scss b/src/plugins/controls/public/control_group/control_group.scss index d1fb6a8d495de..4ad7753a9cb53 100644 --- a/src/plugins/controls/public/control_group/control_group.scss +++ b/src/plugins/controls/public/control_group/control_group.scss @@ -199,3 +199,12 @@ $controlMinWidth: $euiSize * 14; top: (-$euiSizeXS) !important; } } + +.controlGroup--invalidSelectionsTour { + .controlGroup--suppressTourCheckbox { + height: 22px; + &Label { + font-weight: $euiFontWeightMedium; + } + } +} \ No newline at end of file diff --git a/src/plugins/controls/public/control_group/control_group_strings.ts b/src/plugins/controls/public/control_group/control_group_strings.ts index 996d86f7d676d..7b1b3ff3c1169 100644 --- a/src/plugins/controls/public/control_group/control_group_strings.ts +++ b/src/plugins/controls/public/control_group/control_group_strings.ts @@ -10,6 +10,42 @@ import { i18n } from '@kbn/i18n'; import { RANGE_SLIDER_CONTROL } from '../range_slider'; export const ControlGroupStrings = { + invalidControlWarning: { + getTourTitle: () => + i18n.translate('controls.controlGroup.invalidControlWarning.tourStepTitle.default', { + defaultMessage: 'Invalid selections are no longer ignored', + }), + getTourContent: (controlType: string) => { + switch (controlType) { + case RANGE_SLIDER_CONTROL: { + return i18n.translate( + 'controls.controlGroup.invalidControlWarning.tourStepContent.rangeSlider', + { + defaultMessage: 'The selected range is returning no results. Try changing the range.', + } + ); + } + default: { + return i18n.translate( + 'controls.controlGroup.invalidControlWarning.tourStepContent.default', + { + defaultMessage: + 'Some selections are returning no results. Try changing the selections.', + } + ); + } + } + }, + + getDismissButton: () => + i18n.translate('controls.controlGroup.invalidControlWarning.dismissButtonLabel', { + defaultMessage: 'Dismiss', + }), + getSuppressTourLabel: () => + i18n.translate('controls.controlGroup.invalidControlWarning.suppressTourLabel', { + defaultMessage: "Don't show again", + }), + }, manageControl: { getFlyoutCreateTitle: () => i18n.translate('controls.controlGroup.manageControl.createFlyoutTitle', { @@ -258,8 +294,7 @@ export const ControlGroupStrings = { }), getValidateSelectionsSubTitle: () => i18n.translate('controls.controlGroup.management.validate.subtitle', { - defaultMessage: - 'Automatically ignore any control selection that would result in no data.', + defaultMessage: 'Highlight control selections that result in no data.', }), }, controlChaining: { diff --git a/src/plugins/controls/public/control_group/embeddable/control_group_container.tsx b/src/plugins/controls/public/control_group/embeddable/control_group_container.tsx index dcb74fd606154..0663d3a3d9c61 100644 --- a/src/plugins/controls/public/control_group/embeddable/control_group_container.tsx +++ b/src/plugins/controls/public/control_group/embeddable/control_group_container.tsx @@ -5,6 +5,7 @@ * in compliance with, at your election, the Elastic License 2.0 or the Server * Side Public License, v 1. */ + import { compareFilters, COMPARE_ALL_OPTIONS, Filter, uniqFilters } from '@kbn/es-query'; import { isEqual, pick } from 'lodash'; import React, { createContext, useContext } from 'react'; @@ -24,6 +25,7 @@ import { persistableControlGroupInputKeys, } from '../../../common'; import { pluginServices } from '../../services'; +import { ControlsStorageService } from '../../services/storage/types'; import { ControlEmbeddable, ControlInput, ControlOutput } from '../../types'; import { ControlGroup } from '../component/control_group_component'; import { openAddDataControlFlyout } from '../editor/open_add_data_control_flyout'; @@ -86,11 +88,15 @@ export class ControlGroupContainer extends Container< private initialized$ = new BehaviorSubject(false); + private storageService: ControlsStorageService; + private subscriptions: Subscription = new Subscription(); private domNode?: HTMLElement; private recalculateFilters$: Subject; private relevantDataViewId?: string; private lastUsedDataViewId?: string; + private invalidSelectionsState: { [childId: string]: boolean }; + public diffingSubscription: Subscription = new Subscription(); // state management @@ -126,6 +132,8 @@ export class ControlGroupContainer extends Container< ControlGroupChainingSystems[initialInput.chainingSystem]?.getContainerSettings(initialInput) ); + ({ storage: this.storageService } = pluginServices.getServices()); + this.recalculateFilters$ = new Subject(); this.onFiltersPublished$ = new Subject(); this.onControlRemoved$ = new Subject(); @@ -153,6 +161,10 @@ export class ControlGroupContainer extends Container< this.store = reduxEmbeddableTools.store; + this.invalidSelectionsState = this.getChildIds().reduce((prev, id) => { + return { ...prev, [id]: false }; + }, {}); + // when all children are ready setup subscriptions this.untilAllChildrenReady().then(() => { this.recalculateDataViews(); @@ -164,6 +176,32 @@ export class ControlGroupContainer extends Container< this.fieldFilterPredicate = fieldFilterPredicate; } + public canShowInvalidSelectionsWarning = () => + this.storageService.getShowInvalidSelectionWarning() ?? true; + + public suppressInvalidSelectionsWarning = () => { + this.storageService.setShowInvalidSelectionWarning(false); + }; + + public reportInvalidSelections = ({ + id, + hasInvalidSelections, + }: { + id: string; + hasInvalidSelections: boolean; + }) => { + this.invalidSelectionsState = { ...this.invalidSelectionsState, [id]: hasInvalidSelections }; + + const childrenWithInvalidSelections = cachedChildEmbeddableOrder( + this.getInput().panels + ).idsInOrder.filter((childId) => { + return this.invalidSelectionsState[childId]; + }); + this.dispatch.setControlWithInvalidSelectionsId( + childrenWithInvalidSelections.length > 0 ? childrenWithInvalidSelections[0] : undefined + ); + }; + private setupSubscriptions = () => { /** * refresh control order cache and make all panels refreshInputFromParent whenever panel orders change @@ -201,7 +239,9 @@ export class ControlGroupContainer extends Container< * debounce output recalculation */ this.subscriptions.add( - this.recalculateFilters$.pipe(debounceTime(10)).subscribe(() => this.recalculateFilters()) + this.recalculateFilters$.pipe(debounceTime(10)).subscribe(() => { + this.recalculateFilters(); + }) ); }; @@ -211,9 +251,14 @@ export class ControlGroupContainer extends Container< } = this.getState(); if (!persistableControlGroupInputIsEqual(this.getPersistableInput(), lastSavedInput)) { this.updateInput(lastSavedInput); + this.reload(); // this forces the children to update their inputs + perform validation as necessary } } + public reload() { + super.reload(); + } + public getPersistableInput: () => PersistableControlGroupInput & { id: string } = () => { const input = this.getInput(); return pick(input, [...persistableControlGroupInputKeys, 'id']); @@ -284,13 +329,14 @@ export class ControlGroupContainer extends Container< private recalculateFilters = () => { const allFilters: Filter[] = []; let timeslice; - Object.values(this.children).map((child) => { + Object.values(this.children).map((child: ControlEmbeddable) => { const childOutput = child.getOutput() as ControlOutput; allFilters.push(...(childOutput?.filters ?? [])); if (childOutput.timeslice) { timeslice = childOutput.timeslice; } }); + // if filters are different, publish them if ( !compareFilters(this.output.filters ?? [], allFilters ?? [], COMPARE_ALL_OPTIONS) || diff --git a/src/plugins/controls/public/control_group/state/control_group_reducers.ts b/src/plugins/controls/public/control_group/state/control_group_reducers.ts index 617a708fb0fe2..313b21af5745b 100644 --- a/src/plugins/controls/public/control_group/state/control_group_reducers.ts +++ b/src/plugins/controls/public/control_group/state/control_group_reducers.ts @@ -19,6 +19,12 @@ export const controlGroupReducers = { ) => { state.componentState.lastSavedInput = action.payload; }, + setControlWithInvalidSelectionsId: ( + state: WritableDraft, + action: PayloadAction + ) => { + state.componentState.controlWithInvalidSelectionsId = action.payload; + }, setControlStyle: ( state: WritableDraft, action: PayloadAction diff --git a/src/plugins/controls/public/control_group/types.ts b/src/plugins/controls/public/control_group/types.ts index 699b003097fe1..c48aee3f1ace9 100644 --- a/src/plugins/controls/public/control_group/types.ts +++ b/src/plugins/controls/public/control_group/types.ts @@ -42,6 +42,7 @@ export interface ControlGroupSettings { export type ControlGroupComponentState = ControlGroupSettings & { lastSavedInput: PersistableControlGroupInput; + controlWithInvalidSelectionsId?: string; }; export { diff --git a/src/plugins/controls/public/options_list/components/options_list.scss b/src/plugins/controls/public/options_list/components/options_list.scss index 4dca925e952ce..fc1cdf68e3fec 100644 --- a/src/plugins/controls/public/options_list/components/options_list.scss +++ b/src/plugins/controls/public/options_list/components/options_list.scss @@ -15,14 +15,16 @@ font-weight: $euiFontWeightRegular; } - .optionsList__filterValid { + .optionsList__selections { + overflow: hidden !important; + } + + .optionsList__filter { font-weight: $euiFontWeightMedium; } .optionsList__filterInvalid { - color: $euiTextSubduedColor; - text-decoration: line-through; - font-weight: $euiFontWeightRegular; + color: $euiColorWarningText; } .optionsList__negateLabel { @@ -74,12 +76,11 @@ } .optionsList-control-ignored-selection-title { - padding-left: $euiSizeS; + padding-left: $euiSizeM; } .optionsList__selectionInvalid { - text-decoration: line-through; - color: $euiTextSubduedColor; + color: $euiColorWarningText; } .optionslist--loadingMoreGroupLabel { diff --git a/src/plugins/controls/public/options_list/components/options_list_control.test.tsx b/src/plugins/controls/public/options_list/components/options_list_control.test.tsx index 5bfe3edd16c57..65b94fc240eac 100644 --- a/src/plugins/controls/public/options_list/components/options_list_control.test.tsx +++ b/src/plugins/controls/public/options_list/components/options_list_control.test.tsx @@ -8,9 +8,7 @@ import React from 'react'; -import { mountWithIntl } from '@kbn/test-jest-helpers'; -import { findTestSubject } from '@elastic/eui/lib/test'; - +import { render } from '@testing-library/react'; import { OptionsListEmbeddableContext } from '../embeddable/options_list_embeddable'; import { OptionsListComponentState, OptionsListReduxState } from '../types'; import { ControlOutput, OptionsListEmbeddableInput } from '../..'; @@ -37,7 +35,7 @@ describe('Options list control', () => { output: options?.output ?? {}, } as Partial); - return mountWithIntl( + return render( @@ -48,15 +46,15 @@ describe('Options list control', () => { const control = await mountComponent({ explicitInput: { id: 'testExists', exclude: false, existsSelected: true }, }); - const existsOption = findTestSubject(control, 'optionsList-control-testExists'); - expect(existsOption.text()).toBe('Exists'); + const existsOption = control.getByTestId('optionsList-control-testExists'); + expect(existsOption).toHaveTextContent('Exists'); }); test('if exclude = true and existsSelected = true, then the option should read "Does not exist"', async () => { const control = await mountComponent({ explicitInput: { id: 'testDoesNotExist', exclude: true, existsSelected: true }, }); - const existsOption = findTestSubject(control, 'optionsList-control-testDoesNotExist'); - expect(existsOption.text()).toBe('DOES NOT Exist'); + const existsOption = control.getByTestId('optionsList-control-testDoesNotExist'); + expect(existsOption).toHaveTextContent('DOES NOT Exist'); }); }); diff --git a/src/plugins/controls/public/options_list/components/options_list_control.tsx b/src/plugins/controls/public/options_list/components/options_list_control.tsx index 5a358b29c6196..930348d9731fd 100644 --- a/src/plugins/controls/public/options_list/components/options_list_control.tsx +++ b/src/plugins/controls/public/options_list/components/options_list_control.tsx @@ -11,7 +11,15 @@ import classNames from 'classnames'; import { debounce, isEmpty } from 'lodash'; import React, { useCallback, useEffect, useMemo, useState } from 'react'; -import { EuiFilterButton, EuiFilterGroup, EuiInputPopover } from '@elastic/eui'; +import { + EuiFilterButton, + EuiFilterGroup, + EuiFlexGroup, + EuiFlexItem, + EuiInputPopover, + EuiToken, + EuiToolTip, +} from '@elastic/eui'; import { MAX_OPTIONS_LIST_REQUEST_SIZE } from '../types'; import { OptionsListStrings } from './options_list_strings'; @@ -34,7 +42,6 @@ export const OptionsListControl = ({ const error = optionsList.select((state) => state.componentState.error); const isPopoverOpen = optionsList.select((state) => state.componentState.popoverOpen); - const validSelections = optionsList.select((state) => state.componentState.validSelections); const invalidSelections = optionsList.select((state) => state.componentState.invalidSelections); const fieldSpec = optionsList.select((state) => state.componentState.field); @@ -90,74 +97,116 @@ export const OptionsListControl = ({ [loadMoreSubject] ); - const { hasSelections, selectionDisplayNode, validSelectionsCount } = useMemo(() => { - const delimiter = OptionsListStrings.control.getSeparator(fieldSpec?.type); + const delimiter = useMemo( + () => OptionsListStrings.control.getSeparator(fieldSpec?.type), + [fieldSpec?.type] + ); + const { hasSelections, selectionDisplayNode, selectedOptionsCount } = useMemo(() => { return { - hasSelections: !isEmpty(validSelections) || !isEmpty(invalidSelections), - validSelectionsCount: validSelections?.length, + hasSelections: !isEmpty(selectedOptions), + selectedOptionsCount: selectedOptions?.length, selectionDisplayNode: ( - <> - {exclude && ( - <> - - {existsSelected - ? OptionsListStrings.control.getExcludeExists() - : OptionsListStrings.control.getNegate()} - {' '} - - )} - {existsSelected ? ( - - {OptionsListStrings.controlAndPopover.getExists(+Boolean(exclude))} - - ) : ( - <> - {validSelections?.length ? ( - - {validSelections.map((value) => fieldFormatter(value)).join(delimiter)} - - ) : null} - {validSelections?.length && invalidSelections?.length ? delimiter : null} - {invalidSelections?.length ? ( - - {invalidSelections.map((value) => fieldFormatter(value)).join(delimiter)} + + +
+ {exclude && ( + <> + + {existsSelected + ? OptionsListStrings.control.getExcludeExists() + : OptionsListStrings.control.getNegate()} + {' '} + + )} + {existsSelected ? ( + + {OptionsListStrings.controlAndPopover.getExists(+Boolean(exclude))} - ) : null} - + ) : ( + <> + {selectedOptions?.length + ? selectedOptions.map((value: string, i, { length }) => { + const text = `${fieldFormatter(value)}${ + i + 1 === length ? '' : delimiter + } `; + const isInvalid = invalidSelections?.includes(value); + return ( + + {text} + + ); + }) + : null} + + )} +
+
+ {invalidSelections && invalidSelections.length > 0 && ( + + + + + )} - +
), }; - }, [ - exclude, - existsSelected, - validSelections, - invalidSelections, - fieldFormatter, - fieldSpec?.type, - ]); + }, [selectedOptions, exclude, existsSelected, fieldFormatter, delimiter, invalidSelections]); const button = ( - optionsList.dispatch.setPopoverOpen(!isPopoverOpen)} - isSelected={isPopoverOpen} - numActiveFilters={validSelectionsCount} - hasActiveFilters={Boolean(validSelectionsCount)} - textProps={{ className: 'optionsList--selectionText' }} - > - {hasSelections || existsSelected - ? selectionDisplayNode - : placeholder ?? OptionsListStrings.control.getPlaceholder()} - + <> + optionsList.dispatch.setPopoverOpen(!isPopoverOpen)} + isSelected={isPopoverOpen} + numActiveFilters={selectedOptionsCount} + hasActiveFilters={Boolean(selectedOptionsCount)} + aria-label={`${selectedOptions + ?.map((value) => { + const isInvalid = invalidSelections?.includes(value); + return `${ + isInvalid + ? OptionsListStrings.popover.getInvalidSelectionScreenReaderText() + ' ' + : '' + }${fieldFormatter(value)}`; + }) + .join(delimiter)}`} + textProps={{ className: 'optionsList--selectionText' }} + > + {hasSelections || existsSelected + ? selectionDisplayNode + : placeholder ?? OptionsListStrings.control.getPlaceholder()} + + ); return error ? ( diff --git a/src/plugins/controls/public/options_list/components/options_list_popover.test.tsx b/src/plugins/controls/public/options_list/components/options_list_popover.test.tsx index 59f1c9f2b058c..8b39efbc040c2 100644 --- a/src/plugins/controls/public/options_list/components/options_list_popover.test.tsx +++ b/src/plugins/controls/public/options_list/components/options_list_popover.test.tsx @@ -7,10 +7,9 @@ */ import React from 'react'; -import { ReactWrapper } from 'enzyme'; +import { render, RenderResult, within } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; -import { mountWithIntl } from '@kbn/test-jest-helpers'; -import { findTestSubject } from '@elastic/eui/lib/test'; import { FieldSpec } from '@kbn/data-views-plugin/common'; import { pluginServices } from '../../services'; @@ -42,41 +41,36 @@ describe('Options list popover', () => { output: options?.output ?? {}, } as Partial); - return mountWithIntl( + return render( ); } - const clickShowOnlySelections = (popover: ReactWrapper) => { - const showOnlySelectedButton = findTestSubject( - popover, - 'optionsList-control-show-only-selected' - ); - showOnlySelectedButton.simulate('click'); + const clickShowOnlySelections = (popover: RenderResult) => { + const showOnlySelectedButton = popover.getByTestId('optionsList-control-show-only-selected'); + userEvent.click(showOnlySelectedButton); }; test('no available options', async () => { const popover = await mountComponent({ componentState: { availableOptions: [] } }); - const availableOptionsDiv = findTestSubject(popover, 'optionsList-control-available-options'); - const noOptionsDiv = findTestSubject( - availableOptionsDiv, + const availableOptionsDiv = popover.getByTestId('optionsList-control-available-options'); + const noOptionsDiv = within(availableOptionsDiv).getByTestId( 'optionsList-control-noSelectionsMessage' ); - expect(noOptionsDiv.exists()).toBeTruthy(); + expect(noOptionsDiv).toBeInTheDocument(); }); describe('show only selected', () => { test('display error message when the show only selected toggle is true but there are no selections', async () => { const popover = await mountComponent(); clickShowOnlySelections(popover); - const availableOptionsDiv = findTestSubject(popover, 'optionsList-control-available-options'); - const noSelectionsDiv = findTestSubject( - availableOptionsDiv, + const availableOptionsDiv = popover.getByTestId('optionsList-control-available-options'); + const noSelectionsDiv = within(availableOptionsDiv).getByTestId( 'optionsList-control-selectionsEmptyMessage' ); - expect(noSelectionsDiv.exists()).toBeTruthy(); + expect(noSelectionsDiv).toBeInTheDocument(); }); test('show only selected options', async () => { @@ -85,11 +79,11 @@ describe('Options list popover', () => { explicitInput: { selectedOptions: selections }, }); clickShowOnlySelections(popover); - const availableOptions = popover.find( - '[data-test-subj="optionsList-control-available-options"] ul' - ); - availableOptions.children().forEach((child, i) => { - expect(child.text()).toBe(`${selections[i]}. Checked option.`); + const availableOptionsDiv = popover.getByTestId('optionsList-control-available-options'); + const availableOptionsList = within(availableOptionsDiv).getByRole('listbox'); + const availableOptions = within(availableOptionsList).getAllByRole('option'); + availableOptions.forEach((child, i) => { + expect(child).toHaveTextContent(`${selections[i]}. Checked option.`); }); }); @@ -99,16 +93,16 @@ describe('Options list popover', () => { explicitInput: { selectedOptions: selections }, componentState: { field: { type: 'string' } as any as FieldSpec }, }); - let searchBox = findTestSubject(popover, 'optionsList-control-search-input'); - let sortButton = findTestSubject(popover, 'optionsListControl__sortingOptionsButton'); - expect(searchBox.prop('disabled')).toBeFalsy(); - expect(sortButton.prop('disabled')).toBeFalsy(); + let searchBox = popover.getByTestId('optionsList-control-search-input'); + let sortButton = popover.getByTestId('optionsListControl__sortingOptionsButton'); + expect(searchBox).not.toBeDisabled(); + expect(sortButton).not.toBeDisabled(); clickShowOnlySelections(popover); - searchBox = findTestSubject(popover, 'optionsList-control-search-input'); - sortButton = findTestSubject(popover, 'optionsListControl__sortingOptionsButton'); - expect(searchBox.prop('disabled')).toBe(true); - expect(sortButton.prop('disabled')).toBe(true); + searchBox = popover.getByTestId('optionsList-control-search-input'); + sortButton = popover.getByTestId('optionsListControl__sortingOptionsButton'); + expect(searchBox).toBeDisabled(); + expect(sortButton).toBeDisabled(); }); }); @@ -124,23 +118,16 @@ describe('Options list popover', () => { invalidSelections: ['woof'], }, }); - const validSelection = findTestSubject(popover, 'optionsList-control-selection-bark'); - expect(validSelection.find('.euiSelectableListItem__text').text()).toEqual( - 'bark. Checked option.' - ); + const validSelection = popover.getByTestId('optionsList-control-selection-bark'); + expect(validSelection).toHaveTextContent('bark. Checked option.'); expect( - validSelection.find('div[data-test-subj="optionsList-document-count-badge"]').text().trim() - ).toEqual('75'); - const title = findTestSubject(popover, 'optionList__ignoredSelectionLabel').text(); - expect(title).toEqual('Ignored selection'); - const invalidSelection = findTestSubject( - popover, - 'optionsList-control-ignored-selection-woof' - ); - expect(invalidSelection.find('.euiSelectableListItem__text').text()).toEqual( - 'woof. Checked option.' - ); - expect(invalidSelection.hasClass('optionsList__selectionInvalid')).toBe(true); + within(validSelection).getByTestId('optionsList-document-count-badge') + ).toHaveTextContent('75'); + const title = popover.getByTestId('optionList__invalidSelectionLabel'); + expect(title).toHaveTextContent('Invalid selection'); + const invalidSelection = popover.getByTestId('optionsList-control-invalid-selection-woof'); + expect(invalidSelection).toHaveTextContent('woof. Checked option.'); + expect(invalidSelection).toHaveClass('optionsList__selectionInvalid'); }); test('test title when multiple invalid selections', async () => { @@ -152,28 +139,28 @@ describe('Options list popover', () => { invalidSelections: ['woof', 'meow'], }, }); - const title = findTestSubject(popover, 'optionList__ignoredSelectionLabel').text(); - expect(title).toEqual('Ignored selections'); + const title = popover.getByTestId('optionList__invalidSelectionLabel'); + expect(title).toHaveTextContent('Invalid selections'); }); }); describe('include/exclude toggle', () => { test('should default to exclude = false', async () => { const popover = await mountComponent(); - const includeButton = findTestSubject(popover, 'optionsList__includeResults'); - const excludeButton = findTestSubject(popover, 'optionsList__excludeResults'); - expect(includeButton.prop('aria-pressed')).toBe(true); - expect(excludeButton.prop('aria-pressed')).toBe(false); + const includeButton = popover.getByTestId('optionsList__includeResults'); + const excludeButton = popover.getByTestId('optionsList__excludeResults'); + expect(includeButton).toHaveAttribute('aria-pressed', 'true'); + expect(excludeButton).toHaveAttribute('aria-pressed', 'false'); }); test('if exclude = true, select appropriate button in button group', async () => { const popover = await mountComponent({ explicitInput: { exclude: true }, }); - const includeButton = findTestSubject(popover, 'optionsList__includeResults'); - const excludeButton = findTestSubject(popover, 'optionsList__excludeResults'); - expect(includeButton.prop('aria-pressed')).toBe(false); - expect(excludeButton.prop('aria-pressed')).toBe(true); + const includeButton = popover.getByTestId('optionsList__includeResults'); + const excludeButton = popover.getByTestId('optionsList__excludeResults'); + expect(includeButton).toHaveAttribute('aria-pressed', 'false'); + expect(excludeButton).toHaveAttribute('aria-pressed', 'true'); }); }); @@ -182,14 +169,16 @@ describe('Options list popover', () => { const popover = await mountComponent({ explicitInput: { existsSelected: true }, }); - const woofOption = findTestSubject(popover, 'optionsList-control-selection-woof'); - woofOption.simulate('click'); + const woofOption = popover.getByTestId('optionsList-control-selection-woof'); + userEvent.click(woofOption); - const availableOptionsDiv = findTestSubject(popover, 'optionsList-control-available-options'); - availableOptionsDiv.children().forEach((child, i) => { - if (child.text() === 'woof') expect(child.prop('aria-pressed')).toBe(true); - else expect(child.prop('aria-pressed')).toBeFalsy(); + const availableOptionsDiv = popover.getByTestId('optionsList-control-available-options'); + const availableOptionsList = within(availableOptionsDiv).getByRole('listbox'); + const selectedOptions = within(availableOptionsList).getAllByRole('option', { + checked: true, }); + expect(selectedOptions).toHaveLength(1); + expect(selectedOptions[0]).toHaveTextContent('woof. Checked option.'); }); test('clicking "Exists" unselects all other selections', async () => { @@ -197,19 +186,18 @@ describe('Options list popover', () => { const popover = await mountComponent({ explicitInput: { existsSelected: false, selectedOptions: selections }, }); - const existsOption = findTestSubject(popover, 'optionsList-control-selection-exists'); - let availableOptionsDiv = findTestSubject(popover, 'optionsList-control-available-options'); - availableOptionsDiv.children().forEach((child, i) => { - if (selections.includes(child.text())) expect(child.prop('aria-pressed')).toBe(true); - else expect(child.prop('aria-pressed')).toBeFalsy(); - }); - - existsOption.simulate('click'); - availableOptionsDiv = findTestSubject(popover, 'optionsList-control-available-options'); - availableOptionsDiv.children().forEach((child, i) => { - if (child.text() === 'Exists (*)') expect(child.prop('aria-pressed')).toBe(true); - else expect(child.prop('aria-pressed')).toBeFalsy(); - }); + const existsOption = popover.getByTestId('optionsList-control-selection-exists'); + let availableOptionsDiv = popover.getByTestId('optionsList-control-available-options'); + let checkedOptions = within(availableOptionsDiv).getAllByRole('option', { checked: true }); + expect(checkedOptions).toHaveLength(2); + expect(checkedOptions[0]).toHaveTextContent('woof. Checked option.'); + expect(checkedOptions[1]).toHaveTextContent('bark. Checked option.'); + + userEvent.click(existsOption); + availableOptionsDiv = popover.getByTestId('optionsList-control-available-options'); + checkedOptions = within(availableOptionsDiv).getAllByRole('option', { checked: true }); + expect(checkedOptions).toHaveLength(1); + expect(checkedOptions[0]).toHaveTextContent('Exists. Checked option.'); }); test('if existsSelected = false and no suggestions, then "Exists" does not show up', async () => { @@ -217,8 +205,8 @@ describe('Options list popover', () => { componentState: { availableOptions: [] }, explicitInput: { existsSelected: false }, }); - const existsOption = findTestSubject(popover, 'optionsList-control-selection-exists'); - expect(existsOption.exists()).toBeFalsy(); + const existsOption = popover.queryByTestId('optionsList-control-selection-exists'); + expect(existsOption).toBeNull(); }); test('if existsSelected = true, "Exists" is the only option when "Show only selected options" is toggled', async () => { @@ -226,10 +214,10 @@ describe('Options list popover', () => { explicitInput: { existsSelected: true }, }); clickShowOnlySelections(popover); - const availableOptions = popover.find( - '[data-test-subj="optionsList-control-available-options"] ul' - ); - expect(availableOptions.text()).toBe('Exists. Checked option.'); + const availableOptionsDiv = popover.getByTestId('optionsList-control-available-options'); + const availableOptionsList = within(availableOptionsDiv).getByRole('listbox'); + const availableOptions = within(availableOptionsList).getAllByRole('option'); + expect(availableOptions[0]).toHaveTextContent('Exists. Checked option.'); }); }); @@ -240,11 +228,13 @@ describe('Options list popover', () => { field: { name: 'Test keyword field', type: 'keyword' } as FieldSpec, }, }); - const sortButton = findTestSubject(popover, 'optionsListControl__sortingOptionsButton'); - sortButton.simulate('click'); + const sortButton = popover.getByTestId('optionsListControl__sortingOptionsButton'); + userEvent.click(sortButton); - const sortingOptionsDiv = findTestSubject(popover, 'optionsListControl__sortingOptions'); - const optionsText = sortingOptionsDiv.find('ul li').map((element) => element.text().trim()); + const sortingOptionsDiv = popover.getByTestId('optionsListControl__sortingOptions'); + const optionsText = within(sortingOptionsDiv) + .getAllByRole('option') + .map((el) => el.textContent); expect(optionsText).toEqual(['By document count. Checked option.', 'Alphabetically']); }); @@ -255,16 +245,18 @@ describe('Options list popover', () => { field: { name: 'Test keyword field', type: 'keyword' } as FieldSpec, }, }); - const sortButton = findTestSubject(popover, 'optionsListControl__sortingOptionsButton'); - sortButton.simulate('click'); + const sortButton = popover.getByTestId('optionsListControl__sortingOptionsButton'); + userEvent.click(sortButton); - const sortingOptionsDiv = findTestSubject(popover, 'optionsListControl__sortingOptions'); - const optionsText = sortingOptionsDiv.find('ul li').map((element) => element.text().trim()); + const sortingOptionsDiv = popover.getByTestId('optionsListControl__sortingOptions'); + const optionsText = within(sortingOptionsDiv) + .getAllByRole('option') + .map((el) => el.textContent); expect(optionsText).toEqual(['By document count', 'Alphabetically. Checked option.']); - const ascendingButton = findTestSubject(popover, 'optionsList__sortOrder_asc').instance(); + const ascendingButton = popover.getByTestId('optionsList__sortOrder_asc'); expect(ascendingButton).toHaveClass('euiButtonGroupButton-isSelected'); - const descendingButton = findTestSubject(popover, 'optionsList__sortOrder_desc').instance(); + const descendingButton = popover.getByTestId('optionsList__sortOrder_desc'); expect(descendingButton).not.toHaveClass('euiButtonGroupButton-isSelected'); }); @@ -272,11 +264,13 @@ describe('Options list popover', () => { const popover = await mountComponent({ componentState: { field: { name: 'Test IP field', type: 'ip' } as FieldSpec }, }); - const sortButton = findTestSubject(popover, 'optionsListControl__sortingOptionsButton'); - sortButton.simulate('click'); + const sortButton = popover.getByTestId('optionsListControl__sortingOptionsButton'); + userEvent.click(sortButton); - const sortingOptionsDiv = findTestSubject(popover, 'optionsListControl__sortingOptions'); - const optionsText = sortingOptionsDiv.find('ul li').map((element) => element.text().trim()); + const sortingOptionsDiv = popover.getByTestId('optionsListControl__sortingOptions'); + const optionsText = within(sortingOptionsDiv) + .getAllByRole('option') + .map((el) => el.textContent); expect(optionsText).toEqual(['By document count. Checked option.']); }); @@ -284,11 +278,13 @@ describe('Options list popover', () => { const popover = await mountComponent({ componentState: { field: { name: 'Test date field', type: 'date' } as FieldSpec }, }); - const sortButton = findTestSubject(popover, 'optionsListControl__sortingOptionsButton'); - sortButton.simulate('click'); + const sortButton = popover.getByTestId('optionsListControl__sortingOptionsButton'); + userEvent.click(sortButton); - const sortingOptionsDiv = findTestSubject(popover, 'optionsListControl__sortingOptions'); - const optionsText = sortingOptionsDiv.find('ul li').map((element) => element.text().trim()); + const sortingOptionsDiv = popover.getByTestId('optionsListControl__sortingOptions'); + const optionsText = within(sortingOptionsDiv) + .getAllByRole('option') + .map((el) => el.textContent); expect(optionsText).toEqual(['By document count. Checked option.', 'By date']); }); @@ -296,11 +292,13 @@ describe('Options list popover', () => { const popover = await mountComponent({ componentState: { field: { name: 'Test number field', type: 'number' } as FieldSpec }, }); - const sortButton = findTestSubject(popover, 'optionsListControl__sortingOptionsButton'); - sortButton.simulate('click'); + const sortButton = popover.getByTestId('optionsListControl__sortingOptionsButton'); + userEvent.click(sortButton); - const sortingOptionsDiv = findTestSubject(popover, 'optionsListControl__sortingOptions'); - const optionsText = sortingOptionsDiv.find('ul li').map((element) => element.text().trim()); + const sortingOptionsDiv = popover.getByTestId('optionsListControl__sortingOptions'); + const optionsText = within(sortingOptionsDiv) + .getAllByRole('option') + .map((el) => el.textContent); expect(optionsText).toEqual(['By document count. Checked option.', 'Numerically']); }); }); @@ -310,8 +308,8 @@ describe('Options list popover', () => { const popover = await mountComponent({ componentState: { field: { name: 'Test keyword field', type: 'keyword' } as FieldSpec }, }); - const warning = findTestSubject(popover, 'optionsList-allow-expensive-queries-warning'); - expect(warning).toEqual({}); + const warning = popover.queryByTestId('optionsList-allow-expensive-queries-warning'); + expect(warning).toBeNull(); }); test('ensure warning icon shows up when testAllowExpensiveQueries = false', async () => { @@ -324,8 +322,8 @@ describe('Options list popover', () => { allowExpensiveQueries: false, }, }); - const warning = findTestSubject(popover, 'optionsList-allow-expensive-queries-warning'); - expect(warning.getDOMNode()).toBeInstanceOf(HTMLDivElement); + const warning = popover.getByTestId('optionsList-allow-expensive-queries-warning'); + expect(warning).toBeInstanceOf(HTMLDivElement); }); }); @@ -340,8 +338,8 @@ describe('Options list popover', () => { const popover = await mountComponent({ explicitInput, }); - const test = findTestSubject(popover, testSubject); - expect(test.exists()).toBeFalsy(); + const test = popover.queryByTestId(testSubject); + expect(test).toBeNull(); }; test('can hide exists option', async () => { diff --git a/src/plugins/controls/public/options_list/components/options_list_popover_invalid_selections.tsx b/src/plugins/controls/public/options_list/components/options_list_popover_invalid_selections.tsx index 7ff64482b2c60..4de8a2f483ea0 100644 --- a/src/plugins/controls/public/options_list/components/options_list_popover_invalid_selections.tsx +++ b/src/plugins/controls/public/options_list/components/options_list_popover_invalid_selections.tsx @@ -9,16 +9,19 @@ import React, { useEffect, useState } from 'react'; import { - EuiSelectableOption, + EuiFlexGroup, + EuiFlexItem, + EuiIcon, + EuiScreenReaderOnly, EuiSelectable, + EuiSelectableOption, EuiSpacer, EuiTitle, - EuiScreenReaderOnly, } from '@elastic/eui'; -import { OptionsListStrings } from './options_list_strings'; -import { useOptionsList } from '../embeddable/options_list_embeddable'; import { useFieldFormatter } from '../../hooks/use_field_formatter'; +import { useOptionsList } from '../embeddable/options_list_embeddable'; +import { OptionsListStrings } from './options_list_strings'; export const OptionsListPopoverInvalidSelections = () => { const optionsList = useOptionsList(); @@ -40,7 +43,7 @@ export const OptionsListPopoverInvalidSelections = () => { label: fieldFormatter(key), checked: 'on', className: 'optionsList__selectionInvalid', - 'data-test-subj': `optionsList-control-ignored-selection-${key}`, + 'data-test-subj': `optionsList-control-invalid-selection-${key}`, prepend: (
@@ -60,13 +63,25 @@ export const OptionsListPopoverInvalidSelections = () => { - + + + + + + + + + i18n.translate('controls.optionsList.control.invalidSelectionWarningLabel', { + defaultMessage: + '{invalidSelectionCount} {invalidSelectionCount, plural, one {selection returns} other {selections return}} no results.', + values: { + invalidSelectionCount, + }, + }), }, editor: { getSelectionOptionsTitle: () => @@ -43,7 +51,7 @@ export const OptionsListStrings = { multi: { getLabel: () => i18n.translate('controls.optionsList.editor.multiSelectLabel', { - defaultMessage: 'Allow multiple selections', + defaultMessage: 'Allow multiple selections', }), }, single: { @@ -195,19 +203,19 @@ export const OptionsListStrings = { getInvalidSelectionsSectionAriaLabel: (fieldName: string, invalidSelectionCount: number) => i18n.translate('controls.optionsList.popover.invalidSelectionsAriaLabel', { defaultMessage: - 'Ignored {invalidSelectionCount, plural, one {selection} other {selections}} for {fieldName}', + 'Invalid {invalidSelectionCount, plural, one {selection} other {selections}} for {fieldName}', values: { fieldName, invalidSelectionCount }, }), getInvalidSelectionsSectionTitle: (invalidSelectionCount: number) => i18n.translate('controls.optionsList.popover.invalidSelectionsSectionTitle', { defaultMessage: - 'Ignored {invalidSelectionCount, plural, one {selection} other {selections}}', + 'Invalid {invalidSelectionCount, plural, one {selection} other {selections}}', values: { invalidSelectionCount }, }), getInvalidSelectionsLabel: (selectedOptions: number) => i18n.translate('controls.optionsList.popover.invalidSelectionsLabel', { defaultMessage: - '{selectedOptions} {selectedOptions, plural, one {selection} other {selections}} ignored', + '{selectedOptions} {selectedOptions, plural, one {selection} other {selections}} invalid', values: { selectedOptions }, }), getInvalidSelectionScreenReaderText: () => diff --git a/src/plugins/controls/public/options_list/embeddable/options_list_embeddable.tsx b/src/plugins/controls/public/options_list/embeddable/options_list_embeddable.tsx index 0426523c09107..ea3d4d80399c5 100644 --- a/src/plugins/controls/public/options_list/embeddable/options_list_embeddable.tsx +++ b/src/plugins/controls/public/options_list/embeddable/options_list_embeddable.tsx @@ -6,41 +6,42 @@ * Side Public License, v 1. */ -import ReactDOM from 'react-dom'; -import { batch } from 'react-redux'; import deepEqual from 'fast-deep-equal'; import { isEmpty, isEqual } from 'lodash'; -import { merge, Subject, Subscription, switchMap, tap } from 'rxjs'; import React, { createContext, useContext } from 'react'; -import { debounceTime, map, distinctUntilChanged, skip } from 'rxjs/operators'; +import ReactDOM from 'react-dom'; +import { batch } from 'react-redux'; +import { merge, Subject, Subscription, switchMap, tap } from 'rxjs'; +import { debounceTime, distinctUntilChanged, map, skip } from 'rxjs/operators'; +import { DataView, FieldSpec } from '@kbn/data-views-plugin/public'; +import { Embeddable, IContainer } from '@kbn/embeddable-plugin/public'; import { - Filter, - compareFilters, + buildExistsFilter, buildPhraseFilter, buildPhrasesFilter, + compareFilters, COMPARE_ALL_OPTIONS, - buildExistsFilter, + Filter, } from '@kbn/es-query'; import { i18n } from '@kbn/i18n'; -import { DataView, FieldSpec } from '@kbn/data-views-plugin/public'; -import { Embeddable, IContainer } from '@kbn/embeddable-plugin/public'; -import { KibanaThemeProvider } from '@kbn/react-kibana-context-theme'; import { ReduxEmbeddableTools, ReduxToolsPackage } from '@kbn/presentation-util-plugin/public'; +import { KibanaThemeProvider } from '@kbn/react-kibana-context-theme'; import { + ControlGroupContainer, ControlInput, ControlOutput, - OPTIONS_LIST_CONTROL, OptionsListEmbeddableInput, + OPTIONS_LIST_CONTROL, } from '../..'; import { pluginServices } from '../../services'; -import { IClearableControl } from '../../types'; -import { OptionsListControl } from '../components/options_list_control'; import { ControlsDataViewsService } from '../../services/data_views/types'; import { ControlsOptionsListService } from '../../services/options_list/types'; -import { MIN_OPTIONS_LIST_REQUEST_SIZE, OptionsListReduxState } from '../types'; +import { IClearableControl } from '../../types'; +import { OptionsListControl } from '../components/options_list_control'; import { getDefaultComponentState, optionsListReducers } from '../options_list_reducers'; +import { MIN_OPTIONS_LIST_REQUEST_SIZE, OptionsListReduxState } from '../types'; const diffDataFetchProps = ( last?: OptionsListDataFetchProps, @@ -83,6 +84,7 @@ export class OptionsListEmbeddable { public readonly type = OPTIONS_LIST_CONTROL; public deferEmbeddableLoad = true; + public parent: ControlGroupContainer; private subscriptions: Subscription = new Subscription(); private node?: HTMLElement; @@ -113,6 +115,7 @@ export class OptionsListEmbeddable parent?: IContainer ) { super(input, output, parent); + this.parent = parent as ControlGroupContainer; // Destructure controls services ({ dataViews: this.dataViewsService, optionsList: this.optionsListService } = @@ -130,7 +133,6 @@ export class OptionsListEmbeddable reducers: optionsListReducers, initialComponentState: getDefaultComponentState(), }); - this.select = reduxEmbeddableTools.select; this.getState = reduxEmbeddableTools.getState; this.dispatch = reduxEmbeddableTools.dispatch; @@ -142,17 +144,17 @@ export class OptionsListEmbeddable private initialize = async () => { const { selectedOptions: initialSelectedOptions } = this.getInput(); - if (!initialSelectedOptions) this.setInitializationFinished(); + if (initialSelectedOptions) { + const filters = await this.buildFilter(); + this.dispatch.publishFilters(filters); + } + this.setInitializationFinished(); this.dispatch.setAllowExpensiveQueries( await this.optionsListService.getAllowExpensiveQueries() ); this.runOptionsListQuery().then(async () => { - if (initialSelectedOptions) { - await this.buildFilter(); - this.setInitializationFinished(); - } this.setupSubscriptions(); }); }; @@ -324,6 +326,7 @@ export class OptionsListEmbeddable }, this.abortController.signal ); + if (this.optionsListService.optionsListResponseWasFailure(response)) { if (response.error === 'aborted') { // This prevents an aborted request (which can happen, for example, when a user types a search string too quickly) @@ -347,6 +350,7 @@ export class OptionsListEmbeddable validSelections: selectedOptions, totalCardinality, }); + this.reportInvalidSelections(false); } else { const valid: string[] = []; const invalid: string[] = []; @@ -360,14 +364,12 @@ export class OptionsListEmbeddable validSelections: valid, totalCardinality, }); + this.reportInvalidSelections(true); } - // publish filter - const newFilters = await this.buildFilter(); batch(() => { this.dispatch.setErrorMessage(undefined); this.dispatch.setLoading(false); - this.dispatch.publishFilters(newFilters); }); } else { batch(() => { @@ -380,12 +382,18 @@ export class OptionsListEmbeddable } }; + private reportInvalidSelections = (hasInvalidSelections: boolean) => { + this.parent?.reportInvalidSelections({ + id: this.id, + hasInvalidSelections, + }); + }; + private buildFilter = async () => { - const { validSelections } = this.getState().componentState ?? {}; - const { existsSelected } = this.getState().explicitInput ?? {}; + const { existsSelected, selectedOptions } = this.getState().explicitInput ?? {}; const { exclude } = this.getInput(); - if ((!validSelections || isEmpty(validSelections)) && !existsSelected) { + if ((!selectedOptions || isEmpty(selectedOptions)) && !existsSelected) { return []; } const { dataView, field } = await this.getCurrentDataViewAndField(); @@ -394,11 +402,11 @@ export class OptionsListEmbeddable let newFilter: Filter | undefined; if (existsSelected) { newFilter = buildExistsFilter(field, dataView); - } else if (validSelections) { - if (validSelections.length === 1) { - newFilter = buildPhraseFilter(field, validSelections[0], dataView); + } else if (selectedOptions) { + if (selectedOptions.length === 1) { + newFilter = buildPhraseFilter(field, selectedOptions[0], dataView); } else { - newFilter = buildPhrasesFilter(field, validSelections, dataView); + newFilter = buildPhrasesFilter(field, selectedOptions, dataView); } } @@ -411,6 +419,7 @@ export class OptionsListEmbeddable public clearSelections() { this.dispatch.clearSelections({}); + this.reportInvalidSelections(false); } reload = () => { diff --git a/src/plugins/controls/public/options_list/options_list_reducers.ts b/src/plugins/controls/public/options_list/options_list_reducers.ts index 3300072c089f9..488a20313c8d9 100644 --- a/src/plugins/controls/public/options_list/options_list_reducers.ts +++ b/src/plugins/controls/public/options_list/options_list_reducers.ts @@ -50,6 +50,12 @@ export const optionsListReducers = { ) => { state.componentState.allowExpensiveQueries = action.payload; }, + setInvalidSelectionWarningOpen: ( + state: WritableDraft, + action: PayloadAction + ) => { + state.componentState.showInvalidSelectionWarning = action.payload; + }, setPopoverOpen: (state: WritableDraft, action: PayloadAction) => { state.componentState.popoverOpen = action.payload; }, diff --git a/src/plugins/controls/public/options_list/types.ts b/src/plugins/controls/public/options_list/types.ts index 30b561d5e7964..da3f52a4cb870 100644 --- a/src/plugins/controls/public/options_list/types.ts +++ b/src/plugins/controls/public/options_list/types.ts @@ -34,6 +34,7 @@ export interface OptionsListComponentState { popoverOpen: boolean; field?: FieldSpec; error?: string; + showInvalidSelectionWarning?: boolean; } // public only - redux embeddable state type diff --git a/src/plugins/controls/public/range_slider/components/range_slider.scss b/src/plugins/controls/public/range_slider/components/range_slider.scss index c386705382d87..33795ea6f5286 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider.scss +++ b/src/plugins/controls/public/range_slider/components/range_slider.scss @@ -1,14 +1,30 @@ .rangeSliderAnchor__button { .euiFormControlLayout { + align-items: center; box-shadow: none; background-color: transparent; - padding: 0 0 2px 0; .euiFormControlLayout__childrenWrapper { border-top-left-radius: 0; border-bottom-left-radius: 0; border-top-right-radius: $euiBorderRadius - 1px; border-bottom-right-radius: $euiBorderRadius - 1px; + + .euiFormControlLayoutDelimited__delimiter, .euiFormControlLayoutIcons--static { + height: auto !important; + } + } + } + + .rangeSlider__invalidToken { + height: $euiSizeS * 2; + padding: 0 $euiSizeS; + + .euiIcon { + background-color: transparent; + width: $euiSizeS * 2; + border-radius: $euiSizeXS; + padding: 0 calc($euiSizeXS / 2); } } } @@ -24,9 +40,7 @@ } &.rangeSliderAnchor__fieldNumber--invalid { - color: $euiTextSubduedColor; - text-decoration: line-through; - font-weight: $euiFontWeightRegular; + color: $euiColorWarningText; } &:placeholder-shown, &::placeholder { @@ -34,4 +48,4 @@ color: $euiTextSubduedColor; text-decoration: none; } -} \ No newline at end of file +} diff --git a/src/plugins/controls/public/range_slider/components/range_slider_control.tsx b/src/plugins/controls/public/range_slider/components/range_slider_control.tsx index cd879961c5f10..86bf298da91c7 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider_control.tsx +++ b/src/plugins/controls/public/range_slider/components/range_slider_control.tsx @@ -9,7 +9,7 @@ import { debounce } from 'lodash'; import React, { FC, useState, useMemo, useEffect, useCallback, useRef } from 'react'; -import { EuiRangeTick, EuiDualRange, EuiDualRangeProps } from '@elastic/eui'; +import { EuiRangeTick, EuiDualRange, EuiDualRangeProps, EuiToken, EuiToolTip } from '@elastic/eui'; import { RangeValue } from '../../../common/range_slider/types'; import { useRangeSlider } from '../embeddable/range_slider_embeddable'; @@ -18,6 +18,7 @@ import { ControlError } from '../../control_group/component/control_error_compon import './range_slider.scss'; import { MIN_POPOVER_WIDTH } from '../../constants'; import { useFieldFormatter } from '../../hooks/use_field_formatter'; +import { RangeSliderStrings } from './range_slider_strings'; export const RangeSliderControl: FC = () => { /** Controls Services Context */ @@ -164,6 +165,27 @@ export const RangeSliderControl: FC = () => { inputPopoverProps={{ panelMinWidth: MIN_POPOVER_WIDTH, }} + append={ + isInvalid ? ( +
+ + + +
+ ) : undefined + } onMouseUp={() => { // when the pin is dropped (on mouse up), cancel any pending debounced changes and force the change // in value to happen instantly (which, in turn, will re-calculate the min/max for the slider due to diff --git a/src/plugins/controls/public/range_slider/components/range_slider_strings.ts b/src/plugins/controls/public/range_slider/components/range_slider_strings.ts index c37dc49c5306a..a0ed2a051f94a 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider_strings.ts +++ b/src/plugins/controls/public/range_slider/components/range_slider_strings.ts @@ -9,6 +9,12 @@ import { i18n } from '@kbn/i18n'; export const RangeSliderStrings = { + control: { + getInvalidSelectionWarningLabel: () => + i18n.translate('controls.rangeSlider.control.invalidSelectionWarningLabel', { + defaultMessage: 'Selected range returns no results.', + }), + }, editor: { getStepTitle: () => i18n.translate('controls.rangeSlider.editor.stepSizeTitle', { diff --git a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.test.tsx b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.test.tsx index 9629e78dd5285..6a07cd531f8e5 100644 --- a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.test.tsx +++ b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.test.tsx @@ -132,7 +132,7 @@ describe('initialize', () => { await new Promise((resolve) => process.nextTick(resolve)); const reduxState = control.getState(); - expect(reduxState.output.filters?.length).toBe(0); + expect(reduxState.output.filters?.length).toBe(1); expect(reduxState.componentState.isInvalid).toBe(true); }); diff --git a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx index 6ac09cbbaea5a..6efc6e9fc1a65 100644 --- a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx +++ b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx @@ -12,7 +12,7 @@ import React, { createContext, useContext } from 'react'; import ReactDOM from 'react-dom'; import { batch } from 'react-redux'; import { lastValueFrom, Subscription, switchMap } from 'rxjs'; -import { distinctUntilChanged, map, skip } from 'rxjs/operators'; +import { distinctUntilChanged, map } from 'rxjs/operators'; import { DataView, DataViewField } from '@kbn/data-views-plugin/public'; import { Embeddable, IContainer } from '@kbn/embeddable-plugin/public'; @@ -28,6 +28,7 @@ import { ReduxEmbeddableTools, ReduxToolsPackage } from '@kbn/presentation-util- import { KibanaThemeProvider } from '@kbn/react-kibana-context-theme'; import { + ControlGroupContainer, ControlInput, ControlOutput, RangeSliderEmbeddableInput, @@ -81,6 +82,7 @@ export class RangeSliderEmbeddable { public readonly type = RANGE_SLIDER_CONTROL; public deferEmbeddableLoad = true; + public parent: ControlGroupContainer; private subscriptions: Subscription = new Subscription(); private node?: HTMLElement; @@ -92,7 +94,6 @@ export class RangeSliderEmbeddable // Internal data fetching state for this input control. private dataView?: DataView; private field?: DataViewField; - private filters: Filter[] = []; // state management public select: RangeSliderReduxEmbeddableTools['select']; @@ -109,6 +110,7 @@ export class RangeSliderEmbeddable parent?: IContainer ) { super(input, output, parent); // get filters for initial output... + this.parent = parent as ControlGroupContainer; // Destructure controls services ({ data: this.dataService, dataViews: this.dataViewsService } = pluginServices.getServices()); @@ -126,27 +128,23 @@ export class RangeSliderEmbeddable this.dispatch = reduxEmbeddableTools.dispatch; this.onStateChange = reduxEmbeddableTools.onStateChange; this.cleanupStateTools = reduxEmbeddableTools.cleanup; + this.initialize(); } private initialize = async () => { - const initialValue = this.getInput().value; - if (!initialValue) { - this.setInitializationFinished(); - } - - try { - await this.runRangeSliderQuery(); - await this.buildFilter(); - } catch (e) { - this.onLoadingError(e.message); - } - - if (initialValue) { - this.setInitializationFinished(); + const [initialMin, initialMax] = this.getInput().value ?? []; + if (!isEmpty(initialMin) || !isEmpty(initialMax)) { + const filter = await this.buildFilter(); + this.dispatch.publishFilters(filter); } + this.setInitializationFinished(); - this.setupSubscriptions(); + this.runRangeSliderQuery() + .then(async () => { + this.setupSubscriptions(); + }) + .catch((e) => this.onLoadingError(e.message)); }; private setupSubscriptions = () => { @@ -161,18 +159,22 @@ export class RangeSliderEmbeddable filters: newInput.filters, query: newInput.query, })), - distinctUntilChanged(diffDataFetchProps), - skip(1) + distinctUntilChanged(diffDataFetchProps) + ); + + const valueChangePipe = this.getInput$().pipe( + distinctUntilChanged((a, b) => isEqual(a.value ?? ['', ''], b.value ?? ['', ''])) ); - // fetch available min/max when input changes this.subscriptions.add( dataFetchPipe .pipe( - switchMap(async (changes) => { + switchMap(async () => { try { + this.dispatch.setLoading(true); await this.runRangeSliderQuery(); - await this.buildFilter(); + await this.runValidations(); + this.dispatch.setLoading(false); } catch (e) { this.onLoadingError(e.message); } @@ -181,13 +183,21 @@ export class RangeSliderEmbeddable .subscribe() ); - // build filters when value changes + // publish filters when value changes this.subscriptions.add( - this.getInput$() + valueChangePipe .pipe( - distinctUntilChanged((a, b) => isEqual(a.value ?? ['', ''], b.value ?? ['', ''])), - skip(1), // skip the first input update because initial filters will be built by initialize. - switchMap(this.buildFilter) + switchMap(async () => { + try { + this.dispatch.setLoading(true); + const rangeFilter = await this.buildFilter(); + this.dispatch.publishFilters(rangeFilter); + await this.runValidations(); + this.dispatch.setLoading(false); + } catch (e) { + this.onLoadingError(e.message); + } + }) ) .subscribe() ); @@ -228,39 +238,18 @@ export class RangeSliderEmbeddable }; private runRangeSliderQuery = async () => { - this.dispatch.setLoading(true); - const { dataView, field } = await this.getCurrentDataViewAndField(); if (!dataView || !field) return; - const embeddableInput = this.getInput(); - const { ignoreParentSettings, timeRange: globalTimeRange, timeslice } = embeddableInput; - let { filters = [] } = embeddableInput; - - const timeRange = - timeslice !== undefined - ? { - from: new Date(timeslice[0]).toISOString(), - to: new Date(timeslice[1]).toISOString(), - mode: 'absolute' as 'absolute', - } - : globalTimeRange; - if (!ignoreParentSettings?.ignoreTimerange && timeRange) { - const timeFilter = this.dataService.timefilter.createFilter(dataView, timeRange); - if (timeFilter) { - filters = filters.concat(timeFilter); - } - } - - this.filters = filters; const { min, max } = await this.fetchMinMax({ dataView, field, }); - this.dispatch.setMinMax({ - min, - max, + batch(() => { + this.dispatch.setMinMax({ min, max }); + this.dispatch.setDataViewId(dataView.id); + this.dispatch.setErrorMessage(undefined); }); }; @@ -271,15 +260,11 @@ export class RangeSliderEmbeddable dataView: DataView; field: DataViewField; }): Promise<{ min?: number; max?: number }> => { + const { query } = this.getInput(); const searchSource = await this.dataService.searchSource.create(); searchSource.setField('size', 0); searchSource.setField('index', dataView); - - const { ignoreParentSettings, query } = this.getInput(); - - if (!ignoreParentSettings?.ignoreFilters) { - searchSource.setField('filter', this.filters); - } + searchSource.setField('filter', this.getGlobalFilters(dataView)); if (query) { searchSource.setField('query', query); @@ -317,40 +302,25 @@ export class RangeSliderEmbeddable private buildFilter = async () => { const { - componentState: { min: availableMin, max: availableMax }, explicitInput: { value }, } = this.getState(); - const { ignoreParentSettings, query } = this.getInput(); - const [selectedMin, selectedMax] = value ?? ['', '']; - const hasData = availableMin !== undefined && availableMax !== undefined; - const hasLowerSelection = !isEmpty(selectedMin); - const hasUpperSelection = !isEmpty(selectedMax); - const hasEitherSelection = hasLowerSelection || hasUpperSelection; + const [min, max] = [selectedMin, selectedMax].map(parseFloat); const { dataView, field } = await this.getCurrentDataViewAndField(); - if (!dataView || !field) return; + if (!dataView || !field) return []; - if (!hasData || !hasEitherSelection) { - batch(() => { - this.dispatch.setLoading(false); - this.dispatch.setIsInvalid(!ignoreParentSettings?.ignoreValidations && hasEitherSelection); - this.dispatch.setDataViewId(dataView.id); - this.dispatch.publishFilters([]); - this.dispatch.setErrorMessage(undefined); - }); - return; - } + if (isEmpty(selectedMin) && isEmpty(selectedMax)) return []; const params = {} as RangeFilterParams; if (selectedMin) { - params.gte = Math.max(parseFloat(selectedMin), availableMin); + params.gte = min; } if (selectedMax) { - params.lte = Math.min(parseFloat(selectedMax), availableMax); + params.lte = max; } const rangeFilter = buildRangeFilter(field, params, dataView); @@ -358,11 +328,60 @@ export class RangeSliderEmbeddable rangeFilter.meta.type = 'range'; rangeFilter.meta.params = params; + return [rangeFilter]; + }; + + private onLoadingError(errorMessage: string) { + batch(() => { + this.dispatch.setLoading(false); + this.dispatch.publishFilters([]); + this.dispatch.setErrorMessage(errorMessage); + }); + } + + private getGlobalFilters = (dataView: DataView) => { + const { + filters: globalFilters, + ignoreParentSettings, + timeRange: globalTimeRange, + timeslice, + } = this.getInput(); + + const filters: Filter[] = []; + + if (!ignoreParentSettings?.ignoreFilters && globalFilters) { + filters.push(...globalFilters); + } + + const timeRange = + timeslice !== undefined + ? { + from: new Date(timeslice[0]).toISOString(), + to: new Date(timeslice[1]).toISOString(), + mode: 'absolute' as 'absolute', + } + : globalTimeRange; + + if (!ignoreParentSettings?.ignoreTimerange && timeRange) { + const timeFilter = this.dataService.timefilter.createFilter(dataView, timeRange); + if (timeFilter) filters.push(timeFilter); + } + + return filters; + }; + + private runValidations = async () => { + const { dataView } = await this.getCurrentDataViewAndField(); + if (!dataView) return; // Check if new range filter results in no data - if (!ignoreParentSettings?.ignoreValidations) { + const { ignoreParentSettings, query } = this.getInput(); + if (ignoreParentSettings?.ignoreValidations) { + this.dispatch.setIsInvalid(false); + } else { const searchSource = await this.dataService.searchSource.create(); - const filters = [...this.filters, rangeFilter]; + const { filters: rangeFilters = [] } = this.getOutput(); + const filters = this.getGlobalFilters(dataView).concat(rangeFilters); searchSource.setField('size', 0); searchSource.setField('index', dataView); @@ -375,43 +394,33 @@ export class RangeSliderEmbeddable const total = resp?.rawResponse?.hits?.total; const docCount = typeof total === 'number' ? total : total?.value; - if (!docCount) { - batch(() => { - this.dispatch.setLoading(false); - this.dispatch.setIsInvalid(true); - this.dispatch.setDataViewId(dataView.id); - this.dispatch.publishFilters([]); - this.dispatch.setErrorMessage(undefined); - }); - return; - } - } - batch(() => { - this.dispatch.setLoading(false); - this.dispatch.setIsInvalid(false); - this.dispatch.setDataViewId(dataView.id); - this.dispatch.publishFilters([rangeFilter]); - this.dispatch.setErrorMessage(undefined); - }); + const { + explicitInput: { value }, + } = this.getState(); + this.reportInvalidSelections( + !value || (value[0] === '' && value[1] === '') ? false : !docCount // don't set the range slider invalid if it has no selections + ); + } }; - private onLoadingError(errorMessage: string) { - batch(() => { - this.dispatch.setLoading(false); - this.dispatch.publishFilters([]); - this.dispatch.setErrorMessage(errorMessage); + private reportInvalidSelections = (hasInvalidSelections: boolean) => { + this.dispatch.setIsInvalid(hasInvalidSelections); + this.parent?.reportInvalidSelections({ + id: this.id, + hasInvalidSelections, }); - } + }; public clearSelections() { this.dispatch.setSelectedRange(['', '']); } public reload = async () => { + this.dispatch.setLoading(true); try { await this.runRangeSliderQuery(); - await this.buildFilter(); + this.dispatch.setLoading(false); } catch (e) { this.onLoadingError(e.message); } diff --git a/src/plugins/controls/public/services/core/core.story.ts b/src/plugins/controls/public/services/core/core.story.ts index 5549f15461553..dca544758970e 100644 --- a/src/plugins/controls/public/services/core/core.story.ts +++ b/src/plugins/controls/public/services/core/core.story.ts @@ -17,5 +17,6 @@ export const coreServiceFactory: CoreServiceFactory = () => { return { theme: themeServiceMock.createSetupContract(), i18n: corePluginMock.i18n, + notifications: corePluginMock.notifications, }; }; diff --git a/src/plugins/controls/public/services/core/core_service.ts b/src/plugins/controls/public/services/core/core_service.ts index 3a8ac7bb89098..ed63bc54e7a60 100644 --- a/src/plugins/controls/public/services/core/core_service.ts +++ b/src/plugins/controls/public/services/core/core_service.ts @@ -16,10 +16,11 @@ export type CoreServiceFactory = KibanaPluginServiceFactory< >; export const coreServiceFactory: CoreServiceFactory = ({ coreStart }) => { - const { theme, i18n } = coreStart; + const { theme, i18n, notifications } = coreStart; return { theme, i18n, + notifications, }; }; diff --git a/src/plugins/controls/public/services/core/types.ts b/src/plugins/controls/public/services/core/types.ts index f5a769c077163..cd2a167402ec1 100644 --- a/src/plugins/controls/public/services/core/types.ts +++ b/src/plugins/controls/public/services/core/types.ts @@ -11,4 +11,5 @@ import { CoreStart } from '@kbn/core/public'; export interface ControlsCoreService { i18n: CoreStart['i18n']; theme: CoreStart['theme']; + notifications: CoreStart['notifications']; } diff --git a/src/plugins/controls/public/services/plugin_services.story.ts b/src/plugins/controls/public/services/plugin_services.story.ts index e214349095695..1db9d3e201bfd 100644 --- a/src/plugins/controls/public/services/plugin_services.story.ts +++ b/src/plugins/controls/public/services/plugin_services.story.ts @@ -21,6 +21,7 @@ import { httpServiceFactory } from './http/http.stub'; import { optionsListServiceFactory } from './options_list/options_list.story'; import { overlaysServiceFactory } from './overlays/overlays.story'; import { settingsServiceFactory } from './settings/settings.story'; +import { storageServiceFactory } from './storage/storage_service.stub'; import { ControlsServices } from './types'; import { unifiedSearchServiceFactory } from './unified_search/unified_search.story'; @@ -33,7 +34,7 @@ export const providers: PluginServiceProviders = { settings: new PluginServiceProvider(settingsServiceFactory), core: new PluginServiceProvider(coreServiceFactory), embeddable: new PluginServiceProvider(embeddableServiceFactory), - + storage: new PluginServiceProvider(storageServiceFactory), controls: new PluginServiceProvider(controlsServiceFactory), optionsList: new PluginServiceProvider(optionsListServiceFactory), }; diff --git a/src/plugins/controls/public/services/plugin_services.stub.ts b/src/plugins/controls/public/services/plugin_services.stub.ts index 07c24d72ce630..ceadd25c9a349 100644 --- a/src/plugins/controls/public/services/plugin_services.stub.ts +++ b/src/plugins/controls/public/services/plugin_services.stub.ts @@ -26,6 +26,7 @@ import { overlaysServiceFactory } from './overlays/overlays.stub'; import { registry as stubRegistry } from './plugin_services.story'; import { settingsServiceFactory } from './settings/settings.story'; import { unifiedSearchServiceFactory } from './unified_search/unified_search.story'; +import { storageServiceFactory } from './storage/storage_service.stub'; export const providers: PluginServiceProviders = { embeddable: new PluginServiceProvider(embeddableServiceFactory), @@ -37,6 +38,7 @@ export const providers: PluginServiceProviders = { overlays: new PluginServiceProvider(overlaysServiceFactory), settings: new PluginServiceProvider(settingsServiceFactory), core: new PluginServiceProvider(coreServiceFactory), + storage: new PluginServiceProvider(storageServiceFactory), unifiedSearch: new PluginServiceProvider(unifiedSearchServiceFactory), }; diff --git a/src/plugins/controls/public/services/plugin_services.ts b/src/plugins/controls/public/services/plugin_services.ts index e6caa3565ed60..b02a59265c13e 100644 --- a/src/plugins/controls/public/services/plugin_services.ts +++ b/src/plugins/controls/public/services/plugin_services.ts @@ -25,6 +25,7 @@ import { httpServiceFactory } from './http/http_service'; import { optionsListServiceFactory } from './options_list/options_list_service'; import { overlaysServiceFactory } from './overlays/overlays_service'; import { settingsServiceFactory } from './settings/settings_service'; +import { controlsStorageServiceFactory } from './storage/storage_service'; import { unifiedSearchServiceFactory } from './unified_search/unified_search_service'; export const providers: PluginServiceProviders< @@ -39,6 +40,7 @@ export const providers: PluginServiceProviders< optionsList: new PluginServiceProvider(optionsListServiceFactory, ['data', 'http']), overlays: new PluginServiceProvider(overlaysServiceFactory), settings: new PluginServiceProvider(settingsServiceFactory), + storage: new PluginServiceProvider(controlsStorageServiceFactory), core: new PluginServiceProvider(coreServiceFactory), unifiedSearch: new PluginServiceProvider(unifiedSearchServiceFactory), }; diff --git a/src/plugins/controls/public/services/storage/storage_service.stub.ts b/src/plugins/controls/public/services/storage/storage_service.stub.ts new file mode 100644 index 0000000000000..14a2ff13a6138 --- /dev/null +++ b/src/plugins/controls/public/services/storage/storage_service.stub.ts @@ -0,0 +1,19 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { PluginServiceFactory } from '@kbn/presentation-util-plugin/public'; +import { ControlsStorageService } from './types'; + +type StorageServiceFactory = PluginServiceFactory; + +export const storageServiceFactory: StorageServiceFactory = () => { + return { + getShowInvalidSelectionWarning: () => false, + setShowInvalidSelectionWarning: (value: boolean) => null, + }; +}; diff --git a/src/plugins/controls/public/services/storage/storage_service.ts b/src/plugins/controls/public/services/storage/storage_service.ts new file mode 100644 index 0000000000000..db06b3d6a3c00 --- /dev/null +++ b/src/plugins/controls/public/services/storage/storage_service.ts @@ -0,0 +1,30 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { Storage } from '@kbn/kibana-utils-plugin/public'; +import { ControlsStorageService } from './types'; + +const STORAGE_KEY = 'controls:showInvalidSelectionWarning'; + +class StorageService implements ControlsStorageService { + private storage: Storage; + + constructor() { + this.storage = new Storage(localStorage); + } + + getShowInvalidSelectionWarning = () => { + return this.storage.get(STORAGE_KEY); + }; + + setShowInvalidSelectionWarning = (value: boolean) => { + this.storage.set(STORAGE_KEY, value); + }; +} + +export const controlsStorageServiceFactory = () => new StorageService(); diff --git a/src/plugins/controls/public/services/storage/types.ts b/src/plugins/controls/public/services/storage/types.ts new file mode 100644 index 0000000000000..8bae4c70fbe27 --- /dev/null +++ b/src/plugins/controls/public/services/storage/types.ts @@ -0,0 +1,12 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +export interface ControlsStorageService { + getShowInvalidSelectionWarning: () => boolean; + setShowInvalidSelectionWarning: (value: boolean) => void; +} diff --git a/src/plugins/controls/public/services/types.ts b/src/plugins/controls/public/services/types.ts index bb86855302989..9160a63192916 100644 --- a/src/plugins/controls/public/services/types.ts +++ b/src/plugins/controls/public/services/types.ts @@ -15,6 +15,7 @@ import { ControlsHTTPService } from './http/types'; import { ControlsOptionsListService } from './options_list/types'; import { ControlsOverlaysService } from './overlays/types'; import { ControlsSettingsService } from './settings/types'; +import { ControlsStorageService } from './storage/types'; import { ControlsUnifiedSearchService } from './unified_search/types'; export interface ControlsServices { @@ -31,4 +32,5 @@ export interface ControlsServices { // controls plugin's own services controls: ControlsServiceType; optionsList: ControlsOptionsListService; + storage: ControlsStorageService; } diff --git a/test/functional/apps/dashboard_elements/controls/common/control_group_chaining.ts b/test/functional/apps/dashboard_elements/controls/common/control_group_chaining.ts index 7696b6a6f4762..11ec5694e88b9 100644 --- a/test/functional/apps/dashboard_elements/controls/common/control_group_chaining.ts +++ b/test/functional/apps/dashboard_elements/controls/common/control_group_chaining.ts @@ -154,15 +154,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { suggestions: { Fluffy: 6, 'Fee Fee': 3, Rover: 3 }, invalidSelections: ['sylvester'], }); - const suggestions = pick(OPTIONS_LIST_ANIMAL_SOUND_SUGGESTIONS, [ - 'ruff', - 'bark', - 'grrr', - 'bow ow ow', - 'grr', - ]); await dashboardControls.ensureAvailableOptionsEqual(controlIds[2], { - suggestions: { ...suggestions, grr: suggestions.grr - 1 }, + suggestions: {}, invalidSelections: ['meow'], }); }); @@ -188,6 +181,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await dashboardControls.optionsListPopoverSelectOption('cat'); await dashboardControls.optionsListEnsurePopoverIsClosed(controlIds[0]); + await dashboardControls.clearControlSelections(controlIds[1]); await dashboardControls.optionsListOpenPopover(controlIds[1]); expect(await dashboardControls.optionsListPopoverGetAvailableOptionsCount()).to.be(1); await dashboardControls.optionsListOpenPopover(controlIds[2]); @@ -201,7 +195,6 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await dashboardControls.optionsListEnsurePopoverIsClosed(controlIds[0]); await dashboard.waitForRenderComplete(); - await dashboardControls.clearControlSelections(controlIds[1]); await dashboardControls.optionsListOpenPopover(controlIds[1]); expect(await dashboardControls.optionsListPopoverGetAvailableOptionsCount()).to.be(1); await dashboardControls.ensureAvailableOptionsEqual( diff --git a/test/functional/apps/dashboard_elements/controls/common/range_slider.ts b/test/functional/apps/dashboard_elements/controls/common/range_slider.ts index 36fd494864bb1..8819f143a3dc7 100644 --- a/test/functional/apps/dashboard_elements/controls/common/range_slider.ts +++ b/test/functional/apps/dashboard_elements/controls/common/range_slider.ts @@ -36,6 +36,9 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { 'kibana_sample_admin', 'test_logstash_reader', ]); + // disable the invalid selection warning toast + await browser.setLocalStorageItem('controls:showInvalidSelectionWarning', 'false'); + await esArchiver.load('test/functional/fixtures/es_archiver/kibana_sample_data_flights'); await kibanaServer.importExport.load( 'test/functional/fixtures/kbn_archiver/dashboard/current/kibana' @@ -99,7 +102,9 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { additionalSettings: { step: 100 }, }); expect(await dashboardControls.getControlsCount()).to.be(2); - const secondId = (await dashboardControls.getAllControlIds())[1]; + const [firstId, secondId] = await dashboardControls.getAllControlIds(); + await dashboardControls.clearControlSelections(firstId); + await dashboardControls.rangeSliderWaitForLoading(firstId); await dashboardControls.validateRange('placeholder', secondId, '100', '1200'); await dashboardControls.rangeSliderSetLowerBound(secondId, '200'); diff --git a/test/functional/apps/dashboard_elements/controls/options_list/options_list_validation.ts b/test/functional/apps/dashboard_elements/controls/options_list/options_list_validation.ts index d8c1002f485e4..9c222c78715b5 100644 --- a/test/functional/apps/dashboard_elements/controls/options_list/options_list_validation.ts +++ b/test/functional/apps/dashboard_elements/controls/options_list/options_list_validation.ts @@ -111,8 +111,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { }, invalidSelections: ['meow', 'bark'], }); - // only valid selections are applied as filters. - expect(await pieChart.getPieSliceCount()).to.be(1); + // there are no valid selections, so no pie chart is rendered. + expect(await pieChart.expectEmptyPieChart()); }); }); @@ -153,6 +153,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { }, invalidSelections: [], }); + // there are no valid selections, so no pie chart is rendered. + expect(await pieChart.expectEmptyPieChart()); }); }); });