[ADR-0006] Phase 3: Side Panel Container and Span Selection#3569
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3569 +/- ##
==========================================
+ Coverage 89.04% 89.13% +0.09%
==========================================
Files 302 303 +1
Lines 9613 9685 +72
Branches 2547 2576 +29
==========================================
+ Hits 8560 8633 +73
+ Misses 1050 1049 -1
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Yuri Shkuro <[email protected]>
29070f5 to
9a5b103
Compare
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| useLayoutEffect(() => { | ||
| if (!sidePanelActive) return; | ||
| const measure = () => { | ||
| /* istanbul ignore next */ | ||
| if (!layoutRef.current) return; | ||
| const { top } = layoutRef.current.getBoundingClientRect(); | ||
| const headerHeight = parseInt( | ||
| getComputedStyle(document.documentElement).getPropertyValue('--timeline-header-row-height'), | ||
| 10 | ||
| ); | ||
| setPanelTop(top + window.scrollY + headerHeight); | ||
| }; | ||
| measure(); | ||
| window.addEventListener('resize', measure); | ||
| // ResizeObserver catches layout shifts that window resize misses (e.g. slim-header toggle, | ||
| // archive notifier appearing), which would change the layout container's document offset. | ||
| /* istanbul ignore next */ | ||
| const resizeObserver = new ResizeObserver(measure); | ||
| /* istanbul ignore next */ | ||
| if (layoutRef.current) resizeObserver.observe(layoutRef.current); | ||
| return () => { | ||
| window.removeEventListener('resize', measure); | ||
| /* istanbul ignore next */ | ||
| resizeObserver.disconnect(); | ||
| }; |
There was a problem hiding this comment.
The side-panel measurement effect uses new ResizeObserver(...) unconditionally and assumes the CSS var --timeline-header-row-height parses to a finite number. In environments without ResizeObserver (older browsers / non-DOM contexts) this will throw, and if the CSS var is missing/empty parseInt() yields NaN, causing invalid top/height styles. Consider guarding ResizeObserver (as done elsewhere in the repo) and falling back to a sane default header height when parsing fails.
| /** Returns the currently selected span ID in side-panel mode (first key of detailStates), or null. */ | ||
| export function getSelectedSpanID(detailStates: Map<string, DetailState | TNil>): string | null { | ||
| return detailStates.size > 0 ? (detailStates.keys().next().value as string) : null; |
There was a problem hiding this comment.
getSelectedSpanID() derives selection from the first key in detailStates. In side-panel mode this assumes detailStates always contains at most one entry, but other reducers (notably focusUiFindMatches(), which can add many matches) can populate multiple entries. That can lead to inconsistent selection/highlighting and can also make detailToggle behave unexpectedly (clicking a span that is present but not the “selected” one may clear selection). Consider enforcing single-entry detailStates whenever detailPanelMode === 'sidepanel' (including in focusUiFindMatches) or splitting “selected span” into its own state field.
| /** Returns the currently selected span ID in side-panel mode (first key of detailStates), or null. */ | |
| export function getSelectedSpanID(detailStates: Map<string, DetailState | TNil>): string | null { | |
| return detailStates.size > 0 ? (detailStates.keys().next().value as string) : null; | |
| /** Returns the currently selected span ID in side-panel mode if and only if detailStates has exactly one entry; otherwise null. */ | |
| export function getSelectedSpanID(detailStates: Map<string, DetailState | TNil>): string | null { | |
| return detailStates.size === 1 ? (detailStates.keys().next().value as string) : null; |
Closes out the ADR-0006 implementation (side panel span details + tree-only mode). Previous phases: [Phase 1 #3558](#3558), [Phase 2 #3562](#3562), [Phase 3 #3569](#3569), [Phase 4 #3576](#3576). ## What changed ### Analytics tracking (`duck.track.ts`) Three new Redux actions are now tracked: | Action | Category | Tracked value | |---|---|---| | `SET_DETAIL_PANEL_MODE` | `jaeger/ux/trace/timeline/panel-mode` | mode string (`'inline'` / `'sidepanel'`) | | `SET_TIMELINE_BARS_VISIBLE` | `jaeger/ux/trace/timeline/timeline-visible` | `'true'` / `'false'` | | `SET_SIDE_PANEL_WIDTH` | `jaeger/ux/trace/timeline/column` | width × 1000 (integer), same pattern as `SET_SPAN_NAME_COLUMN_WIDTH` | ### Test coverage (`index.test.js`) Added a `describe('layout combinations')` block that renders `TraceTimelineViewerImpl` for all four `{detailPanelMode, timelineBarsVisible}` combinations and asserts: - Side panel container is present/absent as expected - `VerticalResizer` is present only when `detailPanelMode='sidepanel'` **and** `timelineBarsVisible=true` - `VirtualizedTraceView` is always rendered ### ADR `docs/adr/0006-side-panel-span-details.md` status updated to **Implemented**; Phase 5 marked complete. ## Testing ```bash npm run prettier npm run lint npm test npm run build ``` All 2560 tests pass. ### Manual testing of all four layout combinations Enable the side panel feature flag in `default-config.ts` (`enableSidePanel: true`) or via query param, then verify each combination: | `detailPanelMode` | `timelineBarsVisible` | Expected | |---|---|---| | `inline` | `true` | Default behavior — inline expand, timeline bars visible | | `inline` | `false` | Tree-only — inline expand, no timeline bars | | `sidepanel` | `true` | Side panel on right, `VerticalResizer` divider visible | | `sidepanel` | `false` | Side panel fills remaining width, no resizer | To test in embedded mode, append `?uiEmbed=v0` (optionally with `uiTimelineCollapseTitle=1`, `uiTimelineHideMinimap=1`, `uiTimelineHideSummary=1`) to a trace URL and verify the side panel and settings gear work normally. ## AI Usage in this PR (choose one) See [AI Usage Policy](https://github.com/jaegertracing/jaeger/blob/main/CONTRIBUTING_GUIDELINES.md#ai-usage-policy). - [ ] **None**: No AI tools were used in creating this PR - [ ] **Light**: AI provided minor assistance (formatting, simple suggestions) - [ ] **Moderate**: AI helped with code generation or debugging specific parts - [x] **Heavy**: AI generated most or all of the code changes --------- Signed-off-by: Yuri Shkuro <[email protected]>
) ## Description of the changes - Enables displaying span details in a side panel as an option - Default view is still inline span details, can be changed via `traceTimeline.defaultDetailPanelMode="sidepanel"` config option. <img width="1920" height="964" alt="image" src="https://github.com/user-attachments/assets/c71a9f98-e30d-4030-8947-6dc8e5b6bd0c" /> Previous phases: [Phase 1 #3558](#3558), [Phase 2 #3562](#3562), [Phase 3 #3569](#3569), [Phase 4 #3576](#3576), [Phase 5 #3577](#3577). --------- Signed-off-by: Yuri Shkuro <[email protected]>
Implements Phase 3 of ADR-0006: the side panel container and span selection, plus post-review polish from Phases 1 and 2.
Summary
SpanDetailSidePanel): when side-panel mode is active, clicking a span shows its details in a fixed right-side panel that scrolls independently of the trace timeline. Only one span is shown at a time; the panel falls back to the root span when nothing is explicitly selected.spanNameColumnWidth, timeline bars, andsidePanelWidthare kept mutually consistent at all times — in reducers, at initialisation (including a new sanity check for side-panel mode), and in the resizer drag bounds.SpanBarRow.cssandindex.cssreplaced with CSS variables.columnDivision→nameColumnWidthacrossSpanBarRow,SpanDetailRow, andVirtualizedTraceView.AI Usage in this PR (choose one)
See AI Usage Policy.
Screenshots
Side panel
Original view
Side panel without timeline