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

Reduce impact on DOM render #732

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

GertjanVermeir
Copy link

Description
Multiple datePickers on one page can cause the DOM rendering to slow down.
In my current project we have tables with 20 to 100 row that can be edited and we see the DOM becomes very big.

Motivation and Context

  • Amount of nodes in the dom before the change: 71
  • Amount of nodes in the dom after the change: 2

(Done with document.querySelector(".rdt").getElementsByTagName('*').length)

Checklist
[x] I have not included any built dist files (us maintainers do that prior to a new release)
[ ] I have added tests covering my changes
[x] All new and existing tests pass
[ ] My changes required the documentation to be updated
[ ] I have updated the documentation accordingly
[ ] I have updated the TypeScript 1.8 type definitions accordingly
[ ] I have updated the TypeScript 2.0+ type definitions accordingly

@arqex
Copy link
Owner

arqex commented Sep 23, 2020

Hey @GertjanVermeir

Thanks for the PR. This is something we wanted to tackle soon, but unfortunately we can't just return null when the picker is closed. There are some people that might be animating when the picker is open/closed by custom css, if we just return null we will be breaking the closing animation.

We would need to add kind of a timer to return null some time after the picker has been closed, but not instantly. I think the best approach would be create a new wrapper component that handled that timer. It would receive a prop open, if it changes from true to false it would wait a second or so (this parameter should be configurable) and then re-render to return null.

@thomasvaneyck
Copy link

@arqex we would like to help with this issue. We are right now using a patched version internally. @GertjanVermeir clear on what to do?

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.

3 participants