Skip to content

Commit

Permalink
[Dashboard] [Controls] Allow options list suggestions to be sorted (#…
Browse files Browse the repository at this point in the history
…144867)

Closes #140174
Closes #145040
Closes #146086 

## Summary

This PR adds two features to the options list control:
1. A button in the options list popover that gives users the ability to
change how the suggestions are sorted
    <p align="center">
<img
src="https://user-images.githubusercontent.com/8698078/203416853-58f9c909-8909-4902-adf3-59831018c96f.gif"/>
    </p>

2. A per-control setting that disables the ability to dynamically sort
which, if set to `false`, presents the author with the ability to select
one of the four sorting methods for that specific control to use
    <p align="center">
<img
src="https://user-images.githubusercontent.com/8698078/203417193-cd35f264-8c29-4c80-b88b-15da25a1f56c.gif"/>
    </p>

### Design considerations
@elastic/kibana-design 

As noted by Andrea when looking at the preliminary behaviour of this
feature, the `"Show only selected"` toggle has increased in importance
because of the new sorting mechanic - after all, when making selections
and then changing the sort method, your selections can appear to be
"lost" if you have enough unique values in the control's field.

In the original designs, the `"Clear all selections"` button was
**first** in the popover's action bar - however, I found that I kept
accidentally clicking this in my testing when switching between
searching, sorting, making selections, changing sorting, showing only
selected options, etc. etc. I found that the following design felt a lot
more natural for the placement of the `"Clear all selections"` button:


![image](https://user-images.githubusercontent.com/8698078/202318768-cf8a5668-40c4-482f-9eb0-023508866068.png)

Note that, once #143585 is
resolved, this will no longer be as much of a concern because we will be
moving, at the very least, the `"Clear all selections"` to be a floating
action. That being said, this new order for the actions is, in my
opinion, a good compromise in the mean time. Very much open to feedback,
though!

### Video 


https://user-images.githubusercontent.com/8698078/203422674-52aac87c-7295-4eb6-99a5-ee3ffba2756b.mov


### Testing Notes
There are a few things to consider when testing:
1. Does the dynamic sorting give you expected results when sorting
various field types?
- Note that IP fields only support document count sorting, so ensure
that "Alphabetical" sorting does not show up in the sorting list during
either creation or as part of the popover sorting.
2. When setting the `"Allow suggestions to be sorted"` toggle to
`false`, it should always default to `"Document count (descending)"` to
prevent invalid sort selections. For example, consider the following:
    - Create an options list control on some keyword field
- Set the sort to alphabetical (either ascending or descending) in the
popover
    - Edit that control and change it to an IP field
    - Set `"Allow suggestions to be sorted"` to `false
- The default sort should be `"Document count (descending)"` and **not**
`"Alphabetical (descending/ascending)"`, since alphabetical sorting
would be invalid in this case.

**Flaky Test Runner**
<a
href="https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/1585"><img
src="https://user-images.githubusercontent.com/8698078/203428246-13f5ff9a-df0c-4cd5-a4ee-cf7a98792362.png"/></a>



### Checklist

Delete any items that are not applicable to this PR.

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
<p><img
src="https://user-images.githubusercontent.com/8698078/202545715-96daa0ab-8900-45cb-979f-20a83e622597.png"/></p>
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)


### 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: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
Heenawter and kibanamachine authored Nov 24, 2022
1 parent 94b6778 commit 1ed31e1
Show file tree
Hide file tree
Showing 20 changed files with 801 additions and 91 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

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

import { ControlPanelState } from './types';
Expand All @@ -32,7 +33,9 @@ export const ControlPanelDiffSystems: {
}

const {
sort: sortA,
exclude: excludeA,
hideSort: hideSortA,
hideExists: hideExistsA,
hideExclude: hideExcludeA,
selectedOptions: selectedA,
Expand All @@ -42,7 +45,9 @@ export const ControlPanelDiffSystems: {
...inputA
}: Partial<OptionsListEmbeddableInput> = initialInput.explicitInput;
const {
sort: sortB,
exclude: excludeB,
hideSort: hideSortB,
hideExists: hideExistsB,
hideExclude: hideExcludeB,
selectedOptions: selectedB,
Expand All @@ -54,11 +59,13 @@ export const ControlPanelDiffSystems: {

return (
Boolean(excludeA) === Boolean(excludeB) &&
Boolean(hideSortA) === Boolean(hideSortB) &&
Boolean(hideExistsA) === Boolean(hideExistsB) &&
Boolean(hideExcludeA) === Boolean(hideExcludeB) &&
Boolean(singleSelectA) === Boolean(singleSelectB) &&
Boolean(existsSelectedA) === Boolean(existsSelectedB) &&
Boolean(runPastTimeoutA) === Boolean(runPastTimeoutB) &&
deepEqual(sortA ?? DEFAULT_SORT, sortB ?? DEFAULT_SORT) &&
isEqual(selectedA ?? [], selectedB ?? []) &&
deepEqual(inputA, inputB)
);
Expand Down
7 changes: 5 additions & 2 deletions src/plugins/controls/common/options_list/mocks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,20 @@ import { createReduxEmbeddableTools } from '@kbn/presentation-util-plugin/public

import { OptionsListEmbeddable, OptionsListEmbeddableFactory } from '../../public';
import { OptionsListComponentState, OptionsListReduxState } from '../../public/options_list/types';
import { optionsListReducers } from '../../public/options_list/options_list_reducers';
import {
getDefaultComponentState,
optionsListReducers,
} from '../../public/options_list/options_list_reducers';
import { ControlFactory, ControlOutput } from '../../public/types';
import { OptionsListEmbeddableInput } from './types';

const mockOptionsListComponentState = {
...getDefaultComponentState(),
field: undefined,
totalCardinality: 0,
availableOptions: ['woof', 'bark', 'meow', 'quack', 'moo'],
invalidSelections: [],
validSelections: [],
searchString: { value: '', valid: true },
} as OptionsListComponentState;

const mockOptionsListEmbeddableInput = {
Expand Down
31 changes: 31 additions & 0 deletions src/plugins/controls/common/options_list/suggestions_sorting.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* 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 { Direction } from '@elastic/eui';

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

export const DEFAULT_SORT: SortingType = { by: '_count', direction: 'desc' };

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

export const getCompatibleSortingTypes = (type?: string): OptionsListSortBy[] => {
switch (type) {
case 'ip': {
return ['_count'];
}
default: {
return ['_count', '_key'];
}
}
};
6 changes: 5 additions & 1 deletion src/plugins/controls/common/options_list/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
* Side Public License, v 1.
*/

import type { Filter, Query, BoolQuery, TimeRange } from '@kbn/es-query';
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 { DataControlInput } from '../types';

export const OPTIONS_LIST_CONTROL = 'optionsListControl';
Expand All @@ -20,6 +21,8 @@ export interface OptionsListEmbeddableInput extends DataControlInput {
singleSelect?: boolean;
hideExclude?: boolean;
hideExists?: boolean;
hideSort?: boolean;
sort?: SortingType;
exclude?: boolean;
}

Expand Down Expand Up @@ -65,5 +68,6 @@ export interface OptionsListRequestBody {
textFieldName?: string;
searchString?: string;
fieldSpec?: FieldSpec;
sort?: SortingType;
fieldName: string;
}
53 changes: 30 additions & 23 deletions src/plugins/controls/public/control_group/editor/control_editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -243,34 +243,41 @@ export const ControlEditor = ({
/>
</EuiFormRow>
<EuiFormRow label={ControlGroupStrings.manageControl.getWidthInputTitle()}>
<EuiButtonGroup
color="primary"
legend={ControlGroupStrings.management.controlWidth.getWidthSwitchLegend()}
options={CONTROL_WIDTH_OPTIONS}
idSelected={currentWidth}
onChange={(newWidth: string) => {
setCurrentWidth(newWidth as ControlWidth);
updateWidth(newWidth as ControlWidth);
}}
/>
</EuiFormRow>
{updateGrow ? (
<EuiFormRow>
<EuiSwitch
label={ControlGroupStrings.manageControl.getGrowSwitchTitle()}
<>
<EuiButtonGroup
color="primary"
checked={currentGrow}
onChange={() => {
setCurrentGrow(!currentGrow);
updateGrow(!currentGrow);
legend={ControlGroupStrings.management.controlWidth.getWidthSwitchLegend()}
options={CONTROL_WIDTH_OPTIONS}
idSelected={currentWidth}
onChange={(newWidth: string) => {
setCurrentWidth(newWidth as ControlWidth);
updateWidth(newWidth as ControlWidth);
}}
data-test-subj="control-editor-grow-switch"
/>
</EuiFormRow>
) : null}
{updateGrow && (
<>
<EuiSpacer size="s" />
<EuiSwitch
label={ControlGroupStrings.manageControl.getGrowSwitchTitle()}
color="primary"
checked={currentGrow}
onChange={() => {
setCurrentGrow(!currentGrow);
updateGrow(!currentGrow);
}}
data-test-subj="control-editor-grow-switch"
/>
</>
)}
</>
</EuiFormRow>
{CustomSettings && (factory as IEditableControlFactory).controlEditorOptionsComponent && (
<EuiFormRow label={ControlGroupStrings.manageControl.getControlSettingsTitle()}>
<CustomSettings onChange={onTypeEditorChange} initialInput={embeddable?.getInput()} />
<CustomSettings
onChange={onTypeEditorChange}
initialInput={embeddable?.getInput()}
fieldType={fieldRegistry[selectedField].field.type}
/>
</EuiFormRow>
)}
{removeControl && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,11 @@
.optionsList--filterGroup {
width: 100%;
}

.optionsList--hiddenEditorForm {
margin-left: $euiSizeXXL + $euiSizeM;
}

.optionsList--sortPopover {
width: $euiSizeXL * 7;
}
Loading

0 comments on commit 1ed31e1

Please sign in to comment.