Skip to content

Commit

Permalink
addressing pr comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@cauemarcondes
cauemarcondes committed Dec 16, 2020
1 parent 588c76d commit 1e7c498
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 88 deletions.
30 changes: 12 additions & 18 deletions x-pack/plugins/apm/public/components/app/service_details/service_icons/icon_popover.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,34 +10,31 @@ import {
EuiPopover,
EuiPopoverTitle,
} from '@elastic/eui';
import React, { useState } from 'react';
import React from 'react';
import { FETCH_STATUS } from '../../../../hooks/use_fetcher';
import { px } from '../../../../style/variables';

interface IconPopoverProps {
icon: string;
title: string;
children: React.ReactChild;
onClick: (isOpen: boolean) => void;
onClick: () => void;
onClose: () => void;
detailsFetchStatus: FETCH_STATUS;
isOpen: boolean;
icon?: string;
}
export function IconPopover({
icon,
title,
children,
onClick,
onClose,
detailsFetchStatus,
isOpen,
}: IconPopoverProps) {
const [isOpen, setIsOpen] = useState(false);

const togglePopover = () => {
setIsOpen((prevState) => {
const nextState = !prevState;
onClick(nextState);
return nextState;
});
};

if (!icon) {
return null;
}
const isLoading =
detailsFetchStatus === FETCH_STATUS.LOADING ||
detailsFetchStatus === FETCH_STATUS.PENDING;
Expand All @@ -47,15 +44,12 @@ export function IconPopover({
anchorPosition="downCenter"
ownFocus={false}
button={
<EuiButtonEmpty
onClick={togglePopover}
data-test-subj={`popover_${title}`}
>
<EuiButtonEmpty onClick={onClick} data-test-subj={`popover_${title}`}>
<EuiIcon type={icon} size="l" color="black" />
</EuiButtonEmpty>
}
isOpen={isOpen}
closePopover={togglePopover}
closePopover={onClose}
>
<EuiPopoverTitle>{title}</EuiPopoverTitle>
<div style={{ minWidth: px(300) }}>
Expand Down
42 changes: 21 additions & 21 deletions x-pack/plugins/apm/public/components/app/service_details/service_icons/index.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ describe('ServiceIcons', () => {
</Wrapper>
);
expect(getByTestId('loading')).toBeInTheDocument();
expect(queryAllByTestId('java')).toHaveLength(0);
expect(queryAllByTestId('Kubernetes')).toHaveLength(0);
expect(queryAllByTestId('gcp')).toHaveLength(0);
expect(queryAllByTestId('service')).toHaveLength(0);
expect(queryAllByTestId('container')).toHaveLength(0);
expect(queryAllByTestId('cloud')).toHaveLength(0);
});
it("doesn't show any icons", () => {
jest.spyOn(fetcherHook, 'useFetcher').mockReturnValue({
Expand All @@ -80,9 +80,9 @@ describe('ServiceIcons', () => {
</Wrapper>
);
expect(queryAllByTestId('loading')).toHaveLength(0);
expect(queryAllByTestId('java')).toHaveLength(0);
expect(queryAllByTestId('Kubernetes')).toHaveLength(0);
expect(queryAllByTestId('gcp')).toHaveLength(0);
expect(queryAllByTestId('service')).toHaveLength(0);
expect(queryAllByTestId('container')).toHaveLength(0);
expect(queryAllByTestId('cloud')).toHaveLength(0);
});
it('shows service icon', () => {
jest.spyOn(fetcherHook, 'useFetcher').mockReturnValue({
Expand All @@ -99,9 +99,9 @@ describe('ServiceIcons', () => {
</Wrapper>
);
expect(queryAllByTestId('loading')).toHaveLength(0);
expect(getByTestId('java')).toBeInTheDocument();
expect(queryAllByTestId('Kubernetes')).toHaveLength(0);
expect(queryAllByTestId('gcp')).toHaveLength(0);
expect(getByTestId('service')).toBeInTheDocument();
expect(queryAllByTestId('container')).toHaveLength(0);
expect(queryAllByTestId('cloud')).toHaveLength(0);
});
it('shows service and container icons', () => {
jest.spyOn(fetcherHook, 'useFetcher').mockReturnValue({
Expand All @@ -119,9 +119,9 @@ describe('ServiceIcons', () => {
</Wrapper>
);
expect(queryAllByTestId('loading')).toHaveLength(0);
expect(queryAllByTestId('gcp')).toHaveLength(0);
expect(getByTestId('java')).toBeInTheDocument();
expect(getByTestId('Kubernetes')).toBeInTheDocument();
expect(queryAllByTestId('cloud')).toHaveLength(0);
expect(getByTestId('service')).toBeInTheDocument();
expect(getByTestId('container')).toBeInTheDocument();
});
it('shows service, container and cloud icons', () => {
jest.spyOn(fetcherHook, 'useFetcher').mockReturnValue({
Expand All @@ -140,9 +140,9 @@ describe('ServiceIcons', () => {
</Wrapper>
);
expect(queryAllByTestId('loading')).toHaveLength(0);
expect(getByTestId('java')).toBeInTheDocument();
expect(getByTestId('Kubernetes')).toBeInTheDocument();
expect(getByTestId('gcp')).toBeInTheDocument();
expect(getByTestId('service')).toBeInTheDocument();
expect(getByTestId('container')).toBeInTheDocument();
expect(getByTestId('cloud')).toBeInTheDocument();
});
});

Expand Down Expand Up @@ -183,9 +183,9 @@ describe('ServiceIcons', () => {
</Wrapper>
);
expect(queryAllByTestId('loading')).toHaveLength(0);
expect(getByTestId('java')).toBeInTheDocument();
expect(getByTestId('Kubernetes')).toBeInTheDocument();
expect(getByTestId('gcp')).toBeInTheDocument();
expect(getByTestId('service')).toBeInTheDocument();
expect(getByTestId('container')).toBeInTheDocument();
expect(getByTestId('cloud')).toBeInTheDocument();
fireEvent.click(getByTestId('popover_Service'));
expect(getByTestId('loading-content')).toBeInTheDocument();
});
Expand Down Expand Up @@ -219,9 +219,9 @@ describe('ServiceIcons', () => {
</Wrapper>
);
expect(queryAllByTestId('loading')).toHaveLength(0);
expect(getByTestId('java')).toBeInTheDocument();
expect(getByTestId('Kubernetes')).toBeInTheDocument();
expect(getByTestId('gcp')).toBeInTheDocument();
expect(getByTestId('service')).toBeInTheDocument();
expect(getByTestId('container')).toBeInTheDocument();
expect(getByTestId('cloud')).toBeInTheDocument();

fireEvent.click(getByTestId('popover_Service'));
expect(queryAllByTestId('loading-content')).toHaveLength(0);
Expand Down
117 changes: 68 additions & 49 deletions x-pack/plugins/apm/public/components/app/service_details/service_icons/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import { EuiFlexGroup, EuiFlexItem, EuiLoadingSpinner } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import React, { useState } from 'react';
import React, { ReactChild, useState } from 'react';
import { ContainerType } from '../../../../../common/service_metadata';
import { useUrlParams } from '../../../../context/url_params_context/use_url_params';
import { FETCH_STATUS, useFetcher } from '../../../../hooks/use_fetcher';
Expand Down Expand Up @@ -44,12 +44,24 @@ function getContainerIcon(container?: ContainerType) {
}
}

type Icons = 'service' | 'container' | 'cloud';
interface PopoverItem {
key: Icons;
icon?: string;
isVisible: boolean;
title: string;
component: ReactChild;
}

export function ServiceIcons({ serviceName }: Props) {
const {
urlParams: { start, end },
uiFilters,
} = useUrlParams();
const [isPopoverOpen, setIsPopoverOpen] = useState(false);
const [
selectedIconPopover,
setSelectedIconPopover,
] = useState<Icons | null>();

const { data: icons, status: iconsFetchStatus } = useFetcher(
(callApmApi) => {
Expand All @@ -68,7 +80,7 @@ export function ServiceIcons({ serviceName }: Props) {

const { data: details, status: detailsFetchStatus } = useFetcher(
(callApmApi) => {
if (isPopoverOpen && serviceName && start && end) {
if (selectedIconPopover && serviceName && start && end) {
return callApmApi({
endpoint: 'GET /api/apm/services/{serviceName}/metadata/details',
params: {
Expand All @@ -78,12 +90,9 @@ export function ServiceIcons({ serviceName }: Props) {
});
}
},
[isPopoverOpen, serviceName, start, end, uiFilters]
[selectedIconPopover, serviceName, start, end, uiFilters]
);

const cloudIcon = getCloudIcon(icons?.cloudProvider);
const containerIcon = getContainerIcon(icons?.containerType);

const isLoading =
!icons &&
(iconsFetchStatus === FETCH_STATUS.LOADING ||
Expand All @@ -93,50 +102,60 @@ export function ServiceIcons({ serviceName }: Props) {
return <EuiLoadingSpinner data-test-subj="loading" />;
}

const popoverItems: PopoverItem[] = [
{
key: 'service',
icon: getAgentIcon(icons?.agentName) || 'node',
isVisible: !!icons?.agentName,
title: i18n.translate('xpack.apm.serviceIcons.service', {
defaultMessage: 'Service',
}),
component: <ServiceDetails service={details?.service} />,
},
{
key: 'container',
icon: getContainerIcon(icons?.containerType),
isVisible: !!icons?.containerType,
title: i18n.translate('xpack.apm.serviceIcons.container', {
defaultMessage: 'Container',
}),
component: <ContainerDetails container={details?.container} />,
},
{
key: 'cloud',
icon: getCloudIcon(icons?.cloudProvider),
isVisible: !!icons?.cloudProvider,
title: i18n.translate('xpack.apm.serviceIcons.cloud', {
defaultMessage: 'Cloud',
}),
component: <CloudDetails cloud={details?.cloud} />,
},
];

return (
<EuiFlexGroup gutterSize="s" responsive={false}>
{icons?.agentName && (
<EuiFlexItem grow={false} data-test-subj={icons.agentName}>
<IconPopover
detailsFetchStatus={detailsFetchStatus}
onClick={setIsPopoverOpen}
icon={getAgentIcon(icons.agentName) || 'node'}
title={i18n.translate('xpack.apm.serviceIcons.service', {
defaultMessage: 'Service',
})}
>
<ServiceDetails service={details?.service} />
</IconPopover>
</EuiFlexItem>
)}
{containerIcon && (
<EuiFlexItem grow={false} data-test-subj={icons?.containerType}>
<IconPopover
detailsFetchStatus={detailsFetchStatus}
onClick={setIsPopoverOpen}
icon={containerIcon}
title={i18n.translate('xpack.apm.serviceIcons.container', {
defaultMessage: 'Container',
})}
>
<ContainerDetails container={details?.container} />
</IconPopover>
</EuiFlexItem>
)}
{cloudIcon && (
<EuiFlexItem grow={false} data-test-subj={icons?.cloudProvider}>
<IconPopover
detailsFetchStatus={detailsFetchStatus}
onClick={setIsPopoverOpen}
icon={cloudIcon}
title={i18n.translate('xpack.apm.serviceIcons.cloud', {
defaultMessage: 'Cloud',
})}
>
<CloudDetails cloud={details?.cloud} />
</IconPopover>
</EuiFlexItem>
)}
{popoverItems.map((item) => {
if (item.isVisible) {
return (
<EuiFlexItem grow={false} data-test-subj={item.key} key={item.key}>
<IconPopover
isOpen={selectedIconPopover === item.key}
icon={item.icon}
detailsFetchStatus={detailsFetchStatus}
title={item.title}
onClick={() => {
setSelectedIconPopover(item.key);
}}
onClose={() => {
setSelectedIconPopover(null);
}}
>
{item.component}
</IconPopover>
</EuiFlexItem>
);
}
})}
</EuiFlexGroup>
);
}

0 comments on commit 1e7c498

Please sign in to comment.