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

[notifications] add a notification summary list #40

Closed
7 tasks done
sixP-NaraKa opened this issue Jul 23, 2022 · 4 comments · Fixed by #43
Closed
7 tasks done

[notifications] add a notification summary list #40

sixP-NaraKa opened this issue Jul 23, 2022 · 4 comments · Fixed by #43

Comments

@sixP-NaraKa
Copy link
Owner

sixP-NaraKa commented Jul 23, 2022

This means that a user can look into what exactly a notification was about, e.g. was it a new unread message, reaction or a missed call?

Information for each "missed" notification should contain the following:

sixP-NaraKa added a commit that referenced this issue Jul 28, 2022
…, which takes over the existing functionality of sliding content in/out, but in a more flexible and customizable way.
sixP-NaraKa added a commit that referenced this issue Jul 28, 2022
…able pattern to notify new component about new notification events. Show the notifications in the corresponding slider.
@sixP-NaraKa sixP-NaraKa self-assigned this Jul 28, 2022
sixP-NaraKa added a commit that referenced this issue Jul 29, 2022
…s' and add queries and endpoints accordingly. Adjust usage of existing implementation.
@sixP-NaraKa
Copy link
Owner Author

sixP-NaraKa commented Jul 29, 2022

Note:
the in 53f6de6 / f1423e3 implemented persistence is, for now, only saving the notifications during runtime of the "affected" user, meaning that if the user who the notifications belong to is not logged in, they will not be saved into the db.
This has to do with the general way we listen to messages/reactions/calls from other people (done via listeners on the websocket events).

This should probably be changed, though this would then require more effort. For now, only getting these during runtime is fine.

@sixP-NaraKa
Copy link
Owner Author

Proposal on how to handle the above mentioned issue:

  • save every message, reaction and missed/ignored call into the notifications event table with the corresponding data
  • the moved implementation of the saving of the notification record in f1423e3 only needs to be moved again accordingly
  • add a new db column to indicate whether the notification was in the currently opened chatroom for the participant, as we do not need to show this in the summary list (maybe)

Furthermore, after some more consideration, I might just ditch the message notifications, and only do reactions and calls.
Why? Because in the future I would like to have "delivered"/"seen" messages, so chat-tab highlights will stay permanent if the user has not yet seen the containing new messages.

Still requires some more thinking. :)

@sixP-NaraKa
Copy link
Owner Author

The mentioned issue will be handled in #42.

sixP-NaraKa added a commit that referenced this issue Jul 31, 2022
… better output. Use onPush change detection for the notification-summary component.
sixP-NaraKa added a commit that referenced this issue Jul 31, 2022
…pon receiving a voice chat request the notification event will get saved and shown in the UI, regardless if the request was accepted, declined or ignored (might change in the future to only on ignored requests).
@sixP-NaraKa
Copy link
Owner Author

Note:
as mentioned in the commit message of 5820fc6, the notification gets saved and shown immediately for voice chat requests.
This is mainly due to the way this has been implemented - since if we do it as I tested initially, the user who made the request will see the notification in their own list as well, until they refresh the page. The recipient will also only see the notification after a refresh, since we never actually notified them directly, if they were online at the time.

This is again one of the consequences of going the websocket route to handle events and stuff, as mentioned previously.
In the future different ways to handle this will be looked into, as to not repeat the same "mistakes"/"inefficiencies" which exist in some places here currently. :)

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

Successfully merging a pull request may close this issue.

1 participant