Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Attempt a solution for #22 #23

Merged
merged 2 commits into from
Aug 10, 2014
Merged

Attempt a solution for #22 #23

merged 2 commits into from
Aug 10, 2014

Conversation

soumya92
Copy link

@soumya92 soumya92 commented Jul 1, 2014

No description provided.

soumya92 added 2 commits June 30, 2014 17:03
Trying to fix #22. This ensures that checks happen right before
addition, so there’s no way to have multiple notifications with the
same content.
@JanStevens
Copy link
Owner

The timeout is used to add the message in the next apply cycle of angular. You can also force an apply but this will result in errors (hence the docs recommend using timeout instead of apply).

I like your fix because the logic of addMessage should check if the message is already added or not.

I see that some stuff is added to the build files but I cannot find the matching code in the src directory? Can you add all your code changes to the src directory?

Did it work? Any thoughts on if the timeout is really needed or not?

@soumya92
Copy link
Author

Using timeout vs. apply makes complete sense. Probably don't want to call apply within another apply call or something...

There's two commits in the PR, one with the changes to source, and one with the updated built files. I split it up so if you didn't want updated built files, you can ignore the second commit.

@soumya92
Copy link
Author

Also, I have a copy with these changes applied and I'm using it in my application. I haven't had any problems so far.

@JanStevens JanStevens merged commit 7ff698b into JanStevens:master Aug 10, 2014
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants