Skip to content

fix(app): fix session replay sub-event modal stacking and tab conflict#2068

Merged
kodiakhq[bot] merged 8 commits intomainfrom
brandon/session-replay-modal-fix
Apr 8, 2026
Merged

fix(app): fix session replay sub-event modal stacking and tab conflict#2068
kodiakhq[bot] merged 8 commits intomainfrom
brandon/session-replay-modal-fix

Conversation

@brandon-pereira
Copy link
Copy Markdown
Member

@brandon-pereira brandon-pereira commented Apr 7, 2026

Summary

Clicking a log/error event in the session replay event list either reopened the session replay instead of showing event details, or rendered the detail panel behind the replay drawer.

Fixed this by ensuring that isSubPanel is correctly set and using the ZIndexProvider to correctly stack the contexts.

Steps to Reproduce

From the Sessions page:

  1. Go to /sessions, select a session source, open a session card
  2. In the session replay drawer, wait for the event list to load
  3. Click any event row (e.g. a console.error)
  4. Bug A: The detail panel opens behind the session replay drawer (overlay darkens but panel is inaccessible), or
    ESC/close doesn't work correctly

From the Search page (URL conflict):

  1. Go to /search, open any trace row to open the detail side panel
  2. Click the Session Replay tab — this sets sidePanelTab=replay in the URL
  3. In the session event list, click any event row
  4. Bug B: The inner detail panel opens to the Session Replay tab again instead of event details (e.g. Overview/Trace)

When clicking a log/error event inside the session replay event list, the
detail panel was opening behind the session replay drawer (z-index issue)
and, when opened from the search page's Session Replay tab, was re-opening
the session replay instead of showing event details (URL param conflict).

Root causes:
- SessionSubpanel rendered DBRowSidePanel via <Portal> without wrapping it
  in ZIndexContext.Provider, so the sub-panel did not inherit the correct
  z-index value from the parent session replay drawer.
- DBRowSidePanel was not passed isNestedPanel=true, causing it to read the
  sidePanelTab URL param (set to 'replay' by the outer panel) and open to
  the Session Replay tab again instead of event details.
- withOverlay={!isNestedPanel} removed the overlay on nested panels, making
  it impossible to close by clicking outside.

Fixes:
- Wrap Portal-rendered DBRowSidePanel with ZIndexContext.Provider to ensure
  correct z-index propagation through the Portal boundary.
- Pass isNestedPanel=true so the inner panel uses local state for tab
  selection (ignoring the URL param) and renders with an overlay.
- Remove withOverlay={!isNestedPanel} so all drawer instances always render
  with an overlay (click-outside-to-close works).

Also adds:
- data-testid on SessionEventList rows and SessionSidePanel for testability.
- data-testid on the Session Replay tab content wrapper in DBRowSidePanel.
- E2E test that reproduces the exact URL conflict regression by injecting
  sidePanelTab=replay into the URL before opening a session event, then
  asserting the detail panel does not open to the replay tab.
- Seed data: session traces now include routeChange and console.error events
  so the session event list is populated in E2E tests.
@vercel
Copy link
Copy Markdown

vercel bot commented Apr 7, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview, Comment Apr 8, 2026 4:12pm

Request Review

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Apr 7, 2026

🦋 Changeset detected

Latest commit: 6c709ec

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@hyperdx/app Patch
@hyperdx/api Patch
@hyperdx/otel-collector Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

E2E Test Results

All tests passed • 131 passed • 3 skipped • 1067s

Status Count
✅ Passed 131
❌ Failed 0
⚠️ Flaky 2
⏭️ Skipped 3

Tests ran across 4 shards in parallel.

View full report →

setRowId(undefined);
}}
/>
<ZIndexContext.Provider value={contextZIndex}>
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This was the actual file to fix the issue - the rest is adding/improving e2e tests

@brandon-pereira brandon-pereira marked this pull request as ready for review April 7, 2026 20:43
@github-actions github-actions bot added the review/tier-3 Standard — full human review required label Apr 7, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

🟡 Tier 3 — Standard

Introduces new logic, modifies core functionality, or touches areas with non-trivial risk.

Why this tier:

  • Standard feature/fix — introduces new logic or modifies core functionality

Review process: Full human review — logic, architecture, edge cases.
SLA: First-pass feedback within 1 business day.

Stats
  • Files changed: 8
  • Lines changed: 182
  • Branch: brandon/session-replay-modal-fix
  • Author: brandon-pereira

To override this classification, remove the review/tier-3 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@brandon-pereira
Copy link
Copy Markdown
Member Author

@teeohhem not sure you agree, but maybe this should be tier 2. It is a very small change but the surrounding test improvements increased the tier if I had to guess.

@brandon-pereira brandon-pereira requested review from a team and pulpdrew and removed request for a team April 7, 2026 20:45
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

PR Review

Fix is logically sound. The z-index propagation is correct: DBRowSidePanelErrorBoundary already adds +10 to contextZIndex (line 560), so wrapping the inner panel in <ZIndexContext.Provider value={contextZIndex}> ensures the inner drawer renders above the outer session replay panel.

  • ⚠️ Removing withOverlay={!isNestedPanel} affects all callers with isNestedPanel=true — not just SessionSubpanel. At least 3 other components (PodDetailsSidePanel, PatternSidePanel, ContextSidePanel) pass isNestedPanel={true} and previously had no overlay. These nested panels will now show overlays where they didn't before. This is likely an improvement (was buggy to not have one), but warrants a quick visual check in those components to confirm no unintended UI regressions.

✅ No critical bugs or security issues. The core fix is correct.

@pulpdrew
Copy link
Copy Markdown
Contributor

pulpdrew commented Apr 8, 2026

Is it expected that clicking outside of the panel does not close the panel? This seems like a regression compared to the play environment.

Screen.Recording.2026-04-08.at.8.14.23.AM.mov

Also, would it be possible to use show correct breadcrumbs when navigating from Search Page Session Panel --> New Side Drawer? That might be beyond the scope here, perhaps another ticket for that would be good.

Screen.Recording.2026-04-08.at.8.20.49.AM.mov

@elizabetdev
Copy link
Copy Markdown
Contributor

@brandon-pereira, @pulpdrew,

As part of the new Trace Viewers prototype, we want to improve how drawers work.

The idea is to replace stacked/layered drawers with a single right-hand Mantine drawer, where all navigation happens in place. With each new navigation step, a new item is added to the breadcrumb.

Linear ticket: https://linear.app/clickhouse/issue/HDX-3942/single-drawer-navigation-with-breadcrumb-stack

Prototype: #1970
(Some parts are still rough, but it should give a clear idea of what I’m aiming for.)

sessions.mp4

@karl-power will be working on this, and since you, @brandon-pereira, have been doing more work on drawers, maybe you both could coordinate on how to achieve the “single-drawer navigation with breadcrumb stack.”

@brandon-pereira
Copy link
Copy Markdown
Member Author

@brandon-pereira, @pulpdrew,

As part of the new Trace Viewers prototype, we want to improve how drawers work.

The idea is to replace stacked/layered drawers with a single right-hand Mantine drawer, where all navigation happens in place. With each new navigation step, a new item is added to the breadcrumb.

Linear ticket: https://linear.app/clickhouse/issue/HDX-3942/single-drawer-navigation-with-breadcrumb-stack

Prototype: #1970 (Some parts are still rough, but it should give a clear idea of what I’m aiming for.)

sessions.mp4
@karl-power will be working on this, and since you, @brandon-pereira, have been doing more work on drawers, maybe you both could coordinate on how to achieve the “single-drawer navigation with breadcrumb stack.”

Good call @elizabetdev. Given this is a customer reported issue - I believe we should expedite shipping this feature, I will leave the breadcrumbs out of scope and work with @karl-power on the future UI state.

@brandon-pereira
Copy link
Copy Markdown
Member Author

Is it expected that clicking outside of the panel does not close the panel? This seems like a regression compared to the play environment.

Good catch - fixed that. Also improved the e2e test to catch regressions in the future

Copy link
Copy Markdown
Contributor

@pulpdrew pulpdrew left a comment

Choose a reason for hiding this comment

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

LGTM

@kodiakhq kodiakhq bot merged commit 52986a9 into main Apr 8, 2026
17 checks passed
@kodiakhq kodiakhq bot deleted the brandon/session-replay-modal-fix branch April 8, 2026 16:17
knudtty pushed a commit that referenced this pull request Apr 16, 2026
#2068)

## Summary

Clicking a log/error event in the session replay event list either reopened the session replay instead of showing event details, or rendered the detail panel behind the replay drawer.

Fixed this by ensuring that isSubPanel is correctly set and using the ZIndexProvider to correctly stack the contexts.

## Steps to Reproduce

From the Sessions page:

1. Go to /sessions, select a session source, open a session card
2. In the session replay drawer, wait for the event list to load
3. Click any event row (e.g. a console.error)
4. Bug A: The detail panel opens behind the session replay drawer (overlay darkens but panel is inaccessible), or 
ESC/close doesn't work correctly

From the Search page (URL conflict):

1. Go to /search, open any trace row to open the detail side panel
2. Click the Session Replay tab — this sets sidePanelTab=replay in the URL
3. In the session event list, click any event row
4. Bug B: The inner detail panel opens to the Session Replay tab again instead of event details (e.g. Overview/Trace)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge review/tier-3 Standard — full human review required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants