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

set renderAheadOffset={1500} as a prop can greatly boost the performance #36

Closed
leonyhenn opened this issue Apr 30, 2019 · 8 comments
Closed

Comments

@leonyhenn
Copy link

leonyhenn commented Apr 30, 2019

First of all, great emoji component!!! I loved the work you have put on this, it is the emoji component that has the best performance in RN community.

I was building a composer which is under a page with content, so it only pops up when user want to write a comment, and I put the emoji component under the composer, so by pressing a button user can have keyboards down and selecting the emojis.

One of the problem that I find is that the performance really suffers,like the whole page freeze for 3-5 seconds even on the latest Android phone, when navigating to a new page with such setup, and by digging in I find the emoji component takes too long to load and block the entire JS thread. By reading the code and change the renderAheadOffset={1500} to renderAheadOffset={200} or any smaller number this problem solved. I think it might be a better idea to pass the renderAheadOffset as a prop instead of fix it inside the code, cause it is very much performance-related.

Thanks Sujay! Really an amazing emoji component, saved me tons of time, I owe you a beer!

@sskhandek
Copy link
Owner

Hi @leonyhenn , thanks for the warm compliments! Kudos to @rijn for also playing a major part in this keyboard's development. I'd definitely like to have the performance of this keyboard be a lot better.

@rijn do you recall a reason we set the renderAheadOffset to 1500 and not a lower number? Were we potentially concerned about a user trying to scroll very fast through the keyboard?

@leonyhenn
Copy link
Author

@sskhandek Oh actually that was what I was thinking, the user trying to scroll very fast thing. set the renderAheadOffset to 200 have pretty good performance already, 1500 is buttery smooth on scrolling but freezes when entering the page. 200-ish number is the sweet spot for me cause it is the balance point between page loading time and scrolling loading time. It is just an advice, cause it is a component, I think it is a better idea to let developers decides for themselves which side of performance they would like to optimize.

@sskhandek
Copy link
Owner

I think that's a great idea to make it a prop that the user can pass in. @leonyhenn Any chance you'd like to help make this happen with a pull request? It'd be much appreciated :)

@leonyhenn
Copy link
Author

leonyhenn commented May 5, 2019

Hi sorry for the late reply, yeah, sure! My pleasure! @sskhandek

@gutosanches
Copy link
Contributor

Hey guys, sorry I went ahead and made the PR as I also needed this for my project. Hope you don't mind ✌️

@leonyhenn
Copy link
Author

@gutosanches Oh dont worry, i was too busy working on other things recently anyway. Good to know that someone has the same thoughts as I do.

@leonyhenn
Copy link
Author

@gutosanches can you please add to readme on what this props do and some explanation on what number to set, so people can understand?

@sskhandek
Copy link
Owner

Thank you @gutosanches and @leonyhenn for the conversation. I just merged this in!

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

No branches or pull requests

3 participants