Skip to content

[EuiDataGrid] Fix cell popover hidden behind sibling/nested flyouts#9630

Open
acstll wants to merge 4 commits intoelastic:mainfrom
acstll:fix-8801-cell-popover-zindex
Open

[EuiDataGrid] Fix cell popover hidden behind sibling/nested flyouts#9630
acstll wants to merge 4 commits intoelastic:mainfrom
acstll:fix-8801-cell-popover-zindex

Conversation

@acstll
Copy link
Copy Markdown
Contributor

@acstll acstll commented Apr 30, 2026

Summary

Resolves #8801

Fixes a bug where cell popovers are not visible in sibling/nested flyouts because of z-index. For a detailed explanation of the approach, please see #8801 (comment)

API Changes

N/A

Screenshots

The screenshot represent the "after the fix" state. The expanded cell content popover in the screenshot is not visible without the fix.

Screenshot 2026-05-06 at 12 08 04

Impact Assessment

Note: Most PRs should be tested in Kibana to help gauge their Impact before merging.

  • 🔴 Breaking changes — What will break? How many usages in Kibana/Cloud UI are impacted?
  • 💅 Visual changes — May impact style overrides; could require visual testing. Explain and estimate impact.
  • 🧪 Test impact — May break functional or snapshot tests (e.g., HTML structure, class names, default values).
  • 🔧 Hard to integrate — If changes require substantial updates to Kibana, please stage the changes and link them here.

Impact level: 🟢 None

This is a bug fix. It allows removing a workaround, as in elastic/kibana#251434

✔︎ Tested in Kibana → CI run in elastic/kibana#267791

Release Readiness

  • Documentation: {link to docs page(s)}
  • Figma: {link to Figma or issue}
  • Migration guide: {steps or link, for breaking/visual changes or deprecations}
  • Adoption plan (new features): {link to issue/doc or outline who will integrate this and where}

No migration or adoption needed, only the temporary workaround can be removed. Tracking issue: elastic/kibana#252119

QA instructions for reviewer

The only way to test this properly is running Kibana. You need to get to the Traces feature in Discover, which is only available with APM data (you cannot toggle it manually). The page you're looking for is that below, if you don't know how to get there, follow the instructions.

  • checkout the branch in PR [EUI] CI run for #9630 kibana#267791
  • (check instructions below if needed)
  • toggle cell popovers in Traces in the child flyout, check they're visible (happy path)
  • revert the local build of eui to use the latest published version (edit package.json, yarn kbn bootstrap, etc.)
  • toggle cell popovers in Traces in the child flyout, check they're not visible (sad path)
Screenshot 2026-05-06 at 12 04 08

Instructions to see Traces in Discover

  • start Kibana locally, don't forget to run yarn kbn bootstrap
  • create a space with an Observability solution view, in Stack Management \ Spaces, and switch to it
  • run node scripts/synthtrace simple_trace.ts --target=http://elastic:changeme@localhost:9200 in the terminal to feed APM data to ES
  • navigate to Applications \ Traces
  • click on one of the list items e.g. "240rpm/75% 1000ms"
  • scroll down, click on the "Open full trace in Discover" button
  • toggle open the "dialog with details" by clicking the action on the row
  • in the flyout, in the "Trace summary" section, either click on "Expand trace timeline" or click the graphic itself
  • in the new "Trace timeline" flyout, click again on the graphic to open the child flyout
  • in the Overview table/grid, try expanding the cell content (that opens in a popover)
Toggle screenshots Screenshot 2026-05-06 at 12 03 30 Screenshot 2026-05-06 at 12 03 49 Screenshot 2026-05-06 at 12 04 01

Checklist before marking Ready for Review

Reviewer checklist

  • Approved Impact Assessment — Acceptable to merge given the consumer impact.
  • Approved Release Readiness — Docs, Figma, and migration info are sufficient to ship.

acstll added 2 commits April 30, 2026 13:35
Cell expansion popovers were pinned to `levels.header` (1000), the
same value as `levels.flyout`. With multiple unmanaged flyouts open
the second one renders at 1002 and hides the popover. Drop the explicit
`zIndex` so EuiPopover's dynamic `getElementZIndex(button) + 2000`
derivation kicks in, lifting the popover above its host flyout
regardless of how many siblings/children surround it.

Closes elastic#8801
@acstll acstll self-assigned this Apr 30, 2026
@acstll acstll marked this pull request as ready for review May 6, 2026 10:22
@acstll acstll requested a review from a team as a code owner May 6, 2026 10:22
Copilot AI review requested due to automatic review settings May 6, 2026 10:22
@acstll acstll added the support-duty-flywheel Label for PRs, see eui-private #310 label May 6, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes an EuiDataGrid layering bug where cell expansion popovers could render underneath sibling/nested EuiFlyout overlay masks due to using a fixed z-index.

Changes:

  • Removed the fixed zIndex passed to the grid’s cell expansion popover so EuiPopover can derive z-index from the anchor’s stacking context.
  • Simplified DataGrid z-index variables to only track internal sticky header layering (no longer defining a dedicated cellPopover level).
  • Updated the existing RTL unit snapshot expectation and added an upcoming changelog entry.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
packages/eui/src/components/datagrid/data_grid.styles.ts Removes the dedicated cellPopover z-index level and documents the new layering approach; keeps stickyHeader for internal grid layers.
packages/eui/src/components/datagrid/body/cell/data_grid_cell_popover.tsx Stops passing a fixed zIndex so popover stacking follows the anchor context (default EuiPopover behavior).
packages/eui/src/components/datagrid/body/cell/data_grid_cell_popover.test.tsx Updates the inline snapshot z-index value to reflect the new default-derived behavior.
packages/eui/changelogs/upcoming/9630.md Documents the bug fix and the behavioral change in how z-index is determined.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Asserts the cell popover panel rises above an ancestor stacking
context (e.g. a host flyout), which the previous Jest snapshot
couldn't verify under jsdom.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@elasticmachine
Copy link
Copy Markdown
Collaborator

💚 Build Succeeded

History

cc @acstll

@elasticmachine
Copy link
Copy Markdown
Collaborator

💚 Build Succeeded

History

cc @acstll

@weronikaolejniczak
Copy link
Copy Markdown
Contributor

The approach and code lgtm, I'm testing in Kibana right now and validating some edge cases...

Copy link
Copy Markdown
Contributor

@weronikaolejniczak weronikaolejniczak left a comment

Choose a reason for hiding this comment

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

LGTM 🟢 The fix works in Kibana. Thanks for the detailed QA instructions, @acstll 🙏🏻

We have to wait with the merge until the next release is made.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

support-duty-flywheel Label for PRs, see eui-private #310

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[EuiPanel] Invisible when placed in a EuiFlyout above the header

4 participants