Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
js-jankisalvi committed Jul 25, 2023
1 parent 5c1d653 commit ae951cd
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -331,8 +331,11 @@ describe('AssigneesFilterPopover', () => {

expect(
screen.getByText(
`You've selected the maximum number of ${MAX_ASSIGNEES_FILTER_LENGTH} to filter assignees`
`You've selected the maximum number of ${MAX_ASSIGNEES_FILTER_LENGTH} assignees`
)
).toBeInTheDocument();

expect(screen.getByTitle('No assignees')).toHaveAttribute('aria-selected', 'false');
expect(screen.getByTitle('No assignees')).toHaveAttribute('aria-disabled', 'true');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,31 @@ describe('CasesTableFilters ', () => {
expect(screen.getByTestId('maximum-length-warning')).toBeInTheDocument();
});

it('should not show warning message when one of the tags deselected after reaching the limit', async () => {
const newTags = Array(MAX_TAGS_FILTER_LENGTH).fill('coke');
(useGetTags as jest.Mock).mockReturnValue({ data: newTags, isLoading: false });

const ourProps = {
...props,
initial: {
...DEFAULT_FILTER_OPTIONS,
tags: newTags,
},
};

appMockRender.render(<CasesTableFilters {...ourProps} />);

userEvent.click(screen.getByTestId('options-filter-popover-button-Tags'));

await waitForEuiPopoverOpen();

expect(screen.getByTestId('maximum-length-warning')).toBeInTheDocument();

userEvent.click(screen.getAllByTestId(`options-filter-popover-item-${newTags[0]}`)[0]);

expect(screen.queryByTestId('maximum-length-warning')).not.toBeInTheDocument();
});

it('should show warning message when maximum categories selected', async () => {
const newCategories = Array(MAX_CATEGORY_FILTER_LENGTH).fill('snickers');
(useGetCategories as jest.Mock).mockReturnValue({ data: newCategories, isLoading: false });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,20 +228,17 @@ const CasesTableFiltersComponent = ({
selectedOptions={selectedTags}
options={tags}
optionsEmptyLabel={i18n.NO_TAGS_AVAILABLE}
optionsMaxLEngth={MAX_TAGS_FILTER_LENGTH}
optionsMaxLengthLabel={i18n.MAX_SELECTED_FILTER(MAX_TAGS_FILTER_LENGTH, 'tags')}
limit={MAX_TAGS_FILTER_LENGTH}
limitReachedMessage={i18n.MAX_SELECTED_FILTER(MAX_TAGS_FILTER_LENGTH, 'tags')}
/>
<FilterPopover
buttonLabel={i18n.CATEGORIES}
onSelectedOptionsChanged={handleSelectedCategories}
selectedOptions={selectedCategories}
options={categories}
optionsEmptyLabel={i18n.NO_CATEGORIES_AVAILABLE}
optionsMaxLEngth={MAX_CATEGORY_FILTER_LENGTH}
optionsMaxLengthLabel={i18n.MAX_SELECTED_FILTER(
MAX_CATEGORY_FILTER_LENGTH,
'categories'
)}
limit={MAX_CATEGORY_FILTER_LENGTH}
limitReachedMessage={i18n.MAX_SELECTED_FILTER(MAX_CATEGORY_FILTER_LENGTH, 'categories')}
/>
{availableSolutions.length > 1 && (
<SolutionFilter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,10 @@ export const NO_ASSIGNEES = i18n.translate(
}
);

export const MAX_SELECTED_FILTER = (limit: number, field: string) =>
export const MAX_SELECTED_FILTER = (count: number, field: string) =>
i18n.translate('xpack.cases.userProfile.maxSelectedAssigneesFilter', {
defaultMessage: "You've selected the maximum number of {count} to filter {field}",
values: { count: limit, field },
defaultMessage: "You've selected the maximum number of {count} {field}",
values: { count, field },
});

export const SHOW_LESS = i18n.translate('xpack.cases.allCasesView.showLessAvatars', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,8 @@ describe('FilterPopover ', () => {
onSelectedOptionsChanged={onSelectedOptionsChanged}
selectedOptions={[...newTags.slice(0, 3)]}
options={newTags}
optionsMaxLEngth={maxLength}
optionsMaxLengthLabel={maxLengthLabel}
limit={maxLength}
limitReachedMessage={maxLengthLabel}
/>
);

Expand All @@ -134,6 +134,7 @@ describe('FilterPopover ', () => {

expect(getByTestId('maximum-length-warning')).toHaveTextContent(maxLengthLabel);

expect(getByTestId(`options-filter-popover-item-${newTags[3]}`)).toHaveProperty('disabled');
expect(getByTestId(`options-filter-popover-item-${newTags[4]}`)).toHaveProperty('disabled');
});

Expand All @@ -144,7 +145,7 @@ describe('FilterPopover ', () => {
onSelectedOptionsChanged={onSelectedOptionsChanged}
selectedOptions={[newTags[0], newTags[2]]}
options={newTags}
optionsMaxLEngth={maxLength}
limit={maxLength}
/>
);

Expand All @@ -153,6 +154,7 @@ describe('FilterPopover ', () => {
await waitForEuiPopoverOpen();

expect(queryByTestId('maximum-length-warning')).not.toBeInTheDocument();
expect(getByTestId(`options-filter-popover-item-${newTags[3]}`)).toHaveProperty('disabled');
expect(getByTestId(`options-filter-popover-item-${newTags[4]}`)).toHaveProperty('disabled');
});

Expand All @@ -163,6 +165,7 @@ describe('FilterPopover ', () => {
onSelectedOptionsChanged={onSelectedOptionsChanged}
selectedOptions={[newTags[0], newTags[2]]}
options={newTags}
limitReachedMessage={maxLengthLabel}
/>
);

Expand Down
43 changes: 19 additions & 24 deletions x-pack/plugins/cases/public/components/filter_popover/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@

import React, { useCallback, useState } from 'react';
import {
EuiCallOut,
EuiFilterButton,
EuiFilterSelectItem,
EuiFlexGroup,
EuiFlexItem,
EuiHorizontalRule,
EuiPanel,
EuiPopover,
EuiText,
Expand All @@ -22,8 +24,8 @@ interface FilterPopoverProps {
onSelectedOptionsChanged: (value: string[]) => void;
options: string[];
optionsEmptyLabel?: string;
optionsMaxLEngth?: number;
optionsMaxLengthLabel?: string;
limit?: number;
limitReachedMessage?: string;
selectedOptions: string[];
}

Expand All @@ -32,12 +34,6 @@ const ScrollableDiv = styled.div`
overflow: auto;
`;

const WarningPanel = styled(EuiPanel)`
${({ theme }) => `
border: 1px solid ${theme.eui.euiColorLightShade};
`}
`;

const toggleSelectedGroup = (group: string, selectedGroups: string[]): string[] => {
const selectedGroupIndex = selectedGroups.indexOf(group);
if (selectedGroupIndex >= 0) {
Expand All @@ -64,8 +60,8 @@ export const FilterPopoverComponent = ({
options,
optionsEmptyLabel,
selectedOptions,
optionsMaxLEngth,
optionsMaxLengthLabel,
limit,
limitReachedMessage,
}: FilterPopoverProps) => {
const [isPopoverOpen, setIsPopoverOpen] = useState(false);

Expand Down Expand Up @@ -97,25 +93,24 @@ export const FilterPopoverComponent = ({
panelPaddingSize="none"
repositionOnScroll
>
{optionsMaxLEngth && optionsMaxLengthLabel && selectedOptions.length >= optionsMaxLEngth && (
<EuiFlexGroup gutterSize="xs">
<EuiFlexItem grow={true}>
<WarningPanel color="warning" paddingSize="s">
<EuiText size="s" color="warning" data-test-subj="maximum-length-warning">
{optionsMaxLengthLabel}
</EuiText>
</WarningPanel>
</EuiFlexItem>
</EuiFlexGroup>
)}
{limit && limitReachedMessage && selectedOptions.length >= limit ? (
<>
<EuiHorizontalRule margin="none" />
<EuiCallOut
title={limitReachedMessage}
color="warning"
size="s"
data-test-subj="maximum-length-warning"
/>
<EuiHorizontalRule margin="none" />
</>
) : null}
<ScrollableDiv>
{options.map((option, index) => (
<EuiFilterSelectItem
checked={selectedOptions.includes(option) ? 'on' : undefined}
disabled={Boolean(
optionsMaxLEngth &&
selectedOptions.length >= optionsMaxLEngth &&
!selectedOptions.includes(option)
limit && selectedOptions.length >= limit && !selectedOptions.includes(option)
)}
data-test-subj={`options-filter-popover-item-${option}`}
key={`${index}-${option}`}
Expand Down

0 comments on commit ae951cd

Please sign in to comment.