From 2629fa82a871904c745d155b46f49d0e2a1f1d0a Mon Sep 17 00:00:00 2001 From: Davis McPhee Date: Wed, 4 Sep 2024 00:26:49 -0300 Subject: [PATCH] [Discover] [Unified Data Table] Improve Discover / Unified Data Table performance in ES|QL mode (#191249) ## Summary This PR includes several performance improvements for ES|QL mode in Discover: - Memoize `UnifiedDataTableRenderCellValue` to reduce re-renders. - Show the loading spinner immediately on state changes that should trigger a refetch so that changed state isn't applied to the grid before the fetch completes. This also helps to reduce re-renders on state changes. - Replace [`EuiDataGrid` in-memory sorting](https://eui.elastic.co/#/tabular-content/data-grid-advanced#data-grid-in-memory) with a custom implementation using the `@kbn/sort-predicates` package (the same one Lens uses for its data grid). EUI in-memory sorting has a drastic impact on performance because it renders the entire grid off screen in order to parse cell values, detect their schema, and then sort the rows. This can cause rendering delays of several seconds for larger datasets on each render of the data grid. The new approach will sort in memory based on actual field values and pass the sorted rows to the data grid, which is both much more performant as well as improving the sorting behaviour across field types. - Use the `overscanRowCount` option from [react-window](https://github.com/bvaughn/react-window) to render some additional rows outside of view in the data grid, which minimizes pop-in of rows while scrolling quickly. There are also a couple of bug fixes included in the PR as a result of the new sorting behaviour: - Numeric columns are now sorted correctly where previously they used alphabetic sorting. - The "Copy column" action in data grid columns now correctly copies the column values in the current sort order instead of copying them unsorted as it used to. Before - 66,904 `UnifiedDataTableRenderCellValue` render calls: https://github.com/user-attachments/assets/04caca0d-35e7-421c-b8e3-5a37a1d65177 After - 424 `UnifiedDataTableRenderCellValue` render calls: https://github.com/user-attachments/assets/dba1a106-6bd6-4f1b-b60e-4741228c488b Before profile: profile_old After profile: profile_new Before scrolling: https://github.com/user-attachments/assets/8d9dbdbd-df3e-48df-af9a-539deda8d5ea After scrolling: https://github.com/user-attachments/assets/d6309275-e00d-44ee-a59f-2d1e5d30bb60 ### Checklist - [ ] 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) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [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 - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] 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)) - [ ] 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> Co-authored-by: Jatin Kathuria --- .../src/__mocks__/es_hits.ts | 2 + .../src/components/data_table.test.tsx | 1508 ++++++++++------- .../src/components/data_table.tsx | 140 +- .../src/hooks/use_sorting.ts | 113 ++ .../src/utils/get_render_cell_value.tsx | 16 +- packages/kbn-unified-data-table/tsconfig.json | 1 + .../utils/build_state_subscribe.ts | 6 + .../apps/discover/esql/_esql_view.ts | 25 +- .../apps/discover/group5/_url_state.ts | 6 +- .../data_table/index.test.tsx | 6 +- .../common/discover/esql/_esql_view.ts | 14 +- 11 files changed, 1137 insertions(+), 700 deletions(-) create mode 100644 packages/kbn-unified-data-table/src/hooks/use_sorting.ts diff --git a/packages/kbn-discover-utils/src/__mocks__/es_hits.ts b/packages/kbn-discover-utils/src/__mocks__/es_hits.ts index 0cde2c6a00d19..ff5feab164add 100644 --- a/packages/kbn-discover-utils/src/__mocks__/es_hits.ts +++ b/packages/kbn-discover-utils/src/__mocks__/es_hits.ts @@ -72,6 +72,8 @@ const generateFieldValue = (field: DataViewField, index: number) => { return Array.from(field.name).reduce((sum, char) => sum + char.charCodeAt(0) + index, 0); case KBN_FIELD_TYPES.STRING: return `${field.name}_${index}`; + case KBN_FIELD_TYPES._SOURCE: + return { [field.name]: `${field.name}_${index}` }; default: throw new Error(`Unsupported type ${field.type}`); } diff --git a/packages/kbn-unified-data-table/src/components/data_table.test.tsx b/packages/kbn-unified-data-table/src/components/data_table.test.tsx index f78405f12be82..11863df636e86 100644 --- a/packages/kbn-unified-data-table/src/components/data_table.test.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table.test.tsx @@ -16,7 +16,12 @@ import { import { Storage } from '@kbn/kibana-utils-plugin/public'; import { act } from 'react-dom/test-utils'; import { findTestSubject } from '@elastic/eui/lib/test'; -import { buildDataViewMock, deepMockedFields, esHitsMock } from '@kbn/discover-utils/src/__mocks__'; +import { + buildDataViewMock, + deepMockedFields, + esHitsMock, + generateEsHits, +} from '@kbn/discover-utils/src/__mocks__'; import { mountWithIntl } from '@kbn/test-jest-helpers'; import { DataLoadingState, UnifiedDataTable, UnifiedDataTableProps } from './data_table'; import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public'; @@ -30,7 +35,7 @@ import { testTrailingControlColumns, } from '../../__mocks__/external_control_columns'; import { DatatableColumnType } from '@kbn/expressions-plugin/common'; -import { render, screen } from '@testing-library/react'; +import { render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { CELL_CLASS } from '../utils/get_render_cell_value'; import { defaultTimeColumnWidth } from '../constants'; @@ -44,7 +49,9 @@ jest.mock('@kbn/cell-actions', () => ({ useDataGridColumnsCellActions: (prop: unknown) => mockUseDataGridColumnsCellActions(prop), })); -export const dataViewMock = buildDataViewMock({ +const EXTENDED_JEST_TIMEOUT = 10000; + +const dataViewMock = buildDataViewMock({ name: 'the-data-view', fields: deepMockedFields, timeFieldName: '@timestamp', @@ -96,10 +103,11 @@ const DataTable = (props: Partial) => ( const capabilities = capabilitiesServiceMock.createStartContract().capabilities; -const renderDataTable = (props: Partial) => { +const renderDataTable = async (props: Partial) => { const DataTableWrapped = () => { const [columns, setColumns] = useState(props.columns ?? []); const [settings, setSettings] = useState(props.settings); + const [sort, setSort] = useState(props.sort ?? []); const { onSetColumns } = useColumns({ capabilities, @@ -136,18 +144,26 @@ const renderDataTable = (props: Partial) => { }, }); }} + sort={sort} + onSort={setSort as UnifiedDataTableProps['onSort']} /> ); }; render(); + + // EuiDataGrid makes state updates after calling requestAnimationFrame, which can lead + // to "Can't perform a React state update on an unmounted component." warnings in tests, + // so we need to wait for the next animation frame to avoid this + await screen.findByTestId('discoverDocTable'); + await act(() => new Promise((resolve) => requestAnimationFrame(() => resolve()))); }; async function getComponent(props: UnifiedDataTableProps = getProps()) { const component = mountWithIntl(); await act(async () => { - // needed by cell actions to complete async loading + // needed by cell actions to complete async loading and avoid act warning component.update(); }); return component; @@ -210,203 +226,357 @@ describe('UnifiedDataTable', () => { component = await getComponent(); }); - test('no documents are selected initially', async () => { - expect(getSelectedDocNr(component)).toBe(0); - expect(getDisplayedDocNr(component)).toBe(5); - }); + test( + 'no documents are selected initially', + async () => { + expect(getSelectedDocNr(component)).toBe(0); + expect(getDisplayedDocNr(component)).toBe(5); + }, + EXTENDED_JEST_TIMEOUT + ); - test('Allows selection/deselection of multiple documents', async () => { - await toggleDocSelection(component, esHitsMock[0]); - expect(getSelectedDocNr(component)).toBe(1); - await toggleDocSelection(component, esHitsMock[1]); - expect(getSelectedDocNr(component)).toBe(2); - await toggleDocSelection(component, esHitsMock[1]); - expect(getSelectedDocNr(component)).toBe(1); - }); + test( + 'Allows selection/deselection of multiple documents', + async () => { + await toggleDocSelection(component, esHitsMock[0]); + expect(getSelectedDocNr(component)).toBe(1); + await toggleDocSelection(component, esHitsMock[1]); + expect(getSelectedDocNr(component)).toBe(2); + await toggleDocSelection(component, esHitsMock[1]); + expect(getSelectedDocNr(component)).toBe(1); + }, + EXTENDED_JEST_TIMEOUT + ); - test('deselection of all selected documents', async () => { - await toggleDocSelection(component, esHitsMock[0]); - await toggleDocSelection(component, esHitsMock[1]); - expect(getSelectedDocNr(component)).toBe(2); - findTestSubject(component, 'unifiedDataTableSelectionBtn').simulate('click'); - findTestSubject(component, 'dscGridClearSelectedDocuments').simulate('click'); - expect(getSelectedDocNr(component)).toBe(0); - }); + test( + 'deselection of all selected documents', + async () => { + await toggleDocSelection(component, esHitsMock[0]); + await toggleDocSelection(component, esHitsMock[1]); + expect(getSelectedDocNr(component)).toBe(2); + findTestSubject(component, 'unifiedDataTableSelectionBtn').simulate('click'); + findTestSubject(component, 'dscGridClearSelectedDocuments').simulate('click'); + expect(getSelectedDocNr(component)).toBe(0); + }, + EXTENDED_JEST_TIMEOUT + ); - test('showing only selected documents and undo selection', async () => { - await toggleDocSelection(component, esHitsMock[0]); - await toggleDocSelection(component, esHitsMock[1]); - expect(getSelectedDocNr(component)).toBe(2); - findTestSubject(component, 'unifiedDataTableSelectionBtn').simulate('click'); - findTestSubject(component, 'dscGridShowSelectedDocuments').simulate('click'); - expect(getDisplayedDocNr(component)).toBe(2); - findTestSubject(component, 'unifiedDataTableSelectionBtn').simulate('click'); - component.update(); - findTestSubject(component, 'dscGridShowAllDocuments').simulate('click'); - expect(getDisplayedDocNr(component)).toBe(5); - }); + test( + 'showing only selected documents and undo selection', + async () => { + await toggleDocSelection(component, esHitsMock[0]); + await toggleDocSelection(component, esHitsMock[1]); + expect(getSelectedDocNr(component)).toBe(2); + findTestSubject(component, 'unifiedDataTableSelectionBtn').simulate('click'); + findTestSubject(component, 'dscGridShowSelectedDocuments').simulate('click'); + expect(getDisplayedDocNr(component)).toBe(2); + findTestSubject(component, 'unifiedDataTableSelectionBtn').simulate('click'); + component.update(); + findTestSubject(component, 'dscGridShowAllDocuments').simulate('click'); + expect(getDisplayedDocNr(component)).toBe(5); + }, + EXTENDED_JEST_TIMEOUT + ); - test('showing selected documents, underlying data changes, all documents are displayed, selection is gone', async () => { - await toggleDocSelection(component, esHitsMock[0]); - await toggleDocSelection(component, esHitsMock[1]); - expect(getSelectedDocNr(component)).toBe(2); - findTestSubject(component, 'unifiedDataTableSelectionBtn').simulate('click'); - findTestSubject(component, 'dscGridShowSelectedDocuments').simulate('click'); - expect(getDisplayedDocNr(component)).toBe(2); - component.setProps({ - rows: [ - { - _index: 'i', - _id: '6', - _score: 1, - _source: { - date: '2020-20-02T12:12:12.128', - name: 'test6', - extension: 'doc', - bytes: 50, + test( + 'showing selected documents, underlying data changes, all documents are displayed, selection is gone', + async () => { + await toggleDocSelection(component, esHitsMock[0]); + await toggleDocSelection(component, esHitsMock[1]); + expect(getSelectedDocNr(component)).toBe(2); + findTestSubject(component, 'unifiedDataTableSelectionBtn').simulate('click'); + findTestSubject(component, 'dscGridShowSelectedDocuments').simulate('click'); + expect(getDisplayedDocNr(component)).toBe(2); + component.setProps({ + rows: [ + { + _index: 'i', + _id: '6', + _score: 1, + _source: { + date: '2020-20-02T12:12:12.128', + name: 'test6', + extension: 'doc', + bytes: 50, + }, }, - }, - ].map((row) => buildDataTableRecord(row, dataViewMock)), - }); - expect(getDisplayedDocNr(component)).toBe(1); - expect(getSelectedDocNr(component)).toBe(0); - }); + ].map((row) => buildDataTableRecord(row, dataViewMock)), + }); + expect(getDisplayedDocNr(component)).toBe(1); + expect(getSelectedDocNr(component)).toBe(0); + }, + EXTENDED_JEST_TIMEOUT + ); - test('showing only selected documents and remove filter deselecting each doc manually', async () => { - await toggleDocSelection(component, esHitsMock[0]); - findTestSubject(component, 'unifiedDataTableSelectionBtn').simulate('click'); - findTestSubject(component, 'dscGridShowSelectedDocuments').simulate('click'); - expect(getDisplayedDocNr(component)).toBe(1); - await toggleDocSelection(component, esHitsMock[0]); - expect(getDisplayedDocNr(component)).toBe(5); - await toggleDocSelection(component, esHitsMock[0]); - expect(getDisplayedDocNr(component)).toBe(5); - }); + test( + 'showing only selected documents and remove filter deselecting each doc manually', + async () => { + await toggleDocSelection(component, esHitsMock[0]); + findTestSubject(component, 'unifiedDataTableSelectionBtn').simulate('click'); + findTestSubject(component, 'dscGridShowSelectedDocuments').simulate('click'); + expect(getDisplayedDocNr(component)).toBe(1); + await toggleDocSelection(component, esHitsMock[0]); + expect(getDisplayedDocNr(component)).toBe(5); + await toggleDocSelection(component, esHitsMock[0]); + expect(getDisplayedDocNr(component)).toBe(5); + }, + EXTENDED_JEST_TIMEOUT + ); - test('copying selected documents to clipboard as JSON', async () => { - await toggleDocSelection(component, esHitsMock[0]); - findTestSubject(component, 'unifiedDataTableSelectionBtn').simulate('click'); - findTestSubject(component, 'dscGridCopySelectedDocumentsJSON').simulate('click'); - expect(navigator.clipboard.writeText).toHaveBeenCalledWith( - '[{"_index":"i","_id":"1","_score":1,"_type":"_doc","_source":{"date":"2020-20-01T12:12:12.123","message":"test1","bytes":20}}]' - ); - }); + test( + 'copying selected documents to clipboard as JSON', + async () => { + await toggleDocSelection(component, esHitsMock[0]); + findTestSubject(component, 'unifiedDataTableSelectionBtn').simulate('click'); + findTestSubject(component, 'dscGridCopySelectedDocumentsJSON').simulate('click'); + // wait for async copy action to avoid act warning + await act(() => new Promise((resolve) => setTimeout(resolve, 0))); + expect(navigator.clipboard.writeText).toHaveBeenCalledWith( + '[{"_index":"i","_id":"1","_score":1,"_type":"_doc","_source":{"date":"2020-20-01T12:12:12.123","message":"test1","bytes":20}}]' + ); + }, + EXTENDED_JEST_TIMEOUT + ); - test('copying selected documents to clipboard as text', async () => { - await toggleDocSelection(component, esHitsMock[2]); - await toggleDocSelection(component, esHitsMock[1]); - findTestSubject(component, 'unifiedDataTableSelectionBtn').simulate('click'); - findTestSubject(component, 'unifiedDataTableCopyRowsAsText').simulate('click'); - expect(navigator.clipboard.writeText).toHaveBeenCalledWith( - '"\'@timestamp"\t"_index"\t"_score"\tbytes\tdate\textension\tmessage\tname\n-\ti\t1\t-\t"2020-20-01T12:12:12.124"\tjpg\t-\ttest2\n-\ti\t1\t50\t"2020-20-01T12:12:12.124"\tgif\t-\ttest3' - ); - }); + test( + 'copying selected documents to clipboard as text', + async () => { + await toggleDocSelection(component, esHitsMock[2]); + await toggleDocSelection(component, esHitsMock[1]); + findTestSubject(component, 'unifiedDataTableSelectionBtn').simulate('click'); + findTestSubject(component, 'unifiedDataTableCopyRowsAsText').simulate('click'); + // wait for async copy action to avoid act warning + await act(() => new Promise((resolve) => setTimeout(resolve, 0))); + expect(navigator.clipboard.writeText).toHaveBeenCalledWith( + '"\'@timestamp"\t"_index"\t"_score"\tbytes\tdate\textension\tmessage\tname\n-\ti\t1\t-\t"2020-20-01T12:12:12.124"\tjpg\t-\ttest2\n-\ti\t1\t50\t"2020-20-01T12:12:12.124"\tgif\t-\ttest3' + ); + }, + EXTENDED_JEST_TIMEOUT + ); - test('copying selected columns to clipboard as text', async () => { - component = await getComponent({ - ...getProps(), - columns: ['date', 'extension', 'name'], - }); - await toggleDocSelection(component, esHitsMock[2]); - await toggleDocSelection(component, esHitsMock[1]); - findTestSubject(component, 'unifiedDataTableSelectionBtn').simulate('click'); - findTestSubject(component, 'unifiedDataTableCopyRowsAsText').simulate('click'); - expect(navigator.clipboard.writeText).toHaveBeenCalledWith( - '"\'@timestamp"\tdate\textension\tname\n-\t"2020-20-01T12:12:12.124"\tjpg\ttest2\n-\t"2020-20-01T12:12:12.124"\tgif\ttest3' - ); - }); + test( + 'copying selected columns to clipboard as text', + async () => { + component = await getComponent({ + ...getProps(), + columns: ['date', 'extension', 'name'], + }); + await toggleDocSelection(component, esHitsMock[2]); + await toggleDocSelection(component, esHitsMock[1]); + findTestSubject(component, 'unifiedDataTableSelectionBtn').simulate('click'); + findTestSubject(component, 'unifiedDataTableCopyRowsAsText').simulate('click'); + // wait for async copy action to avoid act warning + await act(() => new Promise((resolve) => setTimeout(resolve, 0))); + expect(navigator.clipboard.writeText).toHaveBeenCalledWith( + '"\'@timestamp"\tdate\textension\tname\n-\t"2020-20-01T12:12:12.124"\tjpg\ttest2\n-\t"2020-20-01T12:12:12.124"\tgif\ttest3' + ); + }, + EXTENDED_JEST_TIMEOUT + ); }); describe('edit field button', () => { - it('should render the edit field button if onFieldEdited is provided', async () => { - renderDataTable({ columns: ['message'], onFieldEdited: jest.fn() }); - expect(screen.queryByTestId('dataGridHeaderCellActionGroup-message')).not.toBeInTheDocument(); - userEvent.click(screen.getByRole('button', { name: 'message' })); - expect(screen.getByTestId('dataGridHeaderCellActionGroup-message')).toBeInTheDocument(); - expect(screen.getByTestId('gridEditFieldButton')).toBeInTheDocument(); - }); + it( + 'should render the edit field button if onFieldEdited is provided', + async () => { + await renderDataTable({ columns: ['message'], onFieldEdited: jest.fn() }); + expect( + screen.queryByTestId('dataGridHeaderCellActionGroup-message') + ).not.toBeInTheDocument(); + userEvent.click(screen.getByRole('button', { name: 'message' })); + expect(screen.getByTestId('dataGridHeaderCellActionGroup-message')).toBeInTheDocument(); + expect(screen.getByTestId('gridEditFieldButton')).toBeInTheDocument(); + }, + EXTENDED_JEST_TIMEOUT + ); - it('should not render the edit field button if onFieldEdited is not provided', async () => { - renderDataTable({ columns: ['message'] }); - expect(screen.queryByTestId('dataGridHeaderCellActionGroup-message')).not.toBeInTheDocument(); - userEvent.click(screen.getByRole('button', { name: 'message' })); - expect(screen.getByTestId('dataGridHeaderCellActionGroup-message')).toBeInTheDocument(); - expect(screen.queryByTestId('gridEditFieldButton')).not.toBeInTheDocument(); - }); + it( + 'should not render the edit field button if onFieldEdited is not provided', + async () => { + await renderDataTable({ columns: ['message'] }); + expect( + screen.queryByTestId('dataGridHeaderCellActionGroup-message') + ).not.toBeInTheDocument(); + userEvent.click(screen.getByRole('button', { name: 'message' })); + expect(screen.getByTestId('dataGridHeaderCellActionGroup-message')).toBeInTheDocument(); + expect(screen.queryByTestId('gridEditFieldButton')).not.toBeInTheDocument(); + }, + EXTENDED_JEST_TIMEOUT + ); }); describe('cellActionsTriggerId', () => { - it('should call useDataGridColumnsCellActions with empty params when no cellActionsTriggerId is provided', async () => { - await getComponent({ - ...getProps(), - columns: ['message'], - onFieldEdited: jest.fn(), - }); - expect(mockUseDataGridColumnsCellActions).toHaveBeenCalledWith({ - triggerId: undefined, - getCellValue: expect.any(Function), - fields: undefined, - dataGridRef: expect.any(Object), - metadata: { - dataViewId: 'the-data-view-id', - someKey: 'someValue', - }, - }); - }); + it( + 'should call useDataGridColumnsCellActions with empty params when no cellActionsTriggerId is provided', + async () => { + await getComponent({ + ...getProps(), + columns: ['message'], + onFieldEdited: jest.fn(), + }); + expect(mockUseDataGridColumnsCellActions).toHaveBeenCalledWith({ + triggerId: undefined, + getCellValue: expect.any(Function), + fields: undefined, + dataGridRef: expect.any(Object), + metadata: { + dataViewId: 'the-data-view-id', + someKey: 'someValue', + }, + }); + }, + EXTENDED_JEST_TIMEOUT + ); - it('should call useDataGridColumnsCellActions properly when cellActionsTriggerId defined', async () => { - await getComponent({ - ...getProps(), - columns: ['message'], - onFieldEdited: jest.fn(), - cellActionsTriggerId: 'test', - }); - expect(mockUseDataGridColumnsCellActions).toHaveBeenCalledWith({ - triggerId: 'test', - getCellValue: expect.any(Function), - fields: [ - dataViewMock.getFieldByName('@timestamp')?.toSpec(), - dataViewMock.getFieldByName('message')?.toSpec(), - ], - dataGridRef: expect.any(Object), - metadata: { - dataViewId: 'the-data-view-id', - someKey: 'someValue', - }, - }); - }); + it( + 'should call useDataGridColumnsCellActions properly when cellActionsTriggerId defined', + async () => { + await getComponent({ + ...getProps(), + columns: ['message'], + onFieldEdited: jest.fn(), + cellActionsTriggerId: 'test', + }); + expect(mockUseDataGridColumnsCellActions).toHaveBeenCalledWith({ + triggerId: 'test', + getCellValue: expect.any(Function), + fields: [ + dataViewMock.getFieldByName('@timestamp')?.toSpec(), + dataViewMock.getFieldByName('message')?.toSpec(), + ], + dataGridRef: expect.any(Object), + metadata: { + dataViewId: 'the-data-view-id', + someKey: 'someValue', + }, + }); + }, + EXTENDED_JEST_TIMEOUT + ); }); describe('sorting', () => { - it('should enable in memory sorting with plain records', async () => { - const component = await getComponent({ - ...getProps(), - columns: ['message'], - isPlainRecord: true, - }); + const getButton = (name: string) => screen.getByRole('button', { name }); + const getCellValuesByColumn = () => { + const columns = screen + .getAllByRole('columnheader') + .map((header) => header.dataset.gridcellColumnId!); + const values = screen + .getAllByRole('gridcell') + .map((cell) => cell.querySelector('.unifiedDataTable__cellValue')?.textContent ?? ''); + return values.reduce>((acc, value, i) => { + const column = columns[i % columns.length]; + acc[column] = acc[column] ?? []; + acc[column].push(value); + return acc; + }, {}); + }; - expect( - ( - findTestSubject(component, 'docTable') - .find('EuiDataGridInMemoryRenderer') - .first() - .props() as Record - ).inMemory - ).toMatchInlineSnapshot(` - Object { - "level": "sorting", - } - `); - }); + it( + 'should apply client side sorting in ES|QL mode', + async () => { + await renderDataTable({ + isPlainRecord: true, + columns: ['message'], + rows: generateEsHits(dataViewMock, 10).map((hit) => + buildDataTableRecord(hit, dataViewMock) + ), + }); + let values = getCellValuesByColumn(); + expect(values.message).toEqual([ + 'message_0', + 'message_1', + 'message_2', + 'message_3', + 'message_4', + 'message_5', + 'message_6', + 'message_7', + 'message_8', + 'message_9', + ]); + userEvent.click(getButton('message')); + // Column sort button incorrectly renders as "Sort " instead + // of "Sort Z-A" in Jest tests, so we need to find it by index + userEvent.click(screen.getAllByRole('button', { name: /Sort/ })[2], undefined, { + skipPointerEventsCheck: true, + }); + await waitFor(() => { + values = getCellValuesByColumn(); + expect(values.message).toEqual([ + 'message_9', + 'message_8', + 'message_7', + 'message_6', + 'message_5', + 'message_4', + 'message_3', + 'message_2', + 'message_1', + 'message_0', + ]); + }); + }, + EXTENDED_JEST_TIMEOUT + ); - it('should apply sorting', async () => { - const component = await getComponent({ - ...getProps(), - sort: [['message', 'desc']], - columns: ['message'], - }); + it( + 'should not apply client side sorting if not in ES|QL mode', + async () => { + await renderDataTable({ + columns: ['message'], + rows: generateEsHits(dataViewMock, 10).map((hit) => + buildDataTableRecord(hit, dataViewMock) + ), + }); + let values = getCellValuesByColumn(); + expect(values.message).toEqual([ + 'message_0', + 'message_1', + 'message_2', + 'message_3', + 'message_4', + 'message_5', + 'message_6', + 'message_7', + 'message_8', + 'message_9', + ]); + userEvent.click(getButton('message')); + // Column sort button incorrectly renders as "Sort " instead + // of "Sort Z-A" in Jest tests, so we need to find it by index + userEvent.click(screen.getAllByRole('button', { name: /Sort/ })[2], undefined, { + skipPointerEventsCheck: true, + }); + await waitFor(() => { + values = getCellValuesByColumn(); + expect(values.message).toEqual([ + 'message_0', + 'message_1', + 'message_2', + 'message_3', + 'message_4', + 'message_5', + 'message_6', + 'message_7', + 'message_8', + 'message_9', + ]); + }); + }, + EXTENDED_JEST_TIMEOUT + ); - expect(component.find(EuiDataGrid).last().prop('sorting')).toMatchInlineSnapshot(` + it( + 'should apply sorting', + async () => { + const component = await getComponent({ + ...getProps(), + sort: [['message', 'desc']], + columns: ['message'], + }); + + expect(component.find(EuiDataGrid).last().prop('sorting')).toMatchInlineSnapshot(` Object { "columns": Array [ Object { @@ -417,20 +587,24 @@ describe('UnifiedDataTable', () => { "onSort": [Function], } `); - }); + }, + EXTENDED_JEST_TIMEOUT + ); - it('should not apply unknown sorting', async () => { - const component = await getComponent({ - ...getProps(), - sort: [ - ['bytes', 'desc'], - ['unknown', 'asc'], - ['message', 'desc'], - ], - columns: ['bytes', 'message'], - }); + it( + 'should not apply unknown sorting', + async () => { + const component = await getComponent({ + ...getProps(), + sort: [ + ['bytes', 'desc'], + ['unknown', 'asc'], + ['message', 'desc'], + ], + columns: ['bytes', 'message'], + }); - expect(component.find(EuiDataGrid).last().prop('sorting')).toMatchInlineSnapshot(` + expect(component.find(EuiDataGrid).last().prop('sorting')).toMatchInlineSnapshot(` Object { "columns": Array [ Object { @@ -445,19 +619,24 @@ describe('UnifiedDataTable', () => { "onSort": [Function], } `); - }); + }, + EXTENDED_JEST_TIMEOUT + ); }); describe('display settings', () => { - it('should include additional display settings if onUpdateSampleSize is provided', async () => { - const component = await getComponent({ - ...getProps(), - sampleSizeState: 150, - onUpdateSampleSize: jest.fn(), - onUpdateRowHeight: jest.fn(), - }); - - expect(component.find(EuiDataGrid).first().prop('toolbarVisibility')).toMatchInlineSnapshot(` + it( + 'should include additional display settings if onUpdateSampleSize is provided', + async () => { + const component = await getComponent({ + ...getProps(), + sampleSizeState: 150, + onUpdateSampleSize: jest.fn(), + onUpdateRowHeight: jest.fn(), + }); + + expect(component.find(EuiDataGrid).first().prop('toolbarVisibility')) + .toMatchInlineSnapshot(` Object { "additionalControls": null, "showColumnSelector": false, @@ -482,16 +661,21 @@ describe('UnifiedDataTable', () => { "showSortSelector": true, } `); - }); - - it('should not include additional display settings if onUpdateSampleSize is not provided', async () => { - const component = await getComponent({ - ...getProps(), - sampleSizeState: 200, - onUpdateRowHeight: jest.fn(), - }); + }, + EXTENDED_JEST_TIMEOUT + ); - expect(component.find(EuiDataGrid).first().prop('toolbarVisibility')).toMatchInlineSnapshot(` + it( + 'should not include additional display settings if onUpdateSampleSize is not provided', + async () => { + const component = await getComponent({ + ...getProps(), + sampleSizeState: 200, + onUpdateRowHeight: jest.fn(), + }); + + expect(component.find(EuiDataGrid).first().prop('toolbarVisibility')) + .toMatchInlineSnapshot(` Object { "additionalControls": null, "showColumnSelector": false, @@ -515,16 +699,21 @@ describe('UnifiedDataTable', () => { "showSortSelector": true, } `); - }); - - it('should hide display settings if no handlers provided', async () => { - const component = await getComponent({ - ...getProps(), - onUpdateRowHeight: undefined, - onUpdateSampleSize: undefined, - }); + }, + EXTENDED_JEST_TIMEOUT + ); - expect(component.find(EuiDataGrid).first().prop('toolbarVisibility')).toMatchInlineSnapshot(` + it( + 'should hide display settings if no handlers provided', + async () => { + const component = await getComponent({ + ...getProps(), + onUpdateRowHeight: undefined, + onUpdateSampleSize: undefined, + }); + + expect(component.find(EuiDataGrid).first().prop('toolbarVisibility')) + .toMatchInlineSnapshot(` Object { "additionalControls": null, "showColumnSelector": false, @@ -533,302 +722,358 @@ describe('UnifiedDataTable', () => { "showSortSelector": true, } `); - }); + }, + EXTENDED_JEST_TIMEOUT + ); }); describe('custom control columns', () => { - it('should be able to customise the leading controls', async () => { - const component = await getComponent({ - ...getProps(), - expandedDoc: { - id: 'test', - raw: { - _index: 'test_i', - _id: 'test', + it( + 'should be able to customise the leading controls', + async () => { + const component = await getComponent({ + ...getProps(), + expandedDoc: { + id: 'test', + raw: { + _index: 'test_i', + _id: 'test', + }, + flattened: { test: jest.fn() }, }, - flattened: { test: jest.fn() }, - }, - setExpandedDoc: jest.fn(), - renderDocumentView: jest.fn(), - externalControlColumns: [testLeadingControlColumn], - rowAdditionalLeadingControls: mockRowAdditionalLeadingControls, - }); - - expect(findTestSubject(component, 'test-body-control-column-cell').exists()).toBeTruthy(); - expect( - findTestSubject(component, 'exampleRowControl-visBarVerticalStacked').exists() - ).toBeTruthy(); - expect( - findTestSubject(component, 'unifiedDataTable_additionalRowControl_menuControl').exists() - ).toBeTruthy(); - }); + setExpandedDoc: jest.fn(), + renderDocumentView: jest.fn(), + externalControlColumns: [testLeadingControlColumn], + rowAdditionalLeadingControls: mockRowAdditionalLeadingControls, + }); + + expect(findTestSubject(component, 'test-body-control-column-cell').exists()).toBeTruthy(); + expect( + findTestSubject(component, 'exampleRowControl-visBarVerticalStacked').exists() + ).toBeTruthy(); + expect( + findTestSubject(component, 'unifiedDataTable_additionalRowControl_menuControl').exists() + ).toBeTruthy(); + }, + EXTENDED_JEST_TIMEOUT + ); - it('should be able to customise the trailing controls', async () => { - const component = await getComponent({ - ...getProps(), - expandedDoc: { - id: 'test', - raw: { - _index: 'test_i', - _id: 'test', + it( + 'should be able to customise the trailing controls', + async () => { + const component = await getComponent({ + ...getProps(), + expandedDoc: { + id: 'test', + raw: { + _index: 'test_i', + _id: 'test', + }, + flattened: { test: jest.fn() }, }, - flattened: { test: jest.fn() }, - }, - setExpandedDoc: jest.fn(), - renderDocumentView: jest.fn(), - externalControlColumns: [testLeadingControlColumn], - trailingControlColumns: testTrailingControlColumns, - }); - - expect(findTestSubject(component, 'test-body-control-column-cell').exists()).toBeTruthy(); - expect( - findTestSubject(component, 'test-trailing-column-popover-button').exists() - ).toBeTruthy(); - }); + setExpandedDoc: jest.fn(), + renderDocumentView: jest.fn(), + externalControlColumns: [testLeadingControlColumn], + trailingControlColumns: testTrailingControlColumns, + }); + + expect(findTestSubject(component, 'test-body-control-column-cell').exists()).toBeTruthy(); + expect( + findTestSubject(component, 'test-trailing-column-popover-button').exists() + ).toBeTruthy(); + }, + EXTENDED_JEST_TIMEOUT + ); }); describe('externalControlColumns', () => { - it('should render external leading control columns', async () => { - const component = await getComponent({ - ...getProps(), - expandedDoc: { - id: 'test', - raw: { - _index: 'test_i', - _id: 'test', + it( + 'should render external leading control columns', + async () => { + const component = await getComponent({ + ...getProps(), + expandedDoc: { + id: 'test', + raw: { + _index: 'test_i', + _id: 'test', + }, + flattened: { test: jest.fn() }, }, - flattened: { test: jest.fn() }, - }, - setExpandedDoc: jest.fn(), - renderDocumentView: jest.fn(), - externalControlColumns: [testLeadingControlColumn], - }); + setExpandedDoc: jest.fn(), + renderDocumentView: jest.fn(), + externalControlColumns: [testLeadingControlColumn], + }); - expect(findTestSubject(component, 'docTableExpandToggleColumn').exists()).toBeTruthy(); - expect(findTestSubject(component, 'test-body-control-column-cell').exists()).toBeTruthy(); - }); - }); - - it('should render provided in renderDocumentView DocumentView on expand clicked', async () => { - const expandedDoc = { - id: 'test', - raw: { - _index: 'test_i', - _id: 'test', + expect(findTestSubject(component, 'docTableExpandToggleColumn').exists()).toBeTruthy(); + expect(findTestSubject(component, 'test-body-control-column-cell').exists()).toBeTruthy(); }, - flattened: { test: jest.fn() }, - }; - const columnsMetaOverride = { testField: { type: 'number' as DatatableColumnType } }; - const renderDocumentViewMock = jest.fn((hit: DataTableRecord) => ( -
{hit.id}
- )); - - const component = await getComponent({ - ...getProps(), - expandedDoc, - setExpandedDoc: jest.fn(), - columnsMeta: columnsMetaOverride, - renderDocumentView: renderDocumentViewMock, - externalControlColumns: [testLeadingControlColumn], - }); - - findTestSubject(component, 'docTableExpandToggleColumn').first().simulate('click'); - expect(findTestSubject(component, 'test-document-view').exists()).toBeTruthy(); - expect(renderDocumentViewMock).toHaveBeenLastCalledWith( - expandedDoc, - getProps().rows, - ['_source'], - columnsMetaOverride + EXTENDED_JEST_TIMEOUT ); }); - describe('externalAdditionalControls', () => { - it('should render external additional toolbar controls', async () => { + it( + 'should render provided in renderDocumentView DocumentView on expand clicked', + async () => { + const expandedDoc = { + id: 'test', + raw: { + _index: 'test_i', + _id: 'test', + }, + flattened: { test: jest.fn() }, + }; + const columnsMetaOverride = { testField: { type: 'number' as DatatableColumnType } }; + const renderDocumentViewMock = jest.fn((hit: DataTableRecord) => ( +
{hit.id}
+ )); + const component = await getComponent({ ...getProps(), - columns: ['message'], - externalAdditionalControls: , + expandedDoc, + setExpandedDoc: jest.fn(), + columnsMeta: columnsMetaOverride, + renderDocumentView: renderDocumentViewMock, + externalControlColumns: [testLeadingControlColumn], }); - expect(findTestSubject(component, 'test-additional-control').exists()).toBeTruthy(); - expect(findTestSubject(component, 'dataGridColumnSelectorButton').exists()).toBeTruthy(); - }); + findTestSubject(component, 'docTableExpandToggleColumn').first().simulate('click'); + expect(findTestSubject(component, 'test-document-view').exists()).toBeTruthy(); + expect(renderDocumentViewMock).toHaveBeenLastCalledWith( + expandedDoc, + getProps().rows, + ['_source'], + columnsMetaOverride + ); + }, + EXTENDED_JEST_TIMEOUT + ); + + describe('externalAdditionalControls', () => { + it( + 'should render external additional toolbar controls', + async () => { + const component = await getComponent({ + ...getProps(), + columns: ['message'], + externalAdditionalControls: , + }); + + expect(findTestSubject(component, 'test-additional-control').exists()).toBeTruthy(); + expect(findTestSubject(component, 'dataGridColumnSelectorButton').exists()).toBeTruthy(); + }, + EXTENDED_JEST_TIMEOUT + ); }); describe('externalCustomRenderers', () => { - it('should render only host column with the custom renderer, message should be rendered with the default cell renderer', async () => { - const component = await getComponent({ - ...getProps(), - columns: ['message', 'host'], - externalCustomRenderers: { - host: (props: EuiDataGridCellValueElementProps) => ( -
{props.columnId}
- ), - }, - }); + it( + 'should render only host column with the custom renderer, message should be rendered with the default cell renderer', + async () => { + const component = await getComponent({ + ...getProps(), + columns: ['message', 'host'], + externalCustomRenderers: { + host: (props: EuiDataGridCellValueElementProps) => ( +
{props.columnId}
+ ), + }, + }); - expect(findTestSubject(component, 'test-renderer-host').exists()).toBeTruthy(); - expect(findTestSubject(component, 'test-renderer-message').exists()).toBeFalsy(); - }); + expect(findTestSubject(component, 'test-renderer-host').exists()).toBeTruthy(); + expect(findTestSubject(component, 'test-renderer-message').exists()).toBeFalsy(); + }, + EXTENDED_JEST_TIMEOUT + ); }); describe('renderCustomGridBody', () => { - it('should render custom grid body for each row', async () => { - const component = await getComponent({ - ...getProps(), - columns: ['message', 'host'], - trailingControlColumns: [ - { - id: 'row-details', - - // The header cell should be visually hidden, but available to screen readers - width: 0, - headerCellRender: () => <>, - headerCellProps: { className: 'euiScreenReaderOnly' }, - - // The footer cell can be hidden to both visual & SR users, as it does not contain meaningful information - footerCellProps: { style: { display: 'none' } }, - - // When rendering this custom cell, we'll want to override - // the automatic width/heights calculated by EuiDataGrid - rowCellRender: jest.fn(), - }, - ], - renderCustomGridBody: (props: EuiDataGridCustomBodyProps) => ( -
- -
- ), - }); + it( + 'should render custom grid body for each row', + async () => { + const component = await getComponent({ + ...getProps(), + columns: ['message', 'host'], + trailingControlColumns: [ + { + id: 'row-details', + + // The header cell should be visually hidden, but available to screen readers + width: 0, + headerCellRender: () => <>, + headerCellProps: { className: 'euiScreenReaderOnly' }, + + // The footer cell can be hidden to both visual & SR users, as it does not contain meaningful information + footerCellProps: { style: { display: 'none' } }, + + // When rendering this custom cell, we'll want to override + // the automatic width/heights calculated by EuiDataGrid + rowCellRender: jest.fn(), + }, + ], + renderCustomGridBody: (props: EuiDataGridCustomBodyProps) => ( +
+ +
+ ), + }); - expect(findTestSubject(component, 'test-renderer-custom-grid-body').exists()).toBeTruthy(); - }); + expect(findTestSubject(component, 'test-renderer-custom-grid-body').exists()).toBeTruthy(); + }, + EXTENDED_JEST_TIMEOUT + ); }); describe('componentsTourSteps', () => { - it('should render tour step for the first row of leading control column expandButton', async () => { - const component = await getComponent({ - ...getProps(), - expandedDoc: { - id: 'test', - raw: { - _index: 'test_i', - _id: 'test', + it( + 'should render tour step for the first row of leading control column expandButton', + async () => { + const component = await getComponent({ + ...getProps(), + expandedDoc: { + id: 'test', + raw: { + _index: 'test_i', + _id: 'test', + }, + flattened: { test: jest.fn() }, }, - flattened: { test: jest.fn() }, - }, - setExpandedDoc: jest.fn(), - renderDocumentView: jest.fn(), - componentsTourSteps: { expandButton: 'test-expand' }, - }); - - const gridExpandBtn = findTestSubject(component, 'docTableExpandToggleColumn').first(); - const tourStep = gridExpandBtn.getDOMNode().getAttribute('id'); - expect(tourStep).toEqual('test-expand'); - }); + setExpandedDoc: jest.fn(), + renderDocumentView: jest.fn(), + componentsTourSteps: { expandButton: 'test-expand' }, + }); + + const gridExpandBtn = findTestSubject(component, 'docTableExpandToggleColumn').first(); + const tourStep = gridExpandBtn.getDOMNode().getAttribute('id'); + expect(tourStep).toEqual('test-expand'); + }, + EXTENDED_JEST_TIMEOUT + ); }); describe('renderCustomToolbar', () => { - it('should render a custom toolbar', async () => { - let toolbarParams: Record = {}; - let gridParams: Record = {}; - const renderCustomToolbarMock = jest.fn((props) => { - toolbarParams = props.toolbarProps; - gridParams = props.gridProps; - return
Custom layout
; - }); - const component = await getComponent({ - ...getProps(), - renderCustomToolbar: renderCustomToolbarMock, - }); - - // custom toolbar should be rendered - expect(findTestSubject(component, 'custom-toolbar').exists()).toBe(true); - - expect(renderCustomToolbarMock).toHaveBeenLastCalledWith( - expect.objectContaining({ - toolbarProps: expect.objectContaining({ - hasRoomForGridControls: true, - }), - gridProps: expect.objectContaining({ - additionalControls: null, - }), - }) - ); - - // the default eui controls should be available for custom rendering - expect(toolbarParams?.columnSortingControl).toBeTruthy(); - expect(toolbarParams?.keyboardShortcutsControl).toBeTruthy(); - expect(gridParams?.additionalControls).toBe(null); - - // additional controls become available after selecting a document - act(() => { - component - .find('.euiDataGridRowCell[data-gridcell-column-id="select"] .euiCheckbox__input') - .first() - .simulate('change'); - }); + it( + 'should render a custom toolbar', + async () => { + let toolbarParams: Record = {}; + let gridParams: Record = {}; + const renderCustomToolbarMock = jest.fn((props) => { + toolbarParams = props.toolbarProps; + gridParams = props.gridProps; + return
Custom layout
; + }); + const component = await getComponent({ + ...getProps(), + renderCustomToolbar: renderCustomToolbarMock, + }); + + // custom toolbar should be rendered + expect(findTestSubject(component, 'custom-toolbar').exists()).toBe(true); + + expect(renderCustomToolbarMock).toHaveBeenLastCalledWith( + expect.objectContaining({ + toolbarProps: expect.objectContaining({ + hasRoomForGridControls: true, + }), + gridProps: expect.objectContaining({ + additionalControls: null, + }), + }) + ); + + // the default eui controls should be available for custom rendering + expect(toolbarParams?.columnSortingControl).toBeTruthy(); + expect(toolbarParams?.keyboardShortcutsControl).toBeTruthy(); + expect(gridParams?.additionalControls).toBe(null); + + // additional controls become available after selecting a document + act(() => { + component + .find('.euiDataGridRowCell[data-gridcell-column-id="select"] .euiCheckbox__input') + .first() + .simulate('change'); + }); - expect(toolbarParams?.keyboardShortcutsControl).toBeTruthy(); - expect(gridParams?.additionalControls).toBeTruthy(); - }); + expect(toolbarParams?.keyboardShortcutsControl).toBeTruthy(); + expect(gridParams?.additionalControls).toBeTruthy(); + }, + EXTENDED_JEST_TIMEOUT + ); }); describe('gridStyleOverride', () => { - it('should render the grid with the default style if no gridStyleOverride is provided', async () => { - const component = await getComponent({ - ...getProps(), - }); - - const grid = findTestSubject(component, 'docTable'); + it( + 'should render the grid with the default style if no gridStyleOverride is provided', + async () => { + const component = await getComponent({ + ...getProps(), + }); + + const grid = findTestSubject(component, 'docTable'); + + expect(grid.hasClass('euiDataGrid--bordersHorizontal')).toBeTruthy(); + expect(grid.hasClass('euiDataGrid--fontSizeSmall')).toBeTruthy(); + expect(grid.hasClass('euiDataGrid--paddingSmall')).toBeTruthy(); + expect(grid.hasClass('euiDataGrid--rowHoverHighlight')).toBeTruthy(); + expect(grid.hasClass('euiDataGrid--headerUnderline')).toBeTruthy(); + expect(grid.hasClass('euiDataGrid--stripes')).toBeTruthy(); + }, + EXTENDED_JEST_TIMEOUT + ); - expect(grid.hasClass('euiDataGrid--bordersHorizontal')).toBeTruthy(); - expect(grid.hasClass('euiDataGrid--fontSizeSmall')).toBeTruthy(); - expect(grid.hasClass('euiDataGrid--paddingSmall')).toBeTruthy(); - expect(grid.hasClass('euiDataGrid--rowHoverHighlight')).toBeTruthy(); - expect(grid.hasClass('euiDataGrid--headerUnderline')).toBeTruthy(); - expect(grid.hasClass('euiDataGrid--stripes')).toBeTruthy(); - }); - it('should render the grid with style override if gridStyleOverride is provided', async () => { - const component = await getComponent({ - ...getProps(), - gridStyleOverride: { - stripes: false, - rowHover: 'none', - border: 'none', - }, - }); + it( + 'should render the grid with style override if gridStyleOverride is provided', + async () => { + const component = await getComponent({ + ...getProps(), + gridStyleOverride: { + stripes: false, + rowHover: 'none', + border: 'none', + }, + }); - const grid = findTestSubject(component, 'docTable'); + const grid = findTestSubject(component, 'docTable'); - expect(grid.hasClass('euiDataGrid--stripes')).toBeFalsy(); - expect(grid.hasClass('euiDataGrid--rowHoverHighlight')).toBeFalsy(); - expect(grid.hasClass('euiDataGrid--bordersNone')).toBeTruthy(); - }); + expect(grid.hasClass('euiDataGrid--stripes')).toBeFalsy(); + expect(grid.hasClass('euiDataGrid--rowHoverHighlight')).toBeFalsy(); + expect(grid.hasClass('euiDataGrid--bordersNone')).toBeTruthy(); + }, + EXTENDED_JEST_TIMEOUT + ); }); describe('rowLineHeightOverride', () => { - it('should render the grid with the default row line height if no rowLineHeightOverride is provided', async () => { - const component = await getComponent({ - ...getProps(), - }); - - const gridRowCell = findTestSubject(component, 'dataGridRowCell').first(); - expect(gridRowCell.prop('style')).toMatchObject({ - lineHeight: '1.6em', - }); - }); - it('should render the grid with row line height override if rowLineHeightOverride is provided', async () => { - const component = await getComponent({ - ...getProps(), - rowLineHeightOverride: '24px', - }); + it( + 'should render the grid with the default row line height if no rowLineHeightOverride is provided', + async () => { + const component = await getComponent({ + ...getProps(), + }); + + const gridRowCell = findTestSubject(component, 'dataGridRowCell').first(); + expect(gridRowCell.prop('style')).toMatchObject({ + lineHeight: '1.6em', + }); + }, + EXTENDED_JEST_TIMEOUT + ); - const gridRowCell = findTestSubject(component, 'dataGridRowCell').first(); - expect(gridRowCell.prop('style')).toMatchObject({ - lineHeight: '24px', - }); - }); + it( + 'should render the grid with row line height override if rowLineHeightOverride is provided', + async () => { + const component = await getComponent({ + ...getProps(), + rowLineHeightOverride: '24px', + }); + + const gridRowCell = findTestSubject(component, 'dataGridRowCell').first(); + expect(gridRowCell.prop('style')).toMatchObject({ + lineHeight: '24px', + }); + }, + EXTENDED_JEST_TIMEOUT + ); }); describe('document comparison', () => { @@ -855,6 +1100,11 @@ describe('UnifiedDataTable', () => { await openSelectedRowsMenu(); userEvent.click(await screen.findByTestId('unifiedDataTableCompareSelectedDocuments')); await screen.findByText('Comparing 2 documents'); + // EuiDataGrid makes state updates after calling requestAnimationFrame, which can lead + // to "Can't perform a React state update on an unmounted component." warnings in tests, + // so we need to wait for the next animation frame to avoid this + await screen.findByTestId('unifiedDataTableCompareDocuments'); + await act(() => new Promise((resolve) => requestAnimationFrame(() => resolve()))); }; const getFullScreenButton = () => screen.queryByTestId('dataGridFullScreenButton'); @@ -872,84 +1122,130 @@ describe('UnifiedDataTable', () => { const getCellValues = () => Array.from(document.querySelectorAll(`.${CELL_CLASS}`)).map(({ textContent }) => textContent); - it('should not allow comparison if less than 2 documents are selected', async () => { - renderDataTable({ enableComparisonMode: true }); - expect(getSelectedDocumentsButton()).not.toBeInTheDocument(); - selectDocument(esHitsMock[0]); - expect(getSelectedDocumentsButton()).toBeInTheDocument(); - await openSelectedRowsMenu(); - expect(getCompareDocumentsButton()).not.toBeInTheDocument(); - await closeSelectedRowsMenu(); - selectDocument(esHitsMock[1]); - expect(getSelectedDocumentsButton()).toBeInTheDocument(); - await openSelectedRowsMenu(); - expect(getCompareDocumentsButton()).toBeInTheDocument(); - await closeSelectedRowsMenu(); - }); + it( + 'should not allow comparison if less than 2 documents are selected', + async () => { + await renderDataTable({ enableComparisonMode: true }); + expect(getSelectedDocumentsButton()).not.toBeInTheDocument(); + selectDocument(esHitsMock[0]); + expect(getSelectedDocumentsButton()).toBeInTheDocument(); + await openSelectedRowsMenu(); + expect(getCompareDocumentsButton()).not.toBeInTheDocument(); + await closeSelectedRowsMenu(); + selectDocument(esHitsMock[1]); + expect(getSelectedDocumentsButton()).toBeInTheDocument(); + await openSelectedRowsMenu(); + expect(getCompareDocumentsButton()).toBeInTheDocument(); + await closeSelectedRowsMenu(); + }, + EXTENDED_JEST_TIMEOUT + ); - it('should not allow comparison if comparison mode is disabled', async () => { - renderDataTable({ enableComparisonMode: false }); - selectDocument(esHitsMock[0]); - selectDocument(esHitsMock[1]); - await openSelectedRowsMenu(); - expect(getCompareDocumentsButton()).not.toBeInTheDocument(); - await closeSelectedRowsMenu(); - }); + it( + 'should not allow comparison if comparison mode is disabled', + async () => { + await renderDataTable({ enableComparisonMode: false }); + selectDocument(esHitsMock[0]); + selectDocument(esHitsMock[1]); + await openSelectedRowsMenu(); + expect(getCompareDocumentsButton()).not.toBeInTheDocument(); + await closeSelectedRowsMenu(); + }, + EXTENDED_JEST_TIMEOUT + ); - it('should allow comparison if 2 or more documents are selected and comparison mode is enabled', async () => { - renderDataTable({ enableComparisonMode: true }); - await goToComparisonMode(); - expect(getColumnHeaders()).toEqual(['Field', '1', '2']); - expect(getCellValues()).toEqual(['', '', 'i', 'i', '20', '', '', 'jpg', 'test1', '']); - }); + it( + 'should allow comparison if 2 or more documents are selected and comparison mode is enabled', + async () => { + await renderDataTable({ enableComparisonMode: true }); + await goToComparisonMode(); + expect(getColumnHeaders()).toEqual(['Field', '1', '2']); + expect(getCellValues()).toEqual(['', '', 'i', 'i', '20', '', '', 'jpg', 'test1', '']); + }, + EXTENDED_JEST_TIMEOUT + ); - it('should show full screen button if showFullScreenButton is true', async () => { - renderDataTable({ enableComparisonMode: true, showFullScreenButton: true }); - await goToComparisonMode(); - expect(getFullScreenButton()).toBeInTheDocument(); - }); + it( + 'should show full screen button if showFullScreenButton is true', + async () => { + await renderDataTable({ enableComparisonMode: true, showFullScreenButton: true }); + await goToComparisonMode(); + expect(getFullScreenButton()).toBeInTheDocument(); + }, + EXTENDED_JEST_TIMEOUT + ); - it('should hide full screen button if showFullScreenButton is false', async () => { - renderDataTable({ enableComparisonMode: true, showFullScreenButton: false }); - await goToComparisonMode(); - expect(getFullScreenButton()).not.toBeInTheDocument(); - }); + it( + 'should hide full screen button if showFullScreenButton is false', + async () => { + await renderDataTable({ enableComparisonMode: true, showFullScreenButton: false }); + await goToComparisonMode(); + expect(getFullScreenButton()).not.toBeInTheDocument(); + }, + EXTENDED_JEST_TIMEOUT + ); - it('should render selected fields', async () => { - const columns = ['bytes', 'message']; - renderDataTable({ enableComparisonMode: true, columns }); - await goToComparisonMode(); - expect(getFieldColumns()).toEqual(['@timestamp', ...columns]); - }); + it( + 'should render selected fields', + async () => { + const columns = ['bytes', 'message']; + await renderDataTable({ enableComparisonMode: true, columns }); + await goToComparisonMode(); + expect(getFieldColumns()).toEqual(['@timestamp', ...columns]); + }, + EXTENDED_JEST_TIMEOUT + ); - it('should render all available fields if no fields are selected', async () => { - renderDataTable({ enableComparisonMode: true }); - await goToComparisonMode(); - expect(getFieldColumns()).toEqual(['@timestamp', '_index', 'bytes', 'extension', 'message']); - }); + it( + 'should render all available fields if no fields are selected', + async () => { + await renderDataTable({ enableComparisonMode: true }); + await goToComparisonMode(); + expect(getFieldColumns()).toEqual([ + '@timestamp', + '_index', + 'bytes', + 'extension', + 'message', + ]); + }, + EXTENDED_JEST_TIMEOUT + ); }); describe('getRowIndicator', () => { - it('should render the color indicator control', async () => { - const component = await getComponent({ - ...getProps(), - getRowIndicator: jest.fn(() => ({ color: 'blue', label: 'test' })), - }); - - expect(findTestSubject(component, 'dataGridHeaderCell-colorIndicator').exists()).toBeTruthy(); - expect( - findTestSubject(component, 'unifiedDataTableRowColorIndicatorCell').first().prop('title') - ).toEqual('test'); - }); - - it('should not render the color indicator control by default', async () => { - const component = await getComponent({ - ...getProps(), - getRowIndicator: undefined, - }); + it( + 'should render the color indicator control', + async () => { + const component = await getComponent({ + ...getProps(), + getRowIndicator: jest.fn(() => ({ color: 'blue', label: 'test' })), + }); + + expect( + findTestSubject(component, 'dataGridHeaderCell-colorIndicator').exists() + ).toBeTruthy(); + expect( + findTestSubject(component, 'unifiedDataTableRowColorIndicatorCell').first().prop('title') + ).toEqual('test'); + }, + EXTENDED_JEST_TIMEOUT + ); - expect(findTestSubject(component, 'dataGridHeaderCell-colorIndicator').exists()).toBeFalsy(); - }); + it( + 'should not render the color indicator control by default', + async () => { + const component = await getComponent({ + ...getProps(), + getRowIndicator: undefined, + }); + + expect( + findTestSubject(component, 'dataGridHeaderCell-colorIndicator').exists() + ).toBeFalsy(); + }, + EXTENDED_JEST_TIMEOUT + ); }); describe('columns', () => { @@ -960,66 +1256,90 @@ describe('UnifiedDataTable', () => { const getButton = (name: string) => screen.getByRole('button', { name }); const queryButton = (name: string) => screen.queryByRole('button', { name }); - it('should reset the last column to auto width if only absolute width columns remain', async () => { - renderDataTable({ - columns: ['message', 'extension', 'bytes'], - settings: { - columns: { - extension: { width: 50 }, - bytes: { width: 50 }, + it( + 'should reset the last column to auto width if only absolute width columns remain', + async () => { + await renderDataTable({ + columns: ['message', 'extension', 'bytes'], + settings: { + columns: { + extension: { width: 50 }, + bytes: { width: 50 }, + }, }, - }, - }); - expect(getColumnHeader('message')).toHaveStyle({ width: EUI_DEFAULT_COLUMN_WIDTH }); - expect(getColumnHeader('extension')).toHaveStyle({ width: '50px' }); - expect(getColumnHeader('bytes')).toHaveStyle({ width: '50px' }); - userEvent.click(getButton('message')); - userEvent.click(getButton('Remove column'), undefined, { skipPointerEventsCheck: true }); - expect(queryColumnHeader('message')).not.toBeInTheDocument(); - expect(getColumnHeader('extension')).toHaveStyle({ width: '50px' }); - expect(getColumnHeader('bytes')).toHaveStyle({ width: EUI_DEFAULT_COLUMN_WIDTH }); - }); + }); + expect(getColumnHeader('message')).toHaveStyle({ width: EUI_DEFAULT_COLUMN_WIDTH }); + expect(getColumnHeader('extension')).toHaveStyle({ width: '50px' }); + expect(getColumnHeader('bytes')).toHaveStyle({ width: '50px' }); + userEvent.click(getButton('message')); + userEvent.click(getButton('Remove column'), undefined, { skipPointerEventsCheck: true }); + await waitFor(() => { + expect(queryColumnHeader('message')).not.toBeInTheDocument(); + }); + expect(getColumnHeader('extension')).toHaveStyle({ width: '50px' }); + expect(getColumnHeader('bytes')).toHaveStyle({ width: EUI_DEFAULT_COLUMN_WIDTH }); + }, + EXTENDED_JEST_TIMEOUT + ); - it('should not reset the last column to auto width when there are remaining auto width columns', async () => { - renderDataTable({ - columns: ['message', 'extension', 'bytes'], - settings: { - columns: { - bytes: { width: 50 }, + it( + 'should not reset the last column to auto width when there are remaining auto width columns', + async () => { + await renderDataTable({ + columns: ['message', 'extension', 'bytes'], + settings: { + columns: { + bytes: { width: 50 }, + }, }, - }, - }); - expect(getColumnHeader('message')).toHaveStyle({ width: EUI_DEFAULT_COLUMN_WIDTH }); - expect(getColumnHeader('extension')).toHaveStyle({ width: EUI_DEFAULT_COLUMN_WIDTH }); - expect(getColumnHeader('bytes')).toHaveStyle({ width: '50px' }); - userEvent.click(getButton('message')); - userEvent.click(getButton('Remove column'), undefined, { skipPointerEventsCheck: true }); - expect(queryColumnHeader('message')).not.toBeInTheDocument(); - expect(getColumnHeader('extension')).toHaveStyle({ width: EUI_DEFAULT_COLUMN_WIDTH }); - expect(getColumnHeader('bytes')).toHaveStyle({ width: '50px' }); - }); + }); + expect(getColumnHeader('message')).toHaveStyle({ width: EUI_DEFAULT_COLUMN_WIDTH }); + expect(getColumnHeader('extension')).toHaveStyle({ width: EUI_DEFAULT_COLUMN_WIDTH }); + expect(getColumnHeader('bytes')).toHaveStyle({ width: '50px' }); + userEvent.click(getButton('message')); + userEvent.click(getButton('Remove column'), undefined, { skipPointerEventsCheck: true }); + await waitFor(() => { + expect(queryColumnHeader('message')).not.toBeInTheDocument(); + }); + expect(getColumnHeader('extension')).toHaveStyle({ width: EUI_DEFAULT_COLUMN_WIDTH }); + expect(getColumnHeader('bytes')).toHaveStyle({ width: '50px' }); + }, + EXTENDED_JEST_TIMEOUT + ); - it('should show the reset width button only for absolute width columns, and allow resetting to default width', async () => { - renderDataTable({ - columns: ['message', 'extension'], - settings: { - columns: { - '@timestamp': { width: 50 }, - extension: { width: 50 }, + it( + 'should show the reset width button only for absolute width columns, and allow resetting to default width', + async () => { + await renderDataTable({ + columns: ['message', 'extension'], + settings: { + columns: { + '@timestamp': { width: 50 }, + extension: { width: 50 }, + }, }, - }, - }); - expect(getColumnHeader('@timestamp')).toHaveStyle({ width: '50px' }); - userEvent.click(getButton('@timestamp')); - userEvent.click(getButton('Reset width'), undefined, { skipPointerEventsCheck: true }); - expect(getColumnHeader('@timestamp')).toHaveStyle({ width: `${defaultTimeColumnWidth}px` }); - expect(getColumnHeader('message')).toHaveStyle({ width: EUI_DEFAULT_COLUMN_WIDTH }); - userEvent.click(getButton('message')); - expect(queryButton('Reset width')).not.toBeInTheDocument(); - expect(getColumnHeader('extension')).toHaveStyle({ width: '50px' }); - userEvent.click(getButton('extension')); - userEvent.click(getButton('Reset width'), undefined, { skipPointerEventsCheck: true }); - expect(getColumnHeader('extension')).toHaveStyle({ width: EUI_DEFAULT_COLUMN_WIDTH }); - }); + }); + expect(getColumnHeader('@timestamp')).toHaveStyle({ width: '50px' }); + userEvent.click(getButton('@timestamp')); + userEvent.click(getButton('Reset width'), undefined, { skipPointerEventsCheck: true }); + await waitFor(() => { + expect(getColumnHeader('@timestamp')).toHaveStyle({ + width: `${defaultTimeColumnWidth}px`, + }); + }); + expect(getColumnHeader('message')).toHaveStyle({ width: EUI_DEFAULT_COLUMN_WIDTH }); + userEvent.click(getButton('message')); + expect(queryButton('Reset width')).not.toBeInTheDocument(); + await waitFor(() => { + expect(getColumnHeader('extension')).toHaveStyle({ width: '50px' }); + }); + userEvent.click(getButton('extension')); + userEvent.click(getButton('Reset width'), undefined, { skipPointerEventsCheck: true }); + await waitFor(() => { + expect(getColumnHeader('extension')).toHaveStyle({ width: EUI_DEFAULT_COLUMN_WIDTH }); + }); + }, + EXTENDED_JEST_TIMEOUT + ); }); }); diff --git a/packages/kbn-unified-data-table/src/components/data_table.tsx b/packages/kbn-unified-data-table/src/components/data_table.tsx index 96c5004c61a95..73860538e0593 100644 --- a/packages/kbn-unified-data-table/src/components/data_table.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table.tsx @@ -23,7 +23,6 @@ import { EuiLoadingSpinner, EuiIcon, EuiDataGridRefProps, - EuiDataGridInMemory, EuiDataGridControlColumn, EuiDataGridCustomBodyProps, EuiDataGridStyle, @@ -43,7 +42,7 @@ import { getShouldShowFieldHandler } from '@kbn/discover-utils'; import type { DataViewFieldEditorStart } from '@kbn/data-view-field-editor-plugin/public'; import type { FieldFormatsStart } from '@kbn/field-formats-plugin/public'; import type { ThemeServiceStart } from '@kbn/react-kibana-context-common'; -import type { DataPublicPluginStart } from '@kbn/data-plugin/public'; +import { type DataPublicPluginStart } from '@kbn/data-plugin/public'; import type { DocViewFilterFn } from '@kbn/unified-doc-viewer/types'; import { AdditionalFieldGroups } from '@kbn/unified-field-list'; import { DATA_GRID_DENSITY_STYLE_MAP, useDataGridDensity } from '../hooks/use_data_grid_density'; @@ -91,8 +90,15 @@ import { type ColorIndicatorControlColumnParams, getAdditionalRowControlColumns, } from './custom_control_columns'; +import { useSorting } from '../hooks/use_sorting'; const CONTROL_COLUMN_IDS_DEFAULT = [SELECT_ROW, OPEN_DETAILS]; +const THEME_DEFAULT = { darkMode: false }; +const VIRTUALIZATION_OPTIONS: EuiDataGridProps['virtualizationOptions'] = { + // Allowing some additional rows to be rendered outside + // the view minimizes pop-in when scrolling quickly + overscanRowCount: 20, +}; export type SortOrder = [string, string]; @@ -102,13 +108,6 @@ export enum DataLoadingState { loaded = 'loaded', } -const themeDefault = { darkMode: false }; - -interface SortObj { - id: string; - direction: string; -} - /** * Unified Data Table props */ @@ -484,7 +483,7 @@ export const UnifiedDataTable = ({ }: UnifiedDataTableProps) => { const { fieldFormats, toastNotifications, dataViewFieldEditor, uiSettings, storage, data } = services; - const { darkMode } = useObservable(services.theme?.theme$ ?? of(themeDefault), themeDefault); + const { darkMode } = useObservable(services.theme?.theme$ ?? of(THEME_DEFAULT), THEME_DEFAULT); const dataGridRef = useRef(null); const [isFilterActive, setIsFilterActive] = useState(false); const [isCompareActive, setIsCompareActive] = useState(false); @@ -507,20 +506,55 @@ export const UnifiedDataTable = ({ } }, [isFilterActive, hasSelectedDocs, setIsFilterActive]); + const timeFieldName = dataView.timeFieldName; + const shouldPrependTimeFieldColumn = useCallback( + (activeColumns: string[]) => + canPrependTimeFieldColumn( + activeColumns, + timeFieldName, + columnsMeta, + showTimeCol, + isPlainRecord + ), + [timeFieldName, isPlainRecord, showTimeCol, columnsMeta] + ); + + const visibleColumns = useMemo(() => { + return getVisibleColumns( + displayedColumns, + dataView, + shouldPrependTimeFieldColumn(displayedColumns) + ); + }, [dataView, displayedColumns, shouldPrependTimeFieldColumn]); + + const { sortedRows, sorting } = useSorting({ + rows, + visibleColumns, + columnsMeta, + sort, + dataView, + isPlainRecord, + isSortEnabled, + defaultColumns, + onSort, + }); + const displayedRows = useMemo(() => { - if (!rows) { + if (!sortedRows) { return []; } + if (!isFilterActive || !hasSelectedDocs) { - return rows; + return sortedRows; } - const rowsFiltered = rows.filter((row) => isDocSelected(row.id)); - if (!rowsFiltered.length) { - // in case the selected docs are no longer part of the sample of 500, show all docs - return rows; - } - return rowsFiltered; - }, [rows, isFilterActive, hasSelectedDocs, isDocSelected]); + + const rowsFiltered = sortedRows.filter((row) => isDocSelected(row.id)); + + return rowsFiltered.length + ? rowsFiltered + : // in case the selected docs are no longer part of the sample of 500, show all docs + sortedRows; + }, [sortedRows, isFilterActive, hasSelectedDocs, isDocSelected]); const valueToStringConverter: ValueToStringConverter = useCallback( (rowIndex, columnId, options) => { @@ -709,25 +743,6 @@ export const UnifiedDataTable = ({ [dataView, onFieldEdited, services?.dataViewFieldEditor] ); - const timeFieldName = dataView.timeFieldName; - const shouldPrependTimeFieldColumn = useCallback( - (activeColumns: string[]) => - canPrependTimeFieldColumn( - activeColumns, - timeFieldName, - columnsMeta, - showTimeCol, - isPlainRecord - ), - [timeFieldName, isPlainRecord, showTimeCol, columnsMeta] - ); - - const visibleColumns = useMemo( - () => - getVisibleColumns(displayedColumns, dataView, shouldPrependTimeFieldColumn(displayedColumns)), - [dataView, displayedColumns, shouldPrependTimeFieldColumn] - ); - const getCellValue = useCallback( (fieldName, rowIndex) => displayedRows[rowIndex % displayedRows.length].flattened[fieldName] as Serializable, @@ -848,47 +863,6 @@ export const UnifiedDataTable = ({ [visibleColumns, onSetColumns, shouldPrependTimeFieldColumn] ); - /** - * Sorting - */ - const sortingColumns = useMemo( - () => - sort - .map(([id, direction]) => ({ id, direction })) - .filter(({ id }) => visibleColumns.includes(id)), - [sort, visibleColumns] - ); - - const onTableSort = useCallback( - (sortingColumnsData) => { - if (isSortEnabled) { - if (onSort) { - onSort(sortingColumnsData.map(({ id, direction }: SortObj) => [id, direction])); - } - } - }, - [onSort, isSortEnabled] - ); - - const sorting = useMemo(() => { - if (isSortEnabled) { - // in ES|QL mode, sorting is disabled when in Document view - // ideally we want the @timestamp column to be sortable server side - // but it needs discussion before moving forward like this - if (isPlainRecord && !columns.length) { - return undefined; - } - return { - columns: sortingColumns, - onSort: onTableSort, - }; - } - return { - columns: sortingColumns, - onSort: () => {}, - }; - }, [isSortEnabled, sortingColumns, isPlainRecord, columns.length, onTableSort]); - const canSetExpandedDoc = Boolean(setExpandedDoc && !!renderDocumentView); const leadingControlColumns: EuiDataGridControlColumn[] = useMemo(() => { @@ -1036,12 +1010,6 @@ export const UnifiedDataTable = ({ onUpdateDataGridDensity, ]); - const inMemory = useMemo(() => { - return isPlainRecord && columns.length - ? ({ level: 'sorting' } as EuiDataGridInMemory) - : undefined; - }, [columns.length, isPlainRecord]); - const toolbarVisibility = useMemo( () => defaultColumns @@ -1161,13 +1129,13 @@ export const UnifiedDataTable = ({ sorting={sorting as EuiDataGridSorting} toolbarVisibility={toolbarVisibility} rowHeightsOptions={rowHeightsOptions} - inMemory={inMemory} gridStyle={gridStyle} renderCustomGridBody={renderCustomGridBody} renderCustomToolbar={renderCustomToolbarFn} trailingControlColumns={trailingControlColumns} cellContext={cellContext} renderCellPopover={renderCustomPopover} + virtualizationOptions={VIRTUALIZATION_OPTIONS} /> )} diff --git a/packages/kbn-unified-data-table/src/hooks/use_sorting.ts b/packages/kbn-unified-data-table/src/hooks/use_sorting.ts new file mode 100644 index 0000000000000..d8b1f586f680c --- /dev/null +++ b/packages/kbn-unified-data-table/src/hooks/use_sorting.ts @@ -0,0 +1,113 @@ +/* + * 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 type { DataView } from '@kbn/data-views-plugin/public'; +import type { DataTableRecord } from '@kbn/discover-utils'; +import { getSortingCriteria } from '@kbn/sort-predicates'; +import { useMemo } from 'react'; +import type { EuiDataGridColumnSortingConfig, EuiDataGridProps } from '@elastic/eui'; +import type { SortOrder } from '../components/data_table'; +import type { DataTableColumnsMeta } from '../types'; + +export const useSorting = ({ + rows, + visibleColumns, + columnsMeta, + sort, + dataView, + isPlainRecord, + isSortEnabled, + defaultColumns, + onSort, +}: { + rows: DataTableRecord[] | undefined; + visibleColumns: string[]; + columnsMeta: DataTableColumnsMeta | undefined; + sort: SortOrder[]; + dataView: DataView; + isPlainRecord: boolean; + isSortEnabled: boolean; + defaultColumns: boolean; + onSort: ((sort: string[][]) => void) | undefined; +}) => { + const sortingColumns = useMemo(() => { + return sort + .map(([id, direction]) => ({ id, direction })) + .filter((col) => visibleColumns.includes(col.id)) as EuiDataGridColumnSortingConfig[]; + }, [sort, visibleColumns]); + + const comparators = useMemo(() => { + if (!isPlainRecord || !rows || !sortingColumns.length) { + return; + } + + return sortingColumns.reduce number>>( + (acc, { id, direction }) => { + const field = dataView.fields.getByName(id); + + if (!field) { + return acc; + } + + const sortField = getSortingCriteria( + columnsMeta?.[id]?.type ?? field.type, + id, + dataView.getFormatterForField(field) + ); + + acc.push((a, b) => sortField(a.flattened, b.flattened, direction as 'asc' | 'desc')); + + return acc; + }, + [] + ); + }, [columnsMeta, dataView, isPlainRecord, rows, sortingColumns]); + + const sortedRows = useMemo(() => { + if (!rows || !comparators) { + return rows; + } + + return [...rows].sort((a, b) => { + for (const comparator of comparators) { + const result = comparator(a, b); + + if (result !== 0) { + return result; + } + } + + return 0; + }); + }, [comparators, rows]); + + const sorting = useMemo(() => { + if (!isSortEnabled) { + return { + columns: sortingColumns, + onSort: () => {}, + }; + } + + // in ES|QL mode, sorting is disabled when in Document view + // ideally we want the @timestamp column to be sortable server side + // but it needs discussion before moving forward like this + if (isPlainRecord && defaultColumns) { + return undefined; + } + + return { + columns: sortingColumns, + onSort: (sortingColumnsData) => { + onSort?.(sortingColumnsData.map(({ id, direction }) => [id, direction])); + }, + }; + }, [isSortEnabled, isPlainRecord, defaultColumns, sortingColumns, onSort]); + + return { sortedRows, sorting }; +}; diff --git a/packages/kbn-unified-data-table/src/utils/get_render_cell_value.tsx b/packages/kbn-unified-data-table/src/utils/get_render_cell_value.tsx index 1b856d665d301..3b2badeee91c3 100644 --- a/packages/kbn-unified-data-table/src/utils/get_render_cell_value.tsx +++ b/packages/kbn-unified-data-table/src/utils/get_render_cell_value.tsx @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -import React, { useEffect, useContext } from 'react'; +import React, { useEffect, useContext, memo } from 'react'; import { i18n } from '@kbn/i18n'; import type { DataView, DataViewField } from '@kbn/data-views-plugin/public'; import { @@ -26,6 +26,8 @@ import { DataTablePopoverCellValue } from '../components/data_table_cell_value'; export const CELL_CLASS = 'unifiedDataTable__cellValue'; +const IS_JEST_ENVIRONMENT = typeof jest !== 'undefined'; + export const getRenderCellValueFn = ({ dataView, rows, @@ -49,7 +51,7 @@ export const getRenderCellValueFn = ({ isPlainRecord?: boolean; isCompressed?: boolean; }) => { - return function UnifiedDataTableRenderCellValue({ + const UnifiedDataTableRenderCellValue = ({ rowIndex, columnId, isDetails, @@ -57,7 +59,7 @@ export const getRenderCellValueFn = ({ colIndex, isExpandable, isExpanded, - }: EuiDataGridCellValueElementProps) { + }: EuiDataGridCellValueElementProps) => { const row = rows ? rows[rowIndex] : undefined; const field = dataView.fields.getByName(columnId); const ctx = useContext(UnifiedDataTableContext); @@ -153,6 +155,14 @@ export const getRenderCellValueFn = ({ /> ); }; + + // When memoizing renderCellValue, the following warning is logged in Jest tests: + // Failed prop type: Invalid prop `renderCellValue` supplied to `EuiDataGridCellContent`, expected one of type [function]. + // This is due to incorrect prop type validation that EUI generates for testing components in Jest, + // but is not an actual issue encountered outside of tests + return IS_JEST_ENVIRONMENT + ? UnifiedDataTableRenderCellValue + : memo(UnifiedDataTableRenderCellValue); }; /** diff --git a/packages/kbn-unified-data-table/tsconfig.json b/packages/kbn-unified-data-table/tsconfig.json index ca3372bd40f30..c50d2084efa39 100644 --- a/packages/kbn-unified-data-table/tsconfig.json +++ b/packages/kbn-unified-data-table/tsconfig.json @@ -39,5 +39,6 @@ "@kbn/unified-field-list", "@kbn/core-notifications-browser", "@kbn/core-capabilities-browser-mocks", + "@kbn/sort-predicates", ] } diff --git a/src/plugins/discover/public/application/main/state_management/utils/build_state_subscribe.ts b/src/plugins/discover/public/application/main/state_management/utils/build_state_subscribe.ts index 865992187089a..7285dd8a914ea 100644 --- a/src/plugins/discover/public/application/main/state_management/utils/build_state_subscribe.ts +++ b/src/plugins/discover/public/application/main/state_management/utils/build_state_subscribe.ts @@ -25,6 +25,7 @@ import { DataSourceType, isDataSourceType, } from '../../../../../common/data_sources'; +import { sendLoadingMsg } from '../../hooks/use_saved_search_messages'; /** * Builds a subscribe function for the AppStateContainer, that is executed when the AppState changes in URL @@ -161,6 +162,11 @@ export const buildStateSubscribe = JSON.stringify(logData, null, 2) ); + // Set documents loading to true immediately on state changes since there's a delay + // on the fetch and we don't want to see state changes reflected in the data grid + // until the fetch is complete (it also helps to minimize data grid re-renders) + sendLoadingMsg(dataState.data$.documents$, dataState.data$.documents$.getValue()); + dataState.fetch(); } }; diff --git a/test/functional/apps/discover/esql/_esql_view.ts b/test/functional/apps/discover/esql/_esql_view.ts index 7bdcb1bfe17fb..febe99901f626 100644 --- a/test/functional/apps/discover/esql/_esql_view.ts +++ b/test/functional/apps/discover/esql/_esql_view.ts @@ -7,7 +7,7 @@ */ import expect from '@kbn/expect'; - +import kbnRison from '@kbn/rison'; import { FtrProviderContext } from '../ftr_provider_context'; export default function ({ getService, getPageObjects }: FtrProviderContext) { @@ -51,8 +51,12 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { 'test/functional/fixtures/kbn_archiver/kibana_sample_data_flights_index_pattern' ); await kibanaServer.uiSettings.replace(defaultSettings); + await PageObjects.timePicker.setDefaultAbsoluteRangeViaUiSettings(); await PageObjects.common.navigateToApp('discover'); - await PageObjects.timePicker.setDefaultAbsoluteRange(); + }); + + after(async () => { + await PageObjects.timePicker.resetDefaultAbsoluteRangeViaUiSettings(); }); describe('ES|QL in Discover', () => { @@ -328,8 +332,15 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { describe('with slow queries', () => { it('should show only one entry in inspector for table/visualization', async function () { + const state = kbnRison.encode({ + dataSource: { type: 'esql' }, + query: { esql: 'from kibana_sample_data_flights' }, + }); + await PageObjects.common.navigateToActualUrl('discover', `?_a=${state}`, { + ensureCurrentUrl: false, + }); await PageObjects.discover.selectTextBaseLang(); - const testQuery = `from kibana_sample_data_flights | limit 10`; + const testQuery = `from logstash-* | limit 10`; await monacoEditor.setCodeEditorValue(testQuery); await browser.execute(() => { @@ -470,7 +481,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await retry.waitFor('first cell contains the highest value', async () => { const cell = await dataGrid.getCellElementExcludingControlColumns(0, 0); const text = await cell.getVisibleText(); - return text === '483'; + return text === '17,966'; }); expect(await testSubjects.getVisibleText('dataGridColumnSortingButton')).to.be( @@ -485,7 +496,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await retry.waitFor('first cell contains the same highest value', async () => { const cell = await dataGrid.getCellElementExcludingControlColumns(0, 0); const text = await cell.getVisibleText(); - return text === '483'; + return text === '17,966'; }); await browser.refresh(); @@ -496,7 +507,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await retry.waitFor('first cell contains the same highest value after reload', async () => { const cell = await dataGrid.getCellElementExcludingControlColumns(0, 0); const text = await cell.getVisibleText(); - return text === '483'; + return text === '17,966'; }); await PageObjects.discover.clickNewSearchButton(); @@ -514,7 +525,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { async () => { const cell = await dataGrid.getCellElementExcludingControlColumns(0, 0); const text = await cell.getVisibleText(); - return text === '483'; + return text === '17,966'; } ); diff --git a/test/functional/apps/discover/group5/_url_state.ts b/test/functional/apps/discover/group5/_url_state.ts index 2516f2ed08659..c143b7829239e 100644 --- a/test/functional/apps/discover/group5/_url_state.ts +++ b/test/functional/apps/discover/group5/_url_state.ts @@ -173,7 +173,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.header.waitUntilLoadingHasFinished(); await PageObjects.discover.waitUntilSearchingHasFinished(); - expect(await dataGrid.getRowsText()).to.eql([ + expect((await dataGrid.getRowsText()).slice(0, 6)).to.eql([ 'Sep 22, 2015 @ 20:44:05.521jpg1,808', 'Sep 22, 2015 @ 20:41:53.463png1,969', 'Sep 22, 2015 @ 20:40:22.952jpg1,576', @@ -208,7 +208,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { 'Sep 22, 2015 @ 17:22:12.782css1,583', ]; - expect(await dataGrid.getRowsText()).to.eql(filteredRows); + expect((await dataGrid.getRowsText()).slice(0, 6)).to.eql(filteredRows); expect(await PageObjects.discover.getHitCount()).to.be(totalHitsForTwoFilters); await testSubjects.existOrFail('unsavedChangesBadge'); @@ -217,7 +217,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.header.waitUntilLoadingHasFinished(); await PageObjects.discover.waitUntilSearchingHasFinished(); - expect(await dataGrid.getRowsText()).to.eql(filteredRows); + expect((await dataGrid.getRowsText()).slice(0, 6)).to.eql(filteredRows); expect(await PageObjects.discover.getHitCount()).to.be(totalHitsForTwoFilters); await testSubjects.existOrFail('unsavedChangesBadge'); }); diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/data_table/index.test.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/data_table/index.test.tsx index eb20b408e3ff5..78ffadeb37ff8 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/data_table/index.test.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/data_table/index.test.tsx @@ -229,7 +229,7 @@ describe('unified data table', () => { async () => { const rowHeight = { initial: 2, - new: 1, + new: 4, }; const customMockStore = createMockStore(); @@ -240,7 +240,9 @@ describe('unified data table', () => { }) ); - render(); + render( + + ); expect(await screen.findByTestId('discoverDocTable')).toBeVisible(); diff --git a/x-pack/test_serverless/functional/test_suites/common/discover/esql/_esql_view.ts b/x-pack/test_serverless/functional/test_suites/common/discover/esql/_esql_view.ts index 7cb49894e7de5..d2eebb13acb83 100644 --- a/x-pack/test_serverless/functional/test_suites/common/discover/esql/_esql_view.ts +++ b/x-pack/test_serverless/functional/test_suites/common/discover/esql/_esql_view.ts @@ -50,10 +50,14 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { 'test/functional/fixtures/kbn_archiver/kibana_sample_data_flights_index_pattern' ); await kibanaServer.uiSettings.replace(defaultSettings); + await PageObjects.timePicker.setDefaultAbsoluteRangeViaUiSettings(); await PageObjects.svlCommonPage.loginAsAdmin(); await PageObjects.common.navigateToApp('discover'); await PageObjects.header.waitUntilLoadingHasFinished(); - await PageObjects.timePicker.setDefaultAbsoluteRange(); + }); + + after(async () => { + await PageObjects.timePicker.resetDefaultAbsoluteRangeViaUiSettings(); }); describe('ES|QL in Discover', () => { @@ -453,7 +457,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await retry.waitFor('first cell contains the highest value', async () => { const cell = await dataGrid.getCellElementExcludingControlColumns(0, 0); const text = await cell.getVisibleText(); - return text === '483'; + return text === '17,966'; }); expect(await testSubjects.getVisibleText('dataGridColumnSortingButton')).to.be( @@ -468,7 +472,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await retry.waitFor('first cell contains the same highest value', async () => { const cell = await dataGrid.getCellElementExcludingControlColumns(0, 0); const text = await cell.getVisibleText(); - return text === '483'; + return text === '17,966'; }); await browser.refresh(); @@ -479,7 +483,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await retry.waitFor('first cell contains the same highest value after reload', async () => { const cell = await dataGrid.getCellElementExcludingControlColumns(0, 0); const text = await cell.getVisibleText(); - return text === '483'; + return text === '17,966'; }); await PageObjects.discover.clickNewSearchButton(); @@ -497,7 +501,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { async () => { const cell = await dataGrid.getCellElementExcludingControlColumns(0, 0); const text = await cell.getVisibleText(); - return text === '483'; + return text === '17,966'; } );