Skip to content

Conversation

@cee-chen
Copy link
Contributor

@cee-chen cee-chen commented Jun 30, 2022

Summary

closes #6002

This PR adds support for virtualizationOptions.onItemsRendered as well as virtualizationOptions.className (figured why not, since I was here). It also adds better documentation and type-checking for the virtualizationOptions prop.

Checklist

- [ ] Checked in both light and dark modes
- [ ] Checked in mobile
- [ ] Checked in Chrome, Safari, Edge, and Firefox
- [ ] Props have proper autodocs and playground toggles
- [ ] 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

…Name` props

- there were previously getting overriden by our own prop settings, but should allow being merged
@cee-chen cee-chen requested a review from chandlerprall June 30, 2022 21:29
@cee-chen
Copy link
Contributor Author

@alexwizp I believe this should satisfy your use case - Please let me know if not! :)

@cee-chen cee-chen changed the title [EuiDataGrid] Update virtualizationProps to respect onItemsRendered and className [EuiDataGrid] Update virtualizationOptions to respect onItemsRendered and className Jun 30, 2022
@cee-chen cee-chen force-pushed the datagrid/onitemsrendered branch from f8ba96d to 105d090 Compare June 30, 2022 21:39
@kibanamachine
Copy link

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

@cee-chen cee-chen force-pushed the datagrid/onitemsrendered branch from 105d090 to 9827c1c Compare July 1, 2022 16:09
@kibanamachine
Copy link

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

Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

Pulled & tested locally to ensure supported virtualizationOptions options were still applied. Everything checks out 👍

@cee-chen cee-chen enabled auto-merge (squash) July 5, 2022 18:36
@kibanamachine
Copy link

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

@cee-chen cee-chen merged commit 6113201 into elastic:main Jul 5, 2022
@cee-chen cee-chen deleted the datagrid/onitemsrendered branch July 5, 2022 19:20
@alexwizp
Copy link
Contributor

@constancecchen thanks, but I see this callback doesn't happen on if we refresh table and data doesn't changed. I understand that this is absolutely true from an optimisation perspective but it makes impossible for us to use this callback 😢

@cee-chen
Copy link
Contributor Author

if we refresh table and data doesn't changed

Sorry, I'm not totally clear on what "refresh table" means. Are you completely unmounting and remounting the data grid via the key prop? If so, this callback should fire. How else are you refreshing the table?

Also, having this convo in a closed PR may not make a lot of sense, so feel free to re-open #6002 and continue the discussion there! 🙂

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 this pull request may close these issues.

[DataGrid] Provide OnRender callback

4 participants