Skip to content

Commit

Permalink
[ObsUX][Infra] Fix loading spinner and tooltip for host from APM (#19…
Browse files Browse the repository at this point in the history
…1908)

Closes #191579

### Summary

When working on finding the root cause of the bug, another bug related
was discovered on the host detected by APM tooltip, it was introduced by
this [PR](#191104) when we check
for `metadataLoading` to finish, as we don’t do it for the asset details
page the loading spinner wasn’t shown there when refreshing the page,
but the tooltip shows because is still waiting for the metadata request
to finish in order to retrieve the integration.
To fix this, we don't apply loading spinner to the asset name, but we do
it for the tooltip

BEFORE


https://github.com/user-attachments/assets/3a946e84-1256-440c-bb65-56df30d8d3a5

AFTER


https://github.com/user-attachments/assets/689a6583-c66d-4e96-8605-9cce6c9aff24
  • Loading branch information
MiriamAparicio authored Sep 3, 2024
1 parent 7c29676 commit 051f04c
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 72 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,17 @@ import {
import { css } from '@emotion/react';
import type { InventoryItemType } from '@kbn/metrics-data-access-plugin/common/inventory_models/types';
import { PageTitleWithPopover } from './page_title_with_popover';

type Props = Pick<EuiPageHeaderProps, 'tabs' | 'title' | 'rightSideItems'> & {
hasSystemIntegration: boolean;
assetType: InventoryItemType;
metadataLoading: boolean;
loading: boolean;
};

export const FlyoutHeader = ({
title,
tabs = [],
rightSideItems = [],
hasSystemIntegration,
assetType,
metadataLoading,
loading,
}: Props) => {
const { euiTheme } = useEuiTheme();
Expand All @@ -55,19 +52,10 @@ export const FlyoutHeader = ({
`}
>
<EuiTitle size="xs">
{metadataLoading || loading ? (
{loading ? (
<EuiLoadingSpinner size="m" />
) : (
<h4>
{assetType === 'host' ? (
<PageTitleWithPopover
hasSystemMetrics={hasSystemIntegration}
name={title ?? ''}
/>
) : (
title
)}
</h4>
<h4>{assetType === 'host' ? <PageTitleWithPopover name={title ?? ''} /> : title}</h4>
)}
</EuiTitle>
</EuiFlexItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,61 +6,65 @@
*/
import React from 'react';

import { EuiText, EuiLink, EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
import { EuiText, EuiLink, EuiFlexGroup, EuiFlexItem, EuiLoadingSpinner } from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n-react';
import { APM_HOST_TROUBLESHOOTING_LINK } from '../constants';
import { Popover } from '../tabs/common/popover';
import { useMetadataStateContext } from '../hooks/use_metadata_state';

export const PageTitleWithPopover = ({
hasSystemMetrics,
name,
}: {
hasSystemMetrics: boolean;
name: string;
}) => {
return !hasSystemMetrics ? (
export const PageTitleWithPopover = ({ name }: { name: string }) => {
const { metadata, loading } = useMetadataStateContext();

return (
<EuiFlexGroup gutterSize="xs" alignItems="center" responsive={false}>
<EuiFlexItem grow={false}>{name}</EuiFlexItem>
<EuiFlexItem grow={false}>
<Popover icon="questionInCircle" data-test-subj="assetDetailsTitleHasSystemMetricsPopover">
<EuiText size="xs">
<p>
<FormattedMessage
id="xpack.infra.assetDetails.title.tooltip.apmHostMessage"
defaultMessage="This host has been detected by {apm}"
values={{
apm: (
<EuiLink
data-test-subj="assetDetailsTitleTooltipApmDocumentationLink"
href=" https://www.elastic.co/guide/en/observability/current/apm.html"
target="_blank"
>
<FormattedMessage
id="xpack.infra.assetDetails.title.tooltip.apmHostMessage.apmDocumentationLink"
defaultMessage="APM"
/>
</EuiLink>
),
}}
/>
</p>
<p>
<EuiLink
data-test-subj="assetDetailsTitleHasSystemMetricsLearnMoreLink"
href={APM_HOST_TROUBLESHOOTING_LINK}
target="_blank"
>
<FormattedMessage
id="xpack.infra.assetDetails.title.tooltip.learnMoreLink"
defaultMessage="Learn more"
/>
</EuiLink>
</p>
</EuiText>
</Popover>
</EuiFlexItem>
{loading ? (
<EuiLoadingSpinner size="m" />
) : (
!metadata?.hasSystemIntegration && (
<EuiFlexItem grow={false}>
<Popover
icon="questionInCircle"
data-test-subj="assetDetailsTitleHasSystemMetricsPopover"
>
<EuiText size="xs">
<p>
<FormattedMessage
id="xpack.infra.assetDetails.title.tooltip.apmHostMessage"
defaultMessage="This host has been detected by {apm}"
values={{
apm: (
<EuiLink
data-test-subj="assetDetailsTitleTooltipApmDocumentationLink"
href=" https://www.elastic.co/guide/en/observability/current/apm.html"
target="_blank"
>
<FormattedMessage
id="xpack.infra.assetDetails.title.tooltip.apmHostMessage.apmDocumentationLink"
defaultMessage="APM"
/>
</EuiLink>
),
}}
/>
</p>
<p>
<EuiLink
data-test-subj="assetDetailsTitleHasSystemMetricsLearnMoreLink"
href={APM_HOST_TROUBLESHOOTING_LINK}
target="_blank"
>
<FormattedMessage
id="xpack.infra.assetDetails.title.tooltip.learnMoreLink"
defaultMessage="Learn more"
/>
</EuiLink>
</p>
</EuiText>
</Popover>
</EuiFlexItem>
)
)}
</EuiFlexGroup>
) : (
<>{name}</>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import { useAssetDetailsUrlState } from '../hooks/use_asset_details_url_state';
import { usePageHeader } from '../hooks/use_page_header';
import { useTabSwitcherContext } from '../hooks/use_tab_switcher';
import type { ContentTemplateProps } from '../types';
import { useMetadataStateContext } from '../hooks/use_metadata_state';

export const Flyout = ({
tabs = [],
Expand All @@ -31,7 +30,6 @@ export const Flyout = ({
const {
services: { telemetry },
} = useKibanaContextForPlugin();
const { metadata, loading: metadataLoading } = useMetadataStateContext();

useEffectOnce(() => {
telemetry.reportAssetDetailsFlyoutViewed({
Expand Down Expand Up @@ -59,9 +57,7 @@ export const Flyout = ({
title={asset.name}
tabs={tabEntries}
rightSideItems={rightSideItems}
hasSystemIntegration={!!metadata?.hasSystemIntegration}
assetType={asset.type}
metadataLoading={metadataLoading}
loading={loading}
/>
</EuiFlyoutHeader>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,7 @@ export const Page = ({ tabs = [], links = [] }: ContentTemplateProps) => {
pageTitle: loading ? (
<EuiLoadingSpinner size="m" />
) : asset.type === 'host' ? (
<PageTitleWithPopover
hasSystemMetrics={!!metadata?.hasSystemIntegration}
name={asset.name}
/>
<PageTitleWithPopover name={asset.name} />
) : (
asset.name
),
Expand Down

0 comments on commit 051f04c

Please sign in to comment.