teodosii / react-notifications-component

Delightful and highly customisable React Component to notify your users
https://teodosii.github.io/react-notifications-component/
MIT License
1.27k stars 73 forks source link

Notifications created in rapid succession may not be removed from the DOM when dismissed #157

Open Smona opened 2 years ago

Smona commented 2 years ago

We ran across an issue in the wild where users couldn't click on some buttons because the notification container was blocking them. It turns out that in some scenarios where notifications are created in quick succession, notifications can end up not being removed from the DOM. These zombie notifications will run through their exit animation, but afterwards they remain, causing the notification container not to shrink back to zero width.

I'm able to reproduce this pretty easily by repeatedly spamming out groups of 6 notifications with a dismiss timeout as quickly as I can. If I pass an onRemoval callback, it's never called for the zombie notification. We're on version 3.4.1 but I tried upgrading to 4.0.1 and the bug was still reproducible.

We'll attempt to hack around this with CSS, but because of the custom store (which is generally great) this needs to be fixed on the RNC side.

Smona commented 2 years ago

This is interesting... I added a transition: visibility 0ms linear 500ms to .rnc__notification-item and visibility: hidden to my animation exit class. I was expecting to at least hide the element after the animation, but after a lot of button mashing I haven't been able to reproduce the bug since making this change. Perhaps there's a CSS event listener somewhere that isn't always being triggered without this rule?

For reference, here's our animation config:

            animationIn: ['animate__animated', 'animate__fadeInRight', 'animate__faster'],
            animationOut: ['animate__animated', 'animate__fadeOutRight', 'animate__faster'],
            // Chosen to complement duration of animate__faster speed above (500ms)
            slidingEnter: {
                duration: 200,
                timingFunction: 'ease-in-out',
                delay: 0,
            },
            slidingExit: {
                duration: 200,
                timingFunction: 'ease-in-out',
                delay: 300,
            },
teodosii commented 2 years ago

Hi Smona,

Thanks for reporting. Will investigate

stefanb2 commented 1 year ago

I ran into the same issue.

BUT in my case I can't reproduce it on development code, i.e. in the Dev Env of my app, only on production code, i.e. when the app code is deployed. CORRECTION in normal operations inside the app, i.e. what a normal user would see, there the development/production code behaves differently.

Now that I added a special button on the home page to generate many notifications in quick succession by clicking on it, I have now reproduced the behavior also in development code.

stefanb2 commented 1 year ago

As a workaround for this problem I now make sure that the notification is removed in my helper function. Here is the gist of the code:

const makeNotification = notification => {
  const duration = notification.timeout ?? 5000;

  Promise.resolve()
    .then(() => Store.addNotification({
      ...notification,
      dismiss: {
        duration,
        ...
      },
      ...
    }))
    .then(id => new Promise(resolve =>
      setTimeout(
        () => {
          // Make sure the notification disappears
          Store.removeNotification(id);
          resolve();
        },
        duration
      )
    ));
};