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

Improve performance when there are many pickers. #125

Merged
merged 6 commits into from
Apr 17, 2022

Conversation

comes
Copy link
Contributor

@comes comes commented Feb 4, 2022

If you have many pickers, the rendering time will be significantly slower. To solve this, this commit provides a solution for this.

the trick is, just create a flatpickr instance, if you need one.

If you have many pickers, the rendering time will be significantly slower. To solve this, this commit provides a solution for this.
@ryangjchandler
Copy link
Contributor

I don't see how this makes a difference with rendering/startup time... an Alpine component will only be initialised once anyway.

Moving the property to a piece of data wouldn't make a difference since it's still only being instantiated once, right?

@comes
Copy link
Contributor Author

comes commented Feb 19, 2022

You're completely right in your opinion when you only have one or two flatpickrs per page. There is nothing wrong.

But, Imagin' you have a huge bunch of pickers on a page. Let's say 100 or 150. The reason for this is a super-big admin screen with a table consisting of a lot of date fields.

In that case, it makes a huge difference, when the component is initialized. It will be initialized for every single picker you add to your page. While loading the page, the flatpickr will be appended to the end of the body tag.

you will save a lot of loading and render time when you do the initialization of flatpickr only if the user really needs it.

@ryangjchandler
Copy link
Contributor

Ah, sorry you're right. Your only initialising the picker on click, instead of automatically.

@comes
Copy link
Contributor Author

comes commented Feb 19, 2022

No probs

@comes
Copy link
Contributor Author

comes commented Mar 14, 2022

dear @ryangjchandler, when do you think, we can merge and close this PR?

@ryangjchandler
Copy link
Contributor

I'll need to give this a test before merging, will do that shortly.

@comes
Copy link
Contributor Author

comes commented Apr 17, 2022

switched to mouseenter, because I did a lot of tests and we do have this in production for 2 weeks. no measurable problems, but way better than instantiate hundreds of pickers, when you only need some of them

@ryangjchandler
Copy link
Contributor

@comes I've made some changes to the code in this PR so that it's slightly more readable and maintainable, would you be able to give it all a go again to make sure it still works as expected.

I've tested myself and can't see any problems.

Copy link
Contributor Author

@comes comes left a comment

Choose a reason for hiding this comment

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

I completely agree with this refactoring.

@ryangjchandler ryangjchandler merged commit 415ad4f into blade-ui-kit:0.x Apr 17, 2022
@ryangjchandler
Copy link
Contributor

This has been released in v0.3.4.

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

Successfully merging this pull request may close these issues.

None yet

2 participants