elastic / eui

Elastic UI Framework 🙌
https://eui.elastic.co/
Other
6.11k stars 843 forks source link

[EuiDataGrid] Provide an `onFullScreenChange` callback #7604

Open davismcphee opened 8 months ago

davismcphee commented 8 months ago

Is your feature request related to a problem? Please describe. We experience z-index issues in Kibana when using the data grid's full screen mode due to its use of fixed positioning. We work around this by applying global styles to reset the z-index values of other elements when the grid is in full screen mode (see https://github.com/elastic/kibana/pull/178788). The issue is that in order to detect when the data grid is in full screen mode, we need to use mutation observers that check for the full screen class since the data grid doesn't offer another way to detect the full screen state.

Describe the solution you'd like It would be much easier to implement our solution without workarounds if the data grid offered something like an onFullScreenChange(isFullScreen: boolean) callback that we could use to determine when our override styles should be applied.

Describe alternatives you've considered The data grid could instead be updated to not use fixed positioning for full screen mode, but this seems like a larger and more difficult change on the EUI side.

Desired timeline This isn't an urgent request since we have a workaround in place, but having it would help us remove some unpleasant tech debt.

cee-chen commented 8 months ago

Hey Davis! I'd be interested in figuring out exactly what is going on with your z-index issues that's causing such a cascade of shenanigans. I also wonder if allowing the datagrid full screen z-index level to be configured/customized by consumers would help here.

As a heads up, as this is a low priority datagrid issue we're unlikely to grab it anytime soon, but we would happily accept a PR on it.

github-actions[bot] commented 8 months ago

👋 Thank you for your suggestion or request! While the EUI team agrees that it's valid, it's unlikely that we will prioritize this issue on our roadmap. We'll leave the issue open if you or anyone else in the community wants to implement it by contributing to EUI. If not, this issue will auto close in one year.

Log | Bot Usage
davismcphee commented 8 months ago

@cee-chen I think it's related to CSS stacking contexts and position: fixed, which tbh I don't really understand well enough to fully explain, but it seems to become an issue when any parent of the data grid has non-static positioning and a defined z-index value, making it so the data grid can't override the parent's z-index. The grid also already uses z-index: 999 in full screen, but other elements with lower z-index values can appear on top regardless, so I don't think customizing the data grid z-index would help here.

We've had similar issues elsewhere in Kibana and usually use these kinds of workarounds to fix them since we don't always control the consuming code to ensure no parents have z-index values, and it doesn't seem like there's another universal solution for this besides unsetting other elements' z-index values. Linking a couple of issues for reference:

And no problem on the priority, it's not a big concern on our end either, really just a convenience to help clean up our implementation. One of us may be able to pick it up when we can find time for some EUI contributions 🙂

github-actions[bot] commented 2 months ago

👋 Hi there - this issue hasn't had any activity in 6 months. If the EUI team has not explicitly expressed that this is something on our roadmap, it's unlikely that we'll pick this issue up. We would sincerely appreciate a PR/community contribution if this is something that matters to you! If not, and there is no further activity on this issue for another 6 months (i.e. it's stale for over a year), the issue will be auto-closed.