From c6c210553487cd9ed1c81f9d72502a5fbf8f004c Mon Sep 17 00:00:00 2001 From: Davis Plumlee <56367316+dplumlee@users.noreply.github.com> Date: Fri, 2 Oct 2020 16:18:55 -0600 Subject: [PATCH] [Security Solution] Changes rules table tag display (#77102) --- .../detection_engine/rules/all/columns.tsx | 22 ++-- .../rules/all/helpers.test.tsx | 13 ++- .../detection_engine/rules/all/helpers.ts | 4 + .../detection_engine/rules/all/index.test.tsx | 50 +++++---- .../tags_filter_popover.tsx | 60 ++++++---- .../rules/all/tag_display.test.tsx | 47 ++++++++ .../rules/all/tag_display.tsx | 104 ++++++++++++++++++ .../detection_engine/rules/translations.ts | 7 ++ 8 files changed, 248 insertions(+), 59 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/tag_display.test.tsx create mode 100644 x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/tag_display.tsx diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/columns.tsx b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/columns.tsx index 3666e16dcbaf8..c0786f5234157 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/columns.tsx +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/columns.tsx @@ -7,7 +7,6 @@ /* eslint-disable react/display-name */ import { - EuiBadge, EuiBasicTableColumn, EuiTableActionsColumnType, EuiText, @@ -24,7 +23,6 @@ import { getEmptyTagValue } from '../../../../../common/components/empty_value'; import { FormattedDate } from '../../../../../common/components/formatted_date'; import { getRuleDetailsUrl } from '../../../../../common/components/link_to/redirect_to_detection_engine'; import { ActionToaster } from '../../../../../common/components/toasters'; -import { TruncatableText } from '../../../../../common/components/truncatable_text'; import { getStatusColor } from '../../../../components/rules/rule_status/helpers'; import { RuleSwitch } from '../../../../components/rules/rule_switch'; import { SeverityBadge } from '../../../../components/rules/severity_badge'; @@ -39,6 +37,7 @@ import { Action } from './reducer'; import { LocalizedDateTooltip } from '../../../../../common/components/localized_date_tooltip'; import * as detectionI18n from '../../translations'; import { LinkAnchor } from '../../../../../common/components/links'; +import { TagsDisplay } from './tag_display'; export const getActions = ( dispatch: React.Dispatch, @@ -207,22 +206,19 @@ export const getColumns = ({ ); }, truncateText: true, - width: '10%', + width: '8%', }, { field: 'tags', name: i18n.COLUMN_TAGS, - render: (value: Rule['tags']) => ( - - {value.map((tag, i) => ( - - {tag} - - ))} - - ), + render: (value: Rule['tags']) => { + if (value.length > 0) { + return ; + } + return getEmptyTagValue(); + }, truncateText: true, - width: '14%', + width: '20%', }, { align: 'center', diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/helpers.test.tsx b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/helpers.test.tsx index 062d7967bf301..e06aea2fb2785 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/helpers.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/helpers.test.tsx @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { bucketRulesResponse, showRulesTable } from './helpers'; +import { bucketRulesResponse, caseInsensitiveSort, showRulesTable } from './helpers'; import { mockRule, mockRuleError } from './__mocks__/mock'; import uuid from 'uuid'; import { Rule, RuleError } from '../../../../containers/detection_engine/rules'; @@ -86,4 +86,15 @@ describe('AllRulesTable Helpers', () => { expect(result).toBeTruthy(); }); }); + + describe('caseInsensitiveSort', () => { + describe('when an array of differently cased tags is passed', () => { + const unsortedTags = ['atest', 'Ctest', 'Btest', 'ctest', 'btest', 'Atest']; + const result = caseInsensitiveSort(unsortedTags); + it('returns an alphabetically sorted array with no regard for casing', () => { + const expected = ['atest', 'Atest', 'Btest', 'btest', 'Ctest', 'ctest']; + expect(result).toEqual(expected); + }); + }); + }); }); diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/helpers.ts b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/helpers.ts index 0ebeb84d57468..c54c9ed9b8ef9 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/helpers.ts +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/helpers.ts @@ -33,3 +33,7 @@ export const showRulesTable = ({ }) => (rulesCustomInstalled != null && rulesCustomInstalled > 0) || (rulesInstalled != null && rulesInstalled > 0); + +export const caseInsensitiveSort = (tags: string[]): string[] => { + return tags.sort((a: string, b: string) => a.toLowerCase().localeCompare(b.toLowerCase())); // Case insensitive +}; diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/index.test.tsx b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/index.test.tsx index 13c6985a30c2b..f8ed57aa7537e 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/index.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/index.test.tsx @@ -5,7 +5,8 @@ */ import React from 'react'; -import { shallow, mount } from 'enzyme'; +import { shallow, mount, ReactWrapper } from 'enzyme'; +import { act } from 'react-dom/test-utils'; import '../../../../../common/mock/match_media'; import '../../../../../common/mock/formatted_relative'; @@ -179,27 +180,34 @@ describe('AllRules', () => { expect(wrapper.find('[title="All rules"]')).toHaveLength(1); }); - it('renders rules tab', async () => { - const wrapper = mount( - - - - ); + describe('rules tab', () => { + let wrapper: ReactWrapper; + beforeEach(() => { + wrapper = mount( + + + + ); + }); - await waitFor(() => { - expect(wrapper.exists('[data-test-subj="monitoring-table"]')).toBeFalsy(); - expect(wrapper.exists('[data-test-subj="rules-table"]')).toBeTruthy(); + it('renders correctly', async () => { + await act(async () => { + await waitFor(() => { + expect(wrapper.exists('[data-test-subj="monitoring-table"]')).toBeFalsy(); + expect(wrapper.exists('[data-test-subj="rules-table"]')).toBeTruthy(); + }); + }); }); }); diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/rules_table_filters/tags_filter_popover.tsx b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/rules_table_filters/tags_filter_popover.tsx index 4fe0bc8f835df..9748dde91d18b 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/rules_table_filters/tags_filter_popover.tsx +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/rules_table_filters/tags_filter_popover.tsx @@ -27,6 +27,7 @@ import { import styled from 'styled-components'; import * as i18n from '../../translations'; import { toggleSelectedGroup } from '../../../../../../common/components/ml_popover/jobs_table/filters/toggle_selected_group'; +import { caseInsensitiveSort } from '../helpers'; interface TagsFilterPopoverProps { selectedTags: string[]; @@ -36,9 +37,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; `; /** @@ -52,9 +63,7 @@ const TagsFilterPopoverComponent = ({ selectedTags, onSelectedTagsChanged, }: TagsFilterPopoverProps) => { - const sortedTags = useMemo(() => { - return tags.sort((a: string, b: string) => a.toLowerCase().localeCompare(b.toLowerCase())); // Case insensitive - }, [tags]); + const sortedTags = useMemo(() => caseInsensitiveSort(tags), [tags]); const [isTagPopoverOpen, setIsTagPopoverOpen] = useState(false); const [searchInput, setSearchInput] = useState(''); const [filterTags, setFilterTags] = useState(sortedTags); @@ -65,8 +74,9 @@ const TagsFilterPopoverComponent = ({ checked={selectedTags.includes(tag) ? 'on' : undefined} key={`${index}-${tag}`} onClick={() => toggleSelectedGroup(tag, selectedTags, onSelectedTagsChanged)} + title={tag} > - {`${tag}`} + {tag} )); }, [onSelectedTagsChanged, selectedTags, filterTags]); @@ -101,25 +111,27 @@ const TagsFilterPopoverComponent = ({ panelPaddingSize="none" repositionOnScroll > - - - - {tagsComponent} - {filterTags.length === 0 && ( - - - - {i18n.NO_TAGS_AVAILABLE} - - - - )} + + + + + {tagsComponent} + {filterTags.length === 0 && ( + + + + {i18n.NO_TAGS_AVAILABLE} + + + + )} + ); }; diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/tag_display.test.tsx b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/tag_display.test.tsx new file mode 100644 index 0000000000000..3e60bb2d3168b --- /dev/null +++ b/x-pack/plugins/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( + + + + ); + }); + 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/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/tag_display.tsx b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/tag_display.tsx new file mode 100644 index 0000000000000..9503f5b884133 --- /dev/null +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/tag_display.tsx @@ -0,0 +1,104 @@ +/* + * 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, { useMemo, useState } from 'react'; +import { EuiPopover, EuiBadgeGroup, EuiBadge, EuiButtonEmpty } from '@elastic/eui'; +import styled from 'styled-components'; +import * as i18n from '../translations'; +import { caseInsensitiveSort } from './helpers'; + +interface TagsDisplayProps { + tags: string[]; +} + +const TagWrapper = styled(EuiBadgeGroup)` + width: 100%; +`; + +const TagPopoverWrapper = styled(EuiBadgeGroup)` + max-height: 200px; + max-width: 600px; + overflow: auto; +`; + +const TagPopoverButton = styled(EuiButtonEmpty)` + font-size: ${({ theme }) => theme.eui.euiFontSizeXS} + font-weight: 500; + height: 20px; +`; + +/** + * @param tags to display for filtering + */ +const TagsDisplayComponent = ({ tags }: TagsDisplayProps) => { + const [isTagPopoverOpen, setIsTagPopoverOpen] = useState(false); + const sortedTags = useMemo(() => caseInsensitiveSort(tags), [tags]); + + return ( + <> + {sortedTags.length <= 3 ? ( + + {sortedTags.map((tag: string, i: number) => ( + + {tag} + + ))} + + ) : ( + + {sortedTags.slice(0, 3).map((tag: string, i: number) => ( + + {tag} + + ))} + + setIsTagPopoverOpen(!isTagPopoverOpen)} + > + {i18n.COLUMN_SEE_ALL_POPOVER} + + } + isOpen={isTagPopoverOpen} + closePopover={() => setIsTagPopoverOpen(!isTagPopoverOpen)} + repositionOnScroll + > + + {sortedTags.map((tag: string, i: number) => ( + + {tag} + + ))} + + + + )} + + ); +}; + +export const TagsDisplay = React.memo(TagsDisplayComponent); + +TagsDisplay.displayName = 'TagsDisplay'; diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/translations.ts b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/translations.ts index 09503fcf1ef0f..f2f3265ead1ba 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/translations.ts +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/translations.ts @@ -315,6 +315,13 @@ export const COLUMN_TAGS = i18n.translate( } ); +export const COLUMN_SEE_ALL_POPOVER = i18n.translate( + 'xpack.securitySolution.detectionEngine.rules.allRules.columns.tagsPopoverTitle', + { + defaultMessage: 'See all', + } +); + export const COLUMN_ACTIVATE = i18n.translate( 'xpack.securitySolution.detectionEngine.rules.allRules.columns.activateTitle', {