From 6d9407be9b65de16a92f197b965b53512c3ddb4a Mon Sep 17 00:00:00 2001 From: Cee Chen <549407+cee-chen@users.noreply.github.com> Date: Wed, 31 Jan 2024 17:04:23 -0800 Subject: [PATCH] [EuiInMemoryTable] Replace basic usages of deprecated ref method with controlled `selection.selected` API (#175722) ## Summary See https://github.com/elastic/eui/pull/7321 EUI will shortly be removing this deprecated ref `setSelection` method in favor of the new controlled `selection.selected` prop. This PR converts basic usages of controlled selection, which should not suffer any UI/UX regressions. **Please help us QA your affected tables to confirm that your table selection still works as before!** --- .../context_editor/index.test.tsx | 17 ++++++++---- .../context_editor/index.tsx | 27 ++++++++++++------- .../components/all_cases/all_cases_list.tsx | 10 +++---- .../public/components/all_cases/table.tsx | 2 +- .../metrics/hosts/components/hosts_table.tsx | 2 -- .../metrics/hosts/hooks/use_hosts_table.tsx | 16 +++-------- .../public/management/components/table.tsx | 13 ++------- 7 files changed, 38 insertions(+), 49 deletions(-) diff --git a/x-pack/packages/kbn-elastic-assistant/impl/data_anonymization_editor/context_editor/index.test.tsx b/x-pack/packages/kbn-elastic-assistant/impl/data_anonymization_editor/context_editor/index.test.tsx index 77ffe90ec8c97..fb52041632718 100644 --- a/x-pack/packages/kbn-elastic-assistant/impl/data_anonymization_editor/context_editor/index.test.tsx +++ b/x-pack/packages/kbn-elastic-assistant/impl/data_anonymization_editor/context_editor/index.test.tsx @@ -12,9 +12,12 @@ import React from 'react'; import { ContextEditor } from '.'; describe('ContextEditor', () => { - const allow = ['field1', 'field2']; + const allow = Array.from({ length: 20 }, (_, i) => `field${i + 1}`); const allowReplacement = ['field1']; - const rawData = { field1: ['value1'], field2: ['value2'] }; + const rawData = allow.reduce( + (acc, field, index) => ({ ...acc, [field]: [`value${index + 1}`] }), + {} + ); const onListUpdated = jest.fn(); @@ -36,13 +39,17 @@ describe('ContextEditor', () => { }); it('renders the select all fields button with the expected count', () => { - expect(screen.getByTestId('selectAllFields')).toHaveTextContent('Select all 2 fields'); + expect(screen.getByTestId('selectAllFields')).toHaveTextContent('Select all 20 fields'); }); it('updates the table selection when "Select all n fields" is clicked', () => { - userEvent.click(screen.getByTestId('selectAllFields')); + // The table select all checkbox should only select the number of rows visible on the page + userEvent.click(screen.getByTestId('checkboxSelectAll')); + expect(screen.getByTestId('selectedFields')).toHaveTextContent('Selected 10 fields'); - expect(screen.getByTestId('selectedFields')).toHaveTextContent('Selected 2 fields'); + // The select all button should select all rows regardless of visibility + userEvent.click(screen.getByTestId('selectAllFields')); + expect(screen.getByTestId('selectedFields')).toHaveTextContent('Selected 20 fields'); }); it('calls onListUpdated with the expected values when the update button is clicked', () => { diff --git a/x-pack/packages/kbn-elastic-assistant/impl/data_anonymization_editor/context_editor/index.tsx b/x-pack/packages/kbn-elastic-assistant/impl/data_anonymization_editor/context_editor/index.tsx index 5e9e0b960f577..e0b19fe26d672 100644 --- a/x-pack/packages/kbn-elastic-assistant/impl/data_anonymization_editor/context_editor/index.tsx +++ b/x-pack/packages/kbn-elastic-assistant/impl/data_anonymization_editor/context_editor/index.tsx @@ -7,7 +7,7 @@ import { EuiInMemoryTable } from '@elastic/eui'; import type { EuiSearchBarProps, EuiTableSelectionType } from '@elastic/eui'; -import React, { useCallback, useMemo, useRef, useState } from 'react'; +import React, { useCallback, useMemo, useState, useRef } from 'react'; import { getColumns } from './get_columns'; import { getRows } from './get_rows'; @@ -59,16 +59,25 @@ const ContextEditorComponent: React.FC = ({ rawData, pageSize = DEFAULT_PAGE_SIZE, }) => { + const isAllSelected = useRef(false); // Must be a ref and not state in order not to re-render `selectionValue`, which fires `onSelectionChange` twice const [selected, setSelection] = useState([]); const selectionValue: EuiTableSelectionType = useMemo( () => ({ selectable: () => true, - onSelectionChange: (newSelection) => setSelection(newSelection), - initialSelected: [], + onSelectionChange: (newSelection) => { + if (isAllSelected.current === true) { + // If passed every possible row (including non-visible ones), EuiInMemoryTable + // will fire `onSelectionChange` with only the visible rows - we need to + // ignore this call when that happens and continue to pass all rows + isAllSelected.current = false; + } else { + setSelection(newSelection); + } + }, + selected, }), - [] + [selected] ); - const tableRef = useRef | null>(null); const columns = useMemo(() => getColumns({ onListUpdated, rawData }), [onListUpdated, rawData]); @@ -83,9 +92,8 @@ const ContextEditorComponent: React.FC = ({ ); const onSelectAll = useCallback(() => { - tableRef.current?.setSelection(rows); // updates selection in the EuiInMemoryTable - - setTimeout(() => setSelection(rows), 0); // updates selection in the component state + isAllSelected.current = true; + setSelection(rows); }, [rows]); const pagination = useMemo(() => { @@ -106,7 +114,7 @@ const ContextEditorComponent: React.FC = ({ totalFields={rows.length} /> ), - [onListUpdated, onReset, onSelectAll, rawData, rows.length, selected] + [onListUpdated, onReset, onSelectAll, rawData, rows, selected] ); return ( @@ -120,7 +128,6 @@ const ContextEditorComponent: React.FC = ({ itemId={FIELDS.FIELD} items={rows} pagination={pagination} - ref={tableRef} search={search} selection={selectionValue} sorting={defaultSort} diff --git a/x-pack/plugins/cases/public/components/all_cases/all_cases_list.tsx b/x-pack/plugins/cases/public/components/all_cases/all_cases_list.tsx index 449f7a190f6f0..735ff95a5edf7 100644 --- a/x-pack/plugins/cases/public/components/all_cases/all_cases_list.tsx +++ b/x-pack/plugins/cases/public/components/all_cases/all_cases_list.tsx @@ -5,8 +5,8 @@ * 2.0. */ -import React, { useCallback, useMemo, useRef, useState } from 'react'; -import type { EuiBasicTable, EuiTableSelectionType } from '@elastic/eui'; +import React, { useCallback, useMemo, useState } from 'react'; +import type { EuiTableSelectionType } from '@elastic/eui'; import { EuiProgress } from '@elastic/eui'; import { difference, head, isEmpty } from 'lodash/fp'; import styled, { css } from 'styled-components'; @@ -112,11 +112,8 @@ export const AllCasesList = React.memo( [queryParams.sortField, queryParams.sortOrder] ); - const tableRef = useRef(null); - const deselectCases = useCallback(() => { setSelectedCases([]); - tableRef.current?.setSelection([]); }, [setSelectedCases]); const tableOnChangeCallback = useCallback( @@ -175,7 +172,7 @@ export const AllCasesList = React.memo( const euiBasicTableSelectionProps = useMemo>( () => ({ onSelectionChange: setSelectedCases, - initialSelected: selectedCases, + selected: selectedCases, selectable: () => !isReadOnlyPermissions(permissions), }), [permissions, selectedCases] @@ -229,7 +226,6 @@ export const AllCasesList = React.memo( selectedCases={selectedCases} selection={euiBasicTableSelectionProps} sorting={sorting} - tableRef={tableRef} tableRowProps={tableRowProps} deselectCases={deselectCases} selectedColumns={selectedColumns} diff --git a/x-pack/plugins/cases/public/components/all_cases/table.tsx b/x-pack/plugins/cases/public/components/all_cases/table.tsx index e50d54b8a9d6f..edd7ab7e9955b 100644 --- a/x-pack/plugins/cases/public/components/all_cases/table.tsx +++ b/x-pack/plugins/cases/public/components/all_cases/table.tsx @@ -35,7 +35,7 @@ interface CasesTableProps { selectedCases: CasesUI; selection: EuiTableSelectionType; sorting: EuiBasicTableProps['sorting']; - tableRef: MutableRefObject; + tableRef?: MutableRefObject; tableRowProps: EuiBasicTableProps['rowProps']; deselectCases: () => void; selectedColumns: CasesColumnSelection[]; diff --git a/x-pack/plugins/infra/public/pages/metrics/hosts/components/hosts_table.tsx b/x-pack/plugins/infra/public/pages/metrics/hosts/components/hosts_table.tsx index bbb8c44a85f7e..7449be91aa029 100644 --- a/x-pack/plugins/infra/public/pages/metrics/hosts/components/hosts_table.tsx +++ b/x-pack/plugins/infra/public/pages/metrics/hosts/components/hosts_table.tsx @@ -33,7 +33,6 @@ export const HostsTable = () => { selection, selectedItemsCount, filterSelectedHosts, - refs, } = useHostsTableContext(); return ( @@ -43,7 +42,6 @@ export const HostsTable = () => { filterSelectedHosts={filterSelectedHosts} /> { } = useKibanaContextForPlugin(); const { dataView } = useMetricsDataViewContext(); - const tableRef = useRef(null); - const closeFlyout = useCallback(() => setProperties({ detailsItemId: null }), [setProperties]); const onSelectionChange = (newSelectedItems: HostNodeRow[]) => { @@ -163,7 +156,6 @@ export const useHostsTable = () => { filterManagerService.addFilters(newFilter); setSelectedItems([]); - tableRef.current?.setSelection([]); }, [dataView, filterManagerService, selectedItems]); const reportHostEntryClick = useCallback( @@ -358,6 +350,7 @@ export const useHostsTable = () => { const selection: EuiTableSelectionType = { onSelectionChange, selectable: (item: HostNodeRow) => !!item.name, + selected: selectedItems, }; return { @@ -373,9 +366,6 @@ export const useHostsTable = () => { selection, selectedItemsCount: selectedItems.length, filterSelectedHosts, - refs: { - tableRef, - }, }; }; diff --git a/x-pack/plugins/saved_objects_tagging/public/management/components/table.tsx b/x-pack/plugins/saved_objects_tagging/public/management/components/table.tsx index 359df30516511..c959d0f41bf4a 100644 --- a/x-pack/plugins/saved_objects_tagging/public/management/components/table.tsx +++ b/x-pack/plugins/saved_objects_tagging/public/management/components/table.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import React, { useRef, useEffect, FC, ReactNode } from 'react'; +import React, { FC, ReactNode } from 'react'; import { EuiInMemoryTable, EuiBasicTableColumn, EuiLink, Query } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n-react'; @@ -57,14 +57,6 @@ export const TagTable: FC = ({ actionBar, actions, }) => { - const tableRef = useRef>(null); - - useEffect(() => { - if (tableRef.current) { - tableRef.current.setSelection(selectedTags); - } - }, [selectedTags]); - const columns: Array> = [ { field: 'name', @@ -144,7 +136,6 @@ export const TagTable: FC = ({ return ( = ({ selection={ allowSelection ? { - initialSelected: selectedTags, + selected: selectedTags, onSelectionChange, } : undefined