rpaschoal / ng-chat

💬 A simple facebook/linkedin lookalike chat module for Angular applications.
MIT License
155 stars 92 forks source link

Error on emitBrowserNotification #57

Closed joselvb closed 6 years ago

joselvb commented 6 years ago

An error is occurring in the emitBrowserNotification function that is caused by attempting to access the empty message array.

let message = window.messages[window.messages.length - 1].message;

I noticed this happens when I send a new message to a user I did not talk to earlier or when I start a new conversation.

rpaschoal commented 6 years ago

Hi @joselvb,

Thank you for reporting this issue.

For now, as a workaround, I would advise you to turn off the browser notifications by setting the property browserNotificationsEnabled to false.

I'll be working on a fix for this in a new release. Just give me a few days. If you're comfortable in implementing the fix yourself and sending the pull request here, please advise and I'll let you work on it if this is a priority for you.

Regardless if you are doing it or me, the fix should be in a new release in a few days (next week most likely).

Cheers!

rpaschoal commented 6 years ago

Hi @joselvb,

The bug was happening because a message that is received when you have the history mode enabled is not pushed to the messages array of the window (This is to avoid the message being duplicate as you would have it saved in your data storage already in your back-end).

The fix was fairly simple, just pass the message received on onMessageReceived to emitBrowserNotification.

Here is the commit to fix the issue: https://github.com/rpaschoal/ng-chat/commit/829ab158cd0bbf3c1dae7c21a86c77cca6c7603f

It will be coming soon with version 1.0.13 but I still have to work around unit tests and implement the other suggestions you've made. Should be a couple days more before I release it.

Cheers!

joselvb commented 6 years ago

Hello @rpaschoal,

Thank you very much for the correction.

I'll wait for the next release to update. For now I've disabled the notifications option.

rpaschoal commented 6 years ago

Hi @joselvb ,

1.0.13 is now on NPM! 🎉

Here is the release notes https://github.com/rpaschoal/ng-chat/releases/tag/1.0.13

Thank you for reporting this issue. I will close it as it is now fixed on 1.0.13.