From c7dad263e2ee21d71bd106befbe26a83be8c5621 Mon Sep 17 00:00:00 2001 From: Pablo Machado Date: Tue, 16 Jan 2024 16:11:46 +0100 Subject: [PATCH] [Security Solution] Refactor SeverityFilterGroup to use EuiSelectable instead of EuiFilterSelectItem (#174912) `EuiFilterSelectItem` was deprecated by the EUI team Flaky test runner: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4879 ## Summary ### Before ### After ### Checklist Delete any items that are not applicable to this PR. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [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 - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) --- .../components/severity/common/index.tsx | 20 ++++-- .../severity/severity_filter_group.test.tsx | 20 +++--- .../severity/severity_filter_group.tsx | 66 ++++++++++--------- 3 files changed, 57 insertions(+), 49 deletions(-) diff --git a/x-pack/plugins/security_solution/public/entity_analytics/components/severity/common/index.tsx b/x-pack/plugins/security_solution/public/entity_analytics/components/severity/common/index.tsx index 0706eb117c5d7..2b498fdca79ac 100644 --- a/x-pack/plugins/security_solution/public/entity_analytics/components/severity/common/index.tsx +++ b/x-pack/plugins/security_solution/public/entity_analytics/components/severity/common/index.tsx @@ -7,7 +7,7 @@ import React from 'react'; -import { EuiHealth, transparentize } from '@elastic/eui'; +import { EuiHealth, EuiTextColor, transparentize } from '@elastic/eui'; import styled, { css } from 'styled-components'; import { euiLightVars } from '@kbn/ui-theme'; @@ -38,17 +38,25 @@ export const RiskScoreLevel: React.FC<{ severity: RiskSeverity; hideBackgroundColor?: boolean; toolTipContent?: JSX.Element; -}> = ({ severity, hideBackgroundColor = false, toolTipContent }) => { + ['data-test-subj']?: string; +}> = ({ + severity, + hideBackgroundColor = false, + toolTipContent, + 'data-test-subj': dataTestSubj, +}) => { const badge = ( - - {severity} - + + + {severity} + + ); diff --git a/x-pack/plugins/security_solution/public/entity_analytics/components/severity/severity_filter_group.test.tsx b/x-pack/plugins/security_solution/public/entity_analytics/components/severity/severity_filter_group.test.tsx index ef978fdf80546..053338c67de6f 100644 --- a/x-pack/plugins/security_solution/public/entity_analytics/components/severity/severity_filter_group.test.tsx +++ b/x-pack/plugins/security_solution/public/entity_analytics/components/severity/severity_filter_group.test.tsx @@ -28,7 +28,7 @@ describe('SeverityFilterGroup', () => { }); it('preserves sort order when severityCount is out of order', () => { - const { getByTestId, getAllByTestId } = render( + const { getByTestId } = render( { fireEvent.click(getByTestId('risk-filter-button')); - expect(getAllByTestId('risk-score').map((ele) => ele.textContent)).toEqual([ - 'Unknown', - 'Low', - 'Moderate', - 'High', - 'Critical', - ]); + expect(getByTestId('risk-filter-selectable').textContent).toEqual( + ['Unknown', 'Low', 'Moderate', 'High', 'Critical'].join('') + ); }); it('sends telemetry when selecting a classification', () => { - const { getByTestId, getAllByTestId } = render( + const { getByTestId } = render( { fireEvent.click(getByTestId('risk-filter-button')); - fireEvent.click(getAllByTestId('risk-score').at(0)!); + fireEvent.click(getByTestId('risk-filter-item-Unknown')); expect(mockedTelemetry.reportEntityRiskFiltered).toHaveBeenCalledTimes(1); }); it('does not send telemetry when deselecting a classification', () => { - const { getByTestId, getAllByTestId } = render( + const { getByTestId } = render( { fireEvent.click(getByTestId('risk-filter-button')); - fireEvent.click(getAllByTestId('risk-score').at(0)!); + fireEvent.click(getByTestId('risk-filter-item-Unknown')); expect(mockedTelemetry.reportEntityRiskFiltered).toHaveBeenCalledTimes(0); }); }); diff --git a/x-pack/plugins/security_solution/public/entity_analytics/components/severity/severity_filter_group.tsx b/x-pack/plugins/security_solution/public/entity_analytics/components/severity/severity_filter_group.tsx index c5373199192a4..9ecb9d9cd6cdb 100644 --- a/x-pack/plugins/security_solution/public/entity_analytics/components/severity/severity_filter_group.tsx +++ b/x-pack/plugins/security_solution/public/entity_analytics/components/severity/severity_filter_group.tsx @@ -10,12 +10,11 @@ import type { FilterChecked } from '@elastic/eui'; import { EuiFilterButton, EuiFilterGroup, - EuiFilterSelectItem, EuiPopover, useGeneratedHtmlId, - useEuiTheme, + EuiSelectable, } from '@elastic/eui'; - +import { i18n } from '@kbn/i18n'; import { SEVERITY_UI_SORT_ORDER } from '../../common/utils'; import type { RiskScoreEntity, RiskSeverity } from '../../../../common/search_strategy'; import type { SeverityCount } from './types'; @@ -27,14 +26,22 @@ interface SeverityItems { risk: RiskSeverity; count: number; checked?: FilterChecked; + label: string; } + +const SEVERITY_FILTER_ARIA_LABEL = i18n.translate( + 'xpack.securitySolution.entityAnalytics.severityFilter.ariaLabel', + { + defaultMessage: 'Select the severity level to filter by', + } +); + export const SeverityFilterGroup: React.FC<{ severityCount: SeverityCount; selectedSeverities: RiskSeverity[]; onSelect: (newSelection: RiskSeverity[]) => void; riskEntity: RiskScoreEntity; }> = ({ severityCount, selectedSeverities, onSelect, riskEntity }) => { - const { euiTheme } = useEuiTheme(); const { telemetry } = useKibana().services; const [isPopoverOpen, setIsPopoverOpen] = useState(false); @@ -55,26 +62,27 @@ export const SeverityFilterGroup: React.FC<{ return SEVERITY_UI_SORT_ORDER.map((severity) => ({ risk: severity, count: severityCount[severity], + label: severity, checked: selectedSeverities.includes(severity) ? checked : undefined, })); }, [severityCount, selectedSeverities]); const updateSeverityFilter = useCallback( - (selectedSeverity: RiskSeverity) => { - const currentSelection = selectedSeverities ?? []; - const isAddingSeverity = !currentSelection.includes(selectedSeverity); - - const newSelection = isAddingSeverity - ? [...currentSelection, selectedSeverity] - : currentSelection.filter((s) => s !== selectedSeverity); - - if (isAddingSeverity) { - telemetry.reportEntityRiskFiltered({ entity: riskEntity, selectedSeverity }); + (newSelection: SeverityItems[], _, changedSeverity: SeverityItems) => { + if (changedSeverity.checked === 'on') { + telemetry.reportEntityRiskFiltered({ + entity: riskEntity, + selectedSeverity: changedSeverity.risk, + }); } - onSelect(newSelection); + const newSelectedSeverities = newSelection + .filter((item) => item.checked === 'on') + .map((item) => item.risk); + + onSelect(newSelectedSeverities); }, - [selectedSeverities, onSelect, telemetry, riskEntity] + [onSelect, riskEntity, telemetry] ); const totalActiveItem = useMemo( @@ -107,21 +115,17 @@ export const SeverityFilterGroup: React.FC<{ closePopover={closePopover} panelPaddingSize="none" > - {/* EUI NOTE: Please use EuiSelectable (which already has height/scrolling built in) - instead of EuiFilterSelectItem (which is pending deprecation). - @see https://elastic.github.io/eui/#/forms/filter-group#multi-select */} -
- {items.map((item, index) => ( - updateSeverityFilter(item.risk)} - > - - - ))} -
+ ( + + )} + > + {(list) =>
{list}
} + );