Skip to content

feat: Split tooltip for adjacent traces in two #104150

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+30 −15
Open

feat: Split tooltip for adjacent traces in two #104150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .../app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {useMemo} from 'react';

import {LinkButton} from '@sentry/scraps/button/linkButton';
import type {TooltipProps} from '@sentry/scraps/tooltip';

import {normalizeDateTimeParams} from 'sentry/components/organizations/pageFilters/parse';
import {IconChevron} from 'sentry/icons';
Expand All @@ -20,11 +21,13 @@ type TraceLinkNavigationButtonProps = {
attributes: TraceItemResponseAttribute[];
currentTraceStartTimestamp: number;
direction: ConnectedTraceConnection;
tooltipProps: TooltipProps;
};

export function TraceLinkNavigationButton({
direction,
attributes,
tooltipProps,
currentTraceStartTimestamp,
}: TraceLinkNavigationButtonProps) {
const organization = useOrganization();
Expand Down Expand Up @@ -89,6 +92,8 @@ export function TraceLinkNavigationButton({
size="xs"
icon={<IconChevron direction={iconDirection} />}
aria-label={ariaLabel}
tooltipProps={tooltipProps}
Copy link
Member

@Lms24 Lms24 Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l (feel free to ignore): Should we inline the toolTipProps here and just adjust message based on direction? based from what I can tell the other props are the same in both cases.

title={tooltipProps.title}
onClick={closeSpanDetailsDrawer}
disabled={!traceId || isTraceLoading || !isTraceAvailable}
to={getTraceDetailsUrl({
Expand Down
40 changes: 25 additions & 15 deletions static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinksNavigation.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import {ButtonBar} from '@sentry/scraps/button';
import {ExternalLink} from '@sentry/scraps/link';
import {Tooltip} from '@sentry/scraps/tooltip';

import {tct} from 'sentry/locale';
import type {TraceRootEventQueryResults} from 'sentry/views/performance/newTraceDetails/traceApi/useTraceRootEvent';
Expand Down Expand Up @@ -28,35 +27,46 @@ export function TraceLinksNavigation({
}

return (
<Tooltip
position="top"
delay={400}
isHoverable
title={tct(
`Go to the previous or next trace of the same session. [link:Learn More]`,
{
link: (
<ExternalLink href="https://docs.sentry.io/concepts/key-terms/tracing/trace-view/#previous-and-next-traces" />
),
}
)}
>
<div>
Copy link
Contributor

@nsdeschenes nsdeschenes Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Is this div needed, or would a Fragment be an alright replacement here?

Copy link
Member Author

@JPeer264 JPeer264 Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great question. The div, or any other element is needed since the ButtonBar is centered by default (and the parent container is a little higher).

There are two solutions:

  • Either wrap it inside a div (or any other element)
  • Or update the style of ButtonBar and use a StyledButtonBar for this component. And "undo" the align-items: center

I went for the first, since there is a Omit<..., 'className'> on the ButtonBar - which is maybe there for a reason.

Copy link
Contributor

@nsdeschenes nsdeschenes Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrrm, alrighty sounds all good to leave as is to me 👍

<ButtonBar merged gap="0">
<TraceLinkNavigationButton
direction="previous"
attributes={rootEventResults.data.attributes}
currentTraceStartTimestamp={
new Date(rootEventResults.data.timestamp).getTime() / 1000
}
tooltipProps={{
position: 'top',
delay: 400,
isHoverable: true,
title: tct(
`Go to the previous trace of the same session. [link:Learn More]`,
{
link: (
<ExternalLink href="https://docs.sentry.io/concepts/key-terms/tracing/trace-view/#previous-and-next-traces" />
),
}
),
}}
/>
<TraceLinkNavigationButton
direction="next"
attributes={rootEventResults.data.attributes}
currentTraceStartTimestamp={
new Date(rootEventResults.data.timestamp).getTime() / 1000
}
tooltipProps={{
position: 'top',
delay: 400,
isHoverable: true,
title: tct(`Go to the next trace of the same session. [link:Learn More]`, {
link: (
<ExternalLink href="https://docs.sentry.io/concepts/key-terms/tracing/trace-view/#previous-and-next-traces" />
),
}),
}}
/>
</ButtonBar>
</Tooltip>
</div>
);
}
Loading