feat(tracing): virtualize spans list and flame chart#60738
Merged
jonmcwest merged 1 commit intoJun 1, 2026
Conversation
2 tasks
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Contributor
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
products/tracing/frontend/components/VirtualizedSpanList/VirtualizedSpanList.tsx:116-133
**`ariaAttributes` declared but never applied**
`react-window` passes `ariaAttributes` to each row component for things like `aria-rowindex` and `aria-setsize` so that screen readers can report the row's position within the full list. By declaring the prop in the type but never destructuring or spreading it onto the wrapper `<div>`, every virtual-list accessibility cue is silently discarded. The same issue exists in `FlameListRow` in `TraceFlameChart.tsx`. Spread `{...ariaAttributes}` onto the outermost `<div>` in each row component to restore the behaviour.
### Issue 2 of 2
products/tracing/frontend/components/VirtualizedSpanList/VirtualizedSpanList.tsx:85-88
`SpanRow` uses `tabIndex={-1}`, which removes the row from the normal Tab order entirely. The previous `LemonTable` rows were keyboard-reachable; this is a regression. `FlameRow` uses `tabIndex={0}` — a consistent choice here would be `tabIndex={0}` with the same `onKeyDown` handler pattern used there.
```suggestion
className="flex h-full items-center cursor-pointer border-b border-border hover:bg-surface-primary-hover"
onClick={onClick}
onKeyDown={(e) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault()
onClick()
}
}}
role="button"
tabIndex={0}
```
Reviews (1): Last reviewed commit: "feat(tracing): virtualize spans list and..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+116
to
+133
| function SpanListRow({ | ||
| index, | ||
| style, | ||
| dataSource, | ||
| onRowClick, | ||
| }: { | ||
| ariaAttributes: Record<string, unknown> | ||
| index: number | ||
| style: CSSProperties | ||
| } & SpanRowProps): JSX.Element { | ||
| const span = dataSource[index] | ||
| return ( | ||
| // eslint-disable-next-line react/forbid-dom-props | ||
| <div style={style} data-index={index} data-row-key={span.uuid}> | ||
| <SpanRow span={span} onClick={() => onRowClick(span)} /> | ||
| </div> | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
ariaAttributes declared but never applied
react-window passes ariaAttributes to each row component for things like aria-rowindex and aria-setsize so that screen readers can report the row's position within the full list. By declaring the prop in the type but never destructuring or spreading it onto the wrapper <div>, every virtual-list accessibility cue is silently discarded. The same issue exists in FlameListRow in TraceFlameChart.tsx. Spread {...ariaAttributes} onto the outermost <div> in each row component to restore the behaviour.
Prompt To Fix With AI
This is a comment left during a code review.
Path: products/tracing/frontend/components/VirtualizedSpanList/VirtualizedSpanList.tsx
Line: 116-133
Comment:
**`ariaAttributes` declared but never applied**
`react-window` passes `ariaAttributes` to each row component for things like `aria-rowindex` and `aria-setsize` so that screen readers can report the row's position within the full list. By declaring the prop in the type but never destructuring or spreading it onto the wrapper `<div>`, every virtual-list accessibility cue is silently discarded. The same issue exists in `FlameListRow` in `TraceFlameChart.tsx`. Spread `{...ariaAttributes}` onto the outermost `<div>` in each row component to restore the behaviour.
How can I resolve this? If you propose a fix, please make it concise.
jonmcwest
force-pushed
the
05-29-feat_tracing_add_keyset_pagination_to_spans_query_endpoint
branch
from
7553e8e to
81c534e
Compare
jonmcwest
force-pushed
the
05-29-feat_tracing_virtualize_spans_list_and_flame_chart
branch
2 times, most recently
from
7a4d5a7 to
5a26b01
Compare
jonmcwest
changed the base branch from
05-29-feat_tracing_add_keyset_pagination_to_spans_query_endpoint
to
graphite-base/60738
jonmcwest
force-pushed
the
05-29-feat_tracing_virtualize_spans_list_and_flame_chart
branch
from
5a26b01 to
f9e09df
Compare
jonmcwest
force-pushed
the
graphite-base/60738
branch
from
81c534e to
1b706e2
Compare
jonmcwest
force-pushed
the
05-29-feat_tracing_virtualize_spans_list_and_flame_chart
branch
from
f9e09df to
08f639e
Compare
jonmcwest
force-pushed
the
05-29-feat_tracing_virtualize_spans_list_and_flame_chart
branch
from
08f639e to
d0ede8a
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Contributor
|
Size Change: +9.57 kB (+0.01%) Total Size: 80.9 MB 📦 View Changed
ℹ️ View Unchanged
|
Contributor
|
🎭 Playwright report · View test results →
These issues are not necessarily caused by your changes. |
jonmcwest
added
stamphog
Graphite App
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
DanielVisca
approved these changes
Jun 1, 2026
Contributor
Author
Merge activity
|
jonmcwest
deleted the
05-29-feat_tracing_virtualize_spans_list_and_flame_chart
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The tracing span list was rendered as a plain
LemonTable, which mounted every row in the DOM at once. For traces with hundreds of spans this caused noticeable jank and made the modal grow unboundedly. Similarly, the flame chart rendered all span rows without any height cap, so long traces pushed the modal off-screen.Changes
Virtualized span list (
VirtualizedSpanList)LemonTableinTracingScenewith a newVirtualizedSpanListcomponent backed byreact-window.onLoadMoreis called automatically.onVisibleRowRangeChangeso the sparkline can highlight the corresponding time window.Virtualized flame chart
TraceFlameChartare now rendered through areact-windowListwithuseDynamicRowHeightso the detail panel (which expands inline when a span is selected) is measured and accounted for.MAX_FLAME_HEIGHT); taller traces scroll inside the chart instead of growing the modal.FlameRow/FlameListRowcomponents.Sparkline visible-range highlight
tracingDataLogicgains avisibleRowRangereducer and avisibleRowDateRangeselector that converts the visible row indices into an earliest-first timestamp range derived from the root spans.TracingSparklineaccepts an optionalvisibleRowDateRangeprop and maps it onto bucket indices to pass ahighlightedRangeto the underlying chart, mirroring the behaviour already present in the logs viewer.Scene layout
SceneContentis given an explicit height (calc(var(--scene-layout-rect-height) - 1rem)) so the virtualized list can fill the remaining space without the page growing.How did you test this code?
Unit tests were added for
tracingDataLogiccovering:nullstate forvisibleRowRangeandvisibleRowDateRange.setVisibleRowRange.clearSpansis dispatched.