feat(replay): Web Vital Breadcrumb Design #76320
Conversation
github-actions
bot
added
the
Scope: Frontend
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #76320 +/- ##
==========================================
+ Coverage 78.15% 78.22% +0.07%
==========================================
Files 6910 6895 -15
Lines 307183 306364 -819
Branches 50351 50225 -126
==========================================
- Hits 240073 239655 -418
+ Misses 60755 60323 -432
- Partials 6355 6386 +31 |
| // 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]}, | ||
| ], | ||
| }, | ||
| }; |
There was a problem hiding this comment.
to be removed once the sdk is updated
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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).
|
|
||
| type RemoveHighlightParams = | ||
| | { | ||
| nodeId: number; | ||
| nodeIds: number[]; |
There was a problem hiding this comment.
Will the new sdk version break the UI in self hosted sentry sending data on this old version/format?
There was a problem hiding this comment.
Looks like this method isn't called directly with data from the sdk, instead something like static/app/utils/replays/hooks/useCrumbHandlers.tsx is fixing the data first with code like this, so we should be good.:
if ('nodeId' in data) {
return {nodeIds: [data.nodeId], annotation: record.data.label};
}
if ('nodeIds' in data) {
return {nodeIds: data.nodeIds, annotation: record.data.label};
}
There was a problem hiding this comment.
a better example is: getNodeIds in static/app/utils/replays/types.tsx
| @@ -38,7 +33,6 @@ export default function useVirtualizedInspector({cache, listRef, expandPathsRef} | |||
| } | |||
| expandPathsRef.current?.set(index, rowState); | |||
| handleDimensionChange(index); | |||
| event.stopPropagation(); | |||
There was a problem hiding this comment.
Does this line need to move up into the onClick callbacks themselves? it should be moved somewhere i think
| 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; | ||
| `; |
There was a problem hiding this comment.
might be able to get away with something like <Button size="zero" borderless>...</Button> instead of having to write so much new css
There was a problem hiding this comment.
we could do that! would look like this instead:
Co-authored-by: Ryan Albrecht <ryan.albrecht@sentry.io>
Bundle ReportChanges will increase total bundle size by 4.65kB ⬆️
|
|
|
||
| const renderCodeSnippet = useCallback(() => { | ||
| return ( | ||
| (!isSpanFrame(frame) || (isSpanFrame(frame) && !isWebVitalFrame(frame))) && |
There was a problem hiding this comment.
nit: Isn't the second isSpanFrame() check redundant? e.g. not a OR (a AND b) == not A OR b
There was a problem hiding this comment.
great catch! boolean logic hurts my brain :(
Implements the newest web vital breadcrumbs design: https://www.figma.com/design/bZIT1O93bdxTNbNWDUvCyQ/Specs%3A-Breadcrumbs-%26-Web-Vitals?node-id=277-336&node-type=CANVAS&t=ccSNvPkjFTZWWpDY-0
For web vitals,
nodeIdwas changed tonodeIds, which means there could be multiple elements associated with a breadcrumb item. Highlighting and code snippet extraction were updated to accept multiplenodeIds. When hovering over a web vital breadcrumb, all the associated elements will be highlighted, and when hovering over a selector, only that associated element would be highlighted.Since the SDK changes have not been merged and released yet, the CLS web vital is currently using web vitals to show what they would look like.
Screen.Recording.2024-08-28.at.7.01.44.PM.mov
Closes #69881