adds tests and addresses comments

elastic · Oct 1, 2020 · bb163a9 · bb163a9
1 parent 6feb5d6
commit bb163a9
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 45 deletions.
diff --git a/.../plugins/security_solution/public/detections/pages/detection_engine/rules/all/columns.tsx b/.../plugins/security_solution/public/detections/pages/detection_engine/rules/all/columns.tsx
@@ -211,7 +211,7 @@ export const getColumns = ({
       field: 'tags',
       name: i18n.COLUMN_TAGS,
       render: (value: Rule['tags']) => {
-        if (value != null && value.length > 0) {
+        if (value.length > 0) {
           return <TagsDisplay tags={value} />;
         }
         return getEmptyTagValue();

diff --git a/...ugins/security_solution/public/detections/pages/detection_engine/rules/all/index.test.tsx b/...ugins/security_solution/public/detections/pages/detection_engine/rules/all/index.test.tsx
@@ -14,8 +14,6 @@ import { TestProviders } from '../../../../../common/mock';
 import { waitFor } from '@testing-library/react';
 import { AllRules } from './index';
 
-const mockTags = ['Elastic', 'Endpoint', 'Data Protection', 'ML', 'Continuous Monitoring'];
-
 jest.mock('react-router-dom', () => {
   const original = jest.requireActual('react-router-dom');
 
@@ -68,7 +66,7 @@ jest.mock('./reducer', () => {
           risk_score: 73,
           rule_id: '571afc56-5ed9-465d-a2a9-045f099f6e7e',
           severity: 'high',
-          tags: mockTags,
+          tags: ['Elastic', 'Endpoint'],
           threat: [],
           throttle: null,
           to: 'now',
@@ -114,7 +112,7 @@ jest.mock('../../../../containers/detection_engine/rules', () => {
             risk_score: 73,
             rule_id: '571afc56-5ed9-465d-a2a9-045f099f6e7e',
             severity: 'high',
-            tags: mockTags,
+            tags: ['Elastic', 'Endpoint'],
             threat: [],
             throttle: null,
             to: 'now',
@@ -211,12 +209,6 @@ describe('AllRules', () => {
         });
       });
     });
-
-    it('visibly renders all tags', () => {
-      for (let i = 0; i < mockTags.length; i++) {
-        expect(wrapper.exists(`[data-test-subj="rules-table-column-tags-${i}"]`)).toBeTruthy();
-      }
-    });
   });
 
   it('renders monitoring tab when monitoring tab clicked', async () => {

diff --git a/...c/detections/pages/detection_engine/rules/all/rules_table_filters/tags_filter_popover.tsx b/...c/detections/pages/detection_engine/rules/all/rules_table_filters/tags_filter_popover.tsx
@@ -36,9 +36,19 @@ interface TagsFilterPopoverProps {
   isLoading: boolean; // TO DO reimplement?
 }
 
+const PopoverContentWrapper = styled.div`
+  width: 275px;
+`;
+
 const ScrollableDiv = styled.div`
   max-height: 250px;
-  overflow: auto;
+  overflow-y: auto;
+`;
+
+const TagOverflowContainer = styled.span`
+  overflow: hidden;
+  text-overflow: ellipsis;
+  white-space: nowrap;
 `;
 
 /**
@@ -65,8 +75,9 @@ const TagsFilterPopoverComponent = ({
         checked={selectedTags.includes(tag) ? 'on' : undefined}
         key={`${index}-${tag}`}
         onClick={() => toggleSelectedGroup(tag, selectedTags, onSelectedTagsChanged)}
+        title={tag}
       >
-        {`${tag}`}
+        <TagOverflowContainer>{tag}</TagOverflowContainer>
       </EuiFilterSelectItem>
     ));
   }, [onSelectedTagsChanged, selectedTags, filterTags]);
@@ -101,25 +112,27 @@ const TagsFilterPopoverComponent = ({
       panelPaddingSize="none"
       repositionOnScroll
     >
-      <EuiPopoverTitle>
-        <EuiFieldSearch
-          placeholder="Search tags"
-          value={searchInput}
-          onChange={onSearchInputChange}
-          isClearable
-          aria-label="Rules tag search"
-        />
-      </EuiPopoverTitle>
-      <ScrollableDiv>{tagsComponent}</ScrollableDiv>
-      {filterTags.length === 0 && (
-        <EuiFlexGroup gutterSize="m" justifyContent="spaceAround">
-          <EuiFlexItem grow={true}>
-            <EuiPanel>
-              <EuiText>{i18n.NO_TAGS_AVAILABLE}</EuiText>
-            </EuiPanel>
-          </EuiFlexItem>
-        </EuiFlexGroup>
-      )}
+      <PopoverContentWrapper>
+        <EuiPopoverTitle>
+          <EuiFieldSearch
+            placeholder="Search tags"
+            value={searchInput}
+            onChange={onSearchInputChange}
+            isClearable
+            aria-label="Rules tag search"
+          />
+        </EuiPopoverTitle>
+        <ScrollableDiv>{tagsComponent}</ScrollableDiv>
+        {filterTags.length === 0 && (
+          <EuiFlexGroup gutterSize="m" justifyContent="spaceAround">
+            <EuiFlexItem grow={true}>
+              <EuiPanel>
+                <EuiText>{i18n.NO_TAGS_AVAILABLE}</EuiText>
+              </EuiPanel>
+            </EuiFlexItem>
+          </EuiFlexGroup>
+        )}
+      </PopoverContentWrapper>
     </EuiPopover>
   );
 };

diff --git a/...security_solution/public/detections/pages/detection_engine/rules/all/tag_display.test.tsx b/...security_solution/public/detections/pages/detection_engine/rules/all/tag_display.test.tsx
@@ -0,0 +1,47 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import React from 'react';
+import { mount, ReactWrapper } from 'enzyme';
+
+import { TagsDisplay } from './tag_display';
+import { TestProviders } from '../../../../../common/mock';
+import { waitFor } from '@testing-library/react';
+
+const mockTags = ['Elastic', 'Endpoint', 'Data Protection', 'ML', 'Continuous Monitoring'];
+
+describe('When tag display loads', () => {
+  let wrapper: ReactWrapper;
+  beforeEach(() => {
+    wrapper = mount(
+      <TestProviders>
+        <TagsDisplay tags={mockTags} />
+      </TestProviders>
+    );
+  });
+  it('visibly renders 3 initial tags', () => {
+    for (let i = 0; i < 3; i++) {
+      expect(wrapper.exists(`[data-test-subj="rules-table-column-tags-${i}"]`)).toBeTruthy();
+    }
+  });
+  describe("when the 'see all' button is clicked", () => {
+    beforeEach(() => {
+      const seeAllButton = wrapper.find('[data-test-subj="tags-display-popover-button"] button');
+      seeAllButton.simulate('click');
+    });
+    it('renders all the tags in the popover', async () => {
+      await waitFor(() => {
+        wrapper.update();
+        expect(wrapper.exists('[data-test-subj="tags-display-popover"]')).toBeTruthy();
+        for (let i = 0; i < mockTags.length; i++) {
+          expect(
+            wrapper.exists(`[data-test-subj="rules-table-column-popover-tags-${i}"]`)
+          ).toBeTruthy();
+        }
+      });
+    });
+  });
+});
diff --git a/...gins/security_solution/public/detections/pages/detection_engine/rules/all/tag_display.tsx b/...gins/security_solution/public/detections/pages/detection_engine/rules/all/tag_display.tsx
@@ -5,18 +5,7 @@
  */
 
 import React, { useState } from 'react';
-import {
-  EuiFlexGroup,
-  EuiFlexItem,
-  EuiPanel,
-  EuiPopover,
-  EuiText,
-  EuiFieldSearch,
-  EuiPopoverTitle,
-  EuiBadgeGroup,
-  EuiBadge,
-  EuiButtonEmpty,
-} from '@elastic/eui';
+import { EuiPopover, EuiBadgeGroup, EuiBadge, EuiButtonEmpty } from '@elastic/eui';
 import styled from 'styled-components';
 import * as i18n from '../translations';
 
@@ -29,7 +18,9 @@ const TagWrapper = styled(EuiBadgeGroup)`
 `;
 
 const TagPopoverWrapper = styled(EuiBadgeGroup)`
-  width: min-content;
+  max-height: 200px;
+  max-width: 600px;
+  overflow: auto;
 `;
 
 const TagPopoverButton = styled(EuiButtonEmpty)`
@@ -73,6 +64,7 @@ const TagsDisplayComponent = ({ tags }: TagsDisplayProps) => {
           <EuiPopover
             ownFocus
             display="block"
+            data-test-subj="tags-display-popover"
             button={
               <TagPopoverButton
                 size="xs"