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

Notification not being removed at end of transition #74

Closed
philraj opened this issue Dec 7, 2016 · 4 comments
Closed

Notification not being removed at end of transition #74

philraj opened this issue Dec 7, 2016 · 4 comments

Comments

@philraj
Copy link

philraj commented Dec 7, 2016

Hi,

I'm having a very rare bug which is extremely difficult to reproduce, but my best guess at what is happening is that react-notification-system is not detecting transitionend in some cases.

We needed to customize the popups a fair bit so we set style={false}, set transition: opacity 0.3s on .notification, and set opacity: 0 on .notification-hidden.

From my understanding of the code, when a NotificationItem is instantiated, it hooks into the transitionend event. When the library decides it's time to autodismiss the popup, it applies the class notification-hidden. If we turn off the styling, we're supposed to make sure that when this class is applied, it kicks off a transition, and at the end of the transition the library calls this._didNotificationRemoved from the NotificationSystem component.

That all seems to be working fine 99% of the time, but for some reason, once in a while notifications will fade out (so the opacity transition did occur) but the DOM node hangs around, and blocks interaction of items underneath the hidden popups. If I manually change the opacity of these stubborn invisible notifications in the dev tools, they get properly removed. So somehow the event listener is working, but misses the first transition.

Do you have any idea what could be causing this?

@philraj
Copy link
Author

philraj commented Dec 7, 2016

Ah, I found the issue. We were setting display: none on the popup notifications wrapper when a certain UI widget was open, and I guess it was preventing the transitionend event from firing. All good now.

@philraj philraj closed this as completed Dec 7, 2016
@philraj
Copy link
Author

philraj commented Dec 8, 2016

Come to think of it, it might be a good idea to put a note in the readme about how the autodismiss relies on the transitionend event, and that it won't be triggered if the notifications container is set to display: none; when the hidden class is applied.

@philraj philraj reopened this Dec 8, 2016
@igorprado
Copy link
Owner

thanks @philraj

There's a note about this: https://github.com/igorprado/react-notification-system#important-1

As you pointed, applying display: none; to parent elements prevent the animation and we may have other cases that happens too. It's hard to predict every scenario, but I'll update the README and make it more clear about this. Thanks!

@philraj
Copy link
Author

philraj commented Dec 16, 2016

Yeah, the display: none; detail is a little too specific to our situation. But I think just saying it depends on the transitionend event explicitly would help anyone in a similar situation.

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

2 participants