Skip to content

Commit 3bf7b8b

Browse files
ryan953andrewshie-sentry
authored andcommitted
ref: Streamline loading replays for hydration diffs in Issue Details (#88028)
Before we had two 'placeholder' thing that would render while we wait for the hydration diff preview to load up: - A `<LoadingIndicator />` while we're lazy-loading the `replayDiffContent.tsx` file - Then `<Placeholder/>` while we load the replay data over ajax This resulted in janky loading swapping one placeholder for another. Also, we weren't properly handling all loading states before, so if the replay was missing or archived it looked janky. Now that `<ReplayLoadingState>` is in place, all cases are handled. For example: | Before | After | | --- | --- | | <img width="846" alt="SCR-20250326-mjkc" src="https://github.com/user-attachments/assets/68053fcf-5a2d-44aa-850f-f85255e5526e" /> | <img width="841" alt="SCR-20250326-mjmr" src="https://github.com/user-attachments/assets/c7ffbbcc-9193-46ea-8f37-1df3e4245dac" /> | Along the way i changed the interface to the new `<ReplayLoadingState>`. Now it takes the result of `useLoadReplayReader` as a prop. This will make it possible to separate loading and rendering, so we can deal with logging that sometimes happens, or in the case of Traces, the loading happens very far away and props get drilled down.
1 parent 9bdc476 commit 3bf7b8b

File tree

4 files changed

+132
-85
lines changed

4 files changed

+132
-85
lines changed
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,23 @@
1+
import styled from '@emotion/styled';
2+
3+
import NegativeSpaceContainer from 'sentry/components/container/negativeSpaceContainer';
14
import ErrorBoundary from 'sentry/components/errorBoundary';
2-
import Placeholder from 'sentry/components/placeholder';
5+
import {REPLAY_LOADING_HEIGHT} from 'sentry/components/events/eventReplay/constants';
6+
import LoadingIndicator from 'sentry/components/loadingIndicator';
7+
import ArchivedReplayAlert from 'sentry/components/replays/alerts/archivedReplayAlert';
8+
import MissingReplayAlert from 'sentry/components/replays/alerts/missingReplayAlert';
39
import {OpenReplayComparisonButton} from 'sentry/components/replays/breadcrumbs/openReplayComparisonButton';
410
import {DiffCompareContextProvider} from 'sentry/components/replays/diff/diffCompareContext';
511
import {ReplaySliderDiff} from 'sentry/components/replays/diff/replaySliderDiff';
12+
import ReplayLoadingState from 'sentry/components/replays/player/replayLoadingState';
613
import {ReplayGroupContextProvider} from 'sentry/components/replays/replayGroupContext';
714
import {t} from 'sentry/locale';
15+
import {space} from 'sentry/styles/space';
816
import type {Event} from 'sentry/types/event';
917
import type {Group} from 'sentry/types/group';
1018
import {getReplayDiffOffsetsFromEvent} from 'sentry/utils/replays/getDiffTimestamps';
1119
import useLoadReplayReader from 'sentry/utils/replays/hooks/useLoadReplayReader';
20+
import useOrganization from 'sentry/utils/useOrganization';
1221
import {SectionKey} from 'sentry/views/issueDetails/streamline/context';
1322
import {InterimSection} from 'sentry/views/issueDetails/streamline/interimSection';
1423

@@ -20,54 +29,88 @@ interface Props {
2029
}
2130

2231
export default function ReplayDiffContent({event, group, orgSlug, replaySlug}: Props) {
23-
const replayContext = useLoadReplayReader({
24-
orgSlug,
32+
const organization = useOrganization();
33+
const readerResult = useLoadReplayReader({
34+
orgSlug: organization.slug,
2535
replaySlug,
36+
clipWindow: undefined,
2637
});
27-
const {fetching, replay} = replayContext;
28-
29-
if (fetching) {
30-
return <Placeholder />;
31-
}
3238

33-
if (!replay) {
34-
return null;
35-
}
39+
const sectionProps = {
40+
type: SectionKey.HYDRATION_DIFF,
41+
title: t('Hydration Error Diff'),
42+
};
3643

37-
const {frameOrEvent, leftOffsetMs, rightOffsetMs} = getReplayDiffOffsetsFromEvent(
38-
replay,
39-
event
40-
);
4144
return (
42-
<InterimSection
43-
type={SectionKey.HYDRATION_DIFF}
44-
title={t('Hydration Error Diff')}
45-
actions={
46-
<OpenReplayComparisonButton
47-
frameOrEvent={frameOrEvent}
48-
initialLeftOffsetMs={leftOffsetMs}
49-
initialRightOffsetMs={rightOffsetMs}
50-
key="open-modal-button"
51-
replay={replay}
52-
size="xs"
53-
surface="issue-details" // TODO: refactor once this component is used in more surfaces
54-
>
55-
{t('Open Diff Viewer')}
56-
</OpenReplayComparisonButton>
57-
}
45+
<ReplayLoadingState
46+
readerResult={readerResult}
47+
renderArchived={() => (
48+
<InterimSection {...sectionProps}>
49+
<ArchivedReplayAlert
50+
message={t('The replay for this event has been deleted.')}
51+
/>
52+
</InterimSection>
53+
)}
54+
renderLoading={() => (
55+
<InterimSection {...sectionProps}>
56+
<StyledNegativeSpaceContainer data-test-id="replay-diff-loading-placeholder">
57+
<LoadingIndicator />
58+
</StyledNegativeSpaceContainer>
59+
</InterimSection>
60+
)}
61+
renderError={() => (
62+
<InterimSection {...sectionProps}>
63+
<MissingReplayAlert orgSlug={orgSlug} />
64+
</InterimSection>
65+
)}
66+
renderMissing={() => (
67+
<InterimSection {...sectionProps}>
68+
<MissingReplayAlert orgSlug={orgSlug} />
69+
</InterimSection>
70+
)}
5871
>
59-
<ErrorBoundary mini>
60-
<ReplayGroupContextProvider groupId={group?.id} eventId={event.id}>
61-
<DiffCompareContextProvider
62-
replay={replay}
63-
frameOrEvent={frameOrEvent}
64-
initialLeftOffsetMs={leftOffsetMs}
65-
initialRightOffsetMs={rightOffsetMs}
72+
{({replay}) => {
73+
const {frameOrEvent, leftOffsetMs, rightOffsetMs} = getReplayDiffOffsetsFromEvent(
74+
replay,
75+
event
76+
);
77+
return (
78+
<InterimSection
79+
{...sectionProps}
80+
actions={
81+
<OpenReplayComparisonButton
82+
frameOrEvent={frameOrEvent}
83+
initialLeftOffsetMs={leftOffsetMs}
84+
initialRightOffsetMs={rightOffsetMs}
85+
key="open-modal-button"
86+
replay={replay}
87+
size="xs"
88+
surface="issue-details" // TODO: refactor once this component is used in more surfaces
89+
>
90+
{t('Open Diff Viewer')}
91+
</OpenReplayComparisonButton>
92+
}
6693
>
67-
<ReplaySliderDiff minHeight="355px" />
68-
</DiffCompareContextProvider>
69-
</ReplayGroupContextProvider>
70-
</ErrorBoundary>
71-
</InterimSection>
94+
<ErrorBoundary mini>
95+
<ReplayGroupContextProvider groupId={group?.id} eventId={event.id}>
96+
<DiffCompareContextProvider
97+
replay={replay}
98+
frameOrEvent={frameOrEvent}
99+
initialLeftOffsetMs={leftOffsetMs}
100+
initialRightOffsetMs={rightOffsetMs}
101+
>
102+
<ReplaySliderDiff minHeight="355px" />
103+
</DiffCompareContextProvider>
104+
</ReplayGroupContextProvider>
105+
</ErrorBoundary>
106+
</InterimSection>
107+
);
108+
}}
109+
</ReplayLoadingState>
72110
);
73111
}
112+
113+
const StyledNegativeSpaceContainer = styled(NegativeSpaceContainer)`
114+
height: ${REPLAY_LOADING_HEIGHT}px;
115+
margin-bottom: ${space(2)};
116+
`;

static/app/components/events/eventHydrationDiff/replayDiffSection.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ export function ReplayDiffSection({event, group, replayId}: Props) {
5454
);
5555
}
5656

57-
export const StyledNegativeSpaceContainer = styled(NegativeSpaceContainer)`
57+
const StyledNegativeSpaceContainer = styled(NegativeSpaceContainer)`
5858
height: ${REPLAY_LOADING_HEIGHT}px;
5959
margin-bottom: ${space(2)};
6060
`;

static/app/components/replays/player/__stories__/replaySlugChooser.tsx

+42-23
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import {css} from '@emotion/react';
33

44
import Providers from 'sentry/components/replays/player/__stories__/providers';
55
import ReplayLoadingState from 'sentry/components/replays/player/replayLoadingState';
6+
import useLoadReplayReader from 'sentry/utils/replays/hooks/useLoadReplayReader';
7+
import useOrganization from 'sentry/utils/useOrganization';
68
import {useSessionStorage} from 'sentry/utils/useSessionStorage';
79

810
type Props =
@@ -14,33 +16,50 @@ export default function ReplaySlugChooser(props: Props) {
1416

1517
const [replaySlug, setReplaySlug] = useSessionStorage('stories:replaySlug', '');
1618

17-
let content = null;
18-
if (replaySlug) {
19-
if (children) {
20-
content = (
21-
<ReplayLoadingState replaySlug={replaySlug}>
19+
const input = (
20+
<input
21+
defaultValue={replaySlug}
22+
onChange={event => {
23+
setReplaySlug(event.target.value);
24+
}}
25+
placeholder="Paste a replaySlug"
26+
css={css`
27+
font-variant-numeric: tabular-nums;
28+
`}
29+
size={34}
30+
/>
31+
);
32+
33+
if (replaySlug && children) {
34+
function Content() {
35+
const organization = useOrganization();
36+
const readerResult = useLoadReplayReader({
37+
orgSlug: organization.slug,
38+
replaySlug,
39+
clipWindow: undefined,
40+
});
41+
return (
42+
<ReplayLoadingState readerResult={readerResult}>
2243
{({replay}) => <Providers replay={replay}>{children}</Providers>}
2344
</ReplayLoadingState>
2445
);
25-
} else if (render) {
26-
content = render(replaySlug);
2746
}
47+
return (
48+
<Fragment>
49+
{input}
50+
<Content />
51+
</Fragment>
52+
);
2853
}
2954

30-
return (
31-
<Fragment>
32-
<input
33-
defaultValue={replaySlug}
34-
onChange={event => {
35-
setReplaySlug(event.target.value);
36-
}}
37-
placeholder="Paste a replaySlug"
38-
css={css`
39-
font-variant-numeric: tabular-nums;
40-
`}
41-
size={34}
42-
/>
43-
{content}
44-
</Fragment>
45-
);
55+
if (replaySlug && render) {
56+
return (
57+
<Fragment>
58+
{input}
59+
{render(replaySlug)}
60+
</Fragment>
61+
);
62+
}
63+
64+
return null;
4665
}

static/app/components/replays/player/replayLoadingState.tsx

+3-18
Original file line numberDiff line numberDiff line change
@@ -3,43 +3,28 @@ import type {ReactNode} from 'react';
33
import LoadingIndicator from 'sentry/components/loadingIndicator';
44
import ArchivedReplayAlert from 'sentry/components/replays/alerts/archivedReplayAlert';
55
import MissingReplayAlert from 'sentry/components/replays/alerts/missingReplayAlert';
6-
import useLoadReplayReader from 'sentry/utils/replays/hooks/useLoadReplayReader';
6+
import type useLoadReplayReader from 'sentry/utils/replays/hooks/useLoadReplayReader';
77
import type ReplayReader from 'sentry/utils/replays/replayReader';
88
import useOrganization from 'sentry/utils/useOrganization';
99

10-
type ClipWindow = {
11-
// When to stop the replay, given it continues into that time
12-
endTimestampMs: number;
13-
14-
// When to start the replay, given its start time is early enough
15-
startTimestampMs: number;
16-
};
17-
1810
type ReplayReaderResult = ReturnType<typeof useLoadReplayReader>;
1911

2012
export default function ReplayLoadingState({
2113
children,
22-
replaySlug,
23-
clipWindow,
14+
readerResult,
2415
renderArchived,
2516
renderError,
2617
renderLoading,
2718
renderMissing,
2819
}: {
2920
children: (props: {replay: ReplayReader}) => ReactNode;
30-
replaySlug: string;
31-
clipWindow?: ClipWindow;
21+
readerResult: ReplayReaderResult;
3222
renderArchived?: (results: ReplayReaderResult) => ReactNode;
3323
renderError?: (results: ReplayReaderResult) => ReactNode;
3424
renderLoading?: (results: ReplayReaderResult) => ReactNode;
3525
renderMissing?: (results: ReplayReaderResult) => ReactNode;
3626
}) {
3727
const organization = useOrganization();
38-
const readerResult = useLoadReplayReader({
39-
orgSlug: organization.slug,
40-
replaySlug,
41-
clipWindow,
42-
});
4328

4429
if (readerResult.fetchError) {
4530
return renderError ? (

0 commit comments

Comments
 (0)