Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ML] Add control to show or hide empty fields in dropdown in Transform #195485

Merged
merged 45 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
3ec0455
Refactor to OptionList
qn895 Oct 2, 2024
4b7dd54
Refactor to OptionListWithStats
qn895 Oct 2, 2024
fcb84ba
Add styling for removal
qn895 Oct 2, 2024
0f6224a
Add for other rest of AD
qn895 Oct 2, 2024
cc0de0e
Update for geo field
qn895 Oct 2, 2024
57377aa
Update styling for DFA
qn895 Oct 2, 2024
e321f9d
Update types and styling
qn895 Oct 3, 2024
29658f7
Update width
qn895 Oct 3, 2024
585b2e3
Revert AIOps change
qn895 Oct 3, 2024
70399ee
Remove commented code
qn895 Oct 3, 2024
f9fd80f
[CI] Auto-commit changed files from 'node scripts/notice'
kibanamachine Oct 3, 2024
0915532
Update translations
qn895 Oct 3, 2024
711409f
Update component
qn895 Oct 3, 2024
a2efa16
FIx types
qn895 Oct 3, 2024
72bb627
Update layout
qn895 Oct 3, 2024
1e16d85
Use formcontrolledlayout instead
qn895 Oct 3, 2024
5a8269d
Update types
qn895 Oct 3, 2024
0636979
Merge branch 'ml-fields-empty-state-controls' into replace-combo-box
qn895 Oct 3, 2024
eecd220
Fix in advanced settings
qn895 Oct 3, 2024
2534c00
Update tests
qn895 Oct 4, 2024
2f75637
Update tests for AD
qn895 Oct 7, 2024
ddd93dd
Update for DFA
qn895 Oct 7, 2024
8d260a4
Update DFA tests
qn895 Oct 7, 2024
b32e595
Fix accessability test
qn895 Oct 7, 2024
0d5c2be
Merge remote-tracking branch 'upstream/main' into replace-combo-box
qn895 Oct 7, 2024
27852b0
Add for transform
qn895 Oct 7, 2024
7903d66
Mkae include empty option always true in tests
qn895 Oct 8, 2024
4013ba5
More transform
qn895 Oct 8, 2024
dd4e9c8
Update label
qn895 Oct 8, 2024
395cdf5
Update for AIops change point detection
qn895 Oct 8, 2024
e20c3ea
Update tests
qn895 Oct 8, 2024
6286cd6
Merge remote-tracking branch 'upstream/main' into ml-transform-aiops-…
qn895 Oct 14, 2024
a406fa9
Update for change point detection
qn895 Oct 14, 2024
3d1960b
Update prepend
qn895 Oct 14, 2024
927df17
Revert aiops changes
qn895 Oct 14, 2024
4ab7645
Update type
qn895 Oct 14, 2024
5f0d5c4
Merge remote-tracking branch 'upstream/main' into ml-transform-aiops-…
qn895 Oct 14, 2024
59e0998
Fix test
qn895 Oct 14, 2024
c96f14c
Fix so that if no populated fields, then show empty fields by default
qn895 Oct 15, 2024
de0c081
Fix comments
qn895 Oct 15, 2024
505b070
Merge remote-tracking branch 'upstream/main' into ml-transform-aiops-…
qn895 Oct 15, 2024
e2b75d2
Merge remote-tracking branch 'upstream/main' into ml-transform-aiops-…
qn895 Oct 15, 2024
c524247
Merge remote-tracking branch 'upstream/main' into ml-transform-aiops-…
qn895 Oct 15, 2024
9a6f4c0
Merge branch 'main' into ml-transform-aiops-fields-empty-option-list
elasticmachine Oct 15, 2024
0884c28
Merge branch 'main' into ml-transform-aiops-fields-empty-option-list
elasticmachine Oct 15, 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
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export const OptionsListPopover = ({
}: OptionsListPopoverProps) => {
const { populatedFields } = useFieldStatsFlyoutContext();

const [showEmptyFields, setShowEmptyFields] = useState(false);
const [showEmptyFields, setShowEmptyFields] = useState(populatedFields?.size > 0 ? false : true);
const id = useMemo(() => htmlIdGenerator()(), []);

const filteredOptions = useMemo(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@

import type { FC } from 'react';
import React, { useMemo, useState } from 'react';
import type { EuiComboBoxOptionOption, EuiComboBoxSingleSelectionShape } from '@elastic/eui';
import type {
EuiComboBoxOptionOption,
EuiComboBoxSingleSelectionShape,
EuiFormControlLayoutProps,
} from '@elastic/eui';
import { EuiInputPopover, htmlIdGenerator, EuiFormControlLayout, EuiFieldText } from '@elastic/eui';
import { css } from '@emotion/react';
import { i18n } from '@kbn/i18n';
Expand All @@ -18,8 +22,6 @@ import type { DropDownLabel } from './types';
const MIN_POPOVER_WIDTH = 400;

export const optionCss = css`
display: flex;
align-items: center;
.euiComboBoxOption__enterBadge {
display: none;
}
Expand All @@ -31,7 +33,8 @@ export const optionCss = css`
}
`;

interface OptionListWithFieldStatsProps {
interface OptionListWithFieldStatsProps
extends Pick<EuiFormControlLayoutProps, 'prepend' | 'compressed'> {
options: DropDownLabel[];
placeholder?: string;
'aria-label'?: string;
Expand All @@ -58,6 +61,8 @@ export const OptionListWithFieldStats: FC<OptionListWithFieldStatsProps> = ({
isDisabled,
isLoading,
isClearable = true,
prepend,
compressed,
'aria-label': ariaLabel,
'data-test-subj': dataTestSubj,
}) => {
Expand All @@ -68,13 +73,13 @@ export const OptionListWithFieldStats: FC<OptionListWithFieldStatsProps> = ({
const comboBoxOptions: DropDownLabel[] = useMemo(
() =>
Array.isArray(options)
? options.map(({ isEmpty, hideTrigger: hideInspectButton, ...o }) => ({
? options.map(({ isEmpty, ...o }) => ({
key: o.key,
Copy link
Member

Choose a reason for hiding this comment

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

as o is spread below, I don't believe key: o.key is needed

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated here 9b9ee5b (#195485)

...o,
css: optionCss,
// Change data-is-empty- because EUI is passing all props to dom element
// so isEmpty is invalid, but we need this info to render option correctly
'data-is-empty': isEmpty,
'data-hide-inspect': hideInspectButton,
}))
: [],
[options]
Expand All @@ -89,6 +94,8 @@ export const OptionListWithFieldStats: FC<OptionListWithFieldStatsProps> = ({
id={popoverId}
input={
<EuiFormControlLayout
prepend={prepend}
compressed={compressed}
fullWidth={fullWidth}
// Adding classname to make functional tests similar to EuiComboBox
className={singleSelection ? 'euiComboBox__inputWrap--plainText' : ''}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ import type { EuiComboBoxOptionOption, EuiSelectableOption } from '@elastic/eui'
import type { Aggregation, Field } from '@kbn/ml-anomaly-utils';

interface BaseOption<T> {
key?: string;
label: string | React.ReactNode;
key?: string;
value?: string | number | string[];
isEmpty?: boolean;
hideTrigger?: boolean;
'data-is-empty'?: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import React from 'react';

import type { EuiComboBoxOptionsListProps, EuiComboBoxOptionOption } from '@elastic/eui';
import { EuiComboBox } from '@elastic/eui';
import { OptionListWithFieldStats } from '@kbn/ml-field-stats-flyout/options_list_with_stats/option_list_with_stats';

interface Props {
options: EuiComboBoxOptionOption[];
Expand All @@ -30,7 +30,7 @@ export const DropDown: React.FC<Props> = ({
isDisabled,
}) => {
return (
<EuiComboBox
<OptionListWithFieldStats
fullWidth
placeholder={placeholder}
singleSelection={{ asPlainText: true }}
Expand All @@ -40,7 +40,6 @@ export const DropDown: React.FC<Props> = ({
isClearable={false}
data-test-subj={testSubj}
isDisabled={isDisabled}
renderOption={renderOption}
/>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,14 @@ import type { EuiComboBoxOptionOption } from '@elastic/eui';
import { EuiSpacer, EuiToolTip } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n-react';
import { FieldStatsInfoButton, useFieldStatsTrigger } from '@kbn/ml-field-stats-flyout';
import { AggListForm } from './list_form';
import { DropDown } from '../aggregation_dropdown';
import type { PivotAggsConfig } from '../../../../common';
import { PivotConfigurationContext } from '../pivot_configuration/pivot_configuration';
import { MAX_NESTING_SUB_AGGS } from '../../../../common/pivot_aggs';
import type { DropDownOptionWithField } from '../step_define/common/get_pivot_dropdown_options';
import type { DropDownOption } from '../../../../common/dropdown';

/**
* Component for managing sub-aggregation of the provided
Expand Down Expand Up @@ -54,11 +57,49 @@ export const SubAggsSection: FC<{ item: PivotAggsConfig }> = ({ item }) => {
}
return nestingLevel <= MAX_NESTING_SUB_AGGS;
}, [item]);
const { handleFieldStatsButtonClick, populatedFields } = useFieldStatsTrigger();

const options = useMemo(() => {
const opts: EuiComboBoxOptionOption[] = [];
state.aggOptions.forEach(({ label, field, options: aggOptions }: DropDownOptionWithField) => {
const isEmpty = populatedFields && field.id ? !populatedFields.has(field.id) : false;

const aggOption: DropDownOption = {
isGroupLabel: true,
key: field.id,
searchableLabel: label,
// @ts-ignore Purposefully passing label as element instead of string
// for more robust rendering
label: (
<FieldStatsInfoButton
isEmpty={populatedFields && !populatedFields.has(field.id)}
field={field}
label={label}
onButtonClick={handleFieldStatsButtonClick}
/>
),
};

if (aggOptions.length) {
opts.push(aggOption);
opts.push(
...aggOptions.map((o) => ({
...o,
isEmpty,
label: o.label,
Copy link
Member

Choose a reason for hiding this comment

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

o is spread here, so label: o.label shouldn't be needed

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated here 9b9ee5b (#195485)

isGroupLabel: false,
searchableLabel: o.label,
}))
);
}
});
return opts;
}, [handleFieldStatsButtonClick, populatedFields, state.aggOptions]);
// @TODO: remove
Copy link
Member

Choose a reason for hiding this comment

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

is this TODO needed still, if so, could it have a bit more explanation?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed here 9b9ee5b (#195485)

const dropdown = (
<DropDown
changeHandler={addSubAggHandler}
options={state.aggOptions}
options={options}
placeholder={i18n.translate('xpack.transform.stepDefineForm.addSubAggregationPlaceholder', {
defaultMessage: 'Add a sub-aggregation ...',
})}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import { EuiFormRow, type EuiComboBoxOptionOption } from '@elastic/eui';

import { i18n } from '@kbn/i18n';
import { useFieldStatsTrigger, FieldStatsInfoButton } from '@kbn/ml-field-stats-flyout';

import { type DropDownOptionWithField } from '../step_define/common/get_pivot_dropdown_options';
import type { DropDownOption } from '../../../../common';
import { AggListForm } from '../aggregation_list';
Expand Down Expand Up @@ -41,28 +40,43 @@ export const PivotConfiguration: FC<StepDefineFormHook['pivotConfig']> = memo(
const { aggList, aggOptions, aggOptionsData, groupByList, groupByOptions, groupByOptionsData } =
state;

const aggOptionsWithFieldStats: EuiComboBoxOptionOption[] = useMemo(
() =>
aggOptions.map(({ label, field, options }: DropDownOptionWithField) => {
const aggOption: DropDownOption = {
isGroupLabelOption: true,
key: field.id,
// @ts-ignore Purposefully passing label as element instead of string
// for more robust rendering
label: (
<FieldStatsInfoButton
isEmpty={populatedFields && !populatedFields.has(field.id)}
field={field}
label={label}
onButtonClick={handleFieldStatsButtonClick}
/>
),
options: options ?? [],
};
return aggOption;
}),
[aggOptions, handleFieldStatsButtonClick, populatedFields]
);
const aggOptionsWithFieldStats: EuiComboBoxOptionOption[] = useMemo(() => {
const opts: EuiComboBoxOptionOption[] = [];
aggOptions.forEach(({ label, field, options }: DropDownOptionWithField) => {
const isEmpty = populatedFields && field.id ? !populatedFields.has(field.id) : false;

const aggOption: DropDownOption = {
isGroupLabel: true,
key: field.id,
searchableLabel: label,
// @ts-ignore Purposefully passing label as element instead of string
// for more robust rendering
label: (
<FieldStatsInfoButton
isEmpty={populatedFields && !populatedFields.has(field.id)}
field={field}
label={label}
onButtonClick={handleFieldStatsButtonClick}
/>
),
};

if (options.length) {
opts.push(aggOption);
opts.push(
...options.map((o) => ({
...o,
isEmpty,
label: o.label,
Copy link
Member

Choose a reason for hiding this comment

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

same comment as above about label: o.label

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated here 9b9ee5b (#195485)

isGroupLabel: false,
searchableLabel: o.label,
}))
);
}
});
return opts;
}, [aggOptions, handleFieldStatsButtonClick, populatedFields]);

return (
<PivotConfigurationContext.Provider value={{ actions, state }}>
<EuiFormRow
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import React, { type FC } from 'react';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n-react';
import { EuiButtonIcon, EuiCallOut, EuiComboBox, EuiCopy, EuiFormRow } from '@elastic/eui';
import { useFieldStatsTrigger } from '@kbn/ml-field-stats-flyout';
import type { DropDownLabel } from '@kbn/ml-field-stats-flyout';
import { OptionListWithFieldStats, useFieldStatsTrigger } from '@kbn/ml-field-stats-flyout';
import type { LatestFunctionService } from './hooks/use_latest_function_config';

interface LatestFunctionFormProps {
Expand Down Expand Up @@ -73,7 +74,7 @@ export const LatestFunctionForm: FC<LatestFunctionFormProps> = ({
>
<>
{latestFunctionService.sortFieldOptions.length > 0 && (
<EuiComboBox
<OptionListWithFieldStats
fullWidth
placeholder={i18n.translate('xpack.transform.stepDefineForm.sortPlaceholder', {
defaultMessage: 'Add a date field ...',
Expand All @@ -83,15 +84,19 @@ export const LatestFunctionForm: FC<LatestFunctionFormProps> = ({
selectedOptions={
latestFunctionService.config.sort ? [latestFunctionService.config.sort] : []
}
onChange={(selected) => {
latestFunctionService.updateLatestFunctionConfig({
sort: { value: selected[0].value, label: selected[0].label as string },
});
onChange={(selected: DropDownLabel[]) => {
if (typeof selected[0].value === 'string') {
latestFunctionService.updateLatestFunctionConfig({
sort: {
value: selected[0].value,
label: selected[0].label?.toString(),
},
});
}
closeFlyout();
}}
isClearable={false}
data-test-subj="transformWizardSortFieldSelector"
renderOption={renderOption}
/>
)}
{latestFunctionService.sortFieldOptions.length === 0 && (
Expand Down
15 changes: 12 additions & 3 deletions x-pack/test/functional/services/transform/wizard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,10 @@ export function TransformWizardProvider({ getService, getPageObjects }: FtrProvi
},

async setSortFieldValue(identificator: string, label: string) {
await comboBox.set('transformWizardSortFieldSelector > comboBoxInput', identificator);
await ml.commonUI.setOptionsListWithFieldStatsValue(
'transformWizardSortFieldSelector > comboBoxInput',
identificator
);
await this.assertSortFieldInputValue(identificator);
},

Expand Down Expand Up @@ -507,7 +510,10 @@ export function TransformWizardProvider({ getService, getPageObjects }: FtrProvi
expectedLabel: string,
expectedIntervalLabel?: string
) {
await comboBox.set('transformGroupBySelection > comboBoxInput', identifier);
await ml.commonUI.setOptionsListWithFieldStatsValue(
'transformGroupBySelection > comboBoxInput',
identifier
);
await this.assertGroupByInputValue([]);
await this.assertGroupByEntryExists(index, expectedLabel, expectedIntervalLabel);
},
Expand Down Expand Up @@ -582,7 +588,10 @@ export function TransformWizardProvider({ getService, getPageObjects }: FtrProvi
formData?: Record<string, any>,
parentSelector = ''
) {
await comboBox.set(this.getAggComboBoxInputSelector(parentSelector), identifier);
await ml.commonUI.setOptionsListWithFieldStatsValue(
this.getAggComboBoxInputSelector(parentSelector),
identifier
);
await this.assertAggregationInputValue([], parentSelector);
await this.assertAggregationEntryExists(index, expectedLabel, parentSelector);

Expand Down