Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[EuiDataGrid] Expansion popover needs more customization #5310

Closed
3 tasks done
cchaos opened this issue Oct 21, 2021 · 5 comments · Fixed by #5640
Closed
3 tasks done

[EuiDataGrid] Expansion popover needs more customization #5310

cchaos opened this issue Oct 21, 2021 · 5 comments · Fixed by #5640

Comments

@cchaos
Copy link
Contributor

cchaos commented Oct 21, 2021

While the expansion popover was originally built purely to be a full view of the cell contents, there is a growing need to make it more complex/custom. Several things that are needed:

@cee-chen
Copy link
Member

cee-chen commented Dec 9, 2021

cc @chandlerprall and @thompsongl

Just wanted to get more convo going on this as I think there's some higher level decisions to make here before just yoloing into the code.

  1. Allow consumers to control the open/closed state

Do we intend to completely hand over the cell popover state to the consumer (i.e., they're now required to be fully in charge of setting their own isOpen state a la EuiPopover), or do we want to only optionally give them the ability to pass in a true/false but fall back to our own internal state management if the consumer does not opt in?

  1. Allow consumers to completely control the popover contents including the custom controls

Just to make sure I'm on the same page - it appears to me that consumers already have complete control over popover contents via the popoverContents API (https://elastic.github.io/eui/#/tabular-content/data-grid-schemas-and-popovers). Is the feature request here then just to allow users to customize the actual popover toggles (aka cell actions)?

Other random thoughts:

Fix the hard-coded high z-index

Edit: Went ahead and opened a PR for this: #5461

@thompsongl
Copy link
Contributor

Thanks, Constance! I also have some thoughts to get down and discuss, but my other in-progress work has consumed my attention this week. I'll make this a priority next week.

@cee-chen
Copy link
Member

cee-chen commented Dec 9, 2021

No worries at all! The last item (z-index) looks pretty straightforward at second glance so I might go ahead and tackle it and leave all the hard questions to you :trollface:

@michaelolo24
Copy link

Was going through some of the items in our backlog and also noticed this: elastic/kibana#115341 @constancecchen , should the open and close popovers help us handle this as well? Thanks!

@cee-chen
Copy link
Member

cee-chen commented Feb 1, 2022

Yes, it will! You will be able to grab closeCellPopover() from the data grid's ref and manually call it in the same action you use to open your modal or perform a navigation action, etc. You can see https://eui.elastic.co/pr_5550/#/tabular-content/data-grid-ref-methods for an example + demo JS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants