mirror of
https://github.com/elastic/kibana.git
synced 2025-04-23 17:28:26 -04:00
[Security Solution] Optimize Cell Actions extra render (#186282)
## Summary This PR optimizes Security Solution Cell actions so that there is no extra re-render of the complete data-grid which can be, in some cases, very expensive. Today Security Solutions Cell actions returns cell actions in 3 steps as show below. This cycle is repeated for every updated in the columns. ```mermaid flowchart TD useDataGridColumnsCellActions --> initialActions[setEmptyCellActions] --> loading{isLoading} loadingTrue[SetLoadingActions] loadingFalse[SetFinalActions] loading --> |yes|loadingTrue loading --> |no|loadingFalse finalActions[outputCellActions] initialActions --> finalActions loadingTrue --> finalActions loadingFalse --> finalActions ``` Each update to above state updates causes the entire data grid to re-render. This PR removes the `loading` state to be returned as it is not very useful for consumers. > [!CAUTION] > I have made an assumption that `Loading` state is not needed by the consumer. Feel free to correct me if that assumption is incorrect. This change make `useDataGridColumnsCellActions` return `emptyActions` initially and then the `changed actions` directly as shown below. There is not intermediate loading state on every render. ```mermaid flowchart TD useDataGridColumnsCellActions --> initialActions[setEmptyCellActions] --> loading{isLoading} loadingFalse[SetFinalActions] noop[No-op] loading --> |yes|noop loading --> |no|loadingFalse finalActions[outputCellActions] initialActions --> finalActions loadingFalse --> finalActions ``` ### Checklist Delete any items that are not applicable to this PR. - [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
This commit is contained in:
parent
3959af2a30
commit
c9bd32623a
2 changed files with 25 additions and 37 deletions
|
@ -10,7 +10,7 @@ import type { JSXElementConstructor, MutableRefObject } from 'react';
|
|||
import React from 'react';
|
||||
import type { EuiDataGridColumnCellActionProps, EuiDataGridRefProps } from '@elastic/eui';
|
||||
import { EuiButtonEmpty, type EuiDataGridColumnCellAction } from '@elastic/eui';
|
||||
import { render, waitFor, act } from '@testing-library/react';
|
||||
import { render, waitFor } from '@testing-library/react';
|
||||
import { renderHook } from '@testing-library/react-hooks';
|
||||
import { makeAction } from '../mocks/helpers';
|
||||
import type { UseDataGridColumnsCellActionsProps } from './use_data_grid_column_cell_actions';
|
||||
|
@ -74,8 +74,8 @@ describe('useDataGridColumnsCellActions', () => {
|
|||
const { result, waitForNextUpdate } = renderHook(useDataGridColumnsCellActions, {
|
||||
initialProps: useDataGridColumnsCellActionsProps,
|
||||
});
|
||||
expect(result.current).toHaveLength(columns.length);
|
||||
expect(result.current[0]).toHaveLength(1); // loader
|
||||
|
||||
expect(result.current).toHaveLength(0);
|
||||
|
||||
await waitForNextUpdate();
|
||||
|
||||
|
@ -83,16 +83,6 @@ describe('useDataGridColumnsCellActions', () => {
|
|||
expect(result.current[0]).toHaveLength(actions.length);
|
||||
});
|
||||
|
||||
it('should render cell actions loading state', async () => {
|
||||
const { result } = renderHook(useDataGridColumnsCellActions, {
|
||||
initialProps: useDataGridColumnsCellActionsProps,
|
||||
});
|
||||
await act(async () => {
|
||||
const cellAction = renderCellAction(result.current[0][0]);
|
||||
expect(cellAction.getByTestId('dataGridColumnCellAction-loading')).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
it('should call getCellValue with the proper params', async () => {
|
||||
const { result, waitForNextUpdate } = renderHook(useDataGridColumnsCellActions, {
|
||||
initialProps: useDataGridColumnsCellActionsProps,
|
||||
|
|
|
@ -7,9 +7,9 @@
|
|||
*/
|
||||
|
||||
import type { MutableRefObject } from 'react';
|
||||
import React, { useCallback, useMemo, useRef } from 'react';
|
||||
import React, { useCallback, useMemo, useRef, useState, useEffect } from 'react';
|
||||
import type { EuiDataGridRefProps } from '@elastic/eui';
|
||||
import { EuiLoadingSpinner, type EuiDataGridColumnCellAction } from '@elastic/eui';
|
||||
import { type EuiDataGridColumnCellAction } from '@elastic/eui';
|
||||
import type { FieldSpec } from '@kbn/data-views-plugin/common';
|
||||
import type {
|
||||
CellAction,
|
||||
|
@ -46,10 +46,6 @@ export type UseDataGridColumnsCellActions<
|
|||
P extends UseDataGridColumnsCellActionsProps = UseDataGridColumnsCellActionsProps
|
||||
> = (props: P) => EuiDataGridColumnCellAction[][];
|
||||
|
||||
// static actions array references to prevent React updates
|
||||
const loadingColumnActions: EuiDataGridColumnCellAction[] = [
|
||||
() => <EuiLoadingSpinner size="s" data-test-subj="dataGridColumnCellAction-loading" />,
|
||||
];
|
||||
const emptyActions: EuiDataGridColumnCellAction[][] = [];
|
||||
|
||||
export const useDataGridColumnsCellActions: UseDataGridColumnsCellActions = ({
|
||||
|
@ -60,6 +56,8 @@ export const useDataGridColumnsCellActions: UseDataGridColumnsCellActions = ({
|
|||
dataGridRef,
|
||||
disabledActionTypes = [],
|
||||
}) => {
|
||||
const [cellActions, setCellActions] = useState<EuiDataGridColumnCellAction[][]>(emptyActions);
|
||||
|
||||
const bulkContexts: CellActionCompatibilityContext[] | undefined = useMemo(() => {
|
||||
if (!triggerId || !fields?.length) {
|
||||
return undefined;
|
||||
|
@ -75,35 +73,35 @@ export const useDataGridColumnsCellActions: UseDataGridColumnsCellActions = ({
|
|||
disabledActionTypes,
|
||||
});
|
||||
|
||||
const columnsCellActions = useMemo<EuiDataGridColumnCellAction[][]>(() => {
|
||||
if (loading) {
|
||||
return fields?.length ? fields.map(() => loadingColumnActions) : emptyActions;
|
||||
}
|
||||
if (!triggerId || !columnsActions?.length || !fields?.length) {
|
||||
return emptyActions;
|
||||
useEffect(() => {
|
||||
// no-op
|
||||
if (loading || !triggerId || !columnsActions?.length || !fields?.length) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Check for a temporary inconsistency because `useBulkLoadActions` takes one render loop before setting `loading` to true.
|
||||
// It will eventually update to a consistent state
|
||||
if (columnsActions.length !== fields.length) {
|
||||
return emptyActions;
|
||||
return;
|
||||
}
|
||||
|
||||
return columnsActions.map((actions, columnIndex) =>
|
||||
actions.map((action) =>
|
||||
createColumnCellAction({
|
||||
action,
|
||||
field: fields[columnIndex],
|
||||
getCellValue,
|
||||
metadata,
|
||||
triggerId,
|
||||
dataGridRef,
|
||||
})
|
||||
setCellActions(
|
||||
columnsActions.map((actions, columnIndex) =>
|
||||
actions.map((action) =>
|
||||
createColumnCellAction({
|
||||
action,
|
||||
field: fields[columnIndex],
|
||||
getCellValue,
|
||||
metadata,
|
||||
triggerId,
|
||||
dataGridRef,
|
||||
})
|
||||
)
|
||||
)
|
||||
);
|
||||
}, [columnsActions, fields, getCellValue, loading, metadata, triggerId, dataGridRef]);
|
||||
|
||||
return columnsCellActions;
|
||||
return cellActions;
|
||||
};
|
||||
|
||||
interface CreateColumnCellActionParams
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue