Skip to content
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

Jump to bottom

ref(replay): Move custom breadcrumbs to breadcrumb list #77962

Merged
merged 4 commits into from
Sep 23, 2024
Merged

ref(replay): Move custom breadcrumbs to breadcrumb list #77962

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1fae5b5
move custom breadcrumbs to breadcrumb list
c298lee Sep 23, 2024
a1ed33d
remove margin & add custom breadcrumb test
c298lee Sep 23, 2024
37b1ff8
turns out order matters for the test
c298lee Sep 23, 2024
64730b6
PR comment
c298lee Sep 23, 2024
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
55 changes: 30 additions & 25 deletions static/app/components/replays/breadcrumbs/breadcrumbItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {CodeSnippet} from 'sentry/components/codeSnippet';
import {Flex} from 'sentry/components/container/flex';
import ErrorBoundary from 'sentry/components/errorBoundary';
import Link from 'sentry/components/links/link';
import ObjectInspector from 'sentry/components/objectInspector';
import PanelItem from 'sentry/components/panels/panelItem';
import {OpenReplayComparisonButton} from 'sentry/components/replays/breadcrumbs/openReplayComparisonButton';
import {useReplayContext} from 'sentry/components/replays/replayContext';
Expand Down Expand Up @@ -88,17 +87,19 @@ function BreadcrumbItem({
{description}
</Description>
) : (
<InspectorWrapper>
<ObjectInspector
data={description}
expandPaths={expandPaths}
onExpand={onInspectorExpanded}
theme={{
TREENODE_FONT_SIZE: '0.7rem',
ARROW_FONT_SIZE: '0.5rem',
<Wrapper>
<StructuredEventData
initialExpandedPaths={expandPaths ?? []}
onToggleExpand={(expandedPaths, path) => {
onInspectorExpanded(
path,
Object.fromEntries(expandedPaths.map(item => [item, true]))
);
}}
data={description}
withAnnotatedText
/>
</InspectorWrapper>
</Wrapper>
);
}, [description, expandPaths, onInspectorExpanded]);

Expand Down Expand Up @@ -295,17 +296,19 @@ function WebVitalData({
}

return (
<StructuredEventData
initialExpandedPaths={expandPaths ?? []}
onToggleExpand={(expandedPaths, path) => {
onInspectorExpanded(
path,
Object.fromEntries(expandedPaths.map(item => [item, true]))
);
}}
data={webVitalData}
withAnnotatedText
/>
<Wrapper>
<StructuredEventData
initialExpandedPaths={expandPaths ?? []}
onToggleExpand={(expandedPaths, path) => {
onInspectorExpanded(
path,
Object.fromEntries(expandedPaths.map(item => [item, true]))
);
}}
data={webVitalData}
withAnnotatedText
/>
</Wrapper>
);
}

Expand Down Expand Up @@ -376,10 +379,6 @@ const CrumbIssueWrapper = styled('div')`
color: ${p => p.theme.subText};
`;

const InspectorWrapper = styled('div')`
font-family: ${p => p.theme.text.familyMono};
`;

const CrumbDetails = styled('div')`
display: flex;
flex-direction: column;
Expand Down Expand Up @@ -521,4 +520,10 @@ const SelectorButton = styled(Button)`
min-height: auto;
`;

const Wrapper = styled('div')`
pre {
margin: 0;
}
`;

export default memo(BreadcrumbItem);
6 changes: 3 additions & 3 deletions static/app/utils/replays/getFrameDetails.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,9 +430,9 @@ const MAPPER_FOR_FRAME: Record<string, (frame) => Details> = {

const MAPPER_DEFAULT = (frame): Details => ({
color: 'gray300',
description: frame.message ?? '',
tabKey: TabKey.CONSOLE,
title: defaultTitle(frame),
description: frame.message ?? frame.data ?? '',
tabKey: TabKey.BREADCRUMBS,
title: toTitleCase(defaultTitle(frame)),
icon: <IconTerminal size="xs" />,
});

Expand Down
10 changes: 6 additions & 4 deletions static/app/utils/replays/replayReader.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,11 @@ describe('ReplayReader', () => {
},
{
method: 'getConsoleFrames',
expected: [
expect.objectContaining({category: 'console'}),
expect.objectContaining({category: 'redux.action'}),
],
expected: [expect.objectContaining({category: 'console'})],
},
{
method: 'getCustomFrames',
expected: [expect.objectContaining({category: 'redux.action'})],
},
{
method: 'getNetworkFrames',
Expand All @@ -196,6 +197,7 @@ describe('ReplayReader', () => {
expected: [
expect.objectContaining({category: 'replay.init'}),
expect.objectContaining({category: 'ui.slowClickDetected'}),
expect.objectContaining({category: 'redux.action'}),
expect.objectContaining({op: 'navigation.navigate'}), // prefer the nav span over the breadcrumb
expect.objectContaining({category: 'ui.click'}),
expect.objectContaining({category: 'ui.click'}),
Expand Down
12 changes: 8 additions & 4 deletions static/app/utils/replays/replayReader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -538,10 +538,7 @@ export default class ReplayReader {
getErrorFrames = () => this._errors;

getConsoleFrames = memoize(() =>
this._sortedBreadcrumbFrames.filter(
frame =>
frame.category === 'console' || !BreadcrumbCategories.includes(frame.category)
)
this._sortedBreadcrumbFrames.filter(frame => frame.category === 'console')
);

getNavigationFrames = memoize(() =>
Expand Down Expand Up @@ -589,11 +586,18 @@ export default class ReplayReader {
this._sortedSpanFrames.filter((frame): frame is MemoryFrame => frame.op === 'memory')
);

getCustomFrames = memoize(() =>
this._sortedBreadcrumbFrames.filter(
frame => !BreadcrumbCategories.includes(frame.category)
Copy link
Member

@ryan953 ryan953 Sep 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This BreadcrumbCategories list looks like it also includes 'console', does that mean console logs will be in both places?

Copy link
Member Author

Choose a reason for hiding this comment

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

Custom frames is anything in the sorted breadcrumbs frames list that isn't in BreadcrumbCategories. So, console logs won't be in the breadcrumbs list

)
);

getChapterFrames = memoize(() =>
this._trimFramesToClipWindow(
[
...this.getPerfFrames(),
...this.getWebVitalFrames(),
...this.getCustomFrames(),
...this._sortedBreadcrumbFrames.filter(frame =>
[
'replay.hydrate-error',
Expand Down
1 change: 1 addition & 0 deletions static/app/utils/replays/types.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@ export const BreadcrumbCategories = [
'device.battery',
'device.connectivity',
'device.orientation',
'feedback',
'navigation',
'replay.init',
'replay.mutations',
Expand Down
8 changes: 8 additions & 0 deletions static/app/views/replays/detail/breadcrumbs/useBreadcrumbFilters.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ const TYPE_TO_LABEL: Record<string, string> = {
tap: 'User Tap',
device: 'Device',
app: 'App',
custom: 'Custom',
};

const OPORCATEGORY_TO_TYPE: Record<string, keyof typeof TYPE_TO_LABEL> = {
Expand Down Expand Up @@ -114,6 +115,13 @@ function useBreadcrumbFilters({frames}: Options): Return {
const type = useMemo(() => decodeList(query.f_b_type), [query.f_b_type]);
const searchTerm = decodeScalar(query.f_b_search, '').toLowerCase();

// add custom breadcrumbs to filter
frames.forEach(frame => {
if (!OPORCATEGORY_TO_TYPE[getFrameOpOrCategory(frame)]) {
c298lee marked this conversation as resolved.
Show resolved Hide resolved
OPORCATEGORY_TO_TYPE[getFrameOpOrCategory(frame)] = 'custom';
}
});

const items = useMemo(() => {
// flips OPORCATERGORY_TO_TYPE and prevents overwriting nav entry, nav entry becomes nav: ['navigation','navigation.push']
const TYPE_TO_OPORCATEGORY = Object.entries(OPORCATEGORY_TO_TYPE).reduce(
Expand Down
Loading