rpaschoal / ng-chat

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

New options #58

Closed joselvb closed 6 years ago

joselvb commented 6 years ago

I would like to give some suggestions for improvements to the component.

There could be a property on the user to populate with an unread message counter and appear in the contact list, just as it appears in the title of a window when it is not in focus.

You might also have an option not to open the window automatically when you receive a message. For example, I send a message to a certain user who is online, that other user's window does not open automatically, or open but by default minimized, in order to have a certain privacy.

rpaschoal commented 6 years ago

Hi @joselvb,

Thank you for the suggestions, always nice to hear from the community!

I will work on these along with the bug that you've found and reported here #57

If you've got any more suggestions, just post them here :)

Cheers!

rpaschoal commented 6 years ago

Hi @joselvb,

I have added the total messages counter to the friends list on this commit: https://github.com/rpaschoal/ng-chat/commit/850e1dde3b0b8b7e2e09ef33a5bfb2338fd14d65

In regards to the behavior of new messages, I will add just a setting to open a new window but minimized (EG: maximizeWindowOnNewMessage or similar). I was considering adding multiple behaviors but the way the component is implemented the counter of unread messages work on the assumption it does have the messages loaded/fetched. I will see if I can implement this today, and if so, I will be releasing 1.0.13 right after.

rpaschoal commented 6 years ago

Hi @joselvb ,

The suggested features are now available on NPM with version 1.0.13. Check out the release notes: https://github.com/rpaschoal/ng-chat/releases/tag/1.0.13

Looking forward to hearing what you think of the new features.

Cheers!

joselvb commented 6 years ago

Thanks for the changes.

I updated the project with the latest release and everything is working correctly.

I would like to make two points for improvement:

  1. You could have a new property in the User class so that we can fill in the amount of unread messages when you load the API contact list for the first time, so that the user can see the contacts who sent you messages while he was offline. At the moment I had to make an implementation that when the user connects, forces sending a null message from the other contacts that sent a message while it was offline. That way the component opens the chat window.

  2. A localization improvement in the notification message that is currently set to 'New message from'. You could have a new property in the Localization interface, like the one that exists today for example 'searchPlaceholder'. That way we can define the message we want to give when the user receives a new message.

rpaschoal commented 6 years ago

Hi @joselvb ,

Great suggestions, do you think you would be able to work on them and contribute to the project?

Have a look at the contributing guidelines https://github.com/rpaschoal/ng-chat/blob/master/CONTRIBUTING.md

LuisReinoso commented 6 years ago

Hi! I could contribute for browser notification message but I think it's useful implement it in two ways.

  1. Using Localization interface.
  2. Through @input().

Variable name: messageBrowserNotification It's ok?

rpaschoal commented 6 years ago

Sure thing @LuisReinoso , just get a pull request for that and if I spot anything that might be worth a refactory I will add some review comments.

Variable name: messageBrowserNotification This term does not sound very natural in English, it will be better if we name it as browserNotificationTitle if you are referring to the message's title.

Looking forward to seeing your PR here @LuisReinoso

Cheers!

LuisReinoso commented 6 years ago

It's ok @rpaschoal. Please review my PR. An observation: Could you format the source code because it has some white space additionally I had some problems to run tests. Could you be more specific in the CONTRIBUTING.md for the execution of the tests.?

rpaschoal commented 6 years ago

Hi @LuisReinoso,

I've just merged your PR to branch 1.0.14 for a new release. Have a look at the branch and latest commits, including the ones you have worked on https://github.com/rpaschoal/ng-chat/tree/1.0.14

I have refactored the unit tests a bit, I've found something that was causing a few of them to fail, it should be all good from now on.

Do you have any other options or new small features you wish to implement for 1.0.14 or shall I create a PR for the master branch and push 1.0.14 to NPM?

Cheers!

LuisReinoso commented 6 years ago

For now I think that would be all for 1.0.14.

rpaschoal commented 6 years ago

I am closing this as I've just published https://github.com/rpaschoal/ng-chat/releases/tag/1.0.14 and the initial suggestions were implemented on https://github.com/rpaschoal/ng-chat/releases/tag/1.0.13.