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] Add new setCellPopoverProps API to renderCellPopover #6632

Merged
merged 6 commits into from
Mar 7, 2023

Conversation

cee-chen
Copy link
Contributor

@cee-chen cee-chen commented Mar 6, 2023

Summary

The Discover team requested an API in elastic/kibana#152480 to allow setting props on the cell popover. This prop will enable them to, e.g. set custom resize CSS on the cell popover panel, making their cell popovers resizable, and additionally set custom min/max height/widths.

The new setCellPopoverProps API behaves exactly the same as setCellProps does for the cell, and allows customizing (almost) any prop that EuiPopover takes, except for the button and closePopover props.

Example usage:

<EuiDataGrid
  renderCellPopover={({ setCellPopoverProps, DefaultCellPopover, ...props }) => {
    // Set a custom className on each cell popover panel
    useEffect(() => {
      setCellPopoverProps({ panelClassName: 'customCellPopover' });
    }, [setCellPopoverProps]);

    // Render popover as-is
    return <DefaultCellPopover {...props} />
  }}
/>

I strongly recommend following along by commit for code review.

QA

General checklist

  • Checked in Chrome, Safari, Edge, and Firefox
  • Props have proper autodocs (using @default if default values are missing) and playground toggles
  • Added documentation
  • Added or updated jest and cypress tests
  • A changelog entry exists and is marked appropriately

- [ ] Checked in both light and dark modes
- [ ] Checked in mobile
- [ ] Checked Code Sandbox works for any docs examples
- [ ] Checked for breaking changes and labeled appropriately
- [ ] Checked for accessibility including keyboard-only and screenreader modes
- [ ] Updated the Figma library counterpart

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6632/

@cee-chen cee-chen marked this pull request as ready for review March 6, 2023 18:49
@cee-chen cee-chen requested review from kertal and jughosta March 6, 2023 18:50
@cee-chen
Copy link
Contributor Author

cee-chen commented Mar 6, 2023

@kertal or @jughosta, any chance either of you would be willing to take a quick look at the API/example usage/docs and let me know if it'll work for you? Thanks!

Copy link
Contributor

@jughosta jughosta left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks a lot for allowing customization for the cell popover!

@jughosta
Copy link
Contributor

jughosta commented Mar 7, 2023

Would it be also possible to add an Eui class to this div? The name does not need to be customizable, just something more reliable if it makes sense. We will need to change its height to 100% for Discover use case.

Screenshot 2023-03-07 at 09 57 31

@cee-chen
Copy link
Contributor Author

cee-chen commented Mar 7, 2023

Unfortunately that <div> comes from EuiPopover itself (you can see it if you go to the popover prod docs) and we'd need to add a new API to EuiPopover to allow customizing it. For now, can you target .customCellPopover > div directly instead via CSS if possible?

Editing to add - I'll go ahead and merge this PR as is (as I don't want to conflate EuiPopover functionality with EuiDataGrid), but please feel free to open a feature request/issue in EUI for this functionality in the interim!

@cee-chen cee-chen merged commit 5498fe4 into elastic:main Mar 7, 2023
@cee-chen cee-chen deleted the datagrid/set-cell-popover-props branch March 7, 2023 18:10
@jughosta
Copy link
Contributor

jughosta commented Mar 7, 2023

.customCellPopover > div can also work for us, all good! Thanks!

1Copenut added a commit to elastic/kibana that referenced this pull request Mar 23, 2023
## Summary

[email protected][email protected]

## [`76.3.0`](https://github.com/elastic/eui/tree/v76.3.0)

- Updated `EuiSkipLink`'s `fallbackDestination` prop to support an array
of query selector strings
([#6646](elastic/eui#6646))

**Bug fixes**

- Fixed `EuiFlyout` to preserve body scrollbar width on open
([#6645](elastic/eui#6645))
- Fixed `EuiImage`'s full screen mode to not scroll jump & to preserve
body scrollbar width on open
([#6645](elastic/eui#6645))
- Fixed `EuiCodeBlock`'s full screen mode to not scroll jump & to
preserve body scrollbar width on open
([#6645](elastic/eui#6645))

## [`76.2.0`](https://github.com/elastic/eui/tree/v76.2.0)

- Added new `renderCustomGridBody` escape hatch rendering prop to
`EuiDataGrid` ([#6624](elastic/eui#6624))

**Bug fixes**

- Fixed visual listbox focus ring bug on non-searchable `EuiSelectable`s
([#6637](elastic/eui#6637))
- Added a legacy `alert` alias for the `warning` `EuiIcon` type
([#6640](elastic/eui#6640))
- Fixed a type definition incorrectly coming from a dev dependency,
which was causing issues for some consuming projects
([#6643](elastic/eui#6643))

## [`76.1.0`](https://github.com/elastic/eui/tree/v76.1.0)

- Added more detailed screen reader instructions to `EuiSelectable`,
`EuiSuggest`, `EuiSelectableTemplateSitewide`, `EuiRange`, and
`EuiDualRange`. ([#6589](elastic/eui#6589))
- Added new `placeholder` prop to `EuiSuperSelect`
([#6630](elastic/eui#6630))
- Added new `setCellPopoverProps` parameter callback to `EuiDataGrid`'s
`renderCellPopover` prop
([#6632](elastic/eui#6632))

**Bug fixes**

- Fixed an ARIA attribute in `EuiSelectableList`
([#6589](elastic/eui#6589))
- Fixed `EuiSelectable` to no longer show active selection state or
respond to the Up/Down arrow keys when focus is inside the selectable
container, but not on the searchbox or listbox.
([#6631](elastic/eui#6631))

---------

Co-authored-by: Tiago Costa <[email protected]>
jennypavlova pushed a commit to jennypavlova/kibana that referenced this pull request Mar 24, 2023
## Summary

[email protected][email protected]

## [`76.3.0`](https://github.com/elastic/eui/tree/v76.3.0)

- Updated `EuiSkipLink`'s `fallbackDestination` prop to support an array
of query selector strings
([elastic#6646](elastic/eui#6646))

**Bug fixes**

- Fixed `EuiFlyout` to preserve body scrollbar width on open
([elastic#6645](elastic/eui#6645))
- Fixed `EuiImage`'s full screen mode to not scroll jump & to preserve
body scrollbar width on open
([elastic#6645](elastic/eui#6645))
- Fixed `EuiCodeBlock`'s full screen mode to not scroll jump & to
preserve body scrollbar width on open
([elastic#6645](elastic/eui#6645))

## [`76.2.0`](https://github.com/elastic/eui/tree/v76.2.0)

- Added new `renderCustomGridBody` escape hatch rendering prop to
`EuiDataGrid` ([elastic#6624](elastic/eui#6624))

**Bug fixes**

- Fixed visual listbox focus ring bug on non-searchable `EuiSelectable`s
([elastic#6637](elastic/eui#6637))
- Added a legacy `alert` alias for the `warning` `EuiIcon` type
([elastic#6640](elastic/eui#6640))
- Fixed a type definition incorrectly coming from a dev dependency,
which was causing issues for some consuming projects
([elastic#6643](elastic/eui#6643))

## [`76.1.0`](https://github.com/elastic/eui/tree/v76.1.0)

- Added more detailed screen reader instructions to `EuiSelectable`,
`EuiSuggest`, `EuiSelectableTemplateSitewide`, `EuiRange`, and
`EuiDualRange`. ([elastic#6589](elastic/eui#6589))
- Added new `placeholder` prop to `EuiSuperSelect`
([elastic#6630](elastic/eui#6630))
- Added new `setCellPopoverProps` parameter callback to `EuiDataGrid`'s
`renderCellPopover` prop
([elastic#6632](elastic/eui#6632))

**Bug fixes**

- Fixed an ARIA attribute in `EuiSelectableList`
([elastic#6589](elastic/eui#6589))
- Fixed `EuiSelectable` to no longer show active selection state or
respond to the Up/Down arrow keys when focus is inside the selectable
container, but not on the searchbox or listbox.
([elastic#6631](elastic/eui#6631))

---------

Co-authored-by: Tiago Costa <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants