[Security Solution] TopN chart styling issue (#109007) (#109283)

* fix topN style * add unit tests for topN * add unit tests * review Co-authored-by: Angela Chuang <[email protected]>
elastic · Aug 20, 2021 · 052b0b1 · 052b0b1
1 parent 786b603
commit 052b0b1
Show file tree

Hide file tree

Showing 6 changed files with 361 additions and 37 deletions.
diff --git a/...gins/security_solution/public/common/components/hover_actions/actions/show_top_n.test.tsx b/...gins/security_solution/public/common/components/hover_actions/actions/show_top_n.test.tsx
@@ -0,0 +1,166 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+import { EuiButtonEmpty, EuiContextMenuItem } from '@elastic/eui';
+import { mount } from 'enzyme';
+import React from 'react';
+import { TestProviders } from '../../../mock';
+import { ShowTopNButton } from './show_top_n';
+
+describe('show topN button', () => {
+  const defaultProps = {
+    field: 'signal.rule.name',
+    onClick: jest.fn(),
+    ownFocus: false,
+    showTopN: false,
+    timelineId: 'timeline-1',
+    value: ['rule_name'],
+  };
+
+  describe('button', () => {
+    test('should show EuiButtonIcon by default', () => {
+      const wrapper = mount(
+        <TestProviders>
+          <ShowTopNButton {...defaultProps} />
+        </TestProviders>
+      );
+      expect(wrapper.find('EuiButtonIcon').exists()).toBeTruthy();
+      expect(wrapper.find('[data-test-subj="show-top-field"]').first().prop('iconType')).toEqual(
+        'visBarVertical'
+      );
+    });
+
+    test('should support EuiButtonEmpty', () => {
+      const testProps = {
+        ...defaultProps,
+        Component: EuiButtonEmpty,
+      };
+      const wrapper = mount(
+        <TestProviders>
+          <ShowTopNButton {...testProps} />
+        </TestProviders>
+      );
+      expect(wrapper.find('EuiButtonIcon').exists()).toBeFalsy();
+      expect(wrapper.find('EuiButtonEmpty').exists()).toBeTruthy();
+      expect(wrapper.find('[data-test-subj="show-top-field"]').first().prop('iconType')).toEqual(
+        'visBarVertical'
+      );
+    });
+
+    test('should support EuiContextMenuItem', () => {
+      const testProps = {
+        ...defaultProps,
+        Component: EuiContextMenuItem,
+      };
+      const wrapper = mount(
+        <TestProviders>
+          <ShowTopNButton {...testProps} />
+        </TestProviders>
+      );
+      expect(wrapper.find('EuiButtonIcon').exists()).toBeFalsy();
+      expect(wrapper.find('EuiContextMenuItem').exists()).toBeTruthy();
+      expect(wrapper.find('[data-test-subj="show-top-field"]').first().prop('icon')).toEqual(
+        'visBarVertical'
+      );
+    });
+  });
+
+  describe('tooltip', () => {
+    test('should show tooltip by default', () => {
+      const wrapper = mount(
+        <TestProviders>
+          <ShowTopNButton {...defaultProps} />
+        </TestProviders>
+      );
+      expect(wrapper.find('EuiToolTip').exists()).toBeTruthy();
+    });
+
+    test('should hide tooltip when topN is showed', () => {
+      const testProps = {
+        ...defaultProps,
+        showTopN: true,
+      };
+      const wrapper = mount(
+        <TestProviders>
+          <ShowTopNButton {...testProps} />
+        </TestProviders>
+      );
+      expect(wrapper.find('EuiToolTip').exists()).toBeFalsy();
+    });
+
+    test('should hide tooltip by setting showTooltip to false', () => {
+      const testProps = {
+        ...defaultProps,
+        showTooltip: false,
+      };
+      const wrapper = mount(
+        <TestProviders>
+          <ShowTopNButton {...testProps} />
+        </TestProviders>
+      );
+      expect(wrapper.find('EuiToolTip').exists()).toBeFalsy();
+    });
+  });
+
+  describe('popover', () => {
+    test('should be able to show topN without a popover', () => {
+      const testProps = {
+        ...defaultProps,
+        enablePopOver: false,
+        showTopN: true,
+      };
+      const wrapper = mount(
+        <TestProviders>
+          <ShowTopNButton {...testProps} />
+        </TestProviders>
+      );
+      expect(wrapper.find('[data-test-subj="top-n"]').exists()).toBeTruthy();
+      expect(wrapper.find('[data-test-subj="showTopNContainer"]').exists()).toBeFalsy();
+    });
+    test('should be able to show topN within a popover', () => {
+      const testProps = {
+        ...defaultProps,
+        enablePopOver: true,
+        showTopN: true,
+      };
+      const wrapper = mount(
+        <TestProviders>
+          <ShowTopNButton {...testProps} />
+        </TestProviders>
+      );
+      expect(wrapper.find('[data-test-subj="top-n"]').exists()).toBeTruthy();
+      expect(wrapper.find('[data-test-subj="showTopNContainer"]').exists()).toBeTruthy();
+    });
+  });
+
+  describe('topN', () => {
+    test('should render with correct props', () => {
+      const onFilterAdded = jest.fn();
+      const testProps = {
+        ...defaultProps,
+        enablePopOver: true,
+        showTopN: true,
+        onFilterAdded,
+      };
+      const wrapper = mount(
+        <TestProviders>
+          <ShowTopNButton {...testProps} />
+        </TestProviders>
+      );
+      expect(wrapper.find('[data-test-subj="top-n"]').prop('field')).toEqual(testProps.field);
+      expect(wrapper.find('[data-test-subj="top-n"]').prop('value')).toEqual(testProps.value);
+      expect(wrapper.find('[data-test-subj="top-n"]').prop('toggleTopN')).toEqual(
+        testProps.onClick
+      );
+      expect(wrapper.find('[data-test-subj="top-n"]').prop('timelineId')).toEqual(
+        testProps.timelineId
+      );
+      expect(wrapper.find('[data-test-subj="top-n"]').prop('onFilterAdded')).toEqual(
+        testProps.onFilterAdded
+      );
+    });
+  });
+});
diff --git a/...k/plugins/security_solution/public/common/components/hover_actions/actions/show_top_n.tsx b/...k/plugins/security_solution/public/common/components/hover_actions/actions/show_top_n.tsx
@@ -6,7 +6,13 @@
  */
 
 import React, { useMemo } from 'react';
-import { EuiButtonEmpty, EuiButtonIcon, EuiContextMenuItem, EuiToolTip } from '@elastic/eui';
+import {
+  EuiButtonEmpty,
+  EuiPopover,
+  EuiButtonIcon,
+  EuiContextMenuItem,
+  EuiToolTip,
+} from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
 import { StatefulTopN } from '../../top_n';
 import { TimelineId } from '../../../../../common/types/timeline';
@@ -23,8 +29,11 @@ const SHOW_TOP = (fieldName: string) =>
   });
 
 interface Props {
-  /** `Component` is only used with `EuiDataGrid`; the grid keeps a reference to `Component` for show / hide functionality */
+  /** When `Component` is used with `EuiDataGrid`; the grid keeps a reference to `Component` for show / hide functionality.
+   * When `Component` is used with `EuiContextMenu`, we pass EuiContextMenuItem to render the right style.
+   */
   Component?: typeof EuiButtonEmpty | typeof EuiButtonIcon | typeof EuiContextMenuItem;
+  enablePopOver?: boolean;
   field: string;
   onClick: () => void;
   onFilterAdded?: () => void;
@@ -38,6 +47,7 @@ interface Props {
 export const ShowTopNButton: React.FC<Props> = React.memo(
   ({
     Component,
+    enablePopOver,
     field,
     onClick,
     onFilterAdded,
@@ -58,7 +68,7 @@ export const ShowTopNButton: React.FC<Props> = React.memo(
         : SourcererScopeName.default;
     const { browserFields, indexPattern } = useSourcererScope(activeScope);
 
-    const button = useMemo(
+    const basicButton = useMemo(
       () =>
         Component ? (
           <Component
@@ -84,32 +94,59 @@ export const ShowTopNButton: React.FC<Props> = React.memo(
       [Component, field, onClick]
     );
 
+    const button = useMemo(
+      () =>
+        showTooltip && !showTopN ? (
+          <EuiToolTip
+            content={
+              <TooltipWithKeyboardShortcut
+                additionalScreenReaderOnlyContext={getAdditionalScreenReaderOnlyContext({
+                  field,
+                  value,
+                })}
+                content={SHOW_TOP(field)}
+                shortcut={SHOW_TOP_N_KEYBOARD_SHORTCUT}
+                showShortcut={ownFocus}
+              />
+            }
+          >
+            {basicButton}
+          </EuiToolTip>
+        ) : (
+          basicButton
+        ),
+      [basicButton, field, ownFocus, showTooltip, showTopN, value]
+    );
+
+    const topNPannel = useMemo(
+      () => (
+        <StatefulTopN
+          browserFields={browserFields}
+          field={field}
+          indexPattern={indexPattern}
+          onFilterAdded={onFilterAdded}
+          timelineId={timelineId ?? undefined}
+          toggleTopN={onClick}
+          value={value}
+        />
+      ),
+      [browserFields, field, indexPattern, onClick, onFilterAdded, timelineId, value]
+    );
+
     return showTopN ? (
-      <StatefulTopN
-        browserFields={browserFields}
-        field={field}
-        indexPattern={indexPattern}
-        onFilterAdded={onFilterAdded}
-        timelineId={timelineId ?? undefined}
-        toggleTopN={onClick}
-        value={value}
-      />
-    ) : showTooltip ? (
-      <EuiToolTip
-        content={
-          <TooltipWithKeyboardShortcut
-            additionalScreenReaderOnlyContext={getAdditionalScreenReaderOnlyContext({
-              field,
-              value,
-            })}
-            content={SHOW_TOP(field)}
-            shortcut={SHOW_TOP_N_KEYBOARD_SHORTCUT}
-            showShortcut={ownFocus}
-          />
-        }
-      >
-        {button}
-      </EuiToolTip>
+      enablePopOver ? (
+        <EuiPopover
+          button={basicButton}
+          isOpen={showTopN}
+          closePopover={onClick}
+          panelClassName="withHoverActions__popover"
+          data-test-subj="showTopNContainer"
+        >
+          {topNPannel}
+        </EuiPopover>
+      ) : (
+        topNPannel
+      )
     ) : (
       button
     );