search

apvarun / toastify-js

Pure JavaScript library for better notification messages
https://apvarun.github.io/toastify-js/
MIT License
2.13k stars 230 forks source link

Mouseout after closing causes error on removeChild #38

Open haydster7 opened 4 years ago

haydster7 commented 4 years ago

If close is set to true and duration is not infinite, and close is clicked, then the mouse is moved off where the toast would have been (or is still animating), a second timeout is created to hide the element. As the element will already be hidden by the time that second timeout executes (due to the immediate close function), the element is null and removeChild throws a TypeError.

I have a couple of suggestions to fix this, not sure what would be preferred.

  1. I tried setting pointer-events to none as soon as closing, then once hidden removed pointer-events, but for some reason the css didnt update or something and the mouseout was still firing. To get around this I explicitly checked the pointer-events attribute before setting the mouseout timeout, that seemed to work well.
  2. Create a variable within the Toastify object called isClosing. Set to true when close is clicked, and set to false once the element is completely hidden. Check for isClosing before setting the mouseout timeout.
apvarun commented 4 years ago

Can you reproduce this in an online IDE? Tried checking this problem and it isn't happening. https://codesandbox.io/s/toastify-closing-toasts-with-timers-z6i9j

mukzen commented 4 years ago

I can observe a similar, maybe related error. It happens when you click on the close icon and move the mouse off from where the toast would have been before the timeout runs out. The culprit sits in line 253, where the timeout callback removes the toast without testing, wether it is still available in DOM. For now it's mostly cosmetical, an inconvenience, but an error stil ;)

BTW: this error is present in the online IDE link you posted...

apvarun commented 4 years ago

@mukzen Thanks for the reaffirmation. Feel free to raise a PR for them same. I believe it just requires to have a check for element before attempting to remove.

EdwinHauspie commented 4 years ago

@mukzen Thanks for the reaffirmation. Feel free to raise a PR for them same. I believe it just requires to have a check for element before attempting to remove.

I believe the better solution would be not to re-instantiate the timeout on mouseleave, after the close button has been clicked.