-
Notifications
You must be signed in to change notification settings - Fork 92
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
use one rAF manager #145
use one rAF manager #145
Conversation
@jheth Hey Joe 👋 Would you mind trying out this branch? This uses an rAFPoolManager to use one rAF for all your components on the page. Also, I realized we use a concept called
I short circuited the rAF calls if |
This looks great! I'll give it a try. |
@jheth I'll wait for your confirmation before merging 👍 |
@snewcomer The branch is much improved over the previous where I was seeing rAF for each LazyImage component I had on the page. However, I'm not seeing the rAF ever stop, even after all images are loaded. This is a performance chart on page load with 10 lazy image components on the page. This is a close up, showing a single rAF is being used, which is much better than before. 👍 This is a performance chart after scrolling to the bottom of the page and all images had been scrolled into the viewport. I don't see a difference from the first chart. I would have expected no more rAF requests. |
I do not have |
@jheth Also you can test this behavior (of turning off rAF after first entering the viewport) in the dummy app on the rAF route. I'll merge as is if you don't mind and we can make further improvements in the future. Thanks again for everything here. |
Huge thanks to the work done in #103
Also splits out the recursive rAF function to it's own service.
close #103