Skip to content

Commit

Permalink
[Security Solution] - remove styled-components and cleanup for event …
Browse files Browse the repository at this point in the history
…viewer and data table components (#206523)

## Summary

This PR originally aimed at replacing the usages `styled-components`
with `@emotion/react` in the
`security_solution/public/common/components/events_viewer` folder. I
quickly realized removing some of these would require a small refactor.
This lead to making a few more changes, as many properties were actually
unused so a cleanup was welcome.

Only 2 small UI changes are introduced in this PR:
- the inspect icon on the top right corner of the tables are now always
visible instead of only visible on hover. I'm aware that this is a
different behavior from the alerts table in the alerts page, but we also
have other tables (like the one on threat intelligence page) where the
icon is always shown. Waiting on @codearos for confirmation here
- the `Grid view` and `Additional filters` button are reversed due to
the simplification of the code

No other UI changes are introduced. No behavior logic has been changed
either.

The biggest code cleanup are:
- removal of a bunch of unused properties and logic
- deletion of the RightTopMenu component: it was used in both
`StatefulEventsViewerComponent` and `getPersistentControlsHook` but none
of the internal logic was overlapping. I don't know how we got there but
its current implementation was overly complex and completely
unnecessary...

#### Alerts page

![Screenshot 2025-01-13 at 4 33
36 PM](https://github.com/user-attachments/assets/c6c588c1-16f1-49f8-bcc0-246fb05f7e10)

#### Rule creation page

![Screenshot 2025-01-13 at 4 34
14 PM](https://github.com/user-attachments/assets/ea2332c3-425a-4960-8bd6-f2d7395cdf34)

#### Host/User/Network events tab

![Screenshot 2025-01-13 at 4 34
27 PM](https://github.com/user-attachments/assets/4194e406-6bff-4a46-bc99-aadd1aea88d7)

#### Host session view tab

![Screenshot 2025-01-13 at 4 34
42 PM](https://github.com/user-attachments/assets/045b3bb2-2681-4089-a303-a77f797f9b90)

### Checklist

- [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

---------

Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
PhilippeOberti and kibanamachine authored Jan 15, 2025
1 parent d9b9425 commit 7087891
Show file tree
Hide file tree
Showing 11 changed files with 155 additions and 444 deletions.
2 changes: 0 additions & 2 deletions packages/kbn-babel-preset/styled_components_files.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,19 @@
*/

import type {
EuiDataGridRefProps,
EuiDataGridColumn,
EuiDataGridCellValueElementProps,
EuiDataGridStyle,
EuiDataGridToolBarVisibilityOptions,
EuiDataGridColumn,
EuiDataGridControlColumn,
EuiDataGridPaginationProps,
EuiDataGridRowHeightsOptions,
EuiDataGridProps,
EuiDataGridRefProps,
EuiDataGridStyle,
EuiDataGridToolBarVisibilityOptions,
} from '@elastic/eui';
import { EuiDataGrid, EuiProgress } from '@elastic/eui';
import { getOr } from 'lodash/fp';
import memoizeOne from 'memoize-one';
import React, { useCallback, useEffect, useMemo, useContext, useRef } from 'react';
import React, { useCallback, useContext, useEffect, useMemo, useRef } from 'react';
import { useDispatch } from 'react-redux';

import styled, { ThemeContext } from 'styled-components';
Expand All @@ -31,8 +30,8 @@ import type {
import { i18n } from '@kbn/i18n';
import {
BrowserFields,
DeprecatedCellValueElementProps,
ColumnHeaderOptions,
DeprecatedCellValueElementProps,
DeprecatedRowRenderer,
TimelineItem,
} from '@kbn/timelines-plugin/common';
Expand Down Expand Up @@ -88,12 +87,9 @@ interface BaseDataTableProps {
loadPage: (newActivePage: number) => void;
renderCellValue: (props: DeprecatedCellValueElementProps) => React.ReactNode;
rowRenderers: DeprecatedRowRenderer[];
hasCrudPermissions?: boolean;
unitCountText: string;
pagination: EuiDataGridPaginationProps & { pageSize: number };
totalItems: number;
rowHeightsOptions?: EuiDataGridRowHeightsOptions;
isEventRenderedView?: boolean;
getFieldBrowser: GetFieldBrowser;
getFieldSpec: (fieldName: string) => FieldSpec | undefined;
cellActionsTriggerId?: string;
Expand All @@ -103,41 +99,43 @@ export type DataTableProps = BaseDataTableProps & Omit<EuiDataGridProps, NonCust

const ES_LIMIT_COUNT = 9999;

const gridStyle = (isEventRenderedView: boolean | undefined = false): EuiDataGridStyle => ({
const gridStyle: EuiDataGridStyle = {
border: 'none',
fontSize: 's',
header: 'underline',
stripes: isEventRenderedView === true,
});
};

const EuiDataGridContainer = styled.div<{ hideLastPage: boolean }>`
ul.euiPagination__list {
li.euiPagination__item:last-child {
${({ hideLastPage }) => `${hideLastPage ? 'display:none' : ''}`};
}
}
div .euiDataGridRowCell {
display: flex;
align-items: center;
}
div .euiDataGridRowCell > [data-focus-lock-disabled] {
display: flex;
align-items: center;
flex-grow: 1;
width: 100%;
}
div .euiDataGridRowCell__content {
flex-grow: 1;
}
div .siemEventsTable__trSupplement--summary {
display: block;
}
`;

const memoizedGetColumnHeaders: (
headers: ColumnHeaderOptions[],
browserFields: BrowserFields,
isEventRenderedView: boolean
browserFields: BrowserFields
) => ColumnHeaderOptions[] = memoizeOne(getColumnHeaders);

// eslint-disable-next-line react/display-name
Expand All @@ -148,7 +146,6 @@ export const DataTableComponent = React.memo<DataTableProps>(
bulkActions = true,
data,
fieldBrowserOptions,
hasCrudPermissions,
id,
leadingControlColumns,
loadPage,
Expand All @@ -157,8 +154,6 @@ export const DataTableComponent = React.memo<DataTableProps>(
pagination,
unitCountText,
totalItems,
rowHeightsOptions,
isEventRenderedView = false,
getFieldBrowser,
getFieldSpec,
cellActionsTriggerId,
Expand All @@ -178,7 +173,7 @@ export const DataTableComponent = React.memo<DataTableProps>(
dataViewId,
} = dataTable;

const columnHeaders = memoizedGetColumnHeaders(columns, browserFields, isEventRenderedView);
const columnHeaders = memoizedGetColumnHeaders(columns, browserFields);

const dataGridRef = useRef<EuiDataGridRefProps>(null);

Expand All @@ -189,18 +184,14 @@ export const DataTableComponent = React.memo<DataTableProps>(
const theme: EuiTheme = useContext(ThemeContext);

const showBulkActions = useMemo(() => {
if (!hasCrudPermissions) {
return false;
}

if (selectedCount === 0 || !showCheckboxes) {
return false;
}
if (typeof bulkActions === 'boolean') {
return bulkActions;
}
return (bulkActions?.customBulkActions?.length || bulkActions?.alertStatusActions) ?? true;
}, [hasCrudPermissions, selectedCount, showCheckboxes, bulkActions]);
}, [selectedCount, showCheckboxes, bulkActions]);

const onResetColumns = useCallback(() => {
dispatch(dataTableActions.updateColumns({ id, columns: defaultColumns }));
Expand Down Expand Up @@ -237,22 +228,18 @@ export const DataTableComponent = React.memo<DataTableProps>(
{isLoading && <EuiProgress size="xs" position="absolute" color="accent" />}
<UnitCount data-test-subj="server-side-event-count">{unitCountText}</UnitCount>
{additionalControls ?? null}
{!isEventRenderedView ? (
getFieldBrowser({
browserFields,
options: fieldBrowserOptions,
columnIds: columnHeaders.map(({ id: columnId }) => columnId),
onResetColumns,
onToggleColumn,
})
) : (
<></>
)}
{getFieldBrowser({
browserFields,
options: fieldBrowserOptions,
columnIds: columnHeaders.map(({ id: columnId }) => columnId),
onResetColumns,
onToggleColumn,
})}
</>
),
},
},
...(showBulkActions || isEventRenderedView
...(showBulkActions
? {
showColumnSelector: false,
showSortSelector: false,
Expand All @@ -269,7 +256,6 @@ export const DataTableComponent = React.memo<DataTableProps>(
isLoading,
unitCountText,
additionalControls,
isEventRenderedView,
getFieldBrowser,
browserFields,
fieldBrowserOptions,
Expand Down Expand Up @@ -468,9 +454,9 @@ export const DataTableComponent = React.memo<DataTableProps>(
id={'body-data-grid'}
data-test-subj="body-data-grid"
aria-label={DATA_TABLE_ARIA_LABEL}
columns={isEventRenderedView ? columnHeaders : columnsWithCellActions}
columns={columnsWithCellActions}
columnVisibility={{ visibleColumns, setVisibleColumns: onSetVisibleColumns }}
gridStyle={gridStyle(isEventRenderedView)}
gridStyle={gridStyle}
leadingControlColumns={leadingControlColumns}
toolbarVisibility={toolbarVisibility}
rowCount={totalItems}
Expand All @@ -479,7 +465,7 @@ export const DataTableComponent = React.memo<DataTableProps>(
onColumnResize={onColumnResize}
pagination={pagination}
ref={dataGridRef}
rowHeightsOptions={rowHeightsOptions}
rowHeightsOptions={undefined}
/>
</EuiDataGridContainer>
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@

import { render } from '@testing-library/react';
import React from 'react';
import { TableId, dataTableActions } from '@kbn/securitysolution-data-table';
import { dataTableActions, TableId } from '@kbn/securitysolution-data-table';
import { HostsType } from '../../../explore/hosts/store/model';
import { TestProviders } from '../../mock';
import type { EventsQueryTabBodyComponentProps } from './events_query_tab_body';
import { EventsQueryTabBody, ALERTS_EVENTS_HISTOGRAM_ID } from './events_query_tab_body';
import { ALERTS_EVENTS_HISTOGRAM_ID, EventsQueryTabBody } from './events_query_tab_body';
import { useGlobalFullScreen } from '../../containers/use_full_screen';
import { licenseService } from '../../hooks/use_license';
import { mockHistory } from '../../mock/router';
Expand Down Expand Up @@ -59,9 +59,13 @@ jest.mock('react-router-dom', () => ({
useLocation: jest.fn().mockReturnValue({ pathname: '/test' }),
}));

const FakeStatefulEventsViewer = ({ additionalFilters }: { additionalFilters: JSX.Element }) => (
const FakeStatefulEventsViewer = ({
topRightMenuOptions,
}: {
topRightMenuOptions: JSX.Element;
}) => (
<div>
{additionalFilters}
{topRightMenuOptions}
{'MockedStatefulEventsViewer'}
</div>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import { useDispatch } from 'react-redux';

import { EuiCheckbox } from '@elastic/eui';
import type { Filter } from '@kbn/es-query';
import { dataTableActions } from '@kbn/securitysolution-data-table';
import type { TableId } from '@kbn/securitysolution-data-table';
import { dataTableActions } from '@kbn/securitysolution-data-table';
import { useIsExperimentalFeatureEnabled } from '../../hooks/use_experimental_features';
import type { CustomBulkAction } from '../../../../common/types';
import { RowRendererValues } from '../../../../common/api/timeline';
Expand Down Expand Up @@ -177,7 +177,7 @@ const EventsQueryTabBodyComponent: React.FC<EventsQueryTabBodyComponentProps> =
/>
)}
<StatefulEventsViewer
additionalFilters={toggleExternalAlertsCheckbox}
topRightMenuOptions={toggleExternalAlertsCheckbox}
cellActionsTriggerId={SecurityCellActionsTrigger.DEFAULT}
start={startDate}
end={endDate}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,9 @@
* 2.0.
*/

import type { ViewSelection } from '@kbn/securitysolution-data-table';
import { TableId } from '@kbn/securitysolution-data-table';
import type { CombineQueries } from '../../lib/kuery';
import { buildTimeRangeFilter, combineQueries } from '../../lib/kuery';

import { EVENTS_TABLE_CLASS_NAME } from './styles';

export const getCombinedFilterQuery = ({
from,
to,
Expand All @@ -25,41 +21,3 @@ export const getCombinedFilterQuery = ({

return combinedQueries ? combinedQueries.filterQuery : undefined;
};

export const resolverIsShowing = (graphEventId: string | undefined): boolean =>
graphEventId != null && graphEventId !== '';

export const EVENTS_COUNT_BUTTON_CLASS_NAME = 'local-events-count-button';

/** Returns `true` when the element, or one of it's children has focus */
export const elementOrChildrenHasFocus = (element: HTMLElement | null | undefined): boolean =>
element === document.activeElement || element?.querySelector(':focus-within') != null;

/** Returns true if the events table has focus */
export const tableHasFocus = (containerElement: HTMLElement | null): boolean =>
elementOrChildrenHasFocus(
containerElement?.querySelector<HTMLDivElement>(`.${EVENTS_TABLE_CLASS_NAME}`)
);

export const isSelectableView = (tableId: string): boolean =>
tableId === TableId.alertsOnAlertsPage || tableId === TableId.alertsOnRuleDetailsPage;

export const isViewSelection = (value: unknown): value is ViewSelection =>
value === 'gridView' || value === 'eventRenderedView';

/** always returns a valid default `ViewSelection` */
export const getDefaultViewSelection = ({
tableId,
value,
}: {
tableId: string;
value: unknown;
}): ViewSelection => {
const defaultViewSelection = 'gridView';

if (!isSelectableView(tableId)) {
return defaultViewSelection;
} else {
return isViewSelection(value) ? value : defaultViewSelection;
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { render } from '@testing-library/react';
import { TestProviders } from '../../mock';

import { mockEventViewerResponse } from './mock';
import { StatefulEventsViewer, type EventsViewerProps } from '.';
import { type EventsViewerProps, StatefulEventsViewer } from '.';
import { eventsDefaultModel } from './default_model';
import { EntityType } from '@kbn/timelines-plugin/common';
import { SourcererScopeName } from '../../../sourcerer/store/model';
Expand Down Expand Up @@ -54,18 +54,17 @@ const to = '2019-08-26T22:10:56.791Z';
const ACTION_BUTTON_COUNT = 4;

const testProps: EventsViewerProps = {
bulkActions: false,
defaultModel: eventsDefaultModel,
end: to,
entityType: EntityType.EVENTS,
indexNames: [],
tableId: TableId.test,
leadingControlColumns: getDefaultControlColumn(ACTION_BUTTON_COUNT),
renderCellValue: DefaultCellRenderer,
rowRenderers: defaultRowRenderers,
sourcererScope: SourcererScopeName.default,
start: from,
bulkActions: false,
hasCrudPermissions: true,
tableId: TableId.test,
};
describe('StatefulEventsViewer', () => {
beforeAll(() => {
Expand Down Expand Up @@ -93,7 +92,7 @@ describe('StatefulEventsViewer', () => {
</TestProviders>
);

expect(wrapper.find(`[data-test-subj="hoverVisibilityContainer"]`).exists()).toBeTruthy();
expect(wrapper.find(`[data-test-subj="inspect-icon-button"]`).exists()).toBeTruthy();
});

test('it closes field editor when unmounted', () => {
Expand All @@ -119,7 +118,7 @@ describe('StatefulEventsViewer', () => {
<TestProviders>
<StatefulEventsViewer
{...testProps}
additionalRightMenuOptions={[<p data-test-subj="right-option" />]}
topRightMenuOptions={[<p data-test-subj="right-option" />]}
/>
</TestProviders>
);
Expand Down
Loading

0 comments on commit 7087891

Please sign in to comment.