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

Bold is too unclear for unread messages. #5953

Closed
ara4n opened this issue Jan 9, 2018 · 11 comments
Closed

Bold is too unclear for unread messages. #5953

ara4n opened this issue Jan 9, 2018 · 11 comments
Labels
P1 S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect

Comments

@ara4n
Copy link
Member

ara4n commented Jan 9, 2018

Long-standing problem, particularly on the dark or Status themes, is that using 'bold' to indicate unread is way too cryptic and hard to spot.

@ghost
Copy link

ghost commented Jan 9, 2018

Maybe show an unread counter on rooms as per #3374? 😇

@lampholder lampholder added T-Defect ui/ux P1 S-Major Severely degrades major functionality or product features, with no satisfactory workaround labels Jan 11, 2018
@lukebarnard1
Copy link
Contributor

I was thinking we could do a small circle to the top-right of the room avatars, although it would end up conflicting with badges a little bit.

@ara4n
Copy link
Member Author

ara4n commented Jan 16, 2018

i think @Matrixcoffee's suggestion of revisiting #3374 is a good one; it's a bit weird that there isn't a way to be told how many unread messages are in a room (only unread notifications). A better solution might be:

  • 10 unread messages (no notifications) = 50% transparent green badge with number 10
  • 10 unread messages (of which 5 were notifs) = solid green badge with number 10
  • 10 unread messages (of which 5 were notifs, 1 was a bing) = solid red badge with number 10

@lampholder wdyt?

Every time we've changed the badge settings it's been a huge can of worms, but i'm not sure that the suggestion here is bad (although we'd end up with a lot more badges flying around the place, and so look busier)

@ara4n
Copy link
Member Author

ara4n commented Jan 16, 2018

oh - the problem here, thinking about it, is that maintaining unread badge counts is surprisingly resource intensive on the server, and doing it for every room (whether notifs are turned on or not) might be completely nightmarish for synapse. On the otherhand, it might simplify the logic and make it easier to maintain (given we're just tracking unread messages rather than unread notifications). @dbkr, any idea?

@dbkr
Copy link
Member

dbkr commented Jan 16, 2018

Yep, indeed this needs server support because the only way the client would be able to know is by paginating all the way back to its last read marker which could be a long way.

I suspect it's not fundamentally difficult and probably not that resource intensive - most of the pushrules stuff is running the push rules themselves, ie. the fact that everyone could have different rules. I suspect the hardest part about this would be incrementing a counter for everyone in the room (or rather, saving that back to the db in a way that's fast and safe).

One gotcha is that not all clients are necessarily able to render the same types of messages, ie. in Riot we avoid bolding the room for message we don't display, but the server doesn't know which messages we're not going to display. This could be another thing solved by all renderable events sharing a 'body' field. ;)

That said, you could also increase the number of messages we get per room in an initial /sync to, say, 11 (I think we currently only sync 2?). This would let you display a correct count most of the time except when you first logged in / cleared cache, at which point if it was more than 10, you'd display '10+' or something. This would obviously increase login time though.

@t3chguy
Copy link
Member

t3chguy commented Jan 16, 2018

One gotcha is that not all clients are necessarily able to render the same types of messages, ie. in Riot we avoid bolding the room for message we don't display, but the server doesn't know which messages we're not going to display. This could be another thing solved by all renderable events sharing a 'body' field. ;)

Except riot also doesn't bold on hidden events (hide parts joins whatever)

@turt2live
Copy link
Member

That said, you could also increase the number of messages we get per room in an initial /sync to, say, 11 (I think we currently only sync 2?).

Setting up a new device takes long enough as is :(

@ghost
Copy link

ghost commented Jan 20, 2018

Do the unread counts need to be exact? Could we get reasonable results by looking at the message depth field?

@t3chguy
Copy link
Member

t3chguy commented Jan 20, 2018

I think if it said 7 unread messages and I open it to find 2 because of hiddenPartsJoins/whatever I'll be wondering what the hell happened

@ghost
Copy link

ghost commented Jan 20, 2018

Right, 7 for 2 is clearly not a reasonable approximation.

So get low counts directly from the sync, possibly with some kind of server-assisted middle ground to deal with the 2-10 event gap. For higher counts, you'd probably be able to get away with a ballpark figure from the message depth.

ara4n added a commit to matrix-org/synapse that referenced this issue Sep 19, 2019
…ations

This is a potential solution to https://github.com/vector-im/riot-web/issues/3374
and element-hq/element-web#5953
as raised by Mozilla at element-hq/element-web#10868.

This lets you define a push rule action which increases the badge count (unread notification)
count on a given room, but doesn't actually send a push for that notification via email or HTTP.
We might want to define this as the default behaviour for group chats in future
to solve https://github.com/vector-im/riot-web/issues/3268 at last.

This is implemented as a string action rather than a tweak because:
 * Other pushers don't care about the tweak, given they won't ever get pushed
 * The DB can store the tweak more efficiently using the existing `notify` table.
 * It avoids breaking the default_notif/highlight_action optimisations.

Clients which generate their own notifs (e.g. desktop notifs from Riot/Web
would need to be aware of the new push action) to uphold it.

An alternative way to do this would be to maintain a `msg_count` alongside
`highlight_count` and `notification_count` in `unread_notifications` in sync responses.
However, doing this by counting the rows in `events` since the `stream_position`
of the user's last read receipt turns out to be painfully slow (~200ms), perhaps
due to the size of the events table.  So instead, we use the highly optimised
existing event_push_actions (and event_push_actions_staging) table to maintain
the counts - using the code paths which already exist for tracking unread
notification counts efficiently.  These queries are typically ~3ms or so.

The biggest issues I see here are:
 * We're slightly repurposing the `notif` field on `event_push_actions` to
   track whether a given action actually sent a `push` or not.  This doesn't
   seem unreasonable, but it's slightly naughty given that previously the
   field explicitly tracked whether `notify` was true for the action (and
   as a result, it was uselessly always set to 1 in the DB).
 * We're going to put more load on the `event_push_actions` table for all the
   random group chats which people had previously muted. In practice i don't
   think there are many of these though.
 * There isn't an MSC for this yet (although this comment could become one).
@jryans jryans removed the Z-UI/UX label Mar 9, 2021
@nadonomy
Copy link
Contributor

Closing given age. Feel free to reopen (with screenshots) if still an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect
Projects
None yet
Development

No branches or pull requests

8 participants