Skip to content

Commit

Permalink
[Cases] Enhance labels in the bulk edit tags flyout (#145811)
Browse files Browse the repository at this point in the history
## 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: #145214

## Screenshots

<img width="640" alt="Screenshot 2022-11-19 at 12 57 50 PM"
src="https://user-images.githubusercontent.com/7871006/203005858-e7270771-4aaf-40a6-9174-306bacfb55fd.png">

<img width="639" alt="Screenshot 2022-11-19 at 12 45 55 PM"
src="https://user-images.githubusercontent.com/7871006/203005874-6a42eff5-2c28-4962-8b53-58b2eb21b97e.png">


### 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 <lcawley@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
3 people authored Nov 23, 2022
1 parent 4c6a915 commit 6d9811e
Show file tree
Hide file tree
Showing 8 changed files with 101 additions and 67 deletions.
19 changes: 16 additions & 3 deletions x-pack/plugins/cases/public/common/translations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,16 +143,15 @@ 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', {
defaultMessage: 'A tag must contain at least one non-space character.',
});

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', {
Expand Down Expand Up @@ -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}');
Original file line number Diff line number Diff line change
Expand Up @@ -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(<EditTagsSelectable {...props} />);

/**
Expand All @@ -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);
Expand All @@ -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(<EditTagsSelectable {...props} tags={[]} />);

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(<EditTagsSelectable {...props} />);

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(<EditTagsSelectable {...props} />);

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();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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';
Expand Down Expand Up @@ -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 (
<EuiSelectableListItem
isFocused={false}
showIcons={false}
onClick={onNewTagClick}
data-test-subj="cases-actions-tags-edit-selectable-add-new-tag"
>
<FormattedMessage
id="xpack.cases.actions.tags.newTagMessage"
defaultMessage="Add {searchValue} as a tag"
values={{ searchValue: <b>{searchValue}</b> }}
/>
</EuiSelectableListItem>
);
});

AddNewTagItem.displayName = 'AddNewTagItem';
const hasPartialMatch = (searchValue: string, options: TagSelectableOption[]) => {
return options.some((option) => option.key?.includes(searchValue));
};

const EditTagsSelectableComponent: React.FC<Props> = ({
selectedCases,
Expand Down Expand Up @@ -328,16 +306,6 @@ const EditTagsSelectableComponent: React.FC<Props> = ({
[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: [] });
Expand All @@ -360,25 +328,6 @@ const EditTagsSelectableComponent: React.FC<Props> = ({
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
Expand All @@ -393,8 +342,7 @@ const EditTagsSelectableComponent: React.FC<Props> = ({
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 },
},
Expand All @@ -409,6 +357,11 @@ const EditTagsSelectableComponent: React.FC<Props> = ({
(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 (
<EuiSelectable
options={optionsWithAddNewTagOption}
Expand All @@ -424,7 +377,8 @@ const EditTagsSelectableComponent: React.FC<Props> = ({
renderOption={renderOption}
listProps={{ showIcons: false }}
onChange={onChange}
noMatchesMessage={<AddNewTagItem searchValue={searchValue ?? ''} onNewItem={onNewItem} />}
noMatchesMessage={i18n.NO_SEARCH_MATCH}
emptyMessage={i18n.NO_TAGS_AVAILABLE}
data-test-subj="cases-actions-tags-edit-selectable"
height="full"
>
Expand Down Expand Up @@ -483,6 +437,19 @@ const EditTagsSelectableComponent: React.FC<Props> = ({
</EuiFlexItem>
</EuiFlexGroup>
<EuiHorizontalRule margin="m" />
{showNoMatchText ? (
<EuiText
size="xs"
color="subdued"
textAlign="center"
css={{
marginBottom: euiTheme.size.s,
}}
data-test-subj="cases-actions-tags-edit-selectable-no-match-label"
>
{i18n.NO_SEARCH_MATCH}
</EuiText>
) : null}
{list}
</>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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',
});
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe('AssignUsers', () => {
it('does not show any assignees when there are none assigned', () => {
appMockRender.render(<AssignUsers {...defaultProps} />);

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', () => {
Expand Down Expand Up @@ -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();
});

Expand All @@ -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();
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
}}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', {
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/cases/public/components/create/tags.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 });

Expand Down Expand Up @@ -39,6 +40,7 @@ const TagsComponent: React.FC<Props> = ({ isLoading }) => {
disabled: isLoading || isLoadingTags,
options,
noSuggestions: false,
customOptionText: i18n.ADD_TAG_CUSTOM_OPTION_LABEL_COMBO_BOX,
},
}}
/>
Expand Down

0 comments on commit 6d9811e

Please sign in to comment.