fix(src/index.js): Overwrite delayHide on scroll
#504
Conversation
|
@aronhelser gentle ping 🙏 Thank you! |
src/index.js
Outdated
|
|
||
| const hideTooltip = (event, hasTarget) => this.hideTooltip(event, hasTarget, { isScroll: true }) | ||
|
|
||
| window.addEventListener('scroll', hideTooltip, isCaptureMode) |
There was a problem hiding this comment.
The function added here no longer matches this.hideTooltip inside removeScrollListener, and I think that's a problem.
There was a problem hiding this comment.
Hi @aronhelser! Thanks so much for taking the time to check the PR 🙏
Ahh I see, so since window.addEventListener('scroll', hideTooltip, isCaptureMode) refers to a different function from this.hideTooltip, window.removeEventListener('scroll', this.hideTooltip) won't be able to clear const hideTooltip, and thus introducing memory leak.
Will need to think about a way around this 👍
Thanks for catching that!
Add new param `options` to `hideTooltip()` and update `addScrollListener()`
to set `options` to `{ isScroll: true }`, so `hideTooltip()` can set
`delayHide` to `0` if `isScroll` is `true`
fix #474
|
Hi @aronhelser! I just updated the PR to use a new method
PTAL again 🙏 Thank you! |
|
Awesome, thanks! |
|
🎉 This PR is included in version 3.11.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
Hurray! Thanks again for the great review, @aronhelser ! Have an amazing weekend! |
Add new param
optionstohideTooltip()and updateaddScrollListener()to set
optionsto{ isScroll: true }, sohideTooltip()can setdelayHideto0ifisScrollistruefix #474