Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Uptime] Improve filter group #88185

Merged
merged 19 commits into from
Jan 21, 2021
Merged

[Uptime] Improve filter group #88185

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
0e55ef9
Unskip "Observer location" test block.
justinkambic Jan 6, 2021
e1c5a39
Commit temp "describe.only" to make flaky test runner go faster.
justinkambic Jan 6, 2021
223eb59
Add optional chain for some potentially-null props.
justinkambic Jan 11, 2021
19f9d07
Make overview filters type partial.
justinkambic Jan 12, 2021
f73911a
Repair broken types.
justinkambic Jan 12, 2021
7e4e098
Remove only call from test.
justinkambic Jan 13, 2021
d03b0a4
Add unit tests and mark areas for improvement in \`FilterGroup\` comp…
justinkambic Jan 13, 2021
ca13fe8
Add aria-label translations and new labels.
justinkambic Jan 13, 2021
ed2c964
Refactor existing tests and add tests for new labels.
justinkambic Jan 13, 2021
0d27256
Fix bug in event handler and update tests.
justinkambic Jan 14, 2021
117b17c
Merge branch 'master' into uptime_improve-filter-group
justinkambic Jan 15, 2021
fbdcbb3
Delete a comment.
justinkambic Jan 15, 2021
19ea207
Delete a comment.
justinkambic Jan 15, 2021
b7345fb
Add some line breaks to help readability.
justinkambic Jan 15, 2021
9b624e4
Merge branch 'master' into uptime_improve-filter-group
kibanamachine Jan 19, 2021
ba1969c
Merge branch 'master' into uptime_improve-filter-group
kibanamachine Jan 20, 2021
04706e9
Merge branch 'master' into uptime_improve-filter-group
justinkambic Jan 20, 2021
60984e9
Merge branch 'uptime_improve-filter-group' of github.com:justinkambic…
justinkambic Jan 20, 2021
8fb99f5
Add additional tests, fix a bug.
justinkambic Jan 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
200 changes: 78 additions & 122 deletions .../public/components/overview/alerts/monitor_expressions/filters_expression_select.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
import React from 'react';
import { shallowWithIntl } from '@kbn/test/jest';
import { FiltersExpressionsSelect } from './filters_expression_select';
import { render } from '../../../../lib/helper/rtl_helpers';
import { filterAriaLabels } from './translations';

describe('filters expression select component', () => {
describe('FiltersExpressionSelect', () => {
it('is empty when no filters available', () => {
const component = shallowWithIntl(
<FiltersExpressionsSelect
Expand All @@ -35,11 +37,27 @@ describe('filters expression select component', () => {
`);
});

it('contains provided new filter values', () => {
const component = shallowWithIntl(
it.each([
[
['observer.geo.name'],
[filterAriaLabels.LOCATION],
[filterAriaLabels.TAG, filterAriaLabels.PORT, filterAriaLabels.SCHEME],
],
[
['observer.geo.name', 'tags'],
[filterAriaLabels.LOCATION, filterAriaLabels.TAG],
[filterAriaLabels.PORT, filterAriaLabels.SCHEME],
],
[
['url.port', 'monitor.type'],
[filterAriaLabels.PORT, filterAriaLabels.SCHEME],
[filterAriaLabels.LOCATION, filterAriaLabels.TAG],
],
])('contains provided new filter values', (newFilters, expectedLabels, absentLabels) => {
const { getByLabelText, queryByLabelText } = render(
<FiltersExpressionsSelect
alertParams={{}}
newFilters={['observer.geo.name']}
newFilters={newFilters}
onRemoveFilter={jest.fn()}
filters={{
tags: [],
Expand All @@ -52,125 +70,63 @@ describe('filters expression select component', () => {
shouldUpdateUrl={false}
/>
);
expect(component).toMatchInlineSnapshot(`
<Fragment>
<EuiFlexGroup
key="filter_location"
>
<EuiFlexItem>
<FilterPopover
btnContent={
<EuiExpression
aria-label="ariaLabel"
color="secondary"
data-test-subj="uptimeCreateStatusAlert.filter_location"
description="From"
onClick={[Function]}
value="any location"
/>
}
disabled={true}
fieldName="observer.geo.name"
forceOpen={false}
id="filter_location"
items={Array []}
loading={false}
onFilterFieldChange={[Function]}
selectedItems={Array []}
setForceOpen={[Function]}
title="Scheme"
/>
</EuiFlexItem>
<EuiFlexItem
grow={false}
>
<EuiButtonIcon
aria-label="Remove filter"
color="danger"
iconType="trash"
onClick={[Function]}
/>
</EuiFlexItem>
<EuiSpacer
size="xs"
/>
</EuiFlexGroup>
<EuiSpacer
size="xs"
/>
</Fragment>
`);
expectedLabels.forEach((label) => expect(getByLabelText(label)));
absentLabels.forEach((label) => expect(queryByLabelText(label)).toBeNull());
});

it('contains provided selected filter values', () => {
const component = shallowWithIntl(
<FiltersExpressionsSelect
alertParams={{}}
newFilters={['tags']}
onRemoveFilter={jest.fn()}
filters={{
tags: ['foo', 'bar'],
ports: [],
schemes: [],
locations: [],
}}
setAlertParams={jest.fn()}
setUpdatedFieldValues={jest.fn()}
shouldUpdateUrl={false}
/>
);
expect(component).toMatchInlineSnapshot(`
<Fragment>
<EuiFlexGroup
key="filter_tags"
>
<EuiFlexItem>
<FilterPopover
btnContent={
<EuiExpression
aria-label="ariaLabel"
color="secondary"
data-test-subj="uptimeCreateStatusAlert.filter_tags"
description="Using"
onClick={[Function]}
value="any tag"
/>
}
disabled={false}
fieldName="tags"
forceOpen={false}
id="filter_tags"
items={
Array [
"foo",
"bar",
]
}
loading={false}
onFilterFieldChange={[Function]}
selectedItems={Array []}
setForceOpen={[Function]}
title="Tags"
/>
</EuiFlexItem>
<EuiFlexItem
grow={false}
>
<EuiButtonIcon
aria-label="Remove filter"
color="danger"
iconType="trash"
onClick={[Function]}
/>
</EuiFlexItem>
<EuiSpacer
size="xs"
/>
</EuiFlexGroup>
<EuiSpacer
size="xs"
it.each([
[
{
tags: ['foo', 'bar'],
ports: [],
schemes: [],
locations: [],
},
filterAriaLabels.TAG,
],
[
{
tags: [],
ports: [5601, 9200],
schemes: [],
locations: [],
},
filterAriaLabels.TAG,
],
[
{
tags: [],
ports: [],
schemes: ['http', 'tcp'],
locations: [],
},
filterAriaLabels.TAG,
],
[
{
tags: [],
ports: [],
schemes: [],
locations: ['nyc', 'fairbanks'],
},
filterAriaLabels.TAG,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will it be filterAriaLabels.TAG in all cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I misunderstood your ask. I will address this.

],
])(
'applies accessible label to filter expressions, and contains selected filters',
async (filters, expectedLabel) => {
const { getByLabelText } = render(
<FiltersExpressionsSelect
alertParams={{}}
newFilters={['tags']}
onRemoveFilter={jest.fn()}
filters={filters}
setAlertParams={jest.fn()}
setUpdatedFieldValues={jest.fn()}
shouldUpdateUrl={false}
/>
</Fragment>
`);
});
);

expect(getByLabelText(expectedLabel));
}
);
});
12 changes: 8 additions & 4 deletions ...ptime/public/components/overview/alerts/monitor_expressions/filters_expression_select.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import React, { useState } from 'react';
import { EuiButtonIcon, EuiExpression, EuiFlexGroup, EuiFlexItem, EuiSpacer } from '@elastic/eui';
import { FilterPopover } from '../../filter_group/filter_popover';
import { filterLabels } from '../../filter_group/translations';
import { alertFilterLabels } from './translations';
import { alertFilterLabels, filterAriaLabels } from './translations';
import { FilterExpressionsSelectProps } from './filters_expression_select_container';
import { OverviewFiltersState } from '../../../../state/reducers/overview_filters';

Expand Down Expand Up @@ -58,6 +58,7 @@ export const FiltersExpressionsSelect: React.FC<Props> = ({

const monitorFilters = [
{
'aria-label': filterAriaLabels.PORT,
onFilterFieldChange,
loading: false,
fieldName: 'url.port',
Expand All @@ -71,18 +72,20 @@ export const FiltersExpressionsSelect: React.FC<Props> = ({
value: selectedPorts.length === 0 ? alertFilterLabels.ANY_PORT : selectedPorts?.join(','),
},
{
'aria-label': filterAriaLabels.TAG,
onFilterFieldChange,
loading: false,
fieldName: 'tags',
id: 'filter_tags',
disabled: tags?.length === 0,
items: tags ?? [],
selectedItems: selectedTags,
title: filterLabels.TAGS,
title: filterLabels.TAG,
description: selectedTags.length === 0 ? alertFilterLabels.WITH : alertFilterLabels.WITH_TAG,
value: selectedTags.length === 0 ? alertFilterLabels.ANY_TAG : selectedTags?.join(','),
},
{
'aria-label': filterAriaLabels.SCHEME,
onFilterFieldChange,
loading: false,
fieldName: 'monitor.type',
Expand All @@ -95,6 +98,7 @@ export const FiltersExpressionsSelect: React.FC<Props> = ({
value: selectedSchemes.length === 0 ? alertFilterLabels.ANY_TYPE : selectedSchemes?.join(','),
},
{
'aria-label': filterAriaLabels.LOCATION,
onFilterFieldChange,
loading: false,
fieldName: 'observer.geo.name',
Expand Down Expand Up @@ -132,7 +136,7 @@ export const FiltersExpressionsSelect: React.FC<Props> = ({
{...item}
btnContent={
<EuiExpression
aria-label={'ariaLabel'}
aria-label={item['aria-label']}
color={'secondary'}
data-test-subj={'uptimeCreateStatusAlert.' + item.id}
description={description}
Expand All @@ -148,7 +152,7 @@ export const FiltersExpressionsSelect: React.FC<Props> = ({
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiButtonIcon
aria-label="Remove filter"
aria-label={alertFilterLabels.REMOVE_FILTER_LABEL}
iconType="trash"
color="danger"
onClick={() => {
Expand Down
19 changes: 19 additions & 0 deletions x-pack/plugins/uptime/public/components/overview/alerts/monitor_expressions/translations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ export const alertFilterLabels = {
ANY_LOCATION: i18n.translate('xpack.uptime.alerts.monitorStatus.filters.anyLocation', {
defaultMessage: 'any location',
}),

REMOVE_FILTER_LABEL: i18n.translate('xpack.uptime.alerts.monitorExpression.label', {
defaultMessage: 'Remove filter',
}),
};

export const statusExpLabels = {
Expand Down Expand Up @@ -82,3 +86,18 @@ export const timeExpLabels = {
}
),
};

export const filterAriaLabels = {
PORT: i18n.translate('xpack.uptime.alerts.monitorStatus.filters.port.label', {
defaultMessage: `Select port filters to apply to the alert's query.`,
}),
TAG: i18n.translate('xpack.uptime.alerts.monitorStatus.filters.tag.label', {
defaultMessage: `Select tag filters to apply to the alert's query.`,
}),
SCHEME: i18n.translate('xpack.uptime.alerts.monitorStatus.filters.scheme.label', {
defaultMessage: `Select protocol scheme filters to apply to the alert's query.`,
}),
LOCATION: i18n.translate('xpack.uptime.alerts.monitorStatus.filters.location.label', {
defaultMessage: `Select location filters to apply to the alert's query.`,
}),
};
Loading