From 6d9811ec7dcd732bc6a5984c35cf628e8ca588f8 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Wed, 23 Nov 2022 14:56:44 +0200 Subject: [PATCH] [Cases] Enhance labels in the bulk edit tags flyout (#145811) ## Summary This PR a) fixes the label when there are no tags, b) adds a label when there are no matches, c) removes a `useEffect` which is not necessary anymore, and d) shortens the help text for the tags field. Fixes: https://github.com/elastic/kibana/issues/145214 ## Screenshots Screenshot 2022-11-19 at 12 57 50 PM Screenshot 2022-11-19 at 12 45 55 PM ### 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 ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) Co-authored-by: lcawl Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- .../cases/public/common/translations.ts | 19 ++++- .../tags/edit_tags_selectable.test.tsx | 45 +++++++++- .../actions/tags/edit_tags_selectable.tsx | 83 ++++++------------- .../components/actions/tags/translations.ts | 10 ++- .../components/assign_users.test.tsx | 6 +- .../case_view/components/edit_tags.tsx | 1 + .../components/case_view/translations.ts | 2 +- .../cases/public/components/create/tags.tsx | 2 + 8 files changed, 101 insertions(+), 67 deletions(-) diff --git a/x-pack/plugins/cases/public/common/translations.ts b/x-pack/plugins/cases/public/common/translations.ts index 97483398782227..08f7c0aa4e8d03 100644 --- a/x-pack/plugins/cases/public/common/translations.ts +++ b/x-pack/plugins/cases/public/common/translations.ts @@ -143,8 +143,7 @@ export const COMMENTS = i18n.translate('xpack.cases.allCases.comments', { }); export const TAGS_HELP = i18n.translate('xpack.cases.createCase.fieldTagsHelpText', { - defaultMessage: - 'Type one or more custom identifying tags for this case. Press enter after each tag to begin a new one.', + defaultMessage: 'Separate tags with a line break.', }); export const TAGS_EMPTY_ERROR = i18n.translate('xpack.cases.createCase.fieldTagsEmptyError', { @@ -152,7 +151,7 @@ export const TAGS_EMPTY_ERROR = i18n.translate('xpack.cases.createCase.fieldTags }); export const NO_TAGS = i18n.translate('xpack.cases.caseView.noTags', { - defaultMessage: 'No tags are currently assigned to this case.', + defaultMessage: 'No tags are added', }); export const TITLE_REQUIRED = i18n.translate('xpack.cases.createCase.titleFieldRequiredError', { @@ -302,3 +301,17 @@ export const DELETED_CASES = (totalCases: number) => values: { totalCases }, defaultMessage: 'Deleted {totalCases, plural, =1 {case} other {{totalCases} cases}}', }); + +export const ADD_TAG_CUSTOM_OPTION_LABEL = (searchValue: string) => + i18n.translate('xpack.cases.configure.addTagCustomOptionLabel', { + defaultMessage: 'Add {searchValue} as a tag', + values: { searchValue }, + }); + +/** + * EUI checkbox replace {searchValue} with the current + * search value. We need to put the template variable + * searchValue in the string but not replace it + * with i18n. + */ +export const ADD_TAG_CUSTOM_OPTION_LABEL_COMBO_BOX = ADD_TAG_CUSTOM_OPTION_LABEL('{searchValue}'); diff --git a/x-pack/plugins/cases/public/components/actions/tags/edit_tags_selectable.test.tsx b/x-pack/plugins/cases/public/components/actions/tags/edit_tags_selectable.test.tsx index 9eea326ba389b1..c18642738b376d 100644 --- a/x-pack/plugins/cases/public/components/actions/tags/edit_tags_selectable.test.tsx +++ b/x-pack/plugins/cases/public/components/actions/tags/edit_tags_selectable.test.tsx @@ -237,7 +237,7 @@ describe('EditTagsSelectable', () => { }); }); - it('adds a partial match correctly', async () => { + it('adds a partial match correctly and does not show the no match label', async () => { const result = appMock.render(); /** @@ -252,6 +252,10 @@ describe('EditTagsSelectable', () => { ).toBeInTheDocument(); }); + expect( + result.queryByTestId('cases-actions-tags-edit-selectable-no-match-label') + ).not.toBeInTheDocument(); + const addNewTagButton = result.getByTestId('cases-actions-tags-edit-selectable-add-new-tag'); userEvent.click(addNewTagButton); @@ -275,4 +279,43 @@ describe('EditTagsSelectable', () => { result.queryByTestId('cases-actions-tags-edit-selectable-add-new-tag') ).not.toBeInTheDocument(); }); + + it('does not show the no match label when the initial tags are empty', async () => { + const result = appMock.render(); + + await waitForComponentToUpdate(); + + expect( + result.queryByTestId('cases-actions-tags-edit-selectable-no-match-label') + ).not.toBeInTheDocument(); + }); + + it('shows the no match label when there is no match', async () => { + const result = appMock.render(); + + await userEvent.type(result.getByPlaceholderText('Search'), 'not-exist', { delay: 1 }); + await waitForComponentToUpdate(); + + expect( + result.getByTestId('cases-actions-tags-edit-selectable-no-match-label') + ).toBeInTheDocument(); + }); + + it('shows the no match label and the add new item when there is space in the search term', async () => { + const result = appMock.render(); + + await userEvent.type(result.getByPlaceholderText('Search'), 'test tag', { delay: 1 }); + + await waitFor(() => { + expect( + result.getByTestId('cases-actions-tags-edit-selectable-add-new-tag') + ).toBeInTheDocument(); + }); + + await waitForComponentToUpdate(); + + expect( + result.getByTestId('cases-actions-tags-edit-selectable-no-match-label') + ).toBeInTheDocument(); + }); }); diff --git a/x-pack/plugins/cases/public/components/actions/tags/edit_tags_selectable.tsx b/x-pack/plugins/cases/public/components/actions/tags/edit_tags_selectable.tsx index bb7b167814c6bb..1bd1ba4b74e88e 100644 --- a/x-pack/plugins/cases/public/components/actions/tags/edit_tags_selectable.tsx +++ b/x-pack/plugins/cases/public/components/actions/tags/edit_tags_selectable.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import React, { useCallback, useMemo, useReducer, useState, useEffect } from 'react'; +import React, { useCallback, useMemo, useReducer, useState } from 'react'; import type { EuiSelectableOption, IconType } from '@elastic/eui'; import { EuiSelectable, @@ -17,11 +17,9 @@ import { EuiHorizontalRule, EuiIcon, EuiHighlight, - EuiSelectableListItem, useEuiTheme, } from '@elastic/eui'; -import { FormattedMessage } from '@kbn/i18n-react'; import { assertNever } from '@kbn/std'; import { isEmpty } from 'lodash'; import type { Case } from '../../../../common'; @@ -237,29 +235,9 @@ const hasExactMatch = (searchValue: string, options: TagSelectableOption[]) => { return options.some((option) => option.key === searchValue); }; -const AddNewTagItem: React.FC<{ searchValue: string; onNewItem: (newTag: string) => void }> = - React.memo(({ searchValue, onNewItem }) => { - const onNewTagClick = useCallback(() => { - onNewItem(searchValue); - }, [onNewItem, searchValue]); - - return ( - - {searchValue} }} - /> - - ); - }); - -AddNewTagItem.displayName = 'AddNewTagItem'; +const hasPartialMatch = (searchValue: string, options: TagSelectableOption[]) => { + return options.some((option) => option.key?.includes(searchValue)); +}; const EditTagsSelectableComponent: React.FC = ({ selectedCases, @@ -328,16 +306,6 @@ const EditTagsSelectableComponent: React.FC = ({ [onChangeTags, state.tags] ); - const onNewItem = useCallback( - (newTag: string) => { - const { selectedTags, unSelectedTags } = getSelectedAndUnselectedTags(options, state.tags); - dispatch({ type: Actions.CHECK_TAG, payload: [newTag] }); - setSearchValue(''); - onChangeTags({ selectedTags: [...selectedTags, newTag], unSelectedTags }); - }, - [onChangeTags, options, state.tags] - ); - const onSelectAll = useCallback(() => { dispatch({ type: Actions.CHECK_TAG, payload: Object.keys(state.tags) }); onChangeTags({ selectedTags: Object.keys(state.tags), unSelectedTags: [] }); @@ -360,25 +328,6 @@ const EditTagsSelectableComponent: React.FC = ({ setSearchValue(value); }, []); - /** - * TODO: Remove hack when PR https://github.com/elastic/eui/pull/6317 - * is merged and the new fix is merged into Kibana. - * - * This is a hack to force a rerender when - * the user adds a new tag. There is a bug in - * the EuiSelectable where a race condition that's causing the search bar - * to not to match terms with the empty string to trigger the reload. - * This means that when a user press the button to add a tag the - * search bar clears but the options are not shown. - */ - const [_, setRerender] = useState(0); - - useEffect(() => { - if (isEmpty(searchValue)) { - setRerender((x) => x + 1); - } - }, [options, setRerender, searchValue]); - /** * While the user searches we need to add the ability * to add the search term as a new tag. The no matches message @@ -393,8 +342,7 @@ const EditTagsSelectableComponent: React.FC = ({ return [ { key: searchValue, - searchableLabel: searchValue, - label: `Add ${searchValue} as a tag`, + label: i18n.ADD_TAG_CUSTOM_OPTION_LABEL(searchValue), 'data-test-subj': 'cases-actions-tags-edit-selectable-add-new-tag', data: { tagIcon: 'empty', newItem: true }, }, @@ -409,6 +357,11 @@ const EditTagsSelectableComponent: React.FC = ({ (tag) => tag.tagState === TagState.CHECKED || tag.tagState === TagState.PARTIAL ).length; + const showNoMatchText = useMemo( + () => !hasPartialMatch(searchValue, options) && Object.keys(state.tags).length > 0, + [options, searchValue, state.tags] + ); + return ( = ({ renderOption={renderOption} listProps={{ showIcons: false }} onChange={onChange} - noMatchesMessage={} + noMatchesMessage={i18n.NO_SEARCH_MATCH} + emptyMessage={i18n.NO_TAGS_AVAILABLE} data-test-subj="cases-actions-tags-edit-selectable" height="full" > @@ -483,6 +437,19 @@ const EditTagsSelectableComponent: React.FC = ({ + {showNoMatchText ? ( + + {i18n.NO_SEARCH_MATCH} + + ) : null} {list} )} diff --git a/x-pack/plugins/cases/public/components/actions/tags/translations.ts b/x-pack/plugins/cases/public/components/actions/tags/translations.ts index 61aadf41129a23..a00292ff11cb43 100644 --- a/x-pack/plugins/cases/public/components/actions/tags/translations.ts +++ b/x-pack/plugins/cases/public/components/actions/tags/translations.ts @@ -6,7 +6,7 @@ */ import { i18n } from '@kbn/i18n'; -export { CANCEL } from '../../../common/translations'; +export { CANCEL, ADD_TAG_CUSTOM_OPTION_LABEL } from '../../../common/translations'; export const EDIT_TAGS = i18n.translate('xpack.cases.actions.tags.edit', { defaultMessage: 'Edit tags', @@ -51,3 +51,11 @@ export const SELECTED_TAGS = (selectedTags: number) => defaultMessage: 'Selected: {selectedTags}', values: { selectedTags }, }); + +export const NO_TAGS_AVAILABLE = i18n.translate('xpack.cases.actions.tags.noTagsAvailable', { + defaultMessage: 'No tags available. To add a tag, enter it in the query bar', +}); + +export const NO_SEARCH_MATCH = i18n.translate('xpack.cases.actions.tags.noTagsMatch', { + defaultMessage: 'No tags match your search', +}); diff --git a/x-pack/plugins/cases/public/components/case_view/components/assign_users.test.tsx b/x-pack/plugins/cases/public/components/case_view/components/assign_users.test.tsx index b14e52c9e51dc2..2f90aff69c60e1 100644 --- a/x-pack/plugins/cases/public/components/case_view/components/assign_users.test.tsx +++ b/x-pack/plugins/cases/public/components/case_view/components/assign_users.test.tsx @@ -46,7 +46,7 @@ describe('AssignUsers', () => { it('does not show any assignees when there are none assigned', () => { appMockRender.render(); - expect(screen.getByText('No users have been assigned.')).toBeInTheDocument(); + expect(screen.getByText('No users are assigned')).toBeInTheDocument(); }); it('does not show the suggest users edit button when the user does not have update permissions', () => { @@ -95,7 +95,7 @@ describe('AssignUsers', () => { expect(screen.getByText('Damaged Raccoon')).toBeInTheDocument(); expect(screen.getByText('Physical Dinosaur')).toBeInTheDocument(); expect(screen.queryByText('Wet Dingo')).not.toBeInTheDocument(); - expect(screen.queryByText('No users have been assigned.')).not.toBeInTheDocument(); + expect(screen.queryByText('No users are assigned')).not.toBeInTheDocument(); expect(screen.queryByTestId('case-view-assignees-loading')).not.toBeInTheDocument(); }); @@ -112,7 +112,7 @@ describe('AssignUsers', () => { expect(screen.getByText('Damaged Raccoon')).toBeInTheDocument(); expect(screen.getByText('Physical Dinosaur')).toBeInTheDocument(); expect(screen.queryByText('Wet Dingo')).not.toBeInTheDocument(); - expect(screen.queryByText('No users have been assigned.')).not.toBeInTheDocument(); + expect(screen.queryByText('No users are assigned')).not.toBeInTheDocument(); expect(screen.queryByTestId('case-view-assignees-loading')).not.toBeInTheDocument(); }); diff --git a/x-pack/plugins/cases/public/components/case_view/components/edit_tags.tsx b/x-pack/plugins/cases/public/components/case_view/components/edit_tags.tsx index 2f8567d14f08dc..a221a43b92ded7 100644 --- a/x-pack/plugins/cases/public/components/case_view/components/edit_tags.tsx +++ b/x-pack/plugins/cases/public/components/case_view/components/edit_tags.tsx @@ -147,6 +147,7 @@ export const EditTags = React.memo(({ isLoading, onSubmit, tags }: EditTagsProps placeholder: '', options, noSuggestions: false, + customOptionText: i18n.ADD_TAG_CUSTOM_OPTION_LABEL_COMBO_BOX, }, }} /> diff --git a/x-pack/plugins/cases/public/components/case_view/translations.ts b/x-pack/plugins/cases/public/components/case_view/translations.ts index c4d0020aa71076..d71c56fc97fcab 100644 --- a/x-pack/plugins/cases/public/components/case_view/translations.ts +++ b/x-pack/plugins/cases/public/components/case_view/translations.ts @@ -186,7 +186,7 @@ export const EDIT_ASSIGNEES_ARIA_LABEL = i18n.translate( ); export const NO_ASSIGNEES = i18n.translate('xpack.cases.caseView.noAssignees', { - defaultMessage: 'No users have been assigned.', + defaultMessage: 'No users are assigned', }); export const ASSIGN_A_USER = i18n.translate('xpack.cases.caseView.assignUser', { diff --git a/x-pack/plugins/cases/public/components/create/tags.tsx b/x-pack/plugins/cases/public/components/create/tags.tsx index 6bae6015e769ba..f3d4319dfea372 100644 --- a/x-pack/plugins/cases/public/components/create/tags.tsx +++ b/x-pack/plugins/cases/public/components/create/tags.tsx @@ -10,6 +10,7 @@ import React, { memo, useMemo } from 'react'; import { getUseField } from '@kbn/es-ui-shared-plugin/static/forms/hook_form_lib'; import { Field } from '@kbn/es-ui-shared-plugin/static/forms/components'; import { useGetTags } from '../../containers/use_get_tags'; +import * as i18n from './translations'; const CommonUseField = getUseField({ component: Field }); @@ -39,6 +40,7 @@ const TagsComponent: React.FC = ({ isLoading }) => { disabled: isLoading || isLoadingTags, options, noSuggestions: false, + customOptionText: i18n.ADD_TAG_CUSTOM_OPTION_LABEL_COMBO_BOX, }, }} />