From d2bdf7260ccbf373e83e1b4cfce1a8506753e6df Mon Sep 17 00:00:00 2001 From: Maxim Palenov Date: Fri, 27 Jan 2023 16:35:10 +0100 Subject: [PATCH] [Security Solution] Migrate rules table tags filter to EuiSelectable (#149508) **Relates to**: https://github.com/elastic/kibana/issues/140263 ## Summary This PR migrates custom tags selector implementation on the rules page which mimics EuiSelectable to **EuiSelectable**. Besides simplification it brings keyboard and accessibility support as well as simplifies accessing the component in e2e tests. *Before:* https://user-images.githubusercontent.com/3775283/214831542-737aa9cf-8f76-4777-a23f-cbbfe0a01825.mov *After:* https://user-images.githubusercontent.com/3775283/214831568-e0809fd7-3c17-4789-8d3a-9ecbe379fb56.mov ### Checklist - [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/)) - [x] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) --- .../e2e/detection_rules/bulk_edit_rules.cy.ts | 12 -- .../cypress/screens/common/controls.ts | 2 + .../cypress/tasks/rules_bulk_edit.ts | 4 +- .../tags_filter_popover.tsx | 149 ++++++++---------- .../detection_engine/rules/translations.ts | 14 ++ 5 files changed, 82 insertions(+), 99 deletions(-) diff --git a/x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules.cy.ts b/x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules.cy.ts index 7aa90997a8101..b3f9a48b3e3ef 100644 --- a/x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules.cy.ts +++ b/x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules.cy.ts @@ -267,12 +267,6 @@ describe('Detection rules, bulk edit', () => { // check if only pre-populated tags exist in the tags filter checkTagsInTagsFilter(prePopulatedTags); - cy.get(EUI_FILTER_SELECT_ITEM) - .should('have.length', prePopulatedTags.length) - .each(($el, index) => { - cy.wrap($el).should('have.text', prePopulatedTags[index]); - }); - selectNumberOfRules(expectedNumberOfCustomRulesToBeEdited); // open add tags form and add 2 new tags @@ -296,12 +290,6 @@ describe('Detection rules, bulk edit', () => { // check if only pre-populated tags exist in the tags filter checkTagsInTagsFilter(prePopulatedTags); - cy.get(EUI_FILTER_SELECT_ITEM) - .should('have.length', prePopulatedTags.length) - .each(($el, index) => { - cy.wrap($el).should('have.text', prePopulatedTags[index]); - }); - selectNumberOfRules(expectedNumberOfCustomRulesToBeEdited); // open add tags form and add 2 new tags diff --git a/x-pack/plugins/security_solution/cypress/screens/common/controls.ts b/x-pack/plugins/security_solution/cypress/screens/common/controls.ts index 2b36103996168..8a2335f81049b 100644 --- a/x-pack/plugins/security_solution/cypress/screens/common/controls.ts +++ b/x-pack/plugins/security_solution/cypress/screens/common/controls.ts @@ -9,6 +9,8 @@ export const TIMELINE_SEARCHBOX = '[data-test-subj="timeline-super-select-search export const EUI_FILTER_SELECT_ITEM = '.euiFilterSelectItem'; +export const EUI_SELECTABLE_LIST_ITEM = '[data-test-subj="euiSelectableList"] li'; + export const EUI_CHECKBOX = '.euiCheckbox__input'; export const COMBO_BOX_INPUT = '[data-test-subj="comboBoxInput"]'; diff --git a/x-pack/plugins/security_solution/cypress/tasks/rules_bulk_edit.ts b/x-pack/plugins/security_solution/cypress/tasks/rules_bulk_edit.ts index e01e72fe140db..b2203d1b1202a 100644 --- a/x-pack/plugins/security_solution/cypress/tasks/rules_bulk_edit.ts +++ b/x-pack/plugins/security_solution/cypress/tasks/rules_bulk_edit.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { TIMELINE_SEARCHBOX, EUI_FILTER_SELECT_ITEM } from '../screens/common/controls'; +import { TIMELINE_SEARCHBOX, EUI_SELECTABLE_LIST_ITEM } from '../screens/common/controls'; import { BULK_ACTIONS_BTN, @@ -226,7 +226,7 @@ export const selectTimelineTemplate = (timelineTitle: string) => { export const checkTagsInTagsFilter = (tags: string[]) => { cy.get(RULES_TAGS_FILTER_BTN).contains(`Tags${tags.length}`).click(); - cy.get(EUI_FILTER_SELECT_ITEM) + cy.get(EUI_SELECTABLE_LIST_ITEM) .should('have.length', tags.length) .each(($el, index) => { cy.wrap($el).should('have.text', tags[index]); diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/rules_table_filters/tags_filter_popover.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/rules_table_filters/tags_filter_popover.tsx index b74cf38cc9266..8784c42920931 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/rules_table_filters/tags_filter_popover.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/rules_table_filters/tags_filter_popover.tsx @@ -5,45 +5,21 @@ * 2.0. */ -import type { ChangeEvent } from 'react'; -import React, { useCallback, useEffect, useMemo, useState } from 'react'; -import { - EuiFilterButton, - EuiFilterSelectItem, - EuiFlexGroup, - EuiFlexItem, - EuiPanel, - EuiPopover, - EuiText, - EuiFieldSearch, - EuiPopoverTitle, -} from '@elastic/eui'; -import styled from 'styled-components'; +import React, { useEffect, useMemo, useState } from 'react'; +import type { EuiSelectableOption } from '@elastic/eui'; +import { EuiFilterButton, EuiPopover, EuiPopoverTitle, EuiSelectable } from '@elastic/eui'; import * as i18n from '../../../../../detections/pages/detection_engine/rules/translations'; import { toggleSelectedGroup } from '../../../../../common/components/ml_popover/jobs_table/filters/toggle_selected_group'; import { caseInsensitiveSort } from '../helpers'; +const TAGS_POPOVER_WIDTH = 274; + interface TagsFilterPopoverProps { selectedTags: string[]; tags: string[]; onSelectedTagsChanged: (newTags: string[]) => void; } -const PopoverContentWrapper = styled.div` - width: 275px; -`; - -const ScrollableDiv = styled.div` - max-height: 250px; - overflow-y: auto; -`; - -const TagOverflowContainer = styled.span` - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; -`; - /** * Popover for selecting tags to filter on * @@ -60,75 +36,78 @@ const TagsFilterPopoverComponent = ({ [selectedTags, tags] ); const [isTagPopoverOpen, setIsTagPopoverOpen] = useState(false); - const [searchInput, setSearchInput] = useState(''); - const [filterTags, setFilterTags] = useState(sortedTags); - - const tagsComponent = useMemo(() => { - return filterTags.map((tag, index) => ( - toggleSelectedGroup(tag, selectedTags, onSelectedTagsChanged)} - title={tag} - > - {tag} - - )); - }, [onSelectedTagsChanged, selectedTags, filterTags]); + const [selectableOptions, setSelectableOptions] = useState(() => { + const selectedTagsSet = new Set(selectedTags); - const onSearchInputChange = useCallback((event: ChangeEvent) => { - setSearchInput(event.target.value); - }, []); + return sortedTags.map((label) => ({ + label, + checked: selectedTagsSet.has(label) ? 'on' : undefined, + })); + }); + const handleSelectableOptionsChange = ( + newOptions: EuiSelectableOption[], + _: unknown, + changedOption: EuiSelectableOption + ) => { + setSelectableOptions(newOptions); + toggleSelectedGroup(changedOption.label, selectedTags, onSelectedTagsChanged); + }; useEffect(() => { - setFilterTags( - sortedTags.filter((tag) => tag.toLowerCase().includes(searchInput.toLowerCase())) - ); - }, [sortedTags, searchInput]); + const selectedTagsSet = new Set(selectedTags); + const newSelectableOptions: EuiSelectableOption[] = sortedTags.map((label) => ({ + label, + checked: selectedTagsSet.has(label) ? 'on' : undefined, + })); + + setSelectableOptions(newSelectableOptions); + }, [sortedTags, selectedTags]); + + const triggerButton = ( + setIsTagPopoverOpen(!isTagPopoverOpen)} + numFilters={tags.length} + isSelected={isTagPopoverOpen} + hasActiveFilters={selectedTags.length > 0} + numActiveFilters={selectedTags.length} + data-test-subj="tags-filter-popover-button" + > + {i18n.TAGS} + + ); return ( setIsTagPopoverOpen(!isTagPopoverOpen)} - numFilters={tags.length} - isSelected={isTagPopoverOpen} - hasActiveFilters={selectedTags.length > 0} - numActiveFilters={selectedTags.length} - > - {i18n.TAGS} - - } + button={triggerButton} isOpen={isTagPopoverOpen} closePopover={() => setIsTagPopoverOpen(!isTagPopoverOpen)} panelPaddingSize="none" repositionOnScroll + panelProps={{ + 'data-test-subj': 'tags-filter-popover', + }} > - - - - - {tagsComponent} - {filterTags.length === 0 && ( - - - - {i18n.NO_TAGS_AVAILABLE} - - - + + {(list, search) => ( +
+ {search} + {list} +
)} - + ); }; 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 e734b9b384ff9..c350e09d26f8f 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 @@ -659,6 +659,20 @@ export const TAGS = i18n.translate( } ); +export const SEARCH_TAGS = i18n.translate( + 'xpack.securitySolution.detectionEngine.rules.allRules.filters.searchTagsPlaceholder', + { + defaultMessage: 'Search tags', + } +); + +export const RULES_TAG_SEARCH = i18n.translate( + 'xpack.securitySolution.detectionEngine.rules.allRules.filters.rulesTagSearchText', + { + defaultMessage: 'Rules tag search', + } +); + export const NO_TAGS_AVAILABLE = i18n.translate( 'xpack.securitySolution.detectionEngine.rules.allRules.filters.noTagsAvailableDescription', {