Skip to content

Commit

Permalink
[Security Solution] integrate CellActions in Events/Alerts DataTables (
Browse files Browse the repository at this point in the history
…#149934)

issue: #145666

## Summary

Migrate DataTable component, used to render the alerts/events data grids
in different pages: Alerts, Explore pages (Hosts, Users, Network), and
also Rule preview. In summary, all data grids but Timeline.

The integration won't modify any action or change any functionality,
everything is supposed to keep working the same way from the User's
perspective. But it has a minor UI update in the actions tooltip layout:

*old*

![old](https://user-images.githubusercontent.com/17747913/215819948-9ef23f3f-58b2-4bd5-a9c3-6c365c2e9921.png)

*new*

![new](https://user-images.githubusercontent.com/17747913/215820225-9a736bf3-db8d-41e2-86c8-b709ef7a5f66.png)

This change was needed since the custom tooltip (old one) was adding too
many Security-related customizations to the generic EuiDataGrid's
_columnCellActions_, and also it had a lot of "action-specific"
complexity, which is against the idea of having an action-agnostic
generic component, to begin with.
So, in order to unify the CellActions user experience (icon, text,
layout...), we needed to use a more uniformed component, that will
render all the actions without any special case.

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
semd and kibanamachine authored Feb 13, 2023
1 parent 6e3551d commit fcc1d48
Show file tree
Hide file tree
Showing 37 changed files with 462 additions and 518 deletions.
9 changes: 3 additions & 6 deletions packages/kbn-cell-actions/src/components/cell_actions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,12 @@ export const CellActions: React.FC<CellActionsProps> = ({
metadata,
className,
}) => {
const extraContentNodeRef = useRef<HTMLDivElement | null>(null);
const nodeRef = useRef<HTMLDivElement | null>(null);

const actionContext: CellActionExecutionContext = useMemo(
() => ({
field,
trigger: { id: triggerId },
extraContentNodeRef,
nodeRef,
metadata,
}),
Expand All @@ -49,8 +47,6 @@ export const CellActions: React.FC<CellActionsProps> = ({
>
{children}
</HoverActionsPopover>

<div ref={extraContentNodeRef} />
</div>
);
}
Expand All @@ -62,16 +58,17 @@ export const CellActions: React.FC<CellActionsProps> = ({
ref={nodeRef}
gutterSize="none"
justifyContent="flexStart"
className={className}
data-test-subj={dataTestSubj}
>
<EuiFlexItem grow={false}>{children}</EuiFlexItem>
<EuiFlexItem grow={false} className={className} data-test-subj={dataTestSubj}>
<EuiFlexItem grow={false}>
<InlineActions
actionContext={actionContext}
showActionTooltips={showActionTooltips}
visibleCellActions={visibleCellActions}
disabledActions={disabledActions}
/>
<div ref={extraContentNodeRef} />
</EuiFlexItem>
</EuiFlexGroup>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@
* Side Public License, v 1.
*/

import React, { JSXElementConstructor } from 'react';
import React, { JSXElementConstructor, MutableRefObject } from 'react';
import {
EuiButtonEmpty,
EuiDataGridColumnCellActionProps,
EuiDataGridRefProps,
type EuiDataGridColumnCellAction,
} from '@elastic/eui';
import { render } from '@testing-library/react';
import { render, waitFor } from '@testing-library/react';
import { act, renderHook } from '@testing-library/react-hooks';
import { makeAction } from '../mocks/helpers';
import {
Expand All @@ -35,10 +36,14 @@ const field1 = { name: 'column1', values: ['0.0', '0.1', '0.2', '0.3'], type: 't
const field2 = { name: 'column2', values: ['1.0', '1.1', '1.2', '1.3'], type: 'keyword' };
const columns = [{ id: field1.name }, { id: field2.name }];

const mockCloseCellPopover = jest.fn();
const useDataGridColumnsCellActionsProps: UseDataGridColumnsCellActionsProps = {
fields: [field1, field2],
triggerId: 'testTriggerId',
metadata: { some: 'value' },
dataGridRef: {
current: { closeCellPopover: mockCloseCellPopover },
} as unknown as MutableRefObject<EuiDataGridRefProps>,
};

const renderCellAction = (
Expand Down Expand Up @@ -115,7 +120,9 @@ describe('useDataGridColumnsCellActions', () => {

cellAction.getByTestId(`dataGridColumnCellAction-${action1.id}`).click();

expect(action1.execute).toHaveBeenCalled();
waitFor(() => {
expect(action1.execute).toHaveBeenCalled();
});
});

it('should execute the action with correct context', async () => {
Expand All @@ -128,23 +135,27 @@ describe('useDataGridColumnsCellActions', () => {

cellAction1.getByTestId(`dataGridColumnCellAction-${action1.id}`).click();

expect(action1.execute).toHaveBeenCalledWith(
expect.objectContaining({
field: { name: field1.name, type: field1.type, value: field1.values[1] },
trigger: { id: useDataGridColumnsCellActionsProps.triggerId },
})
);
await waitFor(() => {
expect(action1.execute).toHaveBeenCalledWith(
expect.objectContaining({
field: { name: field1.name, type: field1.type, value: field1.values[1] },
trigger: { id: useDataGridColumnsCellActionsProps.triggerId },
})
);
});

const cellAction2 = renderCellAction(result.current[1][1], { rowIndex: 2 });

cellAction2.getByTestId(`dataGridColumnCellAction-${action2.id}`).click();

expect(action2.execute).toHaveBeenCalledWith(
expect.objectContaining({
field: { name: field2.name, type: field2.type, value: field2.values[2] },
trigger: { id: useDataGridColumnsCellActionsProps.triggerId },
})
);
await waitFor(() => {
expect(action2.execute).toHaveBeenCalledWith(
expect.objectContaining({
field: { name: field2.name, type: field2.type, value: field2.values[2] },
trigger: { id: useDataGridColumnsCellActionsProps.triggerId },
})
);
});
});

it('should execute the action with correct page value', async () => {
Expand All @@ -157,10 +168,27 @@ describe('useDataGridColumnsCellActions', () => {

cellAction.getByTestId(`dataGridColumnCellAction-${action1.id}`).click();

expect(action1.execute).toHaveBeenCalledWith(
expect.objectContaining({
field: { name: field1.name, type: field1.type, value: field1.values[1] },
})
);
await waitFor(() => {
expect(action1.execute).toHaveBeenCalledWith(
expect.objectContaining({
field: { name: field1.name, type: field1.type, value: field1.values[1] },
})
);
});
});

it('should close popover then action executed', async () => {
const { result, waitForNextUpdate } = renderHook(useDataGridColumnsCellActions, {
initialProps: useDataGridColumnsCellActionsProps,
});
await waitForNextUpdate();

const cellAction = renderCellAction(result.current[0][0], { rowIndex: 25 });

cellAction.getByTestId(`dataGridColumnCellAction-${action1.id}`).click();

await waitFor(() => {
expect(mockCloseCellPopover).toHaveBeenCalled();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@
* Side Public License, v 1.
*/

import React, { useMemo, useRef } from 'react';
import { EuiLoadingSpinner, type EuiDataGridColumnCellAction } from '@elastic/eui';
import React, { MutableRefObject, useCallback, useMemo, useRef } from 'react';
import {
EuiDataGridRefProps,
EuiLoadingSpinner,
type EuiDataGridColumnCellAction,
} from '@elastic/eui';
import type {
CellAction,
CellActionCompatibilityContext,
Expand All @@ -27,11 +31,13 @@ interface BulkField extends Pick<CellActionField, 'name' | 'type'> {
export interface UseDataGridColumnsCellActionsProps
extends Pick<CellActionsProps, 'triggerId' | 'metadata' | 'disabledActions'> {
fields: BulkField[];
dataGridRef: MutableRefObject<EuiDataGridRefProps | null>;
}
export const useDataGridColumnsCellActions = ({
fields,
triggerId,
metadata,
dataGridRef,
disabledActions = [],
}: UseDataGridColumnsCellActionsProps): EuiDataGridColumnCellAction[][] => {
const bulkContexts: CellActionCompatibilityContext[] = useMemo(
Expand All @@ -57,15 +63,22 @@ export const useDataGridColumnsCellActions = ({
}
return columnsActions.map((actions, columnIndex) =>
actions.map((action) =>
createColumnCellAction({ action, metadata, triggerId, field: fields[columnIndex] })
createColumnCellAction({
action,
metadata,
triggerId,
field: fields[columnIndex],
dataGridRef,
})
)
);
}, [columnsActions, fields, loading, metadata, triggerId]);
}, [columnsActions, fields, loading, metadata, triggerId, dataGridRef]);

return columnsCellActions;
};

interface CreateColumnCellActionParams extends Pick<CellActionsProps, 'triggerId' | 'metadata'> {
interface CreateColumnCellActionParams
extends Pick<UseDataGridColumnsCellActionsProps, 'triggerId' | 'metadata' | 'dataGridRef'> {
field: BulkField;
action: CellAction;
}
Expand All @@ -74,36 +87,76 @@ const createColumnCellAction = ({
action,
metadata,
triggerId,
dataGridRef,
}: CreateColumnCellActionParams): EuiDataGridColumnCellAction =>
function ColumnCellAction({ Component, rowIndex }) {
function ColumnCellAction({ Component, rowIndex, isExpanded }) {
const nodeRef = useRef<HTMLAnchorElement | null>(null);
const extraContentNodeRef = useRef<HTMLDivElement | null>(null);
const buttonRef = useRef<HTMLAnchorElement | null>(null);

const { name, type, values } = field;
// rowIndex refers to all pages, we need to use the row index relative to the page to get the value
const value = values[rowIndex % values.length];
const actionContext: CellActionExecutionContext = useMemo(() => {
const { name, type, values } = field;
// rowIndex refers to all pages, we need to use the row index relative to the page to get the value
const value = values[rowIndex % values.length];
return {
field: { name, type, value },
trigger: { id: triggerId },
nodeRef,
metadata,
};
}, [rowIndex]);

const actionContext: CellActionExecutionContext = {
field: { name, type, value },
trigger: { id: triggerId },
extraContentNodeRef,
nodeRef,
metadata,
};
const onClick = useCallback(async () => {
actionContext.nodeRef.current = await closeAndGetCellElement({
dataGrid: dataGridRef.current,
isExpanded,
buttonRef,
});
action.execute(actionContext);
}, [actionContext, isExpanded]);

return (
<Component
buttonRef={nodeRef}
buttonRef={buttonRef}
aria-label={action.getDisplayName(actionContext)}
title={action.getDisplayName(actionContext)}
data-test-subj={`dataGridColumnCellAction-${action.id}`}
iconType={action.getIconType(actionContext)!}
onClick={() => {
action.execute(actionContext);
}}
onClick={onClick}
>
{action.getDisplayName(actionContext)}
<div ref={extraContentNodeRef} />
</Component>
);
};

const closeAndGetCellElement = ({
dataGrid,
isExpanded,
buttonRef,
}: {
dataGrid?: EuiDataGridRefProps | null;
isExpanded: boolean;
buttonRef: MutableRefObject<HTMLAnchorElement | null>;
}): Promise<HTMLElement | null> =>
new Promise((resolve) => {
const gridCellElement = isExpanded
? // if actions popover is expanded the button is outside dataGrid, using euiDataGridRowCell--open class
document.querySelector('div[role="gridcell"].euiDataGridRowCell--open')
: // if not expanded the button is inside the cell, get the parent cell from the button
getParentCellElement(buttonRef.current);
// close the popover if needed
dataGrid?.closeCellPopover();
// closing the popover updates the cell content, get the first child after all updates
setTimeout(() => {
resolve((gridCellElement?.firstElementChild as HTMLElement) ?? null);
});
});

const getParentCellElement = (element?: HTMLElement | null): HTMLElement | null => {
if (element == null) {
return null;
}
if (element.nodeName === 'div' && element.getAttribute('role') === 'gridcell') {
return element;
}
return getParentCellElement(element.parentElement);
};
1 change: 0 additions & 1 deletion packages/kbn-cell-actions/src/mocks/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ export const makeActionContext = (
type: 'keyword',
value: 'some value',
},
extraContentNodeRef: {} as MutableRefObject<HTMLDivElement>,
nodeRef: {} as MutableRefObject<HTMLElement>,
...override,
});
6 changes: 0 additions & 6 deletions packages/kbn-cell-actions/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,16 +95,10 @@ type Metadata = Record<string, unknown> | undefined;

export interface CellActionExecutionContext extends ActionExecutionContext {
field: CellActionField;
/**
* Ref to a DOM node where the action can add custom HTML.
*/
extraContentNodeRef: React.MutableRefObject<HTMLDivElement | null>;

/**
* Ref to the node where the cell action are rendered.
*/
nodeRef: React.MutableRefObject<HTMLElement | null>;

/**
* Extra configurations for actions.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ export type ColumnHeaderOptions = Pick<
| 'isResizable'
> & {
aggregatable?: boolean;
dataTableCellActions?: DataTableCellAction[];
category?: string;
columnHeaderType: ColumnHeaderType;
description?: string | null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
*/

import type { EuiDataGridCellValueElementProps } from '@elastic/eui';
import type { Filter } from '@kbn/es-query';
import type { EcsSecurityExtension as Ecs } from '@kbn/securitysolution-ecs';
import type { ColumnHeaderOptions, RowRenderer } from '../..';
import type { BrowserFields, TimelineNonEcsData } from '../../../search_strategy';
Expand All @@ -18,7 +17,6 @@ export type CellValueElementProps = EuiDataGridCellValueElementProps & {
data: TimelineNonEcsData[];
ecsData?: Ecs;
eventId: string; // _id
globalFilters?: Filter[];
header: ColumnHeaderOptions;
isDraggable: boolean;
isTimeline?: boolean; // Default cell renderer is used for both the alert table and timeline. This allows us to cheaply separate concerns
Expand All @@ -30,5 +28,4 @@ export type CellValueElementProps = EuiDataGridCellValueElementProps & {
truncate?: boolean;
key?: string;
closeCellPopover?: () => void;
enableActions?: boolean;
};
Loading

0 comments on commit fcc1d48

Please sign in to comment.