[APM] UI filters: Change transaction type selector from dropdown to r…

…adio 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 <[email protected]>
elastic · Aug 25, 2020 · 8f22849 · 8f22849
1 parent ed228bc
commit 8f22849
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 44 deletions.
diff --git a/...iew/__jest__/TransactionOverview.test.tsx → ...tionOverview/TransactionOverview.test.tsx b/...iew/__jest__/TransactionOverview.test.tsx → ...tionOverview/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
@@ -121,7 +121,7 @@ export function TransactionOverview() {
         <EuiFlexItem grow={1}>
           <LocalUIFilters {...localFiltersConfig}>
             <TransactionTypeFilter transactionTypes={serviceTransactionTypes} />
-            <EuiSpacer size="xl" />
+            <EuiSpacer size="m" />
             <EuiHorizontalRule margin="none" />
           </LocalUIFilters>
         </EuiFlexItem>

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
@@ -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 (
     <EuiFlexGrid gutterSize="s">
       {value.map((val) => (
         <EuiFlexItem key={val} grow={false}>
-          <button
-            type="button"
+          <EuiBadge
+            color="hollow"
             onClick={() => {
               onRemove(val);
             }}
+            onClickAriaLabel={removeFilterLabel}
+            iconOnClick={() => {
+              onRemove(val);
+            }}
+            iconOnClickAriaLabel={removeFilterLabel}
+            iconType="cross"
+            iconSide="right"
           >
-            <EuiBadge color="hollow">
-              <BadgeText>{val}</BadgeText>
-              <EuiIcon type="cross" />
-            </EuiBadge>
-          </button>
+            <BadgeText>{val}</BadgeText>
+          </EuiBadge>
         </EuiFlexItem>
       ))}
     </EuiFlexGrid>

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
@@ -164,7 +164,7 @@ function Filter({ name, title, options, onChange, value, showCount }: Props) {
             }}
             value={value}
           />
-          <EuiSpacer size="s" />
+          <EuiSpacer size="m" />
         </>
       ) : 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
@@ -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) {
       <EuiSpacer size="s" />
       <EuiHorizontalRule margin="none" />
       <EuiSpacer size="s" />
-      <EuiSelect
+      <EuiRadioGroup
         options={options}
-        value={transactionType}
-        compressed={true}
-        onChange={(event) => {
+        idSelected={transactionType}
+        onChange={(selectedTransactionType) => {
           const newLocation = {
             ...history.location,
             search: fromQuery({
               ...toQuery(history.location.search),
-              transactionType: event.target.value,
+              transactionType: selectedTransactionType,
             }),
           };
           history.push(newLocation);