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.

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

feat(replay): Web Vital Breadcrumb Design #76320

Merged
merged 19 commits into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
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
195 changes: 174 additions & 21 deletions static/app/components/replays/breadcrumbs/breadcrumbItem.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import type {CSSProperties, MouseEvent} from 'react';
import type {CSSProperties, ReactNode} from 'react';
import {isValidElement, memo, useCallback} from 'react';
import styled from '@emotion/styled';
import beautify from 'js-beautify';

import ProjectAvatar from 'sentry/components/avatar/projectAvatar';
import {Button} from 'sentry/components/button';
import {CodeSnippet} from 'sentry/components/codeSnippet';
import {Flex} from 'sentry/components/container/flex';
import ErrorBoundary from 'sentry/components/errorBoundary';
Expand All @@ -13,6 +14,7 @@ import PanelItem from 'sentry/components/panels/panelItem';
import {OpenReplayComparisonButton} from 'sentry/components/replays/breadcrumbs/openReplayComparisonButton';
import {useReplayContext} from 'sentry/components/replays/replayContext';
import {useReplayGroupContext} from 'sentry/components/replays/replayGroupContext';
import StructuredEventData from 'sentry/components/structuredEventData';
import Timeline from 'sentry/components/timeline';
import {useHasNewTimelineUI} from 'sentry/components/timeline/utils';
import {Tooltip} from 'sentry/components/tooltip';
Expand All @@ -21,37 +23,37 @@ import {space} from 'sentry/styles/space';
import type {Extraction} from 'sentry/utils/replays/extractHtml';
import {getReplayDiffOffsetsFromFrame} from 'sentry/utils/replays/getDiffTimestamps';
import getFrameDetails from 'sentry/utils/replays/getFrameDetails';
import useExtractDomNodes from 'sentry/utils/replays/hooks/useExtractDomNodes';
import type ReplayReader from 'sentry/utils/replays/replayReader';
import type {
ErrorFrame,
FeedbackFrame,
HydrationErrorFrame,
ReplayFrame,
WebVitalFrame,
} from 'sentry/utils/replays/types';
import {
isBreadcrumbFrame,
isErrorFrame,
isFeedbackFrame,
isHydrationErrorFrame,
isSpanFrame,
isWebVitalFrame,
} from 'sentry/utils/replays/types';
import type {Color} from 'sentry/utils/theme';
import useOrganization from 'sentry/utils/useOrganization';
import useProjectFromSlug from 'sentry/utils/useProjectFromSlug';
import IconWrapper from 'sentry/views/replays/detail/iconWrapper';
import TimestampButton from 'sentry/views/replays/detail/timestampButton';

type MouseCallback = (frame: ReplayFrame, e: React.MouseEvent<HTMLElement>) => void;
type MouseCallback = (frame: ReplayFrame, nodeId?: number) => void;

const FRAMES_WITH_BUTTONS = ['replay.hydrate-error'];

interface Props {
frame: ReplayFrame;
onClick: null | MouseCallback;
onInspectorExpanded: (
path: string,
expandedState: Record<string, boolean>,
event: MouseEvent<HTMLDivElement>
) => void;
onInspectorExpanded: (path: string, expandedState: Record<string, boolean>) => void;
onMouseEnter: MouseCallback;
onMouseLeave: MouseCallback;
startTimestampMs: number;
Expand Down Expand Up @@ -105,15 +107,31 @@ function BreadcrumbItem({
) : null;
}, [frame, replay]);

const renderCodeSnippet = useCallback(() => {
return extraction?.html ? (
<CodeContainer>
<CodeSnippet language="html" hideCopyButton>
{beautify.html(extraction?.html, {indent_size: 2})}
</CodeSnippet>
</CodeContainer>
const renderWebVital = useCallback(() => {
return isSpanFrame(frame) && isWebVitalFrame(frame) ? (
<WebVitalData
replay={replay}
frame={frame}
expandPaths={expandPaths}
onInspectorExpanded={onInspectorExpanded}
onMouseEnter={onMouseEnter}
onMouseLeave={onMouseLeave}
/>
) : null;
}, [extraction?.html]);
}, [expandPaths, frame, onInspectorExpanded, onMouseEnter, onMouseLeave, replay]);

const renderCodeSnippet = useCallback(() => {
return (!isSpanFrame(frame) || (isSpanFrame(frame) && !isWebVitalFrame(frame))) &&
extraction?.html
? extraction?.html.map(html => (
<CodeContainer key={html}>
c298lee marked this conversation as resolved.
Show resolved Hide resolved
<CodeSnippet language="html" hideCopyButton>
{beautify.html(html, {indent_size: 2})}
</CodeSnippet>
</CodeContainer>
))
: null;
}, [extraction?.html, frame]);

const renderIssueLink = useCallback(() => {
return isErrorFrame(frame) || isFeedbackFrame(frame) ? (
Expand Down Expand Up @@ -143,13 +161,14 @@ function BreadcrumbItem({
data-is-error-frame={isErrorFrame(frame)}
style={style}
className={className}
onClick={e => onClick?.(frame, e)}
onMouseEnter={e => onMouseEnter(frame, e)}
onMouseLeave={e => onMouseLeave(frame, e)}
onClick={() => onClick?.(frame)}
onMouseEnter={() => onMouseEnter(frame)}
onMouseLeave={() => onMouseLeave(frame)}
>
<ErrorBoundary mini>
{renderDescription()}
{renderComparisonButton()}
{renderWebVital()}
{renderCodeSnippet()}
{renderIssueLink()}
</ErrorBoundary>
Expand All @@ -160,9 +179,9 @@ function BreadcrumbItem({
<CrumbItem
data-is-error-frame={isErrorFrame(frame)}
as={onClick && !forceSpan ? 'button' : 'span'}
onClick={e => onClick?.(frame, e)}
onMouseEnter={e => onMouseEnter(frame, e)}
onMouseLeave={e => onMouseLeave(frame, e)}
onClick={() => onClick?.(frame)}
onMouseEnter={() => onMouseEnter(frame)}
onMouseLeave={() => onMouseLeave(frame)}
style={style}
className={className}
>
Expand All @@ -184,6 +203,7 @@ function BreadcrumbItem({
{renderDescription()}
</Flex>
{renderComparisonButton()}
{renderWebVital()}
{renderCodeSnippet()}
{renderIssueLink()}
</CrumbDetails>
Expand All @@ -192,6 +212,116 @@ function BreadcrumbItem({
);
}

function WebVitalData({
replay,
frame,
expandPaths,
onInspectorExpanded,
onMouseEnter,
onMouseLeave,
}: {
expandPaths: string[] | undefined;
frame: WebVitalFrame;
onInspectorExpanded: (path: string, expandedState: Record<string, boolean>) => void;
onMouseEnter: MouseCallback;
onMouseLeave: MouseCallback;
replay: ReplayReader | null;
}) {
// TODO: remove test CLS data once SDK is merged and updated
const clsFrame = {
...frame,
data: {
value: frame.data.value,
size: frame.data.size,
rating: frame.data.rating,
nodeIds: [frame.data.nodeIds, 333, 870],
attributes: [
{value: 0.0123, nodeIds: frame.data.nodeIds ?? [93]},
{value: 0.0345, nodeIds: [333, 870]},
],
Copy link
Member

Choose a reason for hiding this comment

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

some magic numbers will need to be removed tho

},
};

Copy link
Member Author

Choose a reason for hiding this comment

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

to be removed once the sdk is updated

Copy link
Member

Choose a reason for hiding this comment

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

Are older sdk versions sending data in this format? if so we'd need to continue to support it as there are customers out there on that older version

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, the older format is supported too! This is just to demo what it would look like with the newer format since we don't have that yet, with the older format, it'll look more similar to the other web vitals (eg. no layout shifts, only element and value).

const {data: frameToExtraction} = useExtractDomNodes({replay});
const selectors = frameToExtraction?.get(frame)?.selector;

const webVitalData = {value: frame.data.value};
if (
frame.description === 'cumulative-layout-shift' &&
// frame.data.attributes &&
selectors
c298lee marked this conversation as resolved.
Show resolved Hide resolved
) {
const layoutShifts: {[x: string]: ReactNode[]}[] = [];
for (const attr of clsFrame.data.attributes) {
const elements: ReactNode[] = [];
attr.nodeIds?.map(nodeId => {
return selectors.get(nodeId)
? elements.push(
<span
key={nodeId}
onMouseEnter={() => onMouseEnter(clsFrame, nodeId)}
c298lee marked this conversation as resolved.
Show resolved Hide resolved
onMouseLeave={() => onMouseLeave(clsFrame, nodeId)}
>
<ValueObjectKey>{'element'}</ValueObjectKey>
<span>{': '}</span>
c298lee marked this conversation as resolved.
Show resolved Hide resolved
<span>
<SelectorButton>{selectors.get(nodeId)}</SelectorButton>
</span>
</span>
)
: null;
});
// if we can't find the elements associated with the layout shift, we still show the score with element: unknown
if (!elements.length) {
elements.push(
<span>
<ValueObjectKey>{'element'}</ValueObjectKey>
<span>{': '}</span>
<ValueNull>{'unknown'}</ValueNull>
</span>
);
c298lee marked this conversation as resolved.
Show resolved Hide resolved
}
layoutShifts.push({[`score ${attr.value}`]: elements});
}
if (layoutShifts.length) {
webVitalData['Layout shifts'] = layoutShifts;
}
} else if (selectors?.size) {
const vitalKey = 'element';
webVitalData[vitalKey] = (
<span>
{Array.from(selectors).map(([, key]) => {
return (
<span
key={key}
onMouseEnter={() => onMouseEnter(frame)}
onMouseLeave={() => onMouseLeave(frame)}
>
<ValueObjectKey>{'element'}</ValueObjectKey>
<span>{': '}</span>
c298lee marked this conversation as resolved.
Show resolved Hide resolved
<SelectorButton>{key}</SelectorButton>
</span>
);
})}
</span>
);
}

return webVitalData ? (
<StructuredEventData
c298lee marked this conversation as resolved.
Show resolved Hide resolved
initialExpandedPaths={expandPaths ?? []}
onToggleExpand={(expandedPaths, path) => {
onInspectorExpanded(
path,
Object.fromEntries(expandedPaths.map(item => [item, true]))
);
}}
data={webVitalData}
withAnnotatedText
/>
) : null;
}

function CrumbHydrationButton({
replay,
frame,
Expand Down Expand Up @@ -381,4 +511,27 @@ const CodeContainer = styled('div')`
overflow: auto;
`;

const ValueObjectKey = styled('span')`
color: var(--prism-keyword);
`;

const ValueNull = styled('span')`
font-weight: ${p => p.theme.fontWeightBold};
color: var(--prism-property);
`;

const SelectorButton = styled(Button)`
background: none;
border: none;
padding: 0 2px;
border-radius: 2px;
font-weight: ${p => p.theme.fontWeightNormal};
box-shadow: none;
font-size: ${p => p.theme.fontSizeSmall};
color: ${p => p.theme.subText};
margin: 0 ${space(0.5)};
height: auto;
min-height: auto;
`;

Comment on lines +513 to +525
Copy link
Member

Choose a reason for hiding this comment

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

might be able to get away with something like <Button size="zero" borderless>...</Button> instead of having to write so much new css

Copy link
Member Author

Choose a reason for hiding this comment

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

we could do that! would look like this instead:
image

Copy link
Member

Choose a reason for hiding this comment

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

either way i don't mind!

export default memo(BreadcrumbItem);
63 changes: 52 additions & 11 deletions static/app/utils/replays/extractHtml.tsx
Original file line number Diff line number Diff line change
@@ -1,25 +1,66 @@
import type {Mirror} from '@sentry-internal/rrweb-snapshot';

import type {ReplayFrame} from 'sentry/utils/replays/types';
import constructSelector from 'sentry/views/replays/deadRageClick/constructSelector';

export type Extraction = {
frame: ReplayFrame;
html: string | null;
html: string[];
selector: Map<number, string>;
timestamp: number;
};

export default function extractHtml(nodeId: number, mirror: Mirror): string | null {
const node = mirror.getNode(nodeId);
export default function extractHtml(nodeIds: number[], mirror: Mirror): string[] {
const htmlStrings: string[] = [];
for (const nodeId of nodeIds) {
const node = mirror.getNode(nodeId);

const html =
(node && 'outerHTML' in node ? (node.outerHTML as string) : node?.textContent) || '';
// Limit document node depth to 2
let truncated = removeNodesAtLevel(html, 2);
// If still very long and/or removeNodesAtLevel failed, truncate
if (truncated.length > 1500) {
truncated = truncated.substring(0, 1500);
const html =
(node && 'outerHTML' in node ? (node.outerHTML as string) : node?.textContent) ||
'';
// Limit document node depth to 2
let truncated = removeNodesAtLevel(html, 2);
// If still very long and/or removeNodesAtLevel failed, truncate
if (truncated.length > 1500) {
truncated = truncated.substring(0, 1500);
}
if (truncated) {
htmlStrings.push(truncated);
}
}
return truncated ? truncated : null;
return htmlStrings;
}

export function extractSelectorFromNodeIds(
c298lee marked this conversation as resolved.
Show resolved Hide resolved
nodeIds: number[],
mirror: Mirror
): Map<number, string> {
const selectors = new Map<number, string>();
for (const nodeId of nodeIds) {
const node = mirror.getNode(nodeId);

const element = node?.nodeType === Node.ELEMENT_NODE ? (node as HTMLElement) : null;

if (element) {
selectors.set(
nodeId,
constructSelector({
alt: element.attributes.getNamedItem('alt')?.nodeValue ?? '',
aria_label: element.attributes.getNamedItem('aria-label')?.nodeValue ?? '',
class: element.attributes.getNamedItem('class')?.nodeValue?.split(' ') ?? [],
component_name:
element.attributes.getNamedItem('data-sentry-component')?.nodeValue ?? '',
id: element.id,
role: element.attributes.getNamedItem('role')?.nodeValue ?? '',
tag: element.tagName.toLowerCase(),
testid: element.attributes.getNamedItem('data-test-id')?.nodeValue ?? '',
title: element.attributes.getNamedItem('title')?.nodeValue ?? '',
}).selector
);
}
}

return selectors;
}

function removeChildLevel(max: number, collection: HTMLCollection, current: number = 0) {
Expand Down
Loading
Loading