Skip to content
Merged
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
042d15a
Do not ignore invalid selections
nickpeihl Jan 3, 2024
3c14316
All selections use same font weight
nickpeihl Jan 3, 2024
2f6df87
Do not ignore invalid values in range slider
nickpeihl Jan 5, 2024
29599ff
Run validations after control has fully loaded
nickpeihl Jan 30, 2024
017e728
Update tests
nickpeihl Jan 30, 2024
f6c3661
Merge branch 'main' of https://github.com/elastic/kibana into control…
nickpeihl Jan 30, 2024
9146dc2
Remove commented code
nickpeihl Jan 30, 2024
f778f55
Warn user about invalid selections no longer ignored
nickpeihl Jan 31, 2024
1f180b2
Fix range slider subscriptions
nickpeihl Jan 31, 2024
eb18331
Merge remote-tracking branch 'upstream/main' into controls-highlight-…
nickpeihl Jan 31, 2024
56f5d26
Add stub for storage service
nickpeihl Jan 31, 2024
bbca6c8
Suggestion
Heenawter Jan 31, 2024
ec93377
Slightly cleaner implementation
Heenawter Feb 1, 2024
9d6790b
Buggy version
Heenawter Feb 1, 2024
03fa8d3
Better version
Heenawter Feb 2, 2024
b8b9a37
Clean up + add to range slider
Heenawter Feb 2, 2024
d19a28b
Give example of custom text + clean up
Heenawter Feb 2, 2024
8173e16
Use toast instead of tour
nickpeihl Feb 9, 2024
779acf5
Merge pull request #3 from Heenawter/try-to-clean-up-tour-stuff
nickpeihl Feb 12, 2024
811fd64
Merge remote-tracking branch 'upstream/main' into controls-highlight-…
nickpeihl Feb 12, 2024
db0cc43
Match range slider and options list font weights
nickpeihl Feb 12, 2024
213b161
Fix story service
nickpeihl Feb 12, 2024
016ccdd
First attempt to add eui token
nickpeihl Feb 12, 2024
ae917a3
Fix positioning of invalid token
Heenawter Feb 12, 2024
886d5b3
Fix truncation
Heenawter Feb 12, 2024
430d5c7
Add invalid token to range slider
Heenawter Feb 13, 2024
72543a3
Wrap tokens in tooltip
nickpeihl Feb 13, 2024
3b83205
lint
nickpeihl Feb 13, 2024
373479d
Don't show icons for invalid selections in popover menu
nickpeihl Feb 13, 2024
5628911
Fix test with uncleared toast
nickpeihl Feb 13, 2024
134bdaf
Fix toast collision in functional test
nickpeihl Feb 13, 2024
fb2ab34
Address review feedback
nickpeihl Feb 13, 2024
b329226
Simplify invalid selections logic
nickpeihl Feb 13, 2024
be3d10f
Move setting loading state outside of runRangeSliderQuery method
nickpeihl Feb 14, 2024
fe3478f
Merge branch 'main' into controls-highlight-invalid
nickpeihl Feb 14, 2024
c98f322
Remove invalid function
nickpeihl Feb 14, 2024
23fea13
Merge remote-tracking branch 'refs/remotes/origin/controls-highlight-…
nickpeihl Feb 14, 2024
951d463
Add warning icon to invalid selections list group label
nickpeihl Feb 15, 2024
243ddd9
Merge branch 'main' into controls-highlight-invalid
Heenawter Feb 20, 2024
0458fe7
Fix failing test
Heenawter Feb 20, 2024
5a56610
Switch to warning toast + fix text
Heenawter Feb 20, 2024
f1ad19c
Switch back to EuiTour
Heenawter Feb 20, 2024
3237fe2
Add custom range slider text
Heenawter Feb 20, 2024
b5b8c29
Switch invalid selections popover title design
Heenawter Feb 20, 2024
7b9aea6
Fix failing test
Heenawter Feb 21, 2024
b5bf9da
Change body of tour step
Heenawter Feb 21, 2024
15cbff4
Fix copy
Heenawter Feb 21, 2024
6fd9848
Another copy change
Heenawter Feb 22, 2024
d8530a0
Merge branch 'main' into controls-highlight-invalid
Heenawter Feb 22, 2024
3484856
Make options list text match range slider
Heenawter Feb 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/plugins/controls/kibana.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"unifiedSearch",
"uiActions"
],
"extraPublicDirs": ["common"]
"extraPublicDirs": ["common"],
"requiredBundles": ["kibanaUtils"]
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,34 +8,34 @@

import '../control_group.scss';

import {
arrayMove,
SortableContext,
rectSortingStrategy,
sortableKeyboardCoordinates,
} from '@dnd-kit/sortable';
import {
closestCenter,
DndContext,
DragEndEvent,
DragOverlay,
KeyboardSensor,
LayoutMeasuringStrategy,
PointerSensor,
useSensor,
useSensors,
LayoutMeasuringStrategy,
} from '@dnd-kit/core';
import {
arrayMove,
rectSortingStrategy,
SortableContext,
sortableKeyboardCoordinates,
} from '@dnd-kit/sortable';
import { EuiButtonIcon, EuiFlexGroup, EuiFlexItem, EuiPanel } from '@elastic/eui';
import classNames from 'classnames';
import React, { useMemo, useState } from 'react';
import React, { useEffect, useMemo, useState } from 'react';
import { TypedUseSelectorHook, useSelector } from 'react-redux';
import { EuiButtonIcon, EuiFlexGroup, EuiFlexItem, EuiPanel } 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<ControlGroupReduxState>;

Expand All @@ -47,6 +47,15 @@ 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 controlsHaveInvalidSelections = contextSelect(
(state) => state.componentState.controlsHaveInvalidSelections
);

useEffect(() => {
if (controlsHaveInvalidSelections) {
controlGroup.showInvalidSelectionsToast();
}
}, [controlGroup, controlsHaveInvalidSelections]);

const isEditable = viewMode === ViewMode.EDIT;

Expand Down
18 changes: 16 additions & 2 deletions src/plugins/controls/public/control_group/control_group_strings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,21 @@ import { i18n } from '@kbn/i18n';
import { RANGE_SLIDER_CONTROL } from '../range_slider';

export const ControlGroupStrings = {
invalidControlWarning: {
title: i18n.translate('controls.controlGroup.invalidControlWarning.title', {
defaultMessage: 'Selections are returning no results',
}),
text: i18n.translate('controls.controlGroup.invalidControlWarning.text', {
defaultMessage:
'Some control selections are returning no results. Remove them for complete results.',
}),
dismissButtonLabel: i18n.translate(
'controls.controlGroup.invalidControlWarning.dismissButtonLabel',
{
defaultMessage: 'Do not show again',
}
),
},
manageControl: {
getFlyoutCreateTitle: () =>
i18n.translate('controls.controlGroup.manageControl.createFlyoutTitle', {
Expand Down Expand Up @@ -258,8 +273,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: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,18 @@
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { EuiButtonEmpty, EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
import { compareFilters, COMPARE_ALL_OPTIONS, Filter, uniqFilters } from '@kbn/es-query';
import { toMountPoint } from '@kbn/react-kibana-mount';
import { isEqual, pick } from 'lodash';
import React, { createContext, useContext } from 'react';
import ReactDOM from 'react-dom';
import { Provider, TypedUseSelectorHook, useSelector } from 'react-redux';
import { BehaviorSubject, merge, Subject, Subscription } from 'rxjs';
import { debounceTime, distinctUntilChanged, skip } from 'rxjs/operators';

import { OverlayRef } from '@kbn/core/public';
import { OverlayRef, Toast } from '@kbn/core/public';
import { Container, EmbeddableFactory } from '@kbn/embeddable-plugin/public';
import { ReduxEmbeddableTools, ReduxToolsPackage } from '@kbn/presentation-util-plugin/public';
import { KibanaThemeProvider } from '@kbn/react-kibana-context-theme';
Expand Down Expand Up @@ -55,6 +58,8 @@ import {
controlOrdersAreEqual,
} from './control_group_chaining_system';
import { getNextPanelOrder } from './control_group_helpers';
import { ControlGroupStrings } from '../control_group_strings';
import { ControlsStorageService } from '../../services/storage/types';

let flyoutRef: OverlayRef | undefined;
export const setFlyoutRef = (newRef: OverlayRef | undefined) => {
Expand Down Expand Up @@ -86,11 +91,16 @@ export class ControlGroupContainer extends Container<

private initialized$ = new BehaviorSubject(false);

private storageService: ControlsStorageService;

private subscriptions: Subscription = new Subscription();
private domNode?: HTMLElement;
private recalculateFilters$: Subject<null>;
private relevantDataViewId?: string;
private lastUsedDataViewId?: string;
private invalidSelectionsState: { [childId: string]: boolean };
private invalidSelectionsToast?: Toast;

public diffingSubscription: Subscription = new Subscription();

// state management
Expand Down Expand Up @@ -126,6 +136,8 @@ export class ControlGroupContainer extends Container<
ControlGroupChainingSystems[initialInput.chainingSystem]?.getContainerSettings(initialInput)
);

({ storage: this.storageService } = pluginServices.getServices());

this.recalculateFilters$ = new Subject();
this.onFiltersPublished$ = new Subject<Filter[]>();
this.onControlRemoved$ = new Subject<string>();
Expand Down Expand Up @@ -153,6 +165,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();
Expand All @@ -164,6 +180,67 @@ export class ControlGroupContainer extends Container<
this.fieldFilterPredicate = fieldFilterPredicate;
}

public canShowInvalidSelectionsWarning = () =>
this.storageService.getShowInvalidSelectionWarning() ?? true;

public supressInvalidSelectionsWarning = () => {
this.storageService.setShowInvalidSelectionWarning(false);
};

public showInvalidSelectionsToast = () => {
if (!this.canShowInvalidSelectionsWarning()) return;
const {
core: { notifications, theme, i18n },
} = pluginServices.getServices();

// remove any existing toasts to avoid a toast storm
if (this.invalidSelectionsToast) {
notifications.toasts.remove(this.invalidSelectionsToast);
}

this.invalidSelectionsToast = notifications.toasts.add({
title: ControlGroupStrings.invalidControlWarning.title,
text: toMountPoint(
<>
<p>{ControlGroupStrings.invalidControlWarning.text}</p>
<EuiFlexGroup justifyContent="flexEnd" gutterSize="s">
<EuiFlexItem grow={false}>
<EuiButtonEmpty
size="xs"
color="text"
onClick={() => {
this.supressInvalidSelectionsWarning();
if (this.invalidSelectionsToast) {
notifications.toasts.remove(this.invalidSelectionsToast);
}
}}
>
{ControlGroupStrings.invalidControlWarning.dismissButtonLabel}
</EuiButtonEmpty>
</EuiFlexItem>
</EuiFlexGroup>
</>,
{ theme, i18n }
),
});
};

public reportInvalidSelections = ({
id,
hasInvalidSelections,
}: {
id: string;
hasInvalidSelections: boolean;
}) => {
this.invalidSelectionsState = { ...this.invalidSelectionsState, [id]: hasInvalidSelections };

const childrenHaveInvalidSelections = Object.keys(this.getInput().panels).some((childId) => {
return this.invalidSelectionsState[childId];
});

this.dispatch.setControlsHaveInvalidSelections(childrenHaveInvalidSelections);
};

private setupSubscriptions = () => {
/**
* refresh control order cache and make all panels refreshInputFromParent whenever panel orders change
Expand Down Expand Up @@ -201,7 +278,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();
})
);
};

Expand All @@ -211,9 +290,16 @@ 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() {
// reset invalid selections state on reload
this.dispatch.setControlsHaveInvalidSelections(undefined);
super.reload();
}

public getPersistableInput: () => PersistableControlGroupInput & { id: string } = () => {
const input = this.getInput();
return pick(input, [...persistableControlGroupInputKeys, 'id']);
Expand Down Expand Up @@ -284,13 +370,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) ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ export const controlGroupReducers = {
) => {
state.componentState.lastSavedInput = action.payload;
},
setControlsHaveInvalidSelections: (
state: WritableDraft<ControlGroupReduxState>,
action: PayloadAction<ControlGroupComponentState['controlsHaveInvalidSelections']>
) => {
state.componentState.controlsHaveInvalidSelections = action.payload;
},
setControlStyle: (
state: WritableDraft<ControlGroupReduxState>,
action: PayloadAction<ControlGroupInput['controlStyle']>
Expand Down
1 change: 1 addition & 0 deletions src/plugins/controls/public/control_group/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export interface ControlGroupSettings {

export type ControlGroupComponentState = ControlGroupSettings & {
lastSavedInput: PersistableControlGroupInput;
controlsHaveInvalidSelections?: boolean;
};

export {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -78,8 +80,7 @@
}

.optionsList__selectionInvalid {
text-decoration: line-through;
color: $euiTextSubduedColor;
color: $euiColorWarningText;
}

.optionslist--loadingMoreGroupLabel {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Copy link
Contributor

@Heenawter Heenawter Feb 2, 2024

Choose a reason for hiding this comment

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

Switching to RTL - nice!! 🎉 Really good cleanup

import { OptionsListEmbeddableContext } from '../embeddable/options_list_embeddable';
import { OptionsListComponentState, OptionsListReduxState } from '../types';
import { ControlOutput, OptionsListEmbeddableInput } from '../..';
Expand All @@ -37,7 +35,7 @@ describe('Options list control', () => {
output: options?.output ?? {},
} as Partial<OptionsListReduxState>);

return mountWithIntl(
return render(
<OptionsListEmbeddableContext.Provider value={optionsListEmbeddable}>
<OptionsListControl {...defaultProps} />
</OptionsListEmbeddableContext.Provider>
Expand All @@ -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');
});
});
Loading