From e59536adbea365fd622ac0e060a05e84b100453d Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Thu, 20 Aug 2020 16:19:37 +0200 Subject: [PATCH 1/6] changing transaction type filter to radio group --- .../TransactionTypeFilter/index.tsx | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/apm/public/components/shared/LocalUIFilters/TransactionTypeFilter/index.tsx b/x-pack/plugins/apm/public/components/shared/LocalUIFilters/TransactionTypeFilter/index.tsx index afd2d023d16ba..54a08e9d45af5 100644 --- a/x-pack/plugins/apm/public/components/shared/LocalUIFilters/TransactionTypeFilter/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/LocalUIFilters/TransactionTypeFilter/index.tsx @@ -9,7 +9,7 @@ import { EuiTitle, EuiHorizontalRule, EuiSpacer, - EuiSelect, + EuiRadioGroup, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { useUrlParams } from '../../../../hooks/useUrlParams'; @@ -26,8 +26,8 @@ function TransactionTypeFilter({ transactionTypes }: Props) { } = useUrlParams(); const options = transactionTypes.map((type) => ({ - text: type, - value: type, + id: type, + label: type, })); return ( @@ -42,16 +42,15 @@ function TransactionTypeFilter({ transactionTypes }: Props) { - { + idSelected={transactionType} + onChange={(selectedTransactionType) => { const newLocation = { ...history.location, search: fromQuery({ ...toQuery(history.location.search), - transactionType: event.target.value, + transactionType: selectedTransactionType, }), }; history.push(newLocation); From e9affb7b455b713381f17262aac6daa8d25dfcb6 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Fri, 21 Aug 2020 10:30:28 +0200 Subject: [PATCH 2/6] fixing unit test --- .../__jest__/TransactionOverview.test.tsx | 40 ++++++++----------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/TransactionOverview.test.tsx b/x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/TransactionOverview.test.tsx index 9c514e429c374..b8547eaafaa8c 100644 --- a/x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/TransactionOverview.test.tsx +++ b/x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/TransactionOverview.test.tsx @@ -4,25 +4,23 @@ * you may not use this file except in compliance with the Elastic License. */ -import React from 'react'; import { + fireEvent, + getByText, queryByLabelText, render, - getByText, - getByDisplayValue, - queryByDisplayValue, - fireEvent, } from '@testing-library/react'; import { omit } from 'lodash'; -import { history } from '../../../../utils/history'; +import React from 'react'; +import { Router } from 'react-router-dom'; import { TransactionOverview } from '..'; +import { MockApmPluginContextWrapper } from '../../../../context/ApmPluginContext/MockApmPluginContext'; +import { UrlParamsProvider } from '../../../../context/UrlParamsContext'; import { IUrlParams } from '../../../../context/UrlParamsContext/types'; -import * as useServiceTransactionTypesHook from '../../../../hooks/useServiceTransactionTypes'; import * as useFetcherHook from '../../../../hooks/useFetcher'; +import * as useServiceTransactionTypesHook from '../../../../hooks/useServiceTransactionTypes'; +import { history } from '../../../../utils/history'; import { fromQuery } from '../../../shared/Links/url_helpers'; -import { Router } from 'react-router-dom'; -import { UrlParamsProvider } from '../../../../context/UrlParamsContext'; -import { MockApmPluginContextWrapper } from '../../../../context/ApmPluginContext/MockApmPluginContext'; jest.spyOn(history, 'push'); jest.spyOn(history, 'replace'); @@ -85,7 +83,7 @@ describe('TransactionOverview', () => { const FILTER_BY_TYPE_LABEL = 'Transaction type'; describe('when transactionType is selected and multiple transaction types are given', () => { - it('should render dropdown with transaction types', () => { + it('should render radio group with transaction types', () => { const { container } = setup({ serviceTransactionTypes: ['firstType', 'secondType'], urlParams: { @@ -94,9 +92,8 @@ describe('TransactionOverview', () => { }, }); - // secondType is selected in the dropdown - expect(queryByDisplayValue(container, 'secondType')).not.toBeNull(); - expect(queryByDisplayValue(container, 'firstType')).toBeNull(); + expect(getByText(container, 'firstType')).toBeInTheDocument(); + expect(getByText(container, 'secondType')).toBeInTheDocument(); expect(getByText(container, 'firstType')).not.toBeNull(); }); @@ -110,22 +107,19 @@ describe('TransactionOverview', () => { }, }); - expect(queryByDisplayValue(container, 'firstType')).toBeNull(); + expect(history.location.search).toEqual('?transactionType=secondType'); + expect(getByText(container, 'firstType')).toBeInTheDocument(); + expect(getByText(container, 'secondType')).toBeInTheDocument(); - fireEvent.change(getByDisplayValue(container, 'secondType'), { - target: { value: 'firstType' }, - }); + fireEvent.click(getByText(container, 'firstType')); expect(history.push).toHaveBeenCalled(); - - getByDisplayValue(container, 'firstType'); - - expect(queryByDisplayValue(container, 'firstType')).not.toBeNull(); + expect(history.location.search).toEqual('?transactionType=firstType'); }); }); describe('when a transaction type is selected, and there are no other transaction types', () => { - it('should not render a dropdown with transaction types', () => { + it('should not render radio group with transaction types', () => { const { container } = setup({ serviceTransactionTypes: ['firstType'], urlParams: { From 7a672d7eba722a11c9b514a4eb435458ed0d88a7 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Fri, 21 Aug 2020 11:28:39 +0200 Subject: [PATCH 3/6] changing transaction type filter to radio group --- .../apm/public/components/app/TransactionOverview/index.tsx | 2 +- .../public/components/shared/LocalUIFilters/Filter/index.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/apm/public/components/app/TransactionOverview/index.tsx b/x-pack/plugins/apm/public/components/app/TransactionOverview/index.tsx index d9bd3e59d281f..f6eb131a8a733 100644 --- a/x-pack/plugins/apm/public/components/app/TransactionOverview/index.tsx +++ b/x-pack/plugins/apm/public/components/app/TransactionOverview/index.tsx @@ -121,7 +121,7 @@ export function TransactionOverview() { - + diff --git a/x-pack/plugins/apm/public/components/shared/LocalUIFilters/Filter/index.tsx b/x-pack/plugins/apm/public/components/shared/LocalUIFilters/Filter/index.tsx index c13439a3c5928..48ebc2add0053 100644 --- a/x-pack/plugins/apm/public/components/shared/LocalUIFilters/Filter/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/LocalUIFilters/Filter/index.tsx @@ -164,7 +164,7 @@ function Filter({ name, title, options, onChange, value, showCount }: Props) { }} value={value} /> - + ) : null} From 8e113d1521da4477dc291244ea5bf86d52cda840 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Mon, 24 Aug 2020 11:44:31 +0200 Subject: [PATCH 4/6] adding onclick to the badge component --- .../LocalUIFilters/Filter/FilterBadgeList.tsx | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/apm/public/components/shared/LocalUIFilters/Filter/FilterBadgeList.tsx b/x-pack/plugins/apm/public/components/shared/LocalUIFilters/Filter/FilterBadgeList.tsx index 2090a92bf0de4..6cf0e89c20501 100644 --- a/x-pack/plugins/apm/public/components/shared/LocalUIFilters/Filter/FilterBadgeList.tsx +++ b/x-pack/plugins/apm/public/components/shared/LocalUIFilters/Filter/FilterBadgeList.tsx @@ -25,17 +25,16 @@ function FilterBadgeList({ onRemove, value }: Props) { {value.map((val) => ( - + {val} + + ))} From b71350e5f2f043694e442ea5c4f3a34de646404f Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Mon, 24 Aug 2020 13:50:02 +0200 Subject: [PATCH 5/6] adding onclick to the badge component --- .../shared/LocalUIFilters/Filter/FilterBadgeList.tsx | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/apm/public/components/shared/LocalUIFilters/Filter/FilterBadgeList.tsx b/x-pack/plugins/apm/public/components/shared/LocalUIFilters/Filter/FilterBadgeList.tsx index 6cf0e89c20501..4fd63e0a0d253 100644 --- a/x-pack/plugins/apm/public/components/shared/LocalUIFilters/Filter/FilterBadgeList.tsx +++ b/x-pack/plugins/apm/public/components/shared/LocalUIFilters/Filter/FilterBadgeList.tsx @@ -5,7 +5,7 @@ */ import React from 'react'; -import { EuiFlexGrid, EuiFlexItem, EuiBadge, EuiIcon } from '@elastic/eui'; +import { EuiFlexGrid, EuiFlexItem, EuiBadge } from '@elastic/eui'; import styled from 'styled-components'; import { unit, px, truncate } from '../../../../style/variables'; @@ -31,9 +31,14 @@ function FilterBadgeList({ onRemove, value }: Props) { onRemove(val); }} onClickAriaLabel="Remove filter" + iconOnClick={() => { + onRemove(val); + }} + iconOnClickAriaLabel="Remove filter" + iconType="cross" + iconSide="right" > {val} - ))} From 66552320ad878cb950f27db8a488534fbdb15adf Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Tue, 25 Aug 2020 09:15:51 +0200 Subject: [PATCH 6/6] adding i18n to aria --- .../TransactionOverview.test.tsx | 20 +++++++++---------- .../LocalUIFilters/Filter/FilterBadgeList.tsx | 10 ++++++++-- 2 files changed, 18 insertions(+), 12 deletions(-) rename x-pack/plugins/apm/public/components/app/TransactionOverview/{__jest__ => }/TransactionOverview.test.tsx (83%) diff --git a/x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/TransactionOverview.test.tsx b/x-pack/plugins/apm/public/components/app/TransactionOverview/TransactionOverview.test.tsx similarity index 83% rename from x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/TransactionOverview.test.tsx rename to x-pack/plugins/apm/public/components/app/TransactionOverview/TransactionOverview.test.tsx index b8547eaafaa8c..28030dd509835 100644 --- a/x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/TransactionOverview.test.tsx +++ b/x-pack/plugins/apm/public/components/app/TransactionOverview/TransactionOverview.test.tsx @@ -13,14 +13,14 @@ import { import { omit } from 'lodash'; import React from 'react'; import { Router } from 'react-router-dom'; -import { TransactionOverview } from '..'; -import { MockApmPluginContextWrapper } from '../../../../context/ApmPluginContext/MockApmPluginContext'; -import { UrlParamsProvider } from '../../../../context/UrlParamsContext'; -import { IUrlParams } from '../../../../context/UrlParamsContext/types'; -import * as useFetcherHook from '../../../../hooks/useFetcher'; -import * as useServiceTransactionTypesHook from '../../../../hooks/useServiceTransactionTypes'; -import { history } from '../../../../utils/history'; -import { fromQuery } from '../../../shared/Links/url_helpers'; +import { TransactionOverview } from './'; +import { MockApmPluginContextWrapper } from '../../../context/ApmPluginContext/MockApmPluginContext'; +import { UrlParamsProvider } from '../../../context/UrlParamsContext'; +import { IUrlParams } from '../../../context/UrlParamsContext/types'; +import * as useFetcherHook from '../../../hooks/useFetcher'; +import * as useServiceTransactionTypesHook from '../../../hooks/useServiceTransactionTypes'; +import { history } from '../../../utils/history'; +import { fromQuery } from '../../shared/Links/url_helpers'; jest.spyOn(history, 'push'); jest.spyOn(history, 'replace'); @@ -83,7 +83,7 @@ describe('TransactionOverview', () => { const FILTER_BY_TYPE_LABEL = 'Transaction type'; describe('when transactionType is selected and multiple transaction types are given', () => { - it('should render radio group with transaction types', () => { + it('renders a radio group with transaction types', () => { const { container } = setup({ serviceTransactionTypes: ['firstType', 'secondType'], urlParams: { @@ -119,7 +119,7 @@ describe('TransactionOverview', () => { }); describe('when a transaction type is selected, and there are no other transaction types', () => { - it('should not render radio group with transaction types', () => { + it('does not render a radio group with transaction types', () => { const { container } = setup({ serviceTransactionTypes: ['firstType'], urlParams: { diff --git a/x-pack/plugins/apm/public/components/shared/LocalUIFilters/Filter/FilterBadgeList.tsx b/x-pack/plugins/apm/public/components/shared/LocalUIFilters/Filter/FilterBadgeList.tsx index 4fd63e0a0d253..ed8d865d2d288 100644 --- a/x-pack/plugins/apm/public/components/shared/LocalUIFilters/Filter/FilterBadgeList.tsx +++ b/x-pack/plugins/apm/public/components/shared/LocalUIFilters/Filter/FilterBadgeList.tsx @@ -7,6 +7,7 @@ import React from 'react'; import { EuiFlexGrid, EuiFlexItem, EuiBadge } from '@elastic/eui'; import styled from 'styled-components'; +import { i18n } from '@kbn/i18n'; import { unit, px, truncate } from '../../../../style/variables'; const BadgeText = styled.div` @@ -20,6 +21,11 @@ interface Props { onRemove: (val: string) => void; } +const removeFilterLabel = i18n.translate( + 'xpack.apm.uifilter.badge.removeFilter', + { defaultMessage: 'Remove filter' } +); + function FilterBadgeList({ onRemove, value }: Props) { return ( @@ -30,11 +36,11 @@ function FilterBadgeList({ onRemove, value }: Props) { onClick={() => { onRemove(val); }} - onClickAriaLabel="Remove filter" + onClickAriaLabel={removeFilterLabel} iconOnClick={() => { onRemove(val); }} - iconOnClickAriaLabel="Remove filter" + iconOnClickAriaLabel={removeFilterLabel} iconType="cross" iconSide="right" >