From b8c6f0061542299417263d3afc71f09f63d4620f Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Tue, 27 Aug 2019 03:48:54 -0700 Subject: [PATCH] [ML] Data frames: Fixes table sorting. (#43859) (#44070) - EuiInMemoryTable will not correctly reflect prop updates like sorting. So for example, when the component gets mounted with sorting={false} it will never consider a later update to make sorting available after all data is loaded. This PR fixes it by mounting the component only once sorting was set properly. This affected all data frame analytics/transform tables. - This consolidates code where we had multiple custom type definitions for EuiInMemoryTable because it's not based on TypeScript itself yet. The PR adds TypeScript Prop definitions for the component in ml/common/types/eui/in_memory_table.ts based on React propTypes and exposes a MlInMemoryTable component that wraps EuiInMemoryTable. I'll be in contact with the EUI team so they can make use of this for EUI itself. --- .../ml/common/types/eui/in_memory_table.ts | 184 ++++++++++++++++++ .../source_index_preview.tsx | 26 +-- .../components/step_define/pivot_preview.tsx | 19 +- .../components/transform_list/columns.tsx | 21 +- .../expanded_row_preview_pane.tsx | 3 +- .../transform_list/transform_list.tsx | 3 +- .../transform_list/transform_table.tsx | 22 +-- .../components/exploration/exploration.tsx | 22 +-- .../analytics_list/analytics_list.tsx | 3 +- .../analytics_list/analytics_table.tsx | 22 +-- 10 files changed, 241 insertions(+), 84 deletions(-) create mode 100644 x-pack/legacy/plugins/ml/common/types/eui/in_memory_table.ts diff --git a/x-pack/legacy/plugins/ml/common/types/eui/in_memory_table.ts b/x-pack/legacy/plugins/ml/common/types/eui/in_memory_table.ts new file mode 100644 index 00000000000000..b67a18562921b8 --- /dev/null +++ b/x-pack/legacy/plugins/ml/common/types/eui/in_memory_table.ts @@ -0,0 +1,184 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { Component, HTMLAttributes, ReactElement, ReactNode } from 'react'; + +import { CommonProps, EuiInMemoryTable } from '@elastic/eui'; + +// At some point this could maybe solved with a generic . +type Item = any; + +// Not using an enum here because the original HorizontalAlignment is also a union type of string. +type HorizontalAlignment = 'left' | 'center' | 'right'; + +type SortableFunc = (item: Item) => any; +type Sortable = boolean | SortableFunc; +type DATA_TYPES = any; +type FooterFunc = (payload: { items: Item[]; pagination: any }) => ReactNode; +type RenderFunc = (value: any, record?: any) => ReactNode; +export interface FieldDataColumnType { + field: string; + name: ReactNode; + description?: string; + dataType?: DATA_TYPES; + width?: string; + sortable?: Sortable; + align?: HorizontalAlignment; + truncateText?: boolean; + render?: RenderFunc; + footer?: string | ReactElement | FooterFunc; +} + +export interface ComputedColumnType { + render: RenderFunc; + name?: ReactNode; + description?: string; + sortable?: (item: Item) => any; + width?: string; + truncateText?: boolean; +} + +type ICON_TYPES = any; +type IconTypesFunc = (item: Item) => ICON_TYPES; // (item) => oneOf(ICON_TYPES) +type BUTTON_ICON_COLORS = any; +type ButtonIconColorsFunc = (item: Item) => BUTTON_ICON_COLORS; // (item) => oneOf(ICON_BUTTON_COLORS) +interface DefaultItemActionType { + type?: 'icon' | 'button'; + name: string; + description: string; + onClick?(item: Item): void; + href?: string; + target?: string; + available?(item: Item): boolean; + enabled?(item: Item): boolean; + isPrimary?: boolean; + icon?: ICON_TYPES | IconTypesFunc; // required when type is 'icon' + color?: BUTTON_ICON_COLORS | ButtonIconColorsFunc; +} + +interface CustomItemActionType { + render(item: Item, enabled: boolean): ReactNode; + available?(item: Item): boolean; + enabled?(item: Item): boolean; + isPrimary?: boolean; +} + +export interface ExpanderColumnType { + align?: HorizontalAlignment; + width?: string; + isExpander: boolean; + render: RenderFunc; +} + +type SupportedItemActionType = DefaultItemActionType | CustomItemActionType; + +export interface ActionsColumnType { + actions: SupportedItemActionType[]; + name?: ReactNode; + description?: string; + width?: string; +} + +export type ColumnType = + | ActionsColumnType + | ComputedColumnType + | ExpanderColumnType + | FieldDataColumnType; + +type QueryType = any; + +interface Schema { + strict?: boolean; + fields?: Record; + flags?: string[]; +} + +interface SearchBoxConfigPropTypes { + placeholder?: string; + incremental?: boolean; + schema?: Schema; +} + +interface Box { + placeholder?: string; + incremental?: boolean; + // here we enable the user to just assign 'true' to the schema, in which case + // we will auto-generate it out of the columns configuration + schema?: boolean | SearchBoxConfigPropTypes['schema']; +} + +type SearchFiltersFiltersType = any; + +interface ExecuteQueryOptions { + defaultFields: string[]; + isClauseMatcher: () => void; + explain: boolean; +} + +type SearchType = + | boolean + | { + toolsLeft?: ReactNode; + toolsRight?: ReactNode; + defaultQuery?: QueryType; + box?: Box; + filters?: SearchFiltersFiltersType; + onChange?: (arg: any) => void; + executeQueryOptions?: ExecuteQueryOptions; + }; + +interface PageSizeOptions { + pageSizeOptions: number[]; +} +interface InitialPageOptions extends PageSizeOptions { + initialPageIndex: number; + initialPageSize: number; +} +type Pagination = boolean | PageSizeOptions | InitialPageOptions; + +type PropertySortType = any; +type Sorting = boolean | { sort: PropertySortType }; + +type SelectionType = any; + +type ItemIdTypeFunc = (item: Item) => string; +type ItemIdType = + | string // the name of the item id property + | ItemIdTypeFunc; + +export type EuiInMemoryTableProps = CommonProps & { + columns: ColumnType[]; + hasActions?: boolean; + isExpandable?: boolean; + isSelectable?: boolean; + items?: Item[]; + loading?: boolean; + message?: HTMLAttributes; + error?: string; + compressed?: boolean; + search?: SearchType; + pagination?: Pagination; + sorting?: Sorting; + // Set `allowNeutralSort` to false to force column sorting. Defaults to true. + allowNeutralSort?: boolean; + selection?: SelectionType; + itemId?: ItemIdType; + itemIdToExpandedRowMap?: Record; + rowProps?: () => void | Record; + cellProps?: () => void | Record; + onTableChange?: (arg: { + page: { index: number; size: number }; + sort: { field: string; direction: string }; + }) => void; +}; + +interface ComponentWithConstructor extends Component { + new (): Component; +} + +export const MlInMemoryTable = (EuiInMemoryTable as any) as ComponentWithConstructor< + EuiInMemoryTableProps +>; diff --git a/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/source_index_preview/source_index_preview.tsx b/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/source_index_preview/source_index_preview.tsx index 9bda576b8c2605..a08f60c868b7b9 100644 --- a/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/source_index_preview/source_index_preview.tsx +++ b/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/source_index_preview/source_index_preview.tsx @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { FunctionComponent, useState } from 'react'; +import React, { useState } from 'react'; import moment from 'moment-timezone'; import { i18n } from '@kbn/i18n'; @@ -18,8 +18,6 @@ import { EuiCopy, EuiFlexGroup, EuiFlexItem, - EuiInMemoryTable, - EuiInMemoryTableProps, EuiPanel, EuiPopover, EuiPopoverTitle, @@ -30,14 +28,7 @@ import { RIGHT_ALIGNMENT, } from '@elastic/eui'; -// TODO EUI's types for EuiInMemoryTable is missing these props -interface ExpandableTableProps extends EuiInMemoryTableProps { - compressed: boolean; - itemIdToExpandedRowMap: ItemIdToExpandedRowMap; - isExpandable: boolean; -} - -const ExpandableTable = (EuiInMemoryTable as any) as FunctionComponent; +import { ColumnType, MlInMemoryTable } from '../../../../../../common/types/eui/in_memory_table'; import { KBN_FIELD_TYPES } from '../../../../../../common/constants/field_types'; import { Dictionary } from '../../../../../../common/types/common'; @@ -204,13 +195,13 @@ export const SourceIndexPreview: React.SFC = React.memo(({ cellClick, que docFieldsCount = docFields.length; } - const columns = selectedFields.map(k => { - const column = { + const columns: ColumnType[] = selectedFields.map(k => { + const column: ColumnType = { field: `_source["${k}"]`, name: k, sortable: true, truncateText: true, - } as Dictionary; + }; const field = indexPattern.fields.find(f => f.name === k); @@ -315,7 +306,7 @@ export const SourceIndexPreview: React.SFC = React.memo(({ cellClick, que if (columns.length > 0) { sorting = { sort: { - field: columns[0].field, + field: `_source["${selectedFields[0]}"]`, direction: SORT_DIRECTON.ASC, }, }; @@ -425,8 +416,9 @@ export const SourceIndexPreview: React.SFC = React.memo(({ cellClick, que {status !== SOURCE_INDEX_STATUS.LOADING && ( )} - {clearTable === false && ( - 0 && sorting !== false && ( + ; - function sortColumns(groupByArr: PivotGroupByConfig[]) { return (a: string, b: string) => { // make sure groupBy fields are always most left columns @@ -237,7 +229,7 @@ export const PivotPreview: SFC = React.memo(({ aggs, groupBy, const columns = columnKeys .filter(k => typeof dataFramePreviewMappings.properties[k] !== 'undefined') .map(k => { - const column: Dictionary = { + const column: ColumnType = { field: k, name: k, sortable: true, @@ -290,8 +282,9 @@ export const PivotPreview: SFC = React.memo(({ aggs, groupBy, {status !== PIVOT_PREVIEW_STATUS.LOADING && ( )} - {dataFramePreviewData.length > 0 && clearTable === false && ( - 0 && clearTable === false && columns.length > 0 && ( + = ({ transformConfig }) => { return ( { { items={filterActive ? filteredTransforms : transforms} itemId={DataFrameTransformListColumn.id} itemIdToExpandedRowMap={itemIdToExpandedRowMap} - onChange={onTableChange} + onTableChange={onTableChange} pagination={pagination} selection={selection} sorting={sorting} diff --git a/x-pack/legacy/plugins/ml/public/data_frame/pages/transform_management/components/transform_list/transform_table.tsx b/x-pack/legacy/plugins/ml/public/data_frame/pages/transform_management/components/transform_list/transform_table.tsx index aa810105a6a54a..bfb4e86abe8b5a 100644 --- a/x-pack/legacy/plugins/ml/public/data_frame/pages/transform_management/components/transform_list/transform_table.tsx +++ b/x-pack/legacy/plugins/ml/public/data_frame/pages/transform_management/components/transform_list/transform_table.tsx @@ -7,11 +7,11 @@ // This component extends EuiInMemoryTable with some // fixes and TS specs until the changes become available upstream. -import React, { Component, Fragment } from 'react'; +import React, { Fragment } from 'react'; -import { EuiInMemoryTable, EuiInMemoryTableProps, EuiProgress } from '@elastic/eui'; +import { EuiProgress } from '@elastic/eui'; -import { ItemIdToExpandedRowMap } from './common'; +import { MlInMemoryTable } from '../../../../../../common/types/eui/in_memory_table'; // The built in loading progress bar of EuiInMemoryTable causes a full DOM replacement // of the table and doesn't play well with auto-refreshing. That's why we're displaying @@ -73,21 +73,7 @@ const getInitialSorting = (columns: any, sorting: any) => { }; }; -// TODO EUI's types for EuiInMemoryTable is missing these props -interface ExpandableTableProps extends EuiInMemoryTableProps { - itemIdToExpandedRowMap?: ItemIdToExpandedRowMap; - isExpandable?: boolean; - onChange({ page }: { page?: {} | undefined }): void; - loading?: boolean; - compressed?: boolean; - error?: string; -} -interface ComponentWithConstructor extends Component { - new (): Component; -} -const ExpandableTable = (EuiInMemoryTable as any) as ComponentWithConstructor; - -export class TransformTable extends ExpandableTable { +export class TransformTable extends MlInMemoryTable { static getDerivedStateFromProps(nextProps: any, prevState: any) { const derivedState = { ...prevState.prevProps, diff --git a/x-pack/legacy/plugins/ml/public/data_frame_analytics/pages/analytics_exploration/components/exploration/exploration.tsx b/x-pack/legacy/plugins/ml/public/data_frame_analytics/pages/analytics_exploration/components/exploration/exploration.tsx index 0528baf19a0119..48bd31c34dcfa8 100644 --- a/x-pack/legacy/plugins/ml/public/data_frame_analytics/pages/analytics_exploration/components/exploration/exploration.tsx +++ b/x-pack/legacy/plugins/ml/public/data_frame_analytics/pages/analytics_exploration/components/exploration/exploration.tsx @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { FC, FunctionComponent, useEffect, useState } from 'react'; +import React, { FC, useEffect, useState } from 'react'; import moment from 'moment-timezone'; import { i18n } from '@kbn/i18n'; @@ -18,8 +18,6 @@ import { EuiCheckbox, EuiFlexGroup, EuiFlexItem, - EuiInMemoryTable, - EuiInMemoryTableProps, EuiPanel, EuiPopover, EuiPopoverTitle, @@ -32,12 +30,7 @@ import { import euiThemeLight from '@elastic/eui/dist/eui_theme_light.json'; import euiThemeDark from '@elastic/eui/dist/eui_theme_dark.json'; -// TODO EUI's types for EuiInMemoryTable is missing these props -interface ExpandableTableProps extends EuiInMemoryTableProps { - compressed: boolean; -} - -const ExpandableTable = (EuiInMemoryTable as any) as FunctionComponent; +import { ColumnType, MlInMemoryTable } from '../../../../../../common/types/eui/in_memory_table'; import { useUiChromeContext } from '../../../../../contexts/ui/use_ui_chrome_context'; @@ -218,7 +211,7 @@ export const Exploration: FC = React.memo(({ jobId }) => { docFieldsCount = docFields.length; } - const columns = []; + const columns: ColumnType[] = []; if (selectedFields.length > 0 && tableItems.length > 0) { // table cell color coding takes into account: @@ -241,12 +234,12 @@ export const Exploration: FC = React.memo(({ jobId }) => { ...selectedFields .sort(sortColumns(tableItems[0]._source, jobConfig.dest.results_field)) .map(k => { - const column = { + const column: ColumnType = { field: `_source["${k}"]`, name: k, sortable: true, truncateText: true, - } as Record; + }; const render = (d: any, fullItem: EsDoc) => { if (Array.isArray(d) && d.every(item => typeof item === 'string')) { @@ -367,7 +360,7 @@ export const Exploration: FC = React.memo(({ jobId }) => { if (columns.length > 0) { sorting = { sort: { - field: columns[0].field, + field: `_source["${selectedFields[0]}"]`, direction: SORT_DIRECTON.ASC, }, }; @@ -443,7 +436,8 @@ export const Exploration: FC = React.memo(({ jobId }) => { )} {clearTable === false && columns.length > 0 && ( - = ({ = ({ items={filterActive ? filteredAnalytics : analytics} itemId={DataFrameAnalyticsListColumn.id} itemIdToExpandedRowMap={itemIdToExpandedRowMap} - onChange={onTableChange} + onTableChange={onTableChange} pagination={pagination} sorting={sorting} search={search} diff --git a/x-pack/legacy/plugins/ml/public/data_frame_analytics/pages/analytics_management/components/analytics_list/analytics_table.tsx b/x-pack/legacy/plugins/ml/public/data_frame_analytics/pages/analytics_management/components/analytics_list/analytics_table.tsx index ca00191746f06f..006aef70e55280 100644 --- a/x-pack/legacy/plugins/ml/public/data_frame_analytics/pages/analytics_management/components/analytics_list/analytics_table.tsx +++ b/x-pack/legacy/plugins/ml/public/data_frame_analytics/pages/analytics_management/components/analytics_list/analytics_table.tsx @@ -7,11 +7,9 @@ // This component extends EuiInMemoryTable with some // fixes and TS specs until the changes become available upstream. -import React, { Component, Fragment } from 'react'; +import React, { Fragment } from 'react'; -import { EuiInMemoryTable, EuiInMemoryTableProps, EuiProgress } from '@elastic/eui'; - -import { ItemIdToExpandedRowMap } from './common'; +import { EuiProgress } from '@elastic/eui'; // The built in loading progress bar of EuiInMemoryTable causes a full DOM replacement // of the table and doesn't play well with auto-refreshing. That's why we're displaying @@ -73,21 +71,9 @@ const getInitialSorting = (columns: any, sorting: any) => { }; }; -// TODO EUI's types for EuiInMemoryTable is missing these props -interface ExpandableTableProps extends EuiInMemoryTableProps { - itemIdToExpandedRowMap?: ItemIdToExpandedRowMap; - isExpandable?: boolean; - onChange({ page }: { page?: {} | undefined }): void; - loading?: boolean; - compressed?: boolean; - error?: string; -} -interface ComponentWithConstructor extends Component { - new (): Component; -} -const ExpandableTable = (EuiInMemoryTable as any) as ComponentWithConstructor; +import { MlInMemoryTable } from '../../../../../../common/types/eui/in_memory_table'; -export class AnalyticsTable extends ExpandableTable { +export class AnalyticsTable extends MlInMemoryTable { static getDerivedStateFromProps(nextProps: any, prevState: any) { const derivedState = { ...prevState.prevProps,