fix(Sticky): scrollContext changes cause memory leak and incorrect scroll events #2464
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2464 +/- ##
==========================================
+ Coverage 99.74% 99.74% +<.01%
==========================================
Files 154 154
Lines 2699 2705 +6
==========================================
+ Hits 2692 2698 +6
Misses 7 7
Continue to review full report at Codecov.
|
|
Superb, thanks for the very clean and complete PR. |
|
When will this fix be released? |
|
I hope this weekend. My family and I are quite sick at the moment. |
|
@fracmak you've contributed a number of PRs and they are typically very well done. I've added you as a collaborator. You're now able to manage issues and PRs as well as create branches here in the main repo. Merging to master is the only reserved permission. We are very select about who we add and when, so know that we trust your abilities and your judgment as to good code. Please feel free to weigh in on any issues and PRs that you have an opinion about. You have a share of ownership and say here now. Once you accept the invite (see your email), you might want to show the Semantic Org badge publicly on your profile. To do that, navigate to the "people" page and change the "private" dropdown to "public" for your user in the table. Here's a direct link to your user in the table. /cc @layershifter @brianespinosa Please welcome @fracmak (Jay) to our ranks 😄 🎉 |
|
Released in |
|
Thanks, man! I really appreciate your effort. |
|
Thanks @levithomason ! It's an honor to join the team and be trusted in helping manage the project! |
…roll events (Semantic-Org#2464) * fix(Sticky): scrollContext changes cause memory leak and incorrect scroll events * style(Sticky): update lines
This PR is similar to #2458.
When the scrollContext prop changes, we don't change the event listeners so we don't fire scroll events on the new scrollContext, and continue to fire scroll events on the old listener. We also have a memory leak when the scrollContext changes and the component unmounts because it will remove the listener on the wrong scrollContext
I also added an if statement where if the scrollContext is null, it doesn't listen for scroll events similar to the change I made in #2458 so they now behave the same.