prose-im / prose-core-client

Prose core XMPP client manager & protocols.
https://prose.org
Mozilla Public License 2.0
20 stars 3 forks source link

Support unread count in conversations #49

Closed nesium closed 6 months ago

nesium commented 7 months ago

While the proper solution that would work across multiple clients is out of reach at the moment, let's implement a preliminary solution at least. It's a horrible experience to not see when a message comes in, or worse: Hear the sound but not see where it came from. So at least show an unread marker for the affected conversation. It doesn't need to be persisted between launches/reloads but the API could have the final form already.

valeriansaliou commented 7 months ago

I'd agree to that. At least that gives some primitive, unreliable, form of unread count for the local session only, on servers that do not support Inbox.

nesium commented 7 months ago

So, API-wise this has 3 components:

  1. The unread count for a room needs to be communicated
  2. An event needs to be triggered when the unread count changes
  3. A method to mark a conversation/room as read needs to be provided

Re 1: SidebarItem already has a property unreadCount, so we're good there.

Re 2: We'll use the sidebarChanged event there. Also done.

Re 3: I can think of two ways here:

  1. When a new message comes in, the core would increase the unread count for the affected room, then send a sidebarChanged event, as well as a messagesAppended event. The client would then check if the affected room is selected and call e.g. a method room.markAsRead(). The core would then clear the unread count and send another event sidebarChanged. The client would need to take care to not show an unread count for a selected room otherwise we could have a flash in the sidebar between the event and the room.markAsRead() call.

    • Pro: Simple
    • Con: Duplicate event
  2. Alternatively the client could mark a room as selected (e.g. room.setSelected(true|false)). The core would then take the selection into account and would not increase the unread count and thus not send a sidebarChanged if a message in that room was received.

    • Pro: No duplicate event + selection handling might enable other features in the future
    • Con: Client needs to make sure to keep its sidebar selection in sync with the core

What do you think @valeriansaliou?


Other implementation notes:

The plan right now is to keep the unread count transient. That is on reload/restart it would be reset to 0 for all rooms. This is where prose-im/prose-pod-system#9 would come into play: On startup the core would request the unread messages from the server and update the unread counts accordingly.

I could theoretically persist the unread counts on the server in our sidebar items, which would then sync with other open clients of the same user but I don't think it would provide any value since open clients would also be able to keep track of newly received messages (or carbons for that matter).

We could also persist the current unread count locally so that you start in the same state where you left when you've closed the app. But unless we have support for the Inbox XEP each room would then sit in the sidebar with potentially invalid unread counts until you select each room so that its unread count goes to 0 - even if you have seen the messages on another client already.

Another alternative would be to actually use the read markers to determine the number of unread messages. The client would then need to send the IDs of messages that are scrolled into the visible area to the core, so that the core can send read markers to the server and decrease the unread count. This kind of depends on the UI where you could either display individual messages as unread or insert a separator above the oldest unread message. In the latter case I would assume the separator to be transient and inserted the next time the room is displayed above the oldest unread message just above the newest read message (i.e. you can have gaps between read messages, but these are ignored).

We could also do something like load the last n messages for each room after connect and count the unread messages to update the read counts. Then display something like n+ unread messages, where n is a low number since we don't want to load a lot of messages. Basically a poor man's Inbox XEP.

valeriansaliou commented 7 months ago

I think I’d go for solution 1. This is IMHO much cleaner that way, at the expense of more ingress events.

This also means that weird things won’t happen if the implementation of the ‘set selected’ is done in a wrong or incomplete way in a certain app implementation, at least we still get a count that never gets flushed.

nesium commented 7 months ago

New API:

export interface RoomBase {
    // …

    markAsRead(): Promise<void>;
}
valeriansaliou commented 7 months ago

All implemented as of https://github.com/prose-im/prose-app-web/commit/a5105f4192699840f2289f5b1fb7b57149684582 ; it's working great! Feel free to test :)

nesium commented 7 months ago

Hm, it doesn't seem to work for me. I'm not able to get rid of the unread count. It's there when I receive a message with the conversation selected and also stays there when I switch back to a conversation with unread messages. How is it supposed to work?

valeriansaliou commented 7 months ago

The unread count should go away when you're:

  1. Scrolled all the way down in the message view (ie. the latest unread message is visible in view)
  2. The Prose app is in focus, that is, you're currently viewing it and your Web browser has focus

It will auto-go away when those 2 conditions are matched.

If it's still not working for you, could you please provide me with a reproduction scenario/steps?

nesium commented 6 months ago

I think we can close this for the time being.