Skip to content

Commit

Permalink
This PR implements the following changes summarized below to address <e…
Browse files Browse the repository at this point in the history
…lastic#113090>, as proposed [here](elastic#113090 (comment)):

- Configures the `Columns` popover to be consistent with `Discover`
- Changes the `Hide column` action to `Remove column`, to be consistent with `Discover`
- Persists updates to the `Columns` popover order in `local storage`
- Restores the feature to persist column widths in `local storage`

- We now pass `false` to the `allowHide` [EuiDataGrid API](https://elastic.github.io/eui/#/tabular-content/data-grid):

![allow_hide](https://user-images.githubusercontent.com/4459398/136114714-02f25b97-86af-47e5-9adc-1177d5a2c715.png)

This makes all `EuiDataGrid`-based views in the Security Solution consistent with `Discover`'s use of the  `EuiDataGrid` `Columns` popover.

In `7.15`, the `Columns` popover includes the _hide column_ toggle, as shown in the screenshot below:

![alerts_columns_popover_7_15](https://user-images.githubusercontent.com/4459398/136112441-455ddbeb-dea3-4837-81ad-32d6c82c11fe.png)

_Above: The `Columns` popover in the `7.15` `Alerts` table_

The `Columns` popover in `Discover`'s `EuiDataGrid`-based table does not display the hide column toggle, as shown the screenshot below:

![columns_popover_discover](https://user-images.githubusercontent.com/4459398/136112856-7e42c822-2260-4759-ac78-5bea63a171c7.png)

_Above: The `EuiDataGrid` `Columns` popover in `Discover`, in `master`_

Passing `false` to the `allowHide` [EuiDataGrid API](https://elastic.github.io/eui/#/tabular-content/data-grid) API makes the `Columns` popover in all `EuiDataGrid`-based views in the Security Solution consistent with `Discover`, as illustrated by the screenshot below:

![alerts_columns_popover_no_hide](https://user-images.githubusercontent.com/4459398/136112980-d4219fbd-1443-4612-8cdb-b97bee8b97ef.png)

_Above: The `Columns` popover is now consistent with `Discover`_

- The `Hide column` action shown in the `7.15` alerts table is changed to `Remove column`, making it consistent with `Discover`'s use of `EuiDataGrid`

In `7.15`, the `Alerts` table has a `Hide column` action, as shown in the screenshot below:

![hide_column](https://user-images.githubusercontent.com/4459398/136115681-9e0da144-a981-4352-8092-9368d74cd153.png)

_Above: The `Hide Column` action in the `7.15` `Alerts` table_

In `7.15`, clicking the `Hide Column` action shown in the screenshot above hides the column, but does not remove it.

In `7.15`, columns may only be removed by un-checking them in the `Fields` browser, or by un-toggling them in the Alerts / Events details popover. Both of those methods require multiple clicks, and require uses to re-find the field in the modal or popover before it may be toggled for removal.

In `Discover`, users don't hide columns.

In `Discover`, users directly remove columns by clicking the `Remove column` action, shown in the screenshot below:

![discover_remove_column](https://user-images.githubusercontent.com/4459398/136114295-f018a561-f9ee-4ce4-a9c6-0fcd7f71e67b.png)

_Above: The `Remove column` action in `Discover`'s use of `EuiDataGrid` in `master`_

All `EuiDataGrid`-based views in the Security Solution were made consistent with `Discover` by replacing the `Hide column` action with `Remove column`, per the screenshot below:

![remove_column_after](https://user-images.githubusercontent.com/4459398/137047582-3c4d6cb0-ac12-4c50-9c34-0c4ef5536550.png)

_Above: The `Remove column` action in the Alerts table_

Note: the `Remove column` action shown above appears as the last item in the popover because it's specified via the `EuiDataGrid` `EuiDataGridColumnActions` > `additonal` API, which appends additonal actions to the end of popover, after the built-in actions:

![additional](https://user-images.githubusercontent.com/4459398/137047825-625002b3-5cd6-4b3e-87da-e76dbaf2a827.png)

- Persist column order updates to `local storage` when users update the order of columns via the `Columns` popover

The following PR <elastic#110685> restored partial support for persisting columns across page refreshes via `local storage`, but the Redux store was not updated when users sort columns via the `Columns` popover, an shown in the animated gif below:

![ordering_via_columns](https://user-images.githubusercontent.com/4459398/136119497-65f76f49-091c-4a45-b8d3-1e5ef80ccbb2.gif)

_Above: Ordering via the `Columns` popover is not persisted to `local storage` in `7.15`_

This PR utilizes the `setVisibleColumns` [EuiDataGrid API](https://elastic.github.io/eui/#/tabular-content/data-grid) API as a callback to update Redux when the columns are sorted, which will in-turn update `local storage` to persist the new order across page refreshes:

![setVisibleColumns](https://user-images.githubusercontent.com/4459398/136117249-628bb147-a860-4ccf-811a-0e57a99296fb.png)

In previous releases, resized column widths were peristed in `local storage` to persist across page refreshes, as documented in <elastic#110524> :

```
{
   "detections-page":{
      "id":"detections-page",
      "activeTab":"query",
      "prevActiveTab":"query",
      "columns":[
         {
            "category":"base",
            "columnHeaderType":"not-filtered",
            "description":"Date/time when the event originated. This is the date/time extracted from the event, typically representing when the event was generated by the source. If the event source has no original timestamp, this value is typically populated by the first time the event was received by the pipeline. Required field for all events.",
            "example":"2016-05-23T08:05:34.853Z",
            "id":"@timestamp",
            "type":"date",
            "aggregatable":true,
            "width":190
         },
         {
            "category":"cloud",
            "columnHeaderType":"not-filtered",
            "description":"The cloud account or organization id used to identify different entities in a multi-tenant environment. Examples: AWS account id, Google Cloud ORG Id, or other unique identifier.",
            "example":"666777888999",
            "id":"cloud.account.id",
            "type":"string",
            "aggregatable":true,
            "width":180
         },
         {
            "category":"cloud",
            "columnHeaderType":"not-filtered",
            "description":"Availability zone in which this host is running.",
            "example":"us-east-1c",
            "id":"cloud.availability_zone",
            "type":"string",
            "aggregatable":true,
            "width":180
         },
         // ...
         }
      ],
      // ...
   }
}
```

_Above: column widths were persisted to `local storage` in previous release, (going at least back to `7.12`)_

In this PR, we utilize the `onColumnResize` [EuiDataGrid API](https://elastic.github.io/eui/#/tabular-content/data-grid) API as a callback to update Redux when the columns are sorted via the `Columns` popover. Updating Redux will in-turn update `local storage`, so resized columns widths will persist across page refreshes:

![onColumnResize](https://user-images.githubusercontent.com/4459398/136120062-3b0bebce-9c44-47fc-9956-48fe07a30f83.png)

The Alerts page `Trend` chart and table were updated to include the following additional `Stack by` fields (CC @paulewing):

```
process.name
file.name
hash.sha256
```

per the before / after screenshots below:

![alerts-trend-before](https://user-images.githubusercontent.com/4459398/137045011-7da4530b-0259-4fd4-b903-9eee6c26d02f.png)

_Above: The Alerts `Trend` Stack by fields in `7.15` (before)_

![alerts-trend-after](https://user-images.githubusercontent.com/4459398/137045023-d0ae987c-a474-4123-a05b-a6ad2fc52922.png)

_Above: The Alerts `Trend` `Stack by` fields (after the addition of the `process.name`, `file.name`, and `hash.sha256` fields)_

CC: @monina-n @paulewing
  • Loading branch information
andrew-goldstein committed Oct 15, 2021
1 parent e18afaa commit ec54000
Show file tree
Hide file tree
Showing 17 changed files with 351 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ export const alertsStackByOptions: AlertsStackByOption[] = [
{ text: 'signal.rule.name', value: 'signal.rule.name' },
{ text: 'source.ip', value: 'source.ip' },
{ text: 'user.name', value: 'user.name' },
{ text: 'process.name', value: 'process.name' },
{ text: 'file.name', value: 'file.name' },
{ text: 'hash.sha256', value: 'hash.sha256' },
];

export const DEFAULT_STACK_BY_FIELD = 'signal.rule.name';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,7 @@ export type AlertsStackByField =
| 'signal.rule.type'
| 'signal.rule.name'
| 'source.ip'
| 'user.name';
| 'user.name'
| 'process.name'
| 'file.name'
| 'hash.sha256';
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ export const {
setSelected,
setTGridSelectAll,
toggleDetailPanel,
updateColumnOrder,
updateColumns,
updateColumnWidth,
updateIsLoading,
updateItemsPerPage,
updateItemsPerPageOptions,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ import {
removeColumn,
upsertColumn,
applyDeltaToColumnWidth,
updateColumnOrder,
updateColumns,
updateColumnWidth,
updateItemsPerPage,
updateSort,
} from './actions';
Expand Down Expand Up @@ -168,4 +170,35 @@ describe('epicLocalStorage', () => {
);
await waitFor(() => expect(addTimelineInStorageMock).toHaveBeenCalled());
});

it('persists updates to the column order to local storage', async () => {
shallow(
<TestProviders store={store}>
<QueryTabContentComponent {...props} />
</TestProviders>
);
store.dispatch(
updateColumnOrder({
columnIds: ['event.severity', '@timestamp', 'event.category'],
id: 'test',
})
);
await waitFor(() => expect(addTimelineInStorageMock).toHaveBeenCalled());
});

it('persists updates to the column width to local storage', async () => {
shallow(
<TestProviders store={store}>
<QueryTabContentComponent {...props} />
</TestProviders>
);
store.dispatch(
updateColumnWidth({
columnId: 'event.severity',
id: 'test',
width: 123,
})
);
await waitFor(() => expect(addTimelineInStorageMock).toHaveBeenCalled());
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import {
applyDeltaToColumnWidth,
setExcludedRowRendererIds,
updateColumns,
updateColumnOrder,
updateColumnWidth,
updateItemsPerPage,
updateSort,
} from './actions';
Expand All @@ -30,6 +32,8 @@ const timelineActionTypes = [
upsertColumn.type,
applyDeltaToColumnWidth.type,
updateColumns.type,
updateColumnOrder.type,
updateColumnWidth.type,
updateItemsPerPage.type,
updateSort.type,
setExcludedRowRendererIds.type,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,8 @@ const ColumnHeaderComponent: React.FC<ColumneHeaderProps> = ({
id: 0,
items: [
{
icon: <EuiIcon type="eyeClosed" size="s" />,
name: i18n.HIDE_COLUMN,
icon: <EuiIcon type="cross" size="s" />,
name: i18n.REMOVE_COLUMN,
onClick: () => {
dispatch(tGridActions.removeColumn({ id: timelineId, columnId: header.id }));
handleClosePopOverTrigger();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ describe('helpers', () => {
describe('getColumnHeaders', () => {
// additional properties used by `EuiDataGrid`:
const actions = {
showHide: false,
showSortAsc: true,
showSortDesc: true,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { allowSorting } from '../helpers';
const defaultActions: EuiDataGridColumnActions = {
showSortAsc: true,
showSortDesc: true,
showHide: false,
};

const getAllBrowserFields = (browserFields: BrowserFields): Array<Partial<BrowserField>> =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,6 @@ export const FULL_SCREEN = i18n.translate('xpack.timelines.timeline.fullScreenBu
defaultMessage: 'Full screen',
});

export const HIDE_COLUMN = i18n.translate('xpack.timelines.timeline.hideColumnLabel', {
defaultMessage: 'Hide column',
});

export const SORT_AZ = i18n.translate('xpack.timelines.timeline.sortAZLabel', {
defaultMessage: 'Sort A-Z',
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@
*/

import React from 'react';
import { fireEvent, render, screen } from '@testing-library/react';

import { BodyComponent, StatefulBodyProps } from '.';
import { Sort } from './sort';
import { REMOVE_COLUMN } from './column_headers/translations';
import { Direction } from '../../../../common/search_strategy';
import { useMountAppended } from '../../utils/use_mount_appended';
import { defaultHeaders, mockBrowserFields, mockTimelineData, TestProviders } from '../../../mock';
Expand Down Expand Up @@ -272,5 +274,57 @@ describe('Body', () => {
.prop<EuiDataGridColumn[]>('columns')
.find((c) => c.id === 'signal.rule.risk_score')?.cellActions
).toBeUndefined();
test('it does NOT render switches for hiding columns in the `EuiDataGrid` `Columns` popover', async () => {
render(
<TestProviders>
<BodyComponent {...props} />
</TestProviders>
);

// Click the `EuidDataGrid` `Columns` button to open the popover:
fireEvent.click(screen.getByTestId('dataGridColumnSelectorButton'));

// `EuiDataGrid` renders switches for hiding in the `Columns` popover when `showColumnSelector.allowHide` is `true`
const switches = await screen.queryAllByRole('switch');

expect(switches.length).toBe(0); // no switches are rendered, because `allowHide` is `false`
});

test('it dispatches the `REMOVE_COLUMN` action when a user clicks `Remove column` in the column header popover', async () => {
render(
<TestProviders>
<BodyComponent {...props} />
</TestProviders>
);

// click the `@timestamp` column header to display the popover
fireEvent.click(screen.getByText('@timestamp'));

// click the `Remove column` action in the popover
fireEvent.click(await screen.getByText(REMOVE_COLUMN));

expect(mockDispatch).toBeCalledWith({
payload: { columnId: '@timestamp', id: 'timeline-test' },
type: 'x-pack/timelines/t-grid/REMOVE_COLUMN',
});
});

test('it dispatches the `UPDATE_COLUMN_WIDTH` action when a user resizes a column', async () => {
render(
<TestProviders>
<BodyComponent {...props} />
</TestProviders>
);

// simulate resizing the column
fireEvent.mouseDown(screen.getAllByTestId('dataGridColumnResizer')[0]);
fireEvent.mouseMove(screen.getAllByTestId('dataGridColumnResizer')[0]);
fireEvent.mouseUp(screen.getAllByTestId('dataGridColumnResizer')[0]);

expect(mockDispatch).toBeCalledWith({
payload: { columnId: '@timestamp', id: 'timeline-test', width: NaN },
type: 'x-pack/timelines/t-grid/UPDATE_COLUMN_WIDTH',
});
});
});
});
52 changes: 43 additions & 9 deletions x-pack/plugins/timelines/public/components/t_grid/body/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ import { ViewSelection } from '../event_rendered_view/selector';
import { EventRenderedView } from '../event_rendered_view';
import { useDataGridHeightHack } from './height_hack';
import { Filter } from '../../../../../../../src/plugins/data/public';
import { REMOVE_COLUMN } from './column_headers/translations';

const StatefulAlertStatusBulkActions = lazy(
() => import('../toolbar/bulk_actions/alert_status_bulk_actions')
Expand Down Expand Up @@ -497,7 +498,7 @@ export const BodyComponent = React.memo<StatefulBodyProps>(
showFullScreenSelector: false,
}
: {
showColumnSelector: { allowHide: true, allowReorder: true },
showColumnSelector: { allowHide: false, allowReorder: true },
showSortSelector: true,
showFullScreenSelector: true,
}),
Expand Down Expand Up @@ -559,13 +560,32 @@ export const BodyComponent = React.memo<StatefulBodyProps>(
[columnHeaders, dispatch, id, loadPage]
);

const [visibleColumns, setVisibleColumns] = useState(() =>
columnHeaders.map(({ id: cid }) => cid)
); // initializes to the full set of columns
const visibleColumns = useMemo(() => columnHeaders.map(({ id: cid }) => cid), [columnHeaders]); // the full set of columns

useEffect(() => {
setVisibleColumns(columnHeaders.map(({ id: cid }) => cid));
}, [columnHeaders]);
const onColumnResize = useCallback(
({ columnId, width }: { columnId: string; width: number }) => {
dispatch(
tGridActions.updateColumnWidth({
columnId,
id,
width,
})
);
},
[dispatch, id]
);

const onSetVisibleColumns = useCallback(
(newVisibleColumns: string[]) => {
dispatch(
tGridActions.updateColumnOrder({
columnIds: newVisibleColumns,
id,
})
);
},
[dispatch, id]
);

const setEventsLoading = useCallback<SetEventsLoading>(
({ eventIds, isLoading: loading }) => {
Expand Down Expand Up @@ -654,6 +674,19 @@ export const BodyComponent = React.memo<StatefulBodyProps>(

return {
...header,
actions: {
...header.actions,
additional: [
{
iconType: 'cross',
label: REMOVE_COLUMN,
onClick: () => {
dispatch(tGridActions.removeColumn({ id, columnId: header.id }));
},
size: 'xs',
},
],
},
...(hasCellActions(header.id)
? {
cellActions:
Expand All @@ -663,7 +696,7 @@ export const BodyComponent = React.memo<StatefulBodyProps>(
: {}),
};
}),
[columnHeaders, defaultCellActions, browserFields, data, pageSize, id]
[columnHeaders, defaultCellActions, browserFields, data, pageSize, id, dispatch]
);

const renderTGridCellValue = useMemo(() => {
Expand Down Expand Up @@ -761,14 +794,15 @@ export const BodyComponent = React.memo<StatefulBodyProps>(
data-test-subj="body-data-grid"
aria-label={i18n.TGRID_BODY_ARIA_LABEL}
columns={columnsWithCellActions}
columnVisibility={{ visibleColumns, setVisibleColumns }}
columnVisibility={{ visibleColumns, setVisibleColumns: onSetVisibleColumns }}
gridStyle={gridStyle}
leadingControlColumns={leadingTGridControlColumns}
trailingControlColumns={trailingTGridControlColumns}
toolbarVisibility={toolbarVisibility}
rowCount={totalItems}
renderCellValue={renderTGridCellValue}
sorting={{ columns: sortingColumns, onSort }}
onColumnResize={onColumnResize}
pagination={{
pageIndex: activePage,
pageSize,
Expand Down
11 changes: 11 additions & 0 deletions x-pack/plugins/timelines/public/store/t_grid/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,17 @@ export const applyDeltaToColumnWidth = actionCreator<{
delta: number;
}>('APPLY_DELTA_TO_COLUMN_WIDTH');

export const updateColumnOrder = actionCreator<{
columnIds: string[];
id: string;
}>('UPDATE_COLUMN_ORDER');

export const updateColumnWidth = actionCreator<{
columnId: string;
id: string;
width: number;
}>('UPDATE_COLUMN_WIDTH');

export type ToggleDetailPanel = TimelineExpandedDetailType & {
tabType?: TimelineTabs;
timelineId: string;
Expand Down
Loading

0 comments on commit ec54000

Please sign in to comment.