From a4a432ec79bafca4dcf4907a8baecec8fcecca5e Mon Sep 17 00:00:00 2001 From: Catherine Lee <55311782+c298lee@users.noreply.github.com> Date: Fri, 16 Aug 2024 03:24:58 -0400 Subject: [PATCH 01/15] all web vitals except cls --- .../replays/breadcrumbs/breadcrumbItem.tsx | 120 ++++++++++++++++-- .../recursiveStructuredData.tsx | 1 + static/app/utils/replays/extractHtml.tsx | 59 +++++++-- static/app/utils/replays/getFrameDetails.tsx | 37 ++++-- static/app/utils/replays/highlightNode.tsx | 114 +++++++++-------- .../utils/replays/hooks/useCrumbHandlers.tsx | 5 +- .../replays/hooks/useInitialTimeOffsetMs.tsx | 2 +- static/app/utils/replays/replayReader.tsx | 35 +++-- static/app/utils/replays/types.tsx | 8 +- .../detail/breadcrumbs/breadcrumbRow.tsx | 7 +- .../detail/useVirtualizedInspector.tsx | 8 +- 11 files changed, 281 insertions(+), 115 deletions(-) diff --git a/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx b/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx index 5a9f00cecf9de..741e7b60bedd3 100644 --- a/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx +++ b/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx @@ -1,9 +1,10 @@ -import type {CSSProperties, MouseEvent} from 'react'; +import type {CSSProperties} 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'; @@ -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'; @@ -21,18 +23,22 @@ 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'; @@ -47,11 +53,7 @@ const FRAMES_WITH_BUTTONS = ['replay.hydrate-error']; interface Props { frame: ReplayFrame; onClick: null | MouseCallback; - onInspectorExpanded: ( - path: string, - expandedState: Record, - event: MouseEvent - ) => void; + onInspectorExpanded: (path: string, expandedState: Record) => void; onMouseEnter: MouseCallback; onMouseLeave: MouseCallback; startTimestampMs: number; @@ -105,15 +107,31 @@ function BreadcrumbItem({ ) : null; }, [frame, replay]); - const renderCodeSnippet = useCallback(() => { - return extraction?.html ? ( - - - {beautify.html(extraction?.html, {indent_size: 2})} - - + const renderWebVital = useCallback(() => { + return isSpanFrame(frame) && isWebVitalFrame(frame) ? ( + ) : 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 => ( + + + {beautify.html(html, {indent_size: 2})} + + + )) + : null; + }, [extraction?.html, frame]); const renderIssueLink = useCallback(() => { return isErrorFrame(frame) || isFeedbackFrame(frame) ? ( @@ -150,6 +168,7 @@ function BreadcrumbItem({ {renderDescription()} {renderComparisonButton()} + {renderWebVital()} {renderCodeSnippet()} {renderIssueLink()} @@ -184,6 +203,7 @@ function BreadcrumbItem({ {renderDescription()} {renderComparisonButton()} + {renderWebVital()} {renderCodeSnippet()} {renderIssueLink()} @@ -192,6 +212,61 @@ function BreadcrumbItem({ ); } +function WebVitalData({ + replay, + frame, + expandPaths, + onInspectorExpanded, + onMouseEnter, + onMouseLeave, +}: { + expandPaths: string[] | undefined; + frame: WebVitalFrame; + onInspectorExpanded: (path: string, expandedState: Record) => void; + onMouseEnter: MouseCallback; + onMouseLeave: MouseCallback; + replay: ReplayReader | null; +}) { + const {data: frameToExtraction} = useExtractDomNodes({replay}); + const selectors = frameToExtraction?.get(frame)?.selector; + const webVitalData = selectors?.length + ? { + value: frame.data.value, + element: ( + + {selectors.map(selector => { + return ( + onMouseEnter(frame, e)} + onMouseLeave={e => onMouseLeave(frame, e)} + > + {'element'} + {': '} + {selector} + + ); + })} + + ), + } + : null; + + return webVitalData ? ( + { + onInspectorExpanded( + path, + Object.fromEntries(expandedPaths.map(item => [item, true])) + ); + }} + data={webVitalData} + withAnnotatedText + /> + ) : null; +} + function CrumbHydrationButton({ replay, frame, @@ -379,6 +454,23 @@ const CodeContainer = styled('div')` max-height: 400px; max-width: 100%; overflow: auto; + background: ${p => p.theme.red100}; +`; + +const ValueObjectKey = styled('span')` + color: var(--prism-keyword); +`; + +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)}; `; export default memo(BreadcrumbItem); diff --git a/static/app/components/structuredEventData/recursiveStructuredData.tsx b/static/app/components/structuredEventData/recursiveStructuredData.tsx index 18a1c3f4879c2..b55c9b8e60d54 100644 --- a/static/app/components/structuredEventData/recursiveStructuredData.tsx +++ b/static/app/components/structuredEventData/recursiveStructuredData.tsx @@ -16,6 +16,7 @@ type Config = { isNumber?: (value: unknown) => boolean; isString?: (value: unknown) => boolean; renderBoolean?: (value: unknown) => React.ReactNode; + renderHighlight?: (value: unknown) => React.ReactNode; renderNull?: (value: unknown) => React.ReactNode; renderObjectKeys?: (value: string) => string; renderString?: (value: string) => string; diff --git a/static/app/utils/replays/extractHtml.tsx b/static/app/utils/replays/extractHtml.tsx index cb6333177d539..38f3bee63cb09 100644 --- a/static/app/utils/replays/extractHtml.tsx +++ b/static/app/utils/replays/extractHtml.tsx @@ -1,25 +1,62 @@ 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: 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(nodeIds: number[], mirror: Mirror): string[] { + const selectors: 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.push( + 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) { diff --git a/static/app/utils/replays/getFrameDetails.tsx b/static/app/utils/replays/getFrameDetails.tsx index a1e409f112a5e..2ddead628a0f9 100644 --- a/static/app/utils/replays/getFrameDetails.tsx +++ b/static/app/utils/replays/getFrameDetails.tsx @@ -50,6 +50,7 @@ import { } from 'sentry/utils/replays/types'; import {toTitleCase} from 'sentry/utils/string/toTitleCase'; import type {Color} from 'sentry/utils/theme'; +import theme from 'sentry/utils/theme'; import stripURLOrigin from 'sentry/utils/url/stripURLOrigin'; interface Details { @@ -286,31 +287,43 @@ const MAPPER_FOR_FRAME: Record Details> = { case 'good': return { color: 'green300', - description: tct('Good [value]ms', { - value: frame.data.value.toFixed(2), - }), + description: ( + + {tct('[value]ms (Good)', { + value: frame.data.value.toFixed(2), + })} + + ), tabKey: TabKey.NETWORK, - title: toTitleCase(explodeSlug(frame.description)), + title: 'Web Vital: ' + toTitleCase(explodeSlug(frame.description)), icon: , }; case 'needs-improvement': return { color: 'yellow300', - description: tct('Meh [value]ms', { - value: frame.data.value.toFixed(2), - }), + description: ( + + {tct('[value]ms (Meh)', { + value: frame.data.value.toFixed(2), + })} + + ), tabKey: TabKey.NETWORK, - title: toTitleCase(explodeSlug(frame.description)), + title: 'Web Vital: ' + toTitleCase(explodeSlug(frame.description)), icon: , }; default: return { color: 'red300', - description: tct('Poor [value]ms', { - value: frame.data.value.toFixed(2), - }), + description: ( + + {tct('[value]ms (Poor)', { + value: frame.data.value.toFixed(2), + })} + + ), tabKey: TabKey.NETWORK, - title: toTitleCase(explodeSlug(frame.description)), + title: 'Web Vital: ' + toTitleCase(explodeSlug(frame.description)), icon: , }; } diff --git a/static/app/utils/replays/highlightNode.tsx b/static/app/utils/replays/highlightNode.tsx index 40da9a364cf55..4c72fc3cf0591 100644 --- a/static/app/utils/replays/highlightNode.tsx +++ b/static/app/utils/replays/highlightNode.tsx @@ -2,31 +2,31 @@ import type {Replayer} from '@sentry-internal/rrweb'; const DEFAULT_HIGHLIGHT_COLOR = 'rgba(168, 196, 236, 0.75)'; -const highlightsByNodeId: Map = new Map(); +const highlightsByNodeIds: Map = new Map(); const highlightsBySelector: Map = new Map(); type DrawProps = {annotation: string; color: string; spotlight: boolean}; -interface AddHighlightByNodeIdParams extends Partial { - nodeId: number; +interface AddHighlightByNodeIdsParams extends Partial { + nodeIds: number[]; } interface AddHighlightBySelectorParams extends Partial { selector: string; } -type AddHighlightParams = AddHighlightByNodeIdParams | AddHighlightBySelectorParams; +type AddHighlightParams = AddHighlightByNodeIdsParams | AddHighlightBySelectorParams; type RemoveHighlightParams = | { - nodeId: number; + nodeIds: number[]; } | { selector: string; }; export function clearAllHighlights(replayer: Replayer) { - for (const nodeId of highlightsByNodeId.keys()) { - removeHighlightedNode(replayer, {nodeId}); + for (const nodeId of highlightsByNodeIds.keys()) { + removeHighlightedNode(replayer, {nodeIds: [nodeId]}); } for (const selector of highlightsBySelector.keys()) { removeHighlightedNode(replayer, {selector}); @@ -40,11 +40,13 @@ export function clearAllHighlights(replayer: Replayer) { * are creating a new canvas PER highlight. */ export function removeHighlightedNode(replayer: Replayer, props: RemoveHighlightParams) { - if ('nodeId' in props) { - const highlightObj = highlightsByNodeId.get(props.nodeId); - if (highlightObj && replayer.wrapper.contains(highlightObj.canvas)) { - replayer.wrapper.removeChild(highlightObj.canvas); - highlightsByNodeId.delete(props.nodeId); + if ('nodeIds' in props) { + for (const nodeId of props.nodeIds) { + const highlightObj = highlightsByNodeIds.get(nodeId); + if (highlightObj && replayer.wrapper.contains(highlightObj.canvas)) { + replayer.wrapper.removeChild(highlightObj.canvas); + highlightsByNodeIds.delete(nodeId); + } } } else { const highlightObj = highlightsBySelector.get(props.selector); @@ -61,56 +63,60 @@ export function removeHighlightedNode(replayer: Replayer, props: RemoveHighlight export function highlightNode(replayer: Replayer, props: AddHighlightParams) { const {wrapper} = replayer; const mirror = replayer.getMirror(); + const canvases: {canvas: HTMLCanvasElement}[] = []; + + const nodes = + 'nodeIds' in props + ? props.nodeIds.map(nodeId => mirror.getNode(nodeId)) + : [replayer.iframe.contentDocument?.body.querySelector(props.selector)]; + + for (const node of nodes) { + // TODO(replays): There is some sort of race condition here when you "rewind" a replay, + // mirror will be empty and highlight does not get added because node is null + if ( + !node || + !('getBoundingClientRect' in node) || + !replayer.iframe.contentDocument?.body?.contains(node) + ) { + return null; + } - const node = - 'nodeId' in props - ? mirror.getNode(props.nodeId) - : replayer.iframe.contentDocument?.body.querySelector(props.selector); - - // TODO(replays): There is some sort of race condition here when you "rewind" a replay, - // mirror will be empty and highlight does not get added because node is null - if ( - !node || - !('getBoundingClientRect' in node) || - !replayer.iframe.contentDocument?.body?.contains(node) - ) { - return null; - } - - // Create a new canvas with the same dimensions as the iframe. We may need to - // revisit this strategy as we create a new canvas for every highlight. See - // additional notes in removeHighlight() method. - const element = node.nodeType === Node.ELEMENT_NODE ? (node as HTMLElement) : null; + // Create a new canvas with the same dimensions as the iframe. We may need to + // revisit this strategy as we create a new canvas for every highlight. See + // additional notes in removeHighlight() method. + const element = node.nodeType === Node.ELEMENT_NODE ? (node as HTMLElement) : null; - if (!element) { - return null; - } + if (!element) { + return null; + } - const canvas = document.createElement('canvas'); - canvas.width = Number(replayer.iframe.width); - canvas.height = Number(replayer.iframe.height); - canvas.setAttribute('style', 'position:absolute;'); + const canvas = document.createElement('canvas'); + canvas.width = Number(replayer.iframe.width); + canvas.height = Number(replayer.iframe.height); + canvas.setAttribute('style', 'position:absolute;'); - const boundingClientRect = element.getBoundingClientRect(); - const drawProps = { - annotation: props.annotation ?? '', - color: props.color ?? DEFAULT_HIGHLIGHT_COLOR, - spotlight: props.spotlight ?? false, - }; + const boundingClientRect = element.getBoundingClientRect(); + const drawProps = { + annotation: props.annotation ?? '', + color: props.color ?? DEFAULT_HIGHLIGHT_COLOR, + spotlight: props.spotlight ?? false, + }; - drawCtx(canvas, boundingClientRect, drawProps); + drawCtx(canvas, boundingClientRect, drawProps); - if ('nodeId' in props) { - highlightsByNodeId.set(props.nodeId, {canvas}); - } else { - highlightsBySelector.set(props.selector, {canvas}); - } + if ('nodeIds' in props) { + highlightsByNodeIds.set(mirror.getId(node), {canvas}); + } else { + highlightsBySelector.set(props.selector, {canvas}); + } - wrapper.insertBefore(canvas, replayer.iframe); + wrapper.insertBefore(canvas, replayer.iframe); - return { - canvas, - }; + canvases.push({ + canvas, + }); + } + return canvases; } function drawCtx( diff --git a/static/app/utils/replays/hooks/useCrumbHandlers.tsx b/static/app/utils/replays/hooks/useCrumbHandlers.tsx index 073296eece72e..28d726ba3c603 100644 --- a/static/app/utils/replays/hooks/useCrumbHandlers.tsx +++ b/static/app/utils/replays/hooks/useCrumbHandlers.tsx @@ -36,7 +36,10 @@ function getNodeIdAndLabel(record: RecordType) { }; } if ('nodeId' in data) { - return {nodeId: data.nodeId, annotation: record.data.label}; + return {nodeIds: [data.nodeId], annotation: record.data.label}; + } + if ('nodeIds' in data) { + return {nodeIds: data.nodeIds, annotation: record.data.label}; } return undefined; } diff --git a/static/app/utils/replays/hooks/useInitialTimeOffsetMs.tsx b/static/app/utils/replays/hooks/useInitialTimeOffsetMs.tsx index 9eeef11ae98e5..6b2c534449cc1 100644 --- a/static/app/utils/replays/hooks/useInitialTimeOffsetMs.tsx +++ b/static/app/utils/replays/hooks/useInitialTimeOffsetMs.tsx @@ -143,7 +143,7 @@ async function fromListPageQuery({ return { highlight: { annotation: undefined, - nodeId, + nodeIds: [nodeId], spotlight: true, }, offsetMs: firstTimestmpMs - replayStartTimestampMs, diff --git a/static/app/utils/replays/replayReader.tsx b/static/app/utils/replays/replayReader.tsx index f612447fef263..3989bfe36eb6a 100644 --- a/static/app/utils/replays/replayReader.tsx +++ b/static/app/utils/replays/replayReader.tsx @@ -7,7 +7,7 @@ import {defined} from 'sentry/utils'; import domId from 'sentry/utils/domId'; import localStorageWrapper from 'sentry/utils/localStorage'; import clamp from 'sentry/utils/number/clamp'; -import extractHtml from 'sentry/utils/replays/extractHtml'; +import extractHtml, {extractSelectorFromNodeIds} from 'sentry/utils/replays/extractHtml'; import hydrateBreadcrumbs, { replayInitBreadcrumb, } from 'sentry/utils/replays/hydrateBreadcrumbs'; @@ -34,11 +34,12 @@ import type { SlowClickFrame, SpanFrame, VideoEvent, + WebVitalFrame, } from 'sentry/utils/replays/types'; import { BreadcrumbCategories, EventType, - getNodeId, + getNodeIds, IncrementalSource, isDeadClick, isDeadRageClick, @@ -144,16 +145,18 @@ function removeDuplicateNavCrumbs( const extractDomNodes = { shouldVisitFrame: frame => { - const nodeId = getNodeId(frame); - return nodeId !== undefined && nodeId !== -1; + const nodeIds = getNodeIds(frame); + return nodeIds.filter(nodeId => nodeId !== -1).length > 0; }, onVisitFrame: (frame, collection, replayer) => { const mirror = replayer.getMirror(); - const nodeId = getNodeId(frame); - const html = extractHtml(nodeId as number, mirror); + const nodeIds = getNodeIds(frame); + const html = extractHtml(nodeIds as number[], mirror); + const selector = extractSelectorFromNodeIds(nodeIds as number[], mirror); collection.set(frame as ReplayFrame, { frame, html, + selector, timestamp: frame.timestampMs, }); }, @@ -576,7 +579,9 @@ export default class ReplayReader { ) ) ), - ...this._sortedSpanFrames.filter(frame => 'nodeId' in (frame.data ?? {})), + ...this._sortedSpanFrames.filter( + frame => 'nodeId' in (frame.data ?? {}) || 'nodeIds' in (frame.data ?? {}) + ), ].sort(sortFrames) ); @@ -628,7 +633,21 @@ export default class ReplayReader { getWebVitalFrames = memoize(() => { if (this._featureFlags?.includes('session-replay-web-vitals')) { - return this._sortedSpanFrames.filter(isWebVitalFrame); + // sort by largest timestamp first to easily find the last CLS in a burst + const allWebVitals = this._sortedSpanFrames.filter(isWebVitalFrame).reverse(); + let lastTimestamp = 0; + const groupedCls: WebVitalFrame[] = []; + + for (const cls of allWebVitals) { + if (cls.description === 'cumulative-layout-shift') { + if (lastTimestamp === cls.timestampMs) { + groupedCls.push(cls); + } else { + lastTimestamp = cls.timestampMs; + } + } + } + return allWebVitals.filter(frame => !groupedCls.includes(frame)).reverse(); } return []; }); diff --git a/static/app/utils/replays/types.tsx b/static/app/utils/replays/types.tsx index 466a1ef8d5e29..f1af185861be5 100644 --- a/static/app/utils/replays/types.tsx +++ b/static/app/utils/replays/types.tsx @@ -171,10 +171,12 @@ export function getFrameOpOrCategory(frame: ReplayFrame) { return val; } -export function getNodeId(frame: ReplayFrame) { +export function getNodeIds(frame: ReplayFrame) { return 'data' in frame && frame.data && 'nodeId' in frame.data - ? frame.data.nodeId - : undefined; + ? [frame.data.nodeId] + : 'data' in frame && frame.data && 'nodeIds' in frame.data + ? frame.data.nodeIds + : undefined; } export function isConsoleFrame(frame: BreadcrumbFrame): frame is ConsoleFrame { diff --git a/static/app/views/replays/detail/breadcrumbs/breadcrumbRow.tsx b/static/app/views/replays/detail/breadcrumbs/breadcrumbRow.tsx index f0a0a818ef1bc..f043f8d08e4e0 100644 --- a/static/app/views/replays/detail/breadcrumbs/breadcrumbRow.tsx +++ b/static/app/views/replays/detail/breadcrumbs/breadcrumbRow.tsx @@ -1,4 +1,4 @@ -import type {CSSProperties, MouseEvent} from 'react'; +import type {CSSProperties} from 'react'; import {useCallback} from 'react'; import classNames from 'classnames'; @@ -17,8 +17,7 @@ interface Props { onInspectorExpanded: ( index: number, path: string, - expandedState: Record, - event: MouseEvent + expandedState: Record ) => void; startTimestampMs: number; style: CSSProperties; @@ -42,7 +41,7 @@ export default function BreadcrumbRow({ const {onMouseEnter, onMouseLeave} = useCrumbHandlers(); const handleObjectInspectorExpanded = useCallback( - (path, expandedState, e) => onInspectorExpanded?.(index, path, expandedState, e), + (path, expandedState) => onInspectorExpanded?.(index, path, expandedState), [index, onInspectorExpanded] ); diff --git a/static/app/views/replays/detail/useVirtualizedInspector.tsx b/static/app/views/replays/detail/useVirtualizedInspector.tsx index 564a861c221a7..9c35613ffeca3 100644 --- a/static/app/views/replays/detail/useVirtualizedInspector.tsx +++ b/static/app/views/replays/detail/useVirtualizedInspector.tsx @@ -23,12 +23,7 @@ export default function useVirtualizedInspector({cache, listRef, expandPathsRef} return { expandPaths: expandPathsRef.current, handleDimensionChange: useCallback( - ( - index: number, - path: string, - expandedState: Record, - event: MouseEvent - ) => { + (index: number, path: string, expandedState: Record) => { const rowState = expandPathsRef.current?.get(index) || new Set(); if (expandedState[path]) { rowState.add(path); @@ -38,7 +33,6 @@ export default function useVirtualizedInspector({cache, listRef, expandPathsRef} } expandPathsRef.current?.set(index, rowState); handleDimensionChange(index); - event.stopPropagation(); }, [expandPathsRef, handleDimensionChange] ), From 164696d2f1f8eef4e6780e70903b24a185a89102 Mon Sep 17 00:00:00 2001 From: Catherine Lee <55311782+c298lee@users.noreply.github.com> Date: Fri, 16 Aug 2024 03:30:25 -0400 Subject: [PATCH 02/15] remove extra code --- .../components/structuredEventData/recursiveStructuredData.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/static/app/components/structuredEventData/recursiveStructuredData.tsx b/static/app/components/structuredEventData/recursiveStructuredData.tsx index b55c9b8e60d54..18a1c3f4879c2 100644 --- a/static/app/components/structuredEventData/recursiveStructuredData.tsx +++ b/static/app/components/structuredEventData/recursiveStructuredData.tsx @@ -16,7 +16,6 @@ type Config = { isNumber?: (value: unknown) => boolean; isString?: (value: unknown) => boolean; renderBoolean?: (value: unknown) => React.ReactNode; - renderHighlight?: (value: unknown) => React.ReactNode; renderNull?: (value: unknown) => React.ReactNode; renderObjectKeys?: (value: string) => string; renderString?: (value: string) => string; From 1518533f7d68b94eb8798b8b6e106223b7373c5e Mon Sep 17 00:00:00 2001 From: Catherine Lee <55311782+c298lee@users.noreply.github.com> Date: Fri, 16 Aug 2024 03:30:38 -0400 Subject: [PATCH 03/15] remove extra code --- static/app/components/replays/breadcrumbs/breadcrumbItem.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx b/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx index 741e7b60bedd3..57a253d4d4917 100644 --- a/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx +++ b/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx @@ -454,7 +454,6 @@ const CodeContainer = styled('div')` max-height: 400px; max-width: 100%; overflow: auto; - background: ${p => p.theme.red100}; `; const ValueObjectKey = styled('span')` From a631530375e89a1a3d90b715e156fba4e8ac3703 Mon Sep 17 00:00:00 2001 From: Catherine Lee <55311782+c298lee@users.noreply.github.com> Date: Tue, 27 Aug 2024 00:59:32 -0400 Subject: [PATCH 04/15] working with dummy cls data --- .../replays/breadcrumbs/breadcrumbItem.tsx | 94 ++++++++++++++----- static/app/utils/replays/extractHtml.tsx | 12 ++- .../utils/replays/hooks/useCrumbHandlers.tsx | 12 ++- 3 files changed, 87 insertions(+), 31 deletions(-) diff --git a/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx b/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx index 57a253d4d4917..a7882bb3761cd 100644 --- a/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx +++ b/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx @@ -1,4 +1,4 @@ -import type {CSSProperties} from 'react'; +import type {CSSProperties, ReactNode} from 'react'; import {isValidElement, memo, useCallback} from 'react'; import styled from '@emotion/styled'; import beautify from 'js-beautify'; @@ -46,7 +46,11 @@ 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) => void; +type MouseCallback = ( + frame: ReplayFrame, + _e: React.MouseEvent, + nodeId?: number +) => void; const FRAMES_WITH_BUTTONS = ['replay.hydrate-error']; @@ -227,30 +231,74 @@ function WebVitalData({ onMouseLeave: MouseCallback; replay: ReplayReader | null; }) { + 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]}, + ], + }, + }; const {data: frameToExtraction} = useExtractDomNodes({replay}); - const selectors = frameToExtraction?.get(frame)?.selector; - const webVitalData = selectors?.length - ? { - value: frame.data.value, - element: ( - - {selectors.map(selector => { - return ( - onMouseEnter(frame, e)} - onMouseLeave={e => onMouseLeave(frame, e)} - > - {'element'} - {': '} - {selector} + const selector = frameToExtraction?.get(frame)?.selector; + + let webVitalData; + if ( + frame.description === 'cumulative-layout-shift' && + // frame.data.attributes && + selector + ) { + const layoutShifts: {[x: string]: ReactNode[]}[] = []; + for (const attr of clsFrame.data.attributes) { + const elements: ReactNode[] = []; + attr.nodeIds?.map(nodeId => { + return selector.get(nodeId) + ? elements.push( + onMouseEnter(clsFrame, e, nodeId)} + onMouseLeave={e => onMouseLeave(clsFrame, e, nodeId)} + > + {'element'} + {': '} + + {selector.get(nodeId)} - ); - })} - - ), + + ) + : null; + }); + if (elements.length) { + const key = `score ${attr.value}`; + layoutShifts.push({[key]: elements}); } - : null; + } + webVitalData = {value: frame.data.value, 'Layout shifts': layoutShifts}; + } else if (selector?.size) { + // all web vitals except cls should have at most one associated element + const entry = selector.values().next().value; + webVitalData = { + value: frame.data.value, + element: ( + onMouseEnter(frame, e)} + onMouseLeave={e => onMouseLeave(frame, e)} + > + {'element'} + {': '} + {entry} + + ), + }; + } else { + webVitalData = {value: frame.data.value}; + } return webVitalData ? ( ; timestamp: number; }; @@ -31,15 +31,19 @@ export default function extractHtml(nodeIds: number[], mirror: Mirror): string[] return htmlStrings; } -export function extractSelectorFromNodeIds(nodeIds: number[], mirror: Mirror): string[] { - const selectors: string[] = []; +export function extractSelectorFromNodeIds( + nodeIds: number[], + mirror: Mirror +): Map { + const selectors = new Map(); for (const nodeId of nodeIds) { const node = mirror.getNode(nodeId); const element = node?.nodeType === Node.ELEMENT_NODE ? (node as HTMLElement) : null; if (element) { - selectors.push( + selectors.set( + nodeId, constructSelector({ alt: element.attributes.getNamedItem('alt')?.nodeValue ?? '', aria_label: element.attributes.getNamedItem('aria-label')?.nodeValue ?? '', diff --git a/static/app/utils/replays/hooks/useCrumbHandlers.tsx b/static/app/utils/replays/hooks/useCrumbHandlers.tsx index 28d726ba3c603..395b766870ebd 100644 --- a/static/app/utils/replays/hooks/useCrumbHandlers.tsx +++ b/static/app/utils/replays/hooks/useCrumbHandlers.tsx @@ -59,7 +59,7 @@ function useCrumbHandlers() { }); const onMouseEnter = useCallback( - (record: RecordType) => { + (record: RecordType, _e: React.MouseEvent, nodeId?: number) => { // This debounces the mouseEnter callback in unison with mouseLeave. // We ensure the pointer remains over the target element before dispatching // state events in order to minimize unnecessary renders. This helps during @@ -71,7 +71,9 @@ function useCrumbHandlers() { setCurrentHoverTime(record.offsetMs); } - const metadata = getNodeIdAndLabel(record); + const metadata = nodeId + ? {annotations: undefined, nodeIds: [nodeId]} + : getNodeIdAndLabel(record); if (metadata) { // XXX: Kind of hacky, but mouseLeave does not fire if you move from a // crumb to a tooltip @@ -86,7 +88,7 @@ function useCrumbHandlers() { ); const onMouseLeave = useCallback( - (record: RecordType) => { + (record: RecordType, _e: React.MouseEvent, nodeId: number) => { if (mouseEnterCallback.current.id === record) { // If there is a mouseEnter callback queued and we're leaving the node // just cancel the timeout. @@ -97,7 +99,9 @@ function useCrumbHandlers() { mouseEnterCallback.current.timeoutId = null; } else { setCurrentHoverTime(undefined); - const metadata = getNodeIdAndLabel(record); + const metadata = nodeId + ? {annotations: undefined, nodeIds: [nodeId]} + : getNodeIdAndLabel(record); if (metadata) { removeHighlight(metadata); } From 0705e5f325d7b160e2562d4f69dc545f2c7605b1 Mon Sep 17 00:00:00 2001 From: Catherine Lee <55311782+c298lee@users.noreply.github.com> Date: Tue, 27 Aug 2024 01:00:36 -0400 Subject: [PATCH 05/15] forgot ? --- static/app/utils/replays/hooks/useCrumbHandlers.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/app/utils/replays/hooks/useCrumbHandlers.tsx b/static/app/utils/replays/hooks/useCrumbHandlers.tsx index 395b766870ebd..cfc9da750c95d 100644 --- a/static/app/utils/replays/hooks/useCrumbHandlers.tsx +++ b/static/app/utils/replays/hooks/useCrumbHandlers.tsx @@ -88,7 +88,7 @@ function useCrumbHandlers() { ); const onMouseLeave = useCallback( - (record: RecordType, _e: React.MouseEvent, nodeId: number) => { + (record: RecordType, _e: React.MouseEvent, nodeId?: number) => { if (mouseEnterCallback.current.id === record) { // If there is a mouseEnter callback queued and we're leaving the node // just cancel the timeout. From a04fed1f9deade8ce99e9d219262940a9bac5a95 Mon Sep 17 00:00:00 2001 From: Catherine Lee <55311782+c298lee@users.noreply.github.com> Date: Tue, 27 Aug 2024 12:40:13 -0400 Subject: [PATCH 06/15] adds unknown element for cls --- .../replays/breadcrumbs/breadcrumbItem.tsx | 69 +++++++++++-------- 1 file changed, 41 insertions(+), 28 deletions(-) diff --git a/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx b/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx index a7882bb3761cd..e568ae0976931 100644 --- a/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx +++ b/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx @@ -245,19 +245,19 @@ function WebVitalData({ }, }; const {data: frameToExtraction} = useExtractDomNodes({replay}); - const selector = frameToExtraction?.get(frame)?.selector; + const selectors = frameToExtraction?.get(frame)?.selector; - let webVitalData; + const webVitalData = {value: frame.data.value}; if ( frame.description === 'cumulative-layout-shift' && // frame.data.attributes && - selector + selectors ) { const layoutShifts: {[x: string]: ReactNode[]}[] = []; for (const attr of clsFrame.data.attributes) { const elements: ReactNode[] = []; attr.nodeIds?.map(nodeId => { - return selector.get(nodeId) + return selectors.get(nodeId) ? elements.push( {'element'} {': '} - {selector.get(nodeId)} + {selectors.get(nodeId)} ) : null; }); - if (elements.length) { - const key = `score ${attr.value}`; - layoutShifts.push({[key]: elements}); + if (!elements.length) { + elements.push( + + {'element'} + {': '} + {'unknown'} + + ); } + layoutShifts.push({[`score ${attr.value}`]: elements}); } - webVitalData = {value: frame.data.value, 'Layout shifts': layoutShifts}; - } else if (selector?.size) { - // all web vitals except cls should have at most one associated element - const entry = selector.values().next().value; - webVitalData = { - value: frame.data.value, - element: ( - onMouseEnter(frame, e)} - onMouseLeave={e => onMouseLeave(frame, e)} - > - {'element'} - {': '} - {entry} - - ), - }; - } else { - webVitalData = {value: frame.data.value}; + if (layoutShifts.length) { + webVitalData['Layout shifts'] = layoutShifts; + } + } else if (selectors?.size) { + const vitalKey = 'element'; + webVitalData[vitalKey] = ( + + {Array.from(selectors).map(([, key]) => { + return ( + onMouseEnter(frame, e)} + onMouseLeave={e => onMouseLeave(frame, e)} + > + {'element'} + {': '} + {key} + + ); + })} + + ); } return webVitalData ? ( @@ -508,6 +516,11 @@ 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; From a81d02057323c0716da42aac3254eea5fe6e0d93 Mon Sep 17 00:00:00 2001 From: Catherine Lee <55311782+c298lee@users.noreply.github.com> Date: Wed, 28 Aug 2024 18:00:35 -0400 Subject: [PATCH 07/15] styling --- .../replays/breadcrumbs/breadcrumbItem.tsx | 5 +++ static/app/utils/replays/getFrameDetails.tsx | 31 ++++++------------- 2 files changed, 14 insertions(+), 22 deletions(-) diff --git a/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx b/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx index e568ae0976931..161133e5e9f07 100644 --- a/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx +++ b/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx @@ -231,6 +231,7 @@ function WebVitalData({ onMouseLeave: MouseCallback; replay: ReplayReader | null; }) { + // TODO: remove test CLS data once SDK is merged and updated const clsFrame = { ...frame, data: { @@ -244,6 +245,7 @@ function WebVitalData({ ], }, }; + const {data: frameToExtraction} = useExtractDomNodes({replay}); const selectors = frameToExtraction?.get(frame)?.selector; @@ -273,6 +275,7 @@ function WebVitalData({ ) : 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( @@ -531,6 +534,8 @@ const SelectorButton = styled(Button)` font-size: ${p => p.theme.fontSizeSmall}; color: ${p => p.theme.subText}; margin: 0 ${space(0.5)}; + height: auto; + min-height: auto; `; export default memo(BreadcrumbItem); diff --git a/static/app/utils/replays/getFrameDetails.tsx b/static/app/utils/replays/getFrameDetails.tsx index 2ddead628a0f9..5720b198c218b 100644 --- a/static/app/utils/replays/getFrameDetails.tsx +++ b/static/app/utils/replays/getFrameDetails.tsx @@ -50,7 +50,6 @@ import { } from 'sentry/utils/replays/types'; import {toTitleCase} from 'sentry/utils/string/toTitleCase'; import type {Color} from 'sentry/utils/theme'; -import theme from 'sentry/utils/theme'; import stripURLOrigin from 'sentry/utils/url/stripURLOrigin'; interface Details { @@ -287,13 +286,9 @@ const MAPPER_FOR_FRAME: Record Details> = { case 'good': return { color: 'green300', - description: ( - - {tct('[value]ms (Good)', { - value: frame.data.value.toFixed(2), - })} - - ), + description: tct('[value]ms (Good)', { + value: frame.data.value.toFixed(2), + }), tabKey: TabKey.NETWORK, title: 'Web Vital: ' + toTitleCase(explodeSlug(frame.description)), icon: , @@ -301,13 +296,9 @@ const MAPPER_FOR_FRAME: Record Details> = { case 'needs-improvement': return { color: 'yellow300', - description: ( - - {tct('[value]ms (Meh)', { - value: frame.data.value.toFixed(2), - })} - - ), + description: tct('[value]ms (Meh)', { + value: frame.data.value.toFixed(2), + }), tabKey: TabKey.NETWORK, title: 'Web Vital: ' + toTitleCase(explodeSlug(frame.description)), icon: , @@ -315,13 +306,9 @@ const MAPPER_FOR_FRAME: Record Details> = { default: return { color: 'red300', - description: ( - - {tct('[value]ms (Poor)', { - value: frame.data.value.toFixed(2), - })} - - ), + description: tct('[value]ms (Poor)', { + value: frame.data.value.toFixed(2), + }), tabKey: TabKey.NETWORK, title: 'Web Vital: ' + toTitleCase(explodeSlug(frame.description)), icon: , From 5143a4c7c297c94689a5afd681ea74bd1283c359 Mon Sep 17 00:00:00 2001 From: Catherine Lee <55311782+c298lee@users.noreply.github.com> Date: Wed, 28 Aug 2024 18:04:26 -0400 Subject: [PATCH 08/15] update test --- .../app/utils/replays/hooks/useInitialTimeOffsetMs.spec.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/static/app/utils/replays/hooks/useInitialTimeOffsetMs.spec.tsx b/static/app/utils/replays/hooks/useInitialTimeOffsetMs.spec.tsx index de4d7c6a25762..049f42b64ca7b 100644 --- a/static/app/utils/replays/hooks/useInitialTimeOffsetMs.spec.tsx +++ b/static/app/utils/replays/hooks/useInitialTimeOffsetMs.spec.tsx @@ -196,7 +196,7 @@ describe('useInitialTimeOffsetMs', () => { expect(result.current).toStrictEqual({ highlight: { annotation: undefined, - nodeId: 7, + nodeIds: [7], spotlight: true, }, offsetMs: 5 * 60 * 1000, @@ -240,7 +240,7 @@ describe('useInitialTimeOffsetMs', () => { expect(result.current).toStrictEqual({ highlight: { annotation: undefined, - nodeId: 7, + nodeIds: [7], spotlight: true, }, offsetMs: 5 * 60 * 1000, From ec938ae1b9a3efe3ffcc3e52350431611a94e88e Mon Sep 17 00:00:00 2001 From: Catherine Lee <55311782+c298lee@users.noreply.github.com> Date: Wed, 28 Aug 2024 18:30:59 -0400 Subject: [PATCH 09/15] fix crumbhandler types --- .../replays/breadcrumbs/breadcrumbItem.tsx | 26 ++++++++----------- .../utils/replays/hooks/useCrumbHandlers.tsx | 4 +-- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx b/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx index 161133e5e9f07..5bdb1c17d22f0 100644 --- a/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx +++ b/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx @@ -46,11 +46,7 @@ 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, - nodeId?: number -) => void; +type MouseCallback = (frame: ReplayFrame, nodeId?: number) => void; const FRAMES_WITH_BUTTONS = ['replay.hydrate-error']; @@ -165,9 +161,9 @@ 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)} > {renderDescription()} @@ -183,9 +179,9 @@ function BreadcrumbItem({ 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} > @@ -263,8 +259,8 @@ function WebVitalData({ ? elements.push( onMouseEnter(clsFrame, e, nodeId)} - onMouseLeave={e => onMouseLeave(clsFrame, e, nodeId)} + onMouseEnter={() => onMouseEnter(clsFrame, nodeId)} + onMouseLeave={() => onMouseLeave(clsFrame, nodeId)} > {'element'} {': '} @@ -298,8 +294,8 @@ function WebVitalData({ return ( onMouseEnter(frame, e)} - onMouseLeave={e => onMouseLeave(frame, e)} + onMouseEnter={() => onMouseEnter(frame)} + onMouseLeave={() => onMouseLeave(frame)} > {'element'} {': '} diff --git a/static/app/utils/replays/hooks/useCrumbHandlers.tsx b/static/app/utils/replays/hooks/useCrumbHandlers.tsx index cfc9da750c95d..825fac72e58b1 100644 --- a/static/app/utils/replays/hooks/useCrumbHandlers.tsx +++ b/static/app/utils/replays/hooks/useCrumbHandlers.tsx @@ -59,7 +59,7 @@ function useCrumbHandlers() { }); const onMouseEnter = useCallback( - (record: RecordType, _e: React.MouseEvent, nodeId?: number) => { + (record: RecordType, nodeId?: number) => { // This debounces the mouseEnter callback in unison with mouseLeave. // We ensure the pointer remains over the target element before dispatching // state events in order to minimize unnecessary renders. This helps during @@ -88,7 +88,7 @@ function useCrumbHandlers() { ); const onMouseLeave = useCallback( - (record: RecordType, _e: React.MouseEvent, nodeId?: number) => { + (record: RecordType, nodeId?: number) => { if (mouseEnterCallback.current.id === record) { // If there is a mouseEnter callback queued and we're leaving the node // just cancel the timeout. From 199c9eba4ce13d7cf65880544d4858632efe2c98 Mon Sep 17 00:00:00 2001 From: Catherine Lee <55311782+c298lee@users.noreply.github.com> Date: Thu, 29 Aug 2024 18:01:39 -0400 Subject: [PATCH 10/15] translate and refactor extract html/selector --- .../replays/breadcrumbs/breadcrumbItem.tsx | 46 +++++----- static/app/utils/replays/extractHtml.tsx | 86 ++++++++++--------- static/app/utils/replays/replayReader.tsx | 7 +- 3 files changed, 73 insertions(+), 66 deletions(-) diff --git a/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx b/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx index 5bdb1c17d22f0..541f6d4e93401 100644 --- a/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx +++ b/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx @@ -121,16 +121,16 @@ function BreadcrumbItem({ }, [expandPaths, frame, onInspectorExpanded, onMouseEnter, onMouseLeave, replay]); const renderCodeSnippet = useCallback(() => { - return (!isSpanFrame(frame) || (isSpanFrame(frame) && !isWebVitalFrame(frame))) && - extraction?.html - ? extraction?.html.map(html => ( - - - {beautify.html(html, {indent_size: 2})} - - - )) - : null; + return ( + (!isSpanFrame(frame) || (isSpanFrame(frame) && !isWebVitalFrame(frame))) && + extraction?.html?.map(html => ( + + + {beautify.html(html, {indent_size: 2})} + + + )) + ); }, [extraction?.html, frame]); const renderIssueLink = useCallback(() => { @@ -243,19 +243,19 @@ function WebVitalData({ }; const {data: frameToExtraction} = useExtractDomNodes({replay}); - const selectors = frameToExtraction?.get(frame)?.selector; + const selectors = frameToExtraction?.get(frame)?.selectors; const webVitalData = {value: frame.data.value}; if ( - frame.description === 'cumulative-layout-shift' && - // frame.data.attributes && + clsFrame.description === 'cumulative-layout-shift' && + clsFrame.data.attributes && selectors ) { const layoutShifts: {[x: string]: ReactNode[]}[] = []; for (const attr of clsFrame.data.attributes) { const elements: ReactNode[] = []; - attr.nodeIds?.map(nodeId => { - return selectors.get(nodeId) + attr.nodeIds?.forEach(nodeId => { + selectors.get(nodeId) ? elements.push( - {'element'} + {t('element')} {': '} - {'unknown'} + {t('unknown')} ); } @@ -290,16 +290,18 @@ function WebVitalData({ const vitalKey = 'element'; webVitalData[vitalKey] = ( - {Array.from(selectors).map(([, key]) => { + {Array.from(selectors).map(([value, key]) => { return ( onMouseEnter(frame)} - onMouseLeave={() => onMouseLeave(frame)} + onMouseEnter={() => onMouseEnter(frame, value)} + onMouseLeave={() => onMouseLeave(frame, value)} > - {'element'} + {t('element')} {': '} - {key} + ); })} diff --git a/static/app/utils/replays/extractHtml.tsx b/static/app/utils/replays/extractHtml.tsx index d6db3b364f9c2..014beb3f59acc 100644 --- a/static/app/utils/replays/extractHtml.tsx +++ b/static/app/utils/replays/extractHtml.tsx @@ -6,61 +6,67 @@ import constructSelector from 'sentry/views/replays/deadRageClick/constructSelec export type Extraction = { frame: ReplayFrame; html: string[]; - selector: Map; + selectors: Map; timestamp: number; }; -export default function extractHtml(nodeIds: number[], mirror: Mirror): string[] { +export default function extractHtmlAndSelector( + nodeIds: number[], + mirror: Mirror +): {html: string[]; selectors: Map} { const htmlStrings: string[] = []; + const selectors = new Map(); for (const nodeId of nodeIds) { const node = mirror.getNode(nodeId); + if (node) { + const html = extractHtml(node); + if (html) { + htmlStrings.push(html); + } - 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); + const selector = extractSelector(node); + if (selector) { + selectors.set(nodeId, selector); + } } } - return htmlStrings; + return {html: htmlStrings, selectors}; } -export function extractSelectorFromNodeIds( - nodeIds: number[], - mirror: Mirror -): Map { - const selectors = new Map(); - for (const nodeId of nodeIds) { - const node = mirror.getNode(nodeId); +export function extractHtml(node: Node): string | null { + const html = + ('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) { + return truncated; + } + return null; +} - const element = node?.nodeType === Node.ELEMENT_NODE ? (node as HTMLElement) : null; +export function extractSelector(node: Node): string | null { + 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 - ); - } + if (element) { + return 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; + return null; } function removeChildLevel(max: number, collection: HTMLCollection, current: number = 0) { diff --git a/static/app/utils/replays/replayReader.tsx b/static/app/utils/replays/replayReader.tsx index 3989bfe36eb6a..318c3f59c2fa9 100644 --- a/static/app/utils/replays/replayReader.tsx +++ b/static/app/utils/replays/replayReader.tsx @@ -7,7 +7,7 @@ import {defined} from 'sentry/utils'; import domId from 'sentry/utils/domId'; import localStorageWrapper from 'sentry/utils/localStorage'; import clamp from 'sentry/utils/number/clamp'; -import extractHtml, {extractSelectorFromNodeIds} from 'sentry/utils/replays/extractHtml'; +import extractHtmlandSelector from 'sentry/utils/replays/extractHtml'; import hydrateBreadcrumbs, { replayInitBreadcrumb, } from 'sentry/utils/replays/hydrateBreadcrumbs'; @@ -151,12 +151,11 @@ const extractDomNodes = { onVisitFrame: (frame, collection, replayer) => { const mirror = replayer.getMirror(); const nodeIds = getNodeIds(frame); - const html = extractHtml(nodeIds as number[], mirror); - const selector = extractSelectorFromNodeIds(nodeIds as number[], mirror); + const {html, selectors} = extractHtmlandSelector((nodeIds ?? []) as number[], mirror); collection.set(frame as ReplayFrame, { frame, html, - selector, + selectors, timestamp: frame.timestampMs, }); }, From 99eef0bbb1ff652afd27bc0f73a995f59163feaf Mon Sep 17 00:00:00 2001 From: Catherine Lee <55311782+c298lee@users.noreply.github.com> Date: Thu, 29 Aug 2024 19:52:45 -0400 Subject: [PATCH 11/15] Update static/app/components/replays/breadcrumbs/breadcrumbItem.tsx Co-authored-by: Ryan Albrecht --- static/app/components/replays/breadcrumbs/breadcrumbItem.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx b/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx index 541f6d4e93401..085c4f8065f3e 100644 --- a/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx +++ b/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx @@ -262,7 +262,7 @@ function WebVitalData({ onMouseEnter={() => onMouseEnter(clsFrame, nodeId)} onMouseLeave={() => onMouseLeave(clsFrame, nodeId)} > - {'element'} + {t('element')} {': '} {selectors.get(nodeId)} From f0b23be6e66049b306c10c5324c8cba5ebdb16c4 Mon Sep 17 00:00:00 2001 From: Catherine Lee <55311782+c298lee@users.noreply.github.com> Date: Mon, 2 Sep 2024 20:25:59 -0400 Subject: [PATCH 12/15] more PR comments --- .../replays/breadcrumbs/breadcrumbItem.tsx | 43 ++++++++----------- 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx b/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx index 085c4f8065f3e..cf348e6065816 100644 --- a/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx +++ b/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx @@ -247,8 +247,8 @@ function WebVitalData({ const webVitalData = {value: frame.data.value}; if ( - clsFrame.description === 'cumulative-layout-shift' && - clsFrame.data.attributes && + frame.description === 'cumulative-layout-shift' && + 'attributes' in frame.data && selectors ) { const layoutShifts: {[x: string]: ReactNode[]}[] = []; @@ -287,29 +287,24 @@ function WebVitalData({ webVitalData['Layout shifts'] = layoutShifts; } } else if (selectors?.size) { - const vitalKey = 'element'; - webVitalData[vitalKey] = ( - - {Array.from(selectors).map(([value, key]) => { - return ( - onMouseEnter(frame, value)} - onMouseLeave={() => onMouseLeave(frame, value)} - > - {t('element')} - {': '} - - - ); - })} - - ); + selectors.forEach((key, value) => { + webVitalData[key] = ( + onMouseEnter(frame, value)} + onMouseLeave={() => onMouseLeave(frame, value)} + > + {t('element')} + {': '} + + {key} + + + ); + }); } - return webVitalData ? ( + return ( { @@ -321,7 +316,7 @@ function WebVitalData({ data={webVitalData} withAnnotatedText /> - ) : null; + ); } function CrumbHydrationButton({ From 69c5ab99258462df40e3f0188ae3077fe75ad058 Mon Sep 17 00:00:00 2001 From: Catherine Lee <55311782+c298lee@users.noreply.github.com> Date: Tue, 3 Sep 2024 10:58:50 -0400 Subject: [PATCH 13/15] add stop propagation to onClick --- .../components/replays/breadcrumbs/breadcrumbItem.tsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx b/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx index cf348e6065816..97934139369c6 100644 --- a/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx +++ b/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx @@ -161,7 +161,10 @@ function BreadcrumbItem({ data-is-error-frame={isErrorFrame(frame)} style={style} className={className} - onClick={() => onClick?.(frame)} + onClick={event => { + event.stopPropagation(); + onClick?.(frame); + }} onMouseEnter={() => onMouseEnter(frame)} onMouseLeave={() => onMouseLeave(frame)} > @@ -179,7 +182,10 @@ function BreadcrumbItem({ onClick?.(frame)} + onClick={event => { + event.stopPropagation(); + onClick?.(frame); + }} onMouseEnter={() => onMouseEnter(frame)} onMouseLeave={() => onMouseLeave(frame)} style={style} From 41e47cb549f93cc04bb9b50193cc0fc4fa0dc687 Mon Sep 17 00:00:00 2001 From: Catherine Lee <55311782+c298lee@users.noreply.github.com> Date: Tue, 3 Sep 2024 15:28:39 -0400 Subject: [PATCH 14/15] more nits + use real data --- .../replays/breadcrumbs/breadcrumbItem.tsx | 53 +++++++------------ static/app/utils/replays/extractHtml.tsx | 4 +- static/app/utils/replays/highlightNode.tsx | 10 +--- 3 files changed, 24 insertions(+), 43 deletions(-) diff --git a/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx b/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx index 97934139369c6..15e8963d2bf18 100644 --- a/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx +++ b/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx @@ -233,50 +233,37 @@ function WebVitalData({ 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]}, - ], - }, - }; - const {data: frameToExtraction} = useExtractDomNodes({replay}); const selectors = frameToExtraction?.get(frame)?.selectors; const webVitalData = {value: frame.data.value}; if ( frame.description === 'cumulative-layout-shift' && - 'attributes' in frame.data && + frame.data.attributions && selectors ) { const layoutShifts: {[x: string]: ReactNode[]}[] = []; - for (const attr of clsFrame.data.attributes) { + for (const attr of frame.data.attributions) { const elements: ReactNode[] = []; - attr.nodeIds?.forEach(nodeId => { - selectors.get(nodeId) - ? elements.push( - onMouseEnter(clsFrame, nodeId)} - onMouseLeave={() => onMouseLeave(clsFrame, nodeId)} - > - {t('element')} - {': '} - - {selectors.get(nodeId)} + if ('nodeIds' in attr && Array.isArray(attr.nodeIds)) { + attr.nodeIds.forEach(nodeId => { + selectors.get(nodeId) + ? elements.push( + onMouseEnter(frame, nodeId)} + onMouseLeave={() => onMouseLeave(frame, nodeId)} + > + {t('element')} + {': '} + + {selectors.get(nodeId)} + - - ) - : null; - }); + ) + : 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( diff --git a/static/app/utils/replays/extractHtml.tsx b/static/app/utils/replays/extractHtml.tsx index 014beb3f59acc..aedc99f40d6a1 100644 --- a/static/app/utils/replays/extractHtml.tsx +++ b/static/app/utils/replays/extractHtml.tsx @@ -33,7 +33,7 @@ export default function extractHtmlAndSelector( return {html: htmlStrings, selectors}; } -export function extractHtml(node: Node): string | null { +function extractHtml(node: Node): string | null { const html = ('outerHTML' in node ? (node.outerHTML as string) : node.textContent) || ''; // Limit document node depth to 2 @@ -48,7 +48,7 @@ export function extractHtml(node: Node): string | null { return null; } -export function extractSelector(node: Node): string | null { +function extractSelector(node: Node): string | null { const element = node.nodeType === Node.ELEMENT_NODE ? (node as HTMLElement) : null; if (element) { diff --git a/static/app/utils/replays/highlightNode.tsx b/static/app/utils/replays/highlightNode.tsx index 4c72fc3cf0591..630f20ed9d7ea 100644 --- a/static/app/utils/replays/highlightNode.tsx +++ b/static/app/utils/replays/highlightNode.tsx @@ -63,7 +63,6 @@ export function removeHighlightedNode(replayer: Replayer, props: RemoveHighlight export function highlightNode(replayer: Replayer, props: AddHighlightParams) { const {wrapper} = replayer; const mirror = replayer.getMirror(); - const canvases: {canvas: HTMLCanvasElement}[] = []; const nodes = 'nodeIds' in props @@ -78,7 +77,7 @@ export function highlightNode(replayer: Replayer, props: AddHighlightParams) { !('getBoundingClientRect' in node) || !replayer.iframe.contentDocument?.body?.contains(node) ) { - return null; + continue; } // Create a new canvas with the same dimensions as the iframe. We may need to @@ -87,7 +86,7 @@ export function highlightNode(replayer: Replayer, props: AddHighlightParams) { const element = node.nodeType === Node.ELEMENT_NODE ? (node as HTMLElement) : null; if (!element) { - return null; + continue; } const canvas = document.createElement('canvas'); @@ -111,12 +110,7 @@ export function highlightNode(replayer: Replayer, props: AddHighlightParams) { } wrapper.insertBefore(canvas, replayer.iframe); - - canvases.push({ - canvas, - }); } - return canvases; } function drawCtx( From 43aeaae22ec110bc0b0b90c979ba7d529027d189 Mon Sep 17 00:00:00 2001 From: Catherine Lee <55311782+c298lee@users.noreply.github.com> Date: Tue, 3 Sep 2024 17:07:57 -0400 Subject: [PATCH 15/15] boolean logic :( --- static/app/components/replays/breadcrumbs/breadcrumbItem.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx b/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx index 15e8963d2bf18..5b0fb28f1b191 100644 --- a/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx +++ b/static/app/components/replays/breadcrumbs/breadcrumbItem.tsx @@ -122,7 +122,7 @@ function BreadcrumbItem({ const renderCodeSnippet = useCallback(() => { return ( - (!isSpanFrame(frame) || (isSpanFrame(frame) && !isWebVitalFrame(frame))) && + (!isSpanFrame(frame) || !isWebVitalFrame(frame)) && extraction?.html?.map(html => (