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

toolbarVisibility additionalControls causes Data Grid to re-render #3880

Closed
frank-mendez opened this issue Aug 10, 2020 · 7 comments · Fixed by #3919
Closed

toolbarVisibility additionalControls causes Data Grid to re-render #3880

frank-mendez opened this issue Aug 10, 2020 · 7 comments · Fixed by #3919
Assignees

Comments

@frank-mendez
Copy link

Is there a way the EUI grid will not render when I call functions on my additionalControls?

@chandlerprall
Copy link
Contributor

It depends on how you are defining & passing some (many) of the datagrid's props. #3518 is relevant and may provide you with some insight, although that issue represents a lack of documentation around those props.

If you can provide a minimal example representing the problem you observe (e.g. in a code sandbox, I'll look at which prop(s) are causing additional renders and provide some ways to avoid it.

@frank-mendez
Copy link
Author

I used the example SelectionContext Provider wrapping the Data Grid <SelectionContext.Provider value={rowSelection}>
I have a custom function Select All Button that mimics the SelectionHeaderCell checkbox but everytime I call updateSelectedRows({ action: 'selectAll' }); it re-renders the Data Grid

@frank-mendez
Copy link
Author

frank-mendez commented Aug 11, 2020

Btw, my selectAll function is inside SelectionButton component which renders in toolbarVisibility and additionalControls

@frank-mendez
Copy link
Author

Does using state management tools like Redux instead of Context provider guarantee Data Grid will not re-render or no? Thanks for your help. I appreciate it.

@chandlerprall
Copy link
Contributor

Does using state management tools like Redux instead of Context provider guarantee Data Grid will not re-render or no? Thanks for your help. I appreciate it.

Some can help, others wouldn't - Redux specifically, at least without additional tooling, would not. I've confirmed that the control header example causes every cell to re-render, I am looking into why and if there's another way to present the example without that performance overhead.

Right now, the SelectionContext's value is re-created anything the selection changes, causing a full element re-render. Another approach would be to change that context from being a Set object to a class doing its own state management, allowing the individual cells to listen to changes and update without triggering a grid-wide render.

@frank-mendez
Copy link
Author

Does using state management tools like Redux instead of Context provider guarantee Data Grid will not re-render or no? Thanks for your help. I appreciate it.

Some can help, others wouldn't - Redux specifically, at least without additional tooling, would not. I've confirmed that the control header example causes every cell to re-render, I am looking into why and if there's another way to present the example without that performance overhead.

Right now, the SelectionContext's value is re-created anything the selection changes, causing a full element re-render. Another approach would be to change that context from being a Set object to a class doing its own state management, allowing the individual cells to listen to changes and update without triggering a grid-wide render.

Sounds a good idea. I will try to change the context. I hope it works! Thanks man! Keep doing it!

@chandlerprall
Copy link
Contributor

The alternate path I mentioned would resolve the issue, but is not the root cause. A bug was introduced into the datagrid where any new object passed to pagination would cause every cell to re-render. I've opened a PR to fix this.

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

Successfully merging a pull request may close this issue.

3 participants