Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import deepEqual from 'fast-deep-equal';
import { omit, isEqual } from 'lodash';
import { DEFAULT_SORT } from '../options_list/suggestions_sorting';
import { OPTIONS_LIST_DEFAULT_SORT } from '../options_list/suggestions_sorting';
import { OptionsListEmbeddableInput, OPTIONS_LIST_CONTROL } from '../options_list/types';

import { ControlPanelState } from './types';
Expand Down Expand Up @@ -65,7 +65,7 @@ export const ControlPanelDiffSystems: {
Boolean(singleSelectA) === Boolean(singleSelectB) &&
Boolean(existsSelectedA) === Boolean(existsSelectedB) &&
Boolean(runPastTimeoutA) === Boolean(runPastTimeoutB) &&
deepEqual(sortA ?? DEFAULT_SORT, sortB ?? DEFAULT_SORT) &&
deepEqual(sortA ?? OPTIONS_LIST_DEFAULT_SORT, sortB ?? OPTIONS_LIST_DEFAULT_SORT) &&
isEqual(selectedA ?? [], selectedB ?? []) &&
deepEqual(inputA, inputB)
);
Expand Down
8 changes: 7 additions & 1 deletion src/plugins/controls/common/options_list/mocks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,13 @@ const mockOptionsListComponentState = {
...getDefaultComponentState(),
field: undefined,
totalCardinality: 0,
availableOptions: ['woof', 'bark', 'meow', 'quack', 'moo'],
availableOptions: {
woof: { doc_count: 100 },
bark: { doc_count: 75 },
meow: { doc_count: 50 },
quack: { doc_count: 25 },
moo: { doc_count: 5 },
},
invalidSelections: [],
validSelections: [],
} as OptionsListComponentState;
Expand Down
11 changes: 6 additions & 5 deletions src/plugins/controls/common/options_list/suggestions_sorting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,14 @@ import { Direction } from '@elastic/eui';

export type OptionsListSortBy = '_count' | '_key';

export const DEFAULT_SORT: SortingType = { by: '_count', direction: 'desc' };
export const OPTIONS_LIST_DEFAULT_SORT: OptionsListSortingType = {
by: '_count',
direction: 'desc',
};
Comment on lines +13 to +16
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed that, after this comment was made for the sort PR, I only renamed the specific options list types - however, as I was working on this PR, I figured this rule should apply to options list constants as well. Hence, renaming this to be more specific.


export const sortDirections: Readonly<Direction[]> = ['asc', 'desc'] as const;
export type SortDirection = typeof sortDirections[number];
export interface SortingType {
export interface OptionsListSortingType {
by: OptionsListSortBy;
direction: SortDirection;
direction: Direction;
}

export const getCompatibleSortingTypes = (type?: string): OptionsListSortBy[] => {
Expand Down
12 changes: 8 additions & 4 deletions src/plugins/controls/common/options_list/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,20 @@
import { FieldSpec, DataView, RuntimeFieldSpec } from '@kbn/data-views-plugin/common';
import type { Filter, Query, BoolQuery, TimeRange } from '@kbn/es-query';

import { SortingType } from './suggestions_sorting';
import { OptionsListSortingType } from './suggestions_sorting';
import { DataControlInput } from '../types';

export const OPTIONS_LIST_CONTROL = 'optionsListControl';

export interface OptionsListEmbeddableInput extends DataControlInput {
sort?: OptionsListSortingType;
selectedOptions?: string[];
existsSelected?: boolean;
runPastTimeout?: boolean;
singleSelect?: boolean;
hideExclude?: boolean;
hideExists?: boolean;
hideSort?: boolean;
sort?: SortingType;
exclude?: boolean;
}

Expand All @@ -32,11 +32,15 @@ export type OptionsListField = FieldSpec & {
childFieldName?: string;
};

export interface OptionsListSuggestions {
[key: string]: { doc_count: number };
}

/**
* The Options list response is returned from the serverside Options List route.
*/
export interface OptionsListResponse {
suggestions: string[];
suggestions: OptionsListSuggestions;
totalCardinality: number;
invalidSelections?: string[];
}
Expand All @@ -61,13 +65,13 @@ export type OptionsListRequest = Omit<
*/
export interface OptionsListRequestBody {
runtimeFieldMap?: Record<string, RuntimeFieldSpec>;
sort?: OptionsListSortingType;
filters?: Array<{ bool: BoolQuery }>;
selectedOptions?: string[];
runPastTimeout?: boolean;
parentFieldName?: string;
textFieldName?: string;
searchString?: string;
fieldSpec?: FieldSpec;
sort?: SortingType;
fieldName: string;
}
7 changes: 6 additions & 1 deletion src/plugins/controls/public/__stories__/controls.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,12 @@ const storybookStubOptionsListRequest = async (
setTimeout(
() =>
r({
suggestions: getFlightSearchOptions(request.field.name, request.searchString),
suggestions: getFlightSearchOptions(request.field.name, request.searchString).reduce(
(o, current, index) => {
return { ...o, [current]: { doc_count: index } };
},
{}
),
totalCardinality: 100,
}),
120
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export const OptionsListControl = ({ typeaheadSubject }: { typeaheadSubject: Sub
) : (
<>
{validSelections && (
<span>{validSelections?.join(OptionsListStrings.control.getSeparator())}</span>
<span>{validSelections.join(OptionsListStrings.control.getSeparator())}</span>
)}
{invalidSelections && (
<span className="optionsList__filterInvalid">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { css } from '@emotion/react';

import {
getCompatibleSortingTypes,
DEFAULT_SORT,
OPTIONS_LIST_DEFAULT_SORT,
OptionsListSortBy,
} from '../../../common/options_list/suggestions_sorting';
import { OptionsListStrings } from './options_list_strings';
Expand All @@ -48,8 +48,8 @@ export const OptionsListEditorOptions = ({
fieldType,
}: ControlEditorProps<OptionsListEmbeddableInput>) => {
const [state, setState] = useState<OptionsListEditorState>({
sortDirection: initialInput?.sort?.direction ?? DEFAULT_SORT.direction,
sortBy: initialInput?.sort?.by ?? DEFAULT_SORT.by,
sortDirection: initialInput?.sort?.direction ?? OPTIONS_LIST_DEFAULT_SORT.direction,
sortBy: initialInput?.sort?.by ?? OPTIONS_LIST_DEFAULT_SORT.by,
runPastTimeout: initialInput?.runPastTimeout,
singleSelect: initialInput?.singleSelect,
hideExclude: initialInput?.hideExclude,
Expand All @@ -60,8 +60,12 @@ export const OptionsListEditorOptions = ({
useEffect(() => {
// when field type changes, ensure that the selected sort type is still valid
if (!getCompatibleSortingTypes(fieldType).includes(state.sortBy)) {
onChange({ sort: DEFAULT_SORT });
setState((s) => ({ ...s, sortBy: DEFAULT_SORT.by, sortDirection: DEFAULT_SORT.direction }));
onChange({ sort: OPTIONS_LIST_DEFAULT_SORT });
setState((s) => ({
...s,
sortBy: OPTIONS_LIST_DEFAULT_SORT.by,
sortDirection: OPTIONS_LIST_DEFAULT_SORT.direction,
}));
}
}, [fieldType, onChange, state.sortBy]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ describe('Options list popover', () => {
});

test('no available options', async () => {
const popover = await mountComponent({ componentState: { availableOptions: [] } });
const popover = await mountComponent({ componentState: { availableOptions: {} } });
const availableOptionsDiv = findTestSubject(popover, 'optionsList-control-available-options');
const noOptionsDiv = findTestSubject(
availableOptionsDiv,
Expand Down Expand Up @@ -118,6 +118,43 @@ describe('Options list popover', () => {
expect(sortButton.prop('disabled')).toBe(true);
});

test('test single invalid selection', async () => {
const popover = await mountComponent({
explicitInput: {
selectedOptions: ['bark', 'woof'],
},
componentState: {
availableOptions: {
bark: { doc_count: 75 },
},
validSelections: ['bark'],
invalidSelections: ['woof'],
},
});
const validSelection = findTestSubject(popover, 'optionsList-control-selection-bark');
expect(validSelection.text()).toEqual('bark75');
const title = findTestSubject(popover, 'optionList__ignoredSelectionLabel').text();
expect(title).toEqual('Ignored selection');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because I refactored the optionsListPopoverGetAvailableOptions, it no longer returns "Ignored selection" / "Ignored selections" as part of the selections array. So, I added these unit tests to ensure that we don't lose test coverage for the singular/plural cases of this label 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

This is likely a very good change anyway! Nice cleanup.

const invalidSelection = findTestSubject(popover, 'optionsList-control-ignored-selection-woof');
expect(invalidSelection.text()).toEqual('woof');
expect(invalidSelection.hasClass('optionsList__selectionInvalid')).toBe(true);
});

test('test title when multiple invalid selections', async () => {
const popover = await mountComponent({
explicitInput: { selectedOptions: ['bark', 'woof', 'meow'] },
componentState: {
availableOptions: {
bark: { doc_count: 75 },
},
validSelections: ['bark'],
invalidSelections: ['woof', 'meow'],
},
});
const title = findTestSubject(popover, 'optionList__ignoredSelectionLabel').text();
expect(title).toEqual('Ignored selections');
});

test('should default to exclude = false', async () => {
const popover = await mountComponent();
const includeButton = findTestSubject(popover, 'optionsList__includeResults');
Expand Down Expand Up @@ -172,7 +209,7 @@ describe('Options list popover', () => {

test('if existsSelected = false and no suggestions, then "Exists" does not show up', async () => {
const popover = await mountComponent({
componentState: { availableOptions: [] },
componentState: { availableOptions: {} },
explicitInput: { existsSelected: false },
});
const existsOption = findTestSubject(popover, 'optionsList-control-selection-exists');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ export const OptionsListPopover = ({ width, updateSearchString }: OptionsListPop

return (
<span
id={`control-popover-${id}`}
role="listbox"
id={`control-popover-${id}`}
aria-label={OptionsListStrings.popover.getAriaLabel(fieldName)}
>
<EuiPopoverTitle paddingSize="s">{title}</EuiPopoverTitle>
Expand All @@ -59,10 +59,10 @@ export const OptionsListPopover = ({ width, updateSearchString }: OptionsListPop
/>
)}
<div
style={{ width: width > 300 ? width : undefined }}
className="optionsList __items"
data-option-count={availableOptions?.length ?? 0}
style={{ width: width > 300 ? width : undefined }}
data-test-subj={`optionsList-control-available-options`}
data-option-count={Object.keys(availableOptions ?? {}).length}
>
<OptionsListPopoverSuggestions showOnlySelected={showOnlySelected} />
{!showOnlySelected && invalidSelections && !isEmpty(invalidSelections) && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,11 @@ export const OptionsListPopoverActionBar = ({
display={showOnlySelected ? 'base' : 'empty'}
onClick={() => setShowOnlySelected(!showOnlySelected)}
data-test-subj="optionsList-control-show-only-selected"
aria-label={OptionsListStrings.popover.getClearAllSelectionsButtonTitle()}
aria-label={
showOnlySelected
? OptionsListStrings.popover.getAllOptionsButtonTitle()
: OptionsListStrings.popover.getSelectedOptionsButtonTitle()
}
Comment on lines +115 to +119
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I must have somehow grabbed the wrong aria-label when refactoring the popover to be multiple smaller components - woops! Fixed now :)

/>
</EuiToolTip>
</EuiFlexItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,14 @@ export const OptionsListPopoverInvalidSelections = () => {

// Select current state from Redux using multiple selectors to avoid rerenders.
const invalidSelections = select((state) => state.componentState.invalidSelections);

return (
<>
<EuiSpacer size="s" />
<EuiTitle size="xxs" className="optionsList-control-ignored-selection-title">
<EuiTitle
size="xxs"
className="optionsList-control-ignored-selection-title"
data-test-subj="optionList__ignoredSelectionLabel"
>
<label>
{OptionsListStrings.popover.getInvalidSelectionsSectionTitle(
invalidSelections?.length ?? 0
Expand All @@ -44,6 +47,7 @@ export const OptionsListPopoverInvalidSelections = () => {
className="optionsList__selectionInvalid"
key={index}
onClick={() => dispatch(deselectOption(ignoredSelection))}
aria-label={OptionsListStrings.popover.getInvalidSelectionAriaLabel(ignoredSelection)}
>
{`${ignoredSelection}`}
</EuiFilterSelectItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,13 @@
* Side Public License, v 1.
*/

import React, { useMemo, useState } from 'react';
import React, { useState } from 'react';

import {
EuiButtonGroupOptionProps,
EuiSelectableOption,
EuiPopoverTitle,
EuiButtonGroup,
toSentenceCase,
EuiButtonIcon,
EuiSelectable,
EuiFlexGroup,
Expand All @@ -26,8 +25,7 @@ import { useReduxEmbeddableContext } from '@kbn/presentation-util-plugin/public'

import {
getCompatibleSortingTypes,
DEFAULT_SORT,
sortDirections,
OPTIONS_LIST_DEFAULT_SORT,
OptionsListSortBy,
} from '../../../common/options_list/suggestions_sorting';
import { OptionsListReduxState } from '../types';
Expand All @@ -40,9 +38,6 @@ interface OptionsListSortingPopoverProps {
type SortByItem = EuiSelectableOption & {
data: { sortBy: OptionsListSortBy };
};
type SortOrderItem = EuiButtonGroupOptionProps & {
value: Direction;
};

export const OptionsListPopoverSortingButton = ({
showOnlySelected,
Expand All @@ -57,7 +52,7 @@ export const OptionsListPopoverSortingButton = ({

// Select current state from Redux using multiple selectors to avoid rerenders.
const field = select((state) => state.componentState.field);
const sort = select((state) => state.explicitInput.sort ?? DEFAULT_SORT);
const sort = select((state) => state.explicitInput.sort ?? OPTIONS_LIST_DEFAULT_SORT);

const [isSortingPopoverOpen, setIsSortingPopoverOpen] = useState(false);

Expand All @@ -73,18 +68,20 @@ export const OptionsListPopoverSortingButton = ({
});
});

const sortOrderOptions = useMemo(
() =>
sortDirections.map((key) => {
return {
id: key,
iconType: `sort${toSentenceCase(key)}ending`,
'data-test-subj': `optionsList__sortOrder_${key}`,
label: OptionsListStrings.editorAndPopover.sortOrder[key].getSortOrderLabel(),
} as SortOrderItem;
}),
[]
);
const sortOrderOptions: EuiButtonGroupOptionProps[] = [
{
id: 'asc',
iconType: `sortAscending`,
'data-test-subj': `optionsList__sortOrder_asc`,
label: OptionsListStrings.editorAndPopover.sortOrder.asc.getSortOrderLabel(),
},
{
id: 'desc',
iconType: `sortDescending`,
'data-test-subj': `optionsList__sortOrder_desc`,
label: OptionsListStrings.editorAndPopover.sortOrder.desc.getSortOrderLabel(),
},
];

const onSortByChange = (updatedOptions: SortByItem[]) => {
setSortByOptions(updatedOptions);
Expand Down
Loading