From 8f22849ccdc1354624de6c7b4152d168197f03a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cau=C3=AA=20Marcondes?= <55978943+cauemarcondes@users.noreply.github.com> Date: Tue, 25 Aug 2020 15:15:36 +0100 Subject: [PATCH] [APM] UI filters: Change transaction type selector from dropdown to radio buttons (#75625) * changing transaction type filter to radio group * fixing unit test * changing transaction type filter to radio group * adding onclick to the badge component * adding onclick to the badge component * adding i18n to aria Co-authored-by: Elastic Machine --- .../TransactionOverview.test.tsx | 46 ++++++++----------- .../app/TransactionOverview/index.tsx | 2 +- .../LocalUIFilters/Filter/FilterBadgeList.tsx | 26 +++++++---- .../shared/LocalUIFilters/Filter/index.tsx | 2 +- .../TransactionTypeFilter/index.tsx | 15 +++--- 5 files changed, 47 insertions(+), 44 deletions(-) rename x-pack/plugins/apm/public/components/app/TransactionOverview/{__jest__ => }/TransactionOverview.test.tsx (70%) 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 70% 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 9c514e429c374..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 @@ -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 { TransactionOverview } from '..'; -import { IUrlParams } from '../../../../context/UrlParamsContext/types'; -import * as useServiceTransactionTypesHook from '../../../../hooks/useServiceTransactionTypes'; -import * as useFetcherHook from '../../../../hooks/useFetcher'; -import { fromQuery } from '../../../shared/Links/url_helpers'; +import React from 'react'; import { Router } from 'react-router-dom'; -import { UrlParamsProvider } from '../../../../context/UrlParamsContext'; -import { MockApmPluginContextWrapper } from '../../../../context/ApmPluginContext/MockApmPluginContext'; +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'); @@ -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('renders a 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('does not render a radio group with transaction types', () => { const { container } = setup({ serviceTransactionTypes: ['firstType'], urlParams: { 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/FilterBadgeList.tsx b/x-pack/plugins/apm/public/components/shared/LocalUIFilters/Filter/FilterBadgeList.tsx index 2090a92bf0de4..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 @@ -5,8 +5,9 @@ */ 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 { i18n } from '@kbn/i18n'; import { unit, px, truncate } from '../../../../style/variables'; const BadgeText = styled.div` @@ -20,22 +21,31 @@ interface Props { onRemove: (val: string) => void; } +const removeFilterLabel = i18n.translate( + 'xpack.apm.uifilter.badge.removeFilter', + { defaultMessage: 'Remove filter' } +); + function FilterBadgeList({ onRemove, value }: Props) { return ( {value.map((val) => ( - + {val} + ))} 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} 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);