Skip to content

Commit 5d65d7d

Browse files
authored
ref(replay): Extract components from <BreadcrumbItem> (#94536)
## PR Description This pull request refactors the `BreadcrumbItem` component in the Replays feature to improve its structure, readability, and maintainability. The original component was becoming too large and complex, handling multiple responsibilities within a single file. The goal is to decompose it into smaller, more focused components, each responsible for a specific aspect of the breadcrumb display. <details> <summary><b>Click to see more</b></summary> ### Key Technical Changes The primary technical change is the extraction of several rendering functions within `BreadcrumbItem` into separate React components. Specifically: * `BreadcrumbCodeSnippet`: Handles the display of code snippets associated with breadcrumbs. * `BreadcrumbComparisonButton`: Renders a button to compare replay states related to hydration errors. * `BreadcrumbDescription`: Displays the description of the breadcrumb, including the 'View HTML' button. * `BreadcrumbIssueLink`: Creates a link to the associated issue, if the breadcrumb represents an error or feedback event. * `BreadcrumbWebVital`: Renders web vital data associated with the breadcrumb. These new components are then used within `BreadcrumbItem` to render their respective parts of the breadcrumb. The `isSpanFrame` and `isWebVitalFrame` checks have been updated to ensure correct rendering of code snippets and the 'View HTML' button. ### Architecture Decisions The architectural decision is to follow the Single Responsibility Principle by breaking down the monolithic `BreadcrumbItem` component into smaller, more manageable components. This promotes code reuse, improves testability, and makes it easier to understand and modify the breadcrumb rendering logic. Styled components are used to encapsulate the styling for each new component, maintaining a consistent look and feel. ### Dependencies and Interactions The refactored components depend on the following: * `sentry/components/codeSnippet`, `sentry/components/placeholder`, `sentry/components/core/button`, `sentry/components/core/tooltip`, `sentry/components/structuredEventData`, `sentry/components/timeline`, and `sentry/components/links/link` for UI elements. * `sentry/utils/replays/extractDomNodes`, `sentry/utils/replays/getDiffTimestamps`, `sentry/utils/replays/getFrameDetails`, and `sentry/utils/replays/hooks/useExtractDomNodes` for data extraction and processing. * `sentry/utils/replays/types` for type definitions related to replay frames and events. * `sentry/utils/useOrganization` and `sentry/utils/useProjectFromSlug` for accessing organization and project information. * `sentry/views/replays/detail/timestampButton` and `sentry/views/replays/detail/useVirtualizedInspector` for replay-specific UI elements and functionality. The components interact with the `ReplayContext` and `ReplayGroupContext` to access replay data and group information. ### Risk Considerations The primary risk is that the refactoring might introduce regressions in the rendering of breadcrumbs. Thorough testing is required to ensure that all types of breadcrumbs are rendered correctly and that the new components handle all edge cases. The changes to the `isSpanFrame` and `isWebVitalFrame` checks need to be carefully reviewed to avoid unintended side effects. The removal of the `&:not(:last-child)::before` CSS selector might also have unintended visual consequences. ### Notable Implementation Details The condition `(!isSpanFrame(frame) || !isWebVitalFrame(frame))` in `BreadcrumbCodeSnippet` and `BreadcrumbDescription` was identified as potentially incorrect and should be reviewed carefully. The `@ts-expect-error` comments in `BreadcrumbWebVital` indicate areas where type safety needs to be improved. The removal of the `render*` functions from `BreadcrumbItem` significantly reduces its complexity and improves readability. </details>
1 parent 6d2e7ea commit 5d65d7d

File tree

8 files changed

+421
-352
lines changed

8 files changed

+421
-352
lines changed
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import styled from '@emotion/styled';
2+
import beautify from 'js-beautify';
3+
4+
import {CodeSnippet} from 'sentry/components/codeSnippet';
5+
import Placeholder from 'sentry/components/placeholder';
6+
import type {Extraction} from 'sentry/utils/replays/extractDomNodes';
7+
import type {ReplayFrame} from 'sentry/utils/replays/types';
8+
import {isSpanFrame} from 'sentry/utils/replays/types';
9+
10+
interface Props {
11+
frame: ReplayFrame;
12+
isPending: boolean;
13+
showSnippet: boolean;
14+
extraction?: Extraction;
15+
}
16+
17+
export function BreadcrumbCodeSnippet({
18+
frame,
19+
extraction,
20+
showSnippet,
21+
isPending,
22+
}: Props) {
23+
if (!showSnippet) {
24+
return null;
25+
}
26+
27+
if (isPending) {
28+
return <Placeholder height="34px" />;
29+
}
30+
31+
if (isSpanFrame(frame)) {
32+
return null;
33+
}
34+
35+
return extraction?.html?.map(html => (
36+
<CodeContainer key={html}>
37+
<CodeSnippet language="html" hideCopyButton>
38+
{beautify.html(html, {indent_size: 2})}
39+
</CodeSnippet>
40+
</CodeContainer>
41+
));
42+
}
43+
44+
const CodeContainer = styled('div')`
45+
max-height: 400px;
46+
max-width: 100%;
47+
overflow: auto;
48+
`;
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import {OpenReplayComparisonButton} from 'sentry/components/replays/breadcrumbs/openReplayComparisonButton';
2+
import {t} from 'sentry/locale';
3+
import {getReplayDiffOffsetsFromFrame} from 'sentry/utils/replays/getDiffTimestamps';
4+
import type ReplayReader from 'sentry/utils/replays/replayReader';
5+
import type {ReplayFrame} from 'sentry/utils/replays/types';
6+
import {isBreadcrumbFrame, isHydrationErrorFrame} from 'sentry/utils/replays/types';
7+
8+
interface Props {
9+
frame: ReplayFrame;
10+
replay: ReplayReader | null;
11+
}
12+
13+
export function BreadcrumbComparisonButton({frame, replay}: Props) {
14+
if (!isBreadcrumbFrame(frame) || !isHydrationErrorFrame(frame) || !replay) {
15+
return null;
16+
}
17+
18+
const {frameOrEvent, leftOffsetMs, rightOffsetMs} = getReplayDiffOffsetsFromFrame(
19+
replay,
20+
frame
21+
);
22+
23+
return (
24+
<div>
25+
<OpenReplayComparisonButton
26+
frameOrEvent={frameOrEvent}
27+
initialLeftOffsetMs={leftOffsetMs}
28+
initialRightOffsetMs={rightOffsetMs}
29+
replay={replay}
30+
size="xs"
31+
surface="replay-breadcrumbs"
32+
>
33+
{t('Open Hydration Diff')}
34+
</OpenReplayComparisonButton>
35+
</div>
36+
);
37+
}
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
import type {ReactNode} from 'react';
2+
import {isValidElement} from 'react';
3+
import styled from '@emotion/styled';
4+
5+
import {Button} from 'sentry/components/core/button';
6+
import {Tooltip} from 'sentry/components/core/tooltip';
7+
import StructuredEventData from 'sentry/components/structuredEventData';
8+
import {t} from 'sentry/locale';
9+
import {space} from 'sentry/styles/space';
10+
import type {ReplayFrame, WebVitalFrame} from 'sentry/utils/replays/types';
11+
import {isSpanFrame} from 'sentry/utils/replays/types';
12+
import type {OnExpandCallback} from 'sentry/views/replays/detail/useVirtualizedInspector';
13+
14+
interface Props {
15+
allowShowSnippet: boolean;
16+
description: ReactNode;
17+
frame: ReplayFrame | WebVitalFrame;
18+
onClickViewHtml: (e: React.MouseEvent<HTMLButtonElement>) => void;
19+
onInspectorExpanded: OnExpandCallback;
20+
showSnippet: boolean;
21+
className?: string;
22+
expandPaths?: string[];
23+
}
24+
25+
export function BreadcrumbDescription({
26+
description,
27+
allowShowSnippet,
28+
showSnippet,
29+
frame,
30+
expandPaths,
31+
onInspectorExpanded,
32+
onClickViewHtml,
33+
}: Props) {
34+
if (
35+
typeof description === 'string' ||
36+
(description !== undefined && isValidElement(description))
37+
) {
38+
return (
39+
<DescriptionWrapper>
40+
<Description title={description} showOnlyOnOverflow isHoverable>
41+
{description}
42+
</Description>
43+
44+
{allowShowSnippet &&
45+
!showSnippet &&
46+
frame.data?.nodeId !== undefined &&
47+
!isSpanFrame(frame) && (
48+
<ViewHtmlButton priority="link" onClick={onClickViewHtml} size="xs">
49+
{t('View HTML')}
50+
</ViewHtmlButton>
51+
)}
52+
</DescriptionWrapper>
53+
);
54+
}
55+
56+
return (
57+
<Wrapper>
58+
<StructuredEventData
59+
initialExpandedPaths={expandPaths ?? []}
60+
onToggleExpand={(expandedPaths, path) => {
61+
onInspectorExpanded(
62+
path,
63+
Object.fromEntries(expandedPaths.map(item => [item, true]))
64+
);
65+
}}
66+
data={description}
67+
withAnnotatedText
68+
/>
69+
</Wrapper>
70+
);
71+
}
72+
73+
const Description = styled(Tooltip)`
74+
${p => p.theme.overflowEllipsis};
75+
font-size: 0.7rem;
76+
font-variant-numeric: tabular-nums;
77+
line-height: ${p => p.theme.text.lineHeightBody};
78+
color: ${p => p.theme.subText};
79+
`;
80+
81+
const DescriptionWrapper = styled('div')`
82+
display: flex;
83+
gap: ${space(1)};
84+
justify-content: space-between;
85+
`;
86+
87+
const ViewHtmlButton = styled(Button)`
88+
white-space: nowrap;
89+
`;
90+
91+
const Wrapper = styled('div')`
92+
pre {
93+
margin: 0;
94+
}
95+
`;
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import styled from '@emotion/styled';
2+
3+
import {ProjectAvatar} from 'sentry/components/core/avatar/projectAvatar';
4+
import Link from 'sentry/components/links/link';
5+
import {useReplayGroupContext} from 'sentry/components/replays/replayGroupContext';
6+
import {space} from 'sentry/styles/space';
7+
import type {ErrorFrame, FeedbackFrame, ReplayFrame} from 'sentry/utils/replays/types';
8+
import {isErrorFrame, isFeedbackFrame} from 'sentry/utils/replays/types';
9+
import useOrganization from 'sentry/utils/useOrganization';
10+
import useProjectFromSlug from 'sentry/utils/useProjectFromSlug';
11+
import {makeFeedbackPathname} from 'sentry/views/userFeedback/pathnames';
12+
13+
interface Props {
14+
frame: ReplayFrame;
15+
}
16+
17+
export function BreadcrumbIssueLink({frame}: Props) {
18+
if (!isErrorFrame(frame) && !isFeedbackFrame(frame)) {
19+
return null;
20+
}
21+
22+
return <CrumbErrorIssue frame={frame} />;
23+
}
24+
25+
function CrumbErrorIssue({frame}: {frame: FeedbackFrame | ErrorFrame}) {
26+
const organization = useOrganization();
27+
const project = useProjectFromSlug({organization, projectSlug: frame.data.projectSlug});
28+
const {groupId} = useReplayGroupContext();
29+
30+
const projectAvatar = project ? <ProjectAvatar project={project} size={16} /> : null;
31+
32+
if (String(frame.data.groupId) === groupId) {
33+
return (
34+
<CrumbIssueWrapper>
35+
{projectAvatar}
36+
{frame.data.groupShortId}
37+
</CrumbIssueWrapper>
38+
);
39+
}
40+
41+
return (
42+
<CrumbIssueWrapper>
43+
{projectAvatar}
44+
<Link
45+
to={
46+
isFeedbackFrame(frame)
47+
? {
48+
pathname: makeFeedbackPathname({
49+
path: '/',
50+
organization,
51+
}),
52+
query: {feedbackSlug: `${frame.data.projectSlug}:${frame.data.groupId}`},
53+
}
54+
: `/organizations/${organization.slug}/issues/${frame.data.groupId}/`
55+
}
56+
>
57+
{frame.data.groupShortId}
58+
</Link>
59+
</CrumbIssueWrapper>
60+
);
61+
}
62+
63+
const CrumbIssueWrapper = styled('div')`
64+
display: flex;
65+
align-items: center;
66+
gap: ${space(0.5)};
67+
font-size: ${p => p.theme.fontSize.sm};
68+
color: ${p => p.theme.subText};
69+
`;

0 commit comments

Comments
 (0)