[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 <img width="200" src="https://github.com/elastic/kibana/assets/1490444/233ff8c5-09c4-4afa-a719-1e0cae8f0396"/> <img width="213" src="https://github.com/elastic/kibana/assets/1490444/8cc262d0-047a-4ef7-b182-ab42df6923f9"/> ### After <img width="200" src="https://github.com/elastic/kibana/assets/1490444/d5a822c5-1d52-4880-81c7-83ab82df01e3"/> <img width="210" src="https://github.com/elastic/kibana/assets/1490444/6ce840c0-18d5-42fa-b4ab-734478b023a8"/> ### 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/))
elastic · Jan 16, 2024 · c7dad26 · c7dad26
1 parent 692a8ca
commit c7dad26
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 49 deletions.
diff --git a/...ck/plugins/security_solution/public/entity_analytics/components/severity/common/index.tsx b/...ck/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 = (
     <RiskBadge
       color={euiLightVars.euiColorDanger}
       $severity={severity}
       $hideBackgroundColor={hideBackgroundColor}
-      data-test-subj="risk-score"
+      data-test-subj={dataTestSubj ?? 'risk-score'}
     >
-      <EuiHealth className="eui-alignMiddle" color={RISK_SEVERITY_COLOUR[severity]}>
-        {severity}
-      </EuiHealth>
+      <EuiTextColor color="default">
+        <EuiHealth className="eui-alignMiddle" color={RISK_SEVERITY_COLOUR[severity]}>
+          {severity}
+        </EuiHealth>
+      </EuiTextColor>
     </RiskBadge>
   );
 

diff --git a/...urity_solution/public/entity_analytics/components/severity/severity_filter_group.test.tsx b/...urity_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(
       <TestProviders>
         <SeverityFilterGroup
           selectedSeverities={[]}
@@ -47,17 +47,13 @@ describe('SeverityFilterGroup', () => {
 
     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(
       <TestProviders>
         <SeverityFilterGroup
           selectedSeverities={[]}
@@ -76,12 +72,12 @@ describe('SeverityFilterGroup', () => {
 
     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(
       <TestProviders>
         <SeverityFilterGroup
           selectedSeverities={[
@@ -106,7 +102,7 @@ describe('SeverityFilterGroup', () => {
 
     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/...s/security_solution/public/entity_analytics/components/severity/severity_filter_group.tsx b/...s/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 */}
-        <div className="eui-yScroll" css={{ maxHeight: euiTheme.base * 30 }}>
-          {items.map((item, index) => (
-            <EuiFilterSelectItem
-              data-test-subj={`risk-filter-item-${item.risk}`}
-              checked={item.checked}
-              key={index + item.risk}
-              onClick={() => updateSeverityFilter(item.risk)}
-            >
-              <RiskScoreLevel severity={item.risk} />
-            </EuiFilterSelectItem>
-          ))}
-        </div>
+        <EuiSelectable
+          aria-label={SEVERITY_FILTER_ARIA_LABEL}
+          options={items}
+          onChange={updateSeverityFilter}
+          data-test-subj="risk-filter-selectable"
+          renderOption={(item) => (
+            <RiskScoreLevel data-test-subj={`risk-filter-item-${item.risk}`} severity={item.risk} />
+          )}
+        >
+          {(list) => <div style={{ width: 150 }}>{list}</div>}
+        </EuiSelectable>
       </EuiPopover>
     </EuiFilterGroup>
   );