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

[BUG 4.0.3] When open 2 consecutive notifications I get a console error #71

Closed
tadeubas opened this issue Oct 11, 2016 · 12 comments
Closed
Assignees
Labels

Comments

@tadeubas
Copy link

When my code call 2 times to notificate something I get this error:
Uncaught TypeError: Cannot read property 'style' of null at /bower_components/notific8/dist/notific8.js:110

To fix, need to add this on the two setTimeout functions:

notification = document.getElementById(notificationId);
if (!notification)
        return;
@willsteinmetz
Copy link
Collaborator

Which browser does this occur in? Also, are you queuing the notifications?

@willsteinmetz
Copy link
Collaborator

And are there any other frameworks being used?

@tadeubas
Copy link
Author

Linux Chrome, AngularJS, but I think the error is not related to the browser or the framework, it is the way I use notifications. Try these steps in sequence (remove any notification and notific something 2 times):

notific8('remove');
notific8('test', {theme: 'ruby', sticky: true});
notific8('remove');
notific8('test2', {life: 4000});

@willsteinmetz
Copy link
Collaborator

I will try those out. Those extra details just help with making sure I emulate the environment that the errors occurred in correctly :)

Question, though: why call remove after each one? There isn't really a need to do that since they will remove themselves when they finish. If it's a matter of making sure they're in sequence instead of stacked along the edge, they can set them to queue (config before you call any notifications) instead and set the life to make sure there is only one visible at a time.

@tadeubas
Copy link
Author

Hmm nice to hear about queues!

Today a lot of pages can create a notification and some of them are sticky error messages. Sometimes two or more messages are called and I only check to maintain the last error message on screen, so it will eventually call the remove code and create a notification.

@willsteinmetz
Copy link
Collaborator

I definitely need to look into the bug that you found but it sounds like you should look into the queuing functionality. They even work with sticky notifications (you can mix and match sticky and not sticky as well).

However, you just gave me an idea for a new feature to have the ability to name a notification and have the ability to rip it from the queue by name (that would probably need to be required for this functionality). From the sounds of what you said about possibly creating two or more messages and only keeping the last in the queue, that might be a useful feature to help with that functionality.

@tadeubas
Copy link
Author

@willsteinmetz It is nice to see you making theses changes, thx!

But the fact is that when you set those timeouts for 1ms and 5ms ( https://github.com/willsteinmetz/notific8/blob/develop/dist/notific8.js#L106 - https://github.com/willsteinmetz/notific8/blob/develop/dist/notific8.js#L118 ) and get the element notification = document.getElementById(notificationId); it can be null depending on what happened between 1ms and 5ms, so you should check if the element is null like you did here: https://github.com/willsteinmetz/notific8/blob/develop/dist/notific8.js#L169

It is a problem HARD to reproduce because it depends on the clock of your machine, but on mine i7-3770 @ 3.4GHz x 4 I am getting this error Uncaught TypeError: Cannot read property 'style' of null or Uncaught TypeError: Cannot read property className of null when I call those in sequence:

notific8('remove');
notific8('test', {theme: 'ruby', sticky: true});
notific8('remove');
notific8('test2', {life: 1000});
notific8('remove');
notific8('test3', {theme: 'ruby', sticky: true});
notific8('remove');
notific8('test4', {life: 4000});

@tadeubas
Copy link
Author

And I am not using queues, so queue is FALSE

@willsteinmetz
Copy link
Collaborator

I'm definitely going to look into that problem. I get that it's there. I'm suggesting that queues might be worthwhile for what you're trying to accomplish as well. Either way, I am going to fix that bug.

@willsteinmetz
Copy link
Collaborator

@Tudumanu I just pushed up the fix in version 4.0.4. It should be available now on Bower and NPM.

@tadeubas
Copy link
Author

THX!!!

@willsteinmetz
Copy link
Collaborator

@Tudumanu I'm working on issue #72 now. That might be of interest to you as well.

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

No branches or pull requests

2 participants