Skip to content
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

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

Open
Smona opened this issue Jun 16, 2022 · 4 comments
Assignees

Comments

@Smona
Copy link

Smona commented Jun 16, 2022

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
Copy link
Author

Smona commented Jun 16, 2022

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
Copy link
Owner

Hi Smona,

Thanks for reporting. Will investigate

@teodosii teodosii self-assigned this Jun 24, 2022
@stefanb2
Copy link

stefanb2 commented Feb 27, 2023

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
Copy link

stefanb2 commented Feb 27, 2023

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
      )
    ));
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants