matrix-org / matrix-android-console

Apache License 2.0
26 stars 16 forks source link

When event is received, clear notifications #19

Closed mitchhentges closed 8 years ago

mitchhentges commented 8 years ago

Duplicate from #18, but into correct branch. Original comment:

This works awesomely if there's only one user on the android app. If there's multiple users on the app, then (due to a pre-existing bug), no notifications appear for the other user.

ara4n commented 8 years ago

So the main issue here with the fix is that historical read receipts may cause the notification state to be incorrectly cleared. Separately, it should be multi-account aware (this could be a separate PR however).

The correct implementation is quite subtle - for instance, vector-web calculates unread state as per: https://github.com/matrix-org/matrix-react-sdk/blob/develop/src/Unread.js#L37

Please note (both @ylecollen and @mitchhentges) that for the special case of missed push notifications and missed 'highlight' push notifications (not just unread messages), synapse generates explicit unread_notification fields on the /sync response, which gives you, per-room, whether a notification should be shown for a room (both how many missed highlighted messages and how many missed pushes in general). The datastructure looks like:

unread_notifications: {
    highlight_count: 0
    notification_count: 0
}

Supporting proper synchronised highlights and missed push notifs however is probably a separate PR.

@mitchhentges - up to you if you'd like to get the unread message semantics right, otherwise we'll get to it. It should probably be being calculated by the SDK rather than Console. I'm afraid that I'd prefer that we don't merge a partial solution here.

mitchhentges commented 8 years ago

I'll do it in a new PR, which will involve a refactor (and also make this obsolete). Thanks!