element-hq / element-x-ios

Next generation Matrix client for iOS built with SwiftUI on top of matrix-rust-sdk.
https://element.io/labs/element-x
GNU Affero General Public License v3.0
423 stars 100 forks source link

unsent send receipts should immediately replace ticked send receipts #1101

Closed ara4n closed 1 year ago

ara4n commented 1 year ago

Steps to reproduce

  1. Send two messages one after another
  2. The first one will be marked as sent (with a ticked send receipt circle)
  3. The second one will still be marked as sending (empty send receipt circle).

The fact the first one was sent successfully can be inferred by the fact we are now sending the second however - we don’t need to see both circles, unless the first one hasn’t yet sent. As a result, it makes the timeline pop around and look redundant and confusing:

IMG_0535

Outcome

What did you expect?

Only show SRs if they are the most recent message you sent (or if they are stuck trying to send).

What happened instead?

Duplicated SRs telling me useless information which disappear only when the second SR arrives, causing lots of timeline pops. Instead the SR should disappear as soon as you send a new msg.

You can also get dups where both are in sent state…

IMG_0537

Your phone model

No response

Operating system version

No response

Application version

242

Homeserver

No response

Will you send logs?

No

manuroe commented 1 year ago

@amshakal what do you think about this proposal? It can be a solution to the problem reported on Friday.

We also need to consider when there are several messages queued for sending: do we want to use a temporary line for each empty circles? Every time we use this temporary line, it makes the timeline clunky when it disappears.

nadonomy commented 1 year ago

@manuroe btw @amshakal is out this week so anything timely best to tag @callumu.

ara4n commented 1 year ago

We also need to consider when there are several messages queued for sending: do we want to use a temporary line for each empty circles? Every time we use this temporary line, it makes the timeline clunky when it disappears.

personally I think the negative space caused by the various unsent SRs is a feature, rather than a bug: if you're on bad connectivity and all your messages are stacked up, a) it's your fault, b) you want to know about it, c) the 'unpopping' effect is quite satisfying when they finally send.

The thing i'm objecting to here is that we include (and then unpop) sent SRs, not just unsent ones. Surely we should be copying EW's semantics, which feel fine? I have a feeling that it's probably a bug that we're not.

ara4n commented 1 year ago

the duplicate ticked SRs feel really flakey, hence marking as potential release blocker

VolkerJunginger commented 1 year ago

https://github.com/vector-im/element-x-ios/assets/118339066/30300731-ec56-4a62-ad95-9d0dba0b51cd

I added the animation I made on how this should work.

Message 1 & 2 have been send in a row. Message 3 at a later point in time.

manuroe commented 1 year ago

Conclusion from an early discussion with @Velin92:

We want to display the checkmark icon as less as possible. The information it provides is repetitive. At least, user can deduce when there are send receipts.

We want to display the checkmark icon only once max in the timeline. It must be displayed only on the last message. If the last message is a local echo, we display the empty circle icon on it, no checkmark icon on it. In the future, RRs will replace the checkmark icon.

Other states: We continue to display an error for every send failure. We continue to display an empty circle for every local echo <-- To be validated with @VolkerJunginger

Velin92 commented 1 year ago

Conclusion from an early discussion with @Velin92:

We want to display the checkmark icon as less as possible. The information it provides is repetitive. At least, user can deduce when there are send receipts.

We want to display the checkmark icon only once max in the timeline. It must be displayed only on the last message. If the last message is a local echo, we display the empty circle icon on it, no checkmark icon on it. In the future, RRs will replace the checkmark icon.

Other states: We continue to display an error for every send failure. We continue to display an empty circle for every local echo <-- To be validated with @VolkerJunginger

This is the behaviour with only the last checkmark being displayed, however while doing my tests I noticed something weird with the queueing, essentially the sent message always gets moved at the end of the timeline, which creates this weird jumping, there is anything we can do about it? @jplatte https://github.com/vector-im/element-x-ios/assets/34335419/097a1cab-01f2-46fb-a119-9eaccf9cc40a

jplatte commented 1 year ago

Known issue, top prio for me now.