search

project-robius / robrix

A Matrix chat client written in pure Rust using the Makepad UI toolkit and the Robius app dev framework
MIT License
67 stars 11 forks source link

Send user read receipts based on scroll position #86 #124

Closed alanpoon closed 5 days ago

alanpoon commented 3 weeks ago

https://github.com/project-robius/robrix/issues/86

  1. Send a regular Read receipt is done inside draw_walk function
  2. Send a FullyRead receipt is implemented inside when the user clicks the bottom half of the screen. fully_read
alanpoon commented 3 weeks ago

Hi @alanpoon, thanks so much for the contribution! I really appreciate you looking into this feature.

I've got a couple of questions/comments:

  1. I'm not actually sure what the best policy is for determining whether a user has actually "read" a given message. Is your heuristic here a typical choice, that is, is it common to require a "click in the bottom half of the window" to signify that the user has read a message? I would think it'd be more common to base that on where the user has scrolled to, not where they click or tap. I'm also not sure if we should make it time-based, although I know i did mention that in the original issue Send user read receipts based on scroll position #86, but I'm reconsidering now. Would love to hear your opinion there.
  2. The draw_walk() routine should be as fast and as minimal as possible, so it's not the best place to do something like checking/calculating where a message is and then sending a user read receipt update. Ideally this should be done in a background task, but for the sake of ease, you could do it in the event handler upon a PortalList::scrolled() action, since you're already doing other logic in the event handler currently.
  1. I am taking Element desktop app as a reference. It seems like it registers fully read event when user click on the message. To do so, I would need to compute the click box for the individual item in the scroll list. I couldn't implement this thus I implemented "click bottom half of the screen" and send the fully read event based on the index of content last drawn.
  2. I will look into it.
ZhangHanDong commented 3 weeks ago

86

kevinaboos commented 2 weeks ago
  1. I am taking Element desktop app as a reference. It seems like it registers fully read event when user click on the message. To do so, I would need to compute the click box for the individual item in the scroll list. I couldn't implement this thus I implemented "click bottom half of the screen" and send the fully read event based on the index of content last drawn.

Thanks for the response! Personally I think that determining whether a user has read a given message should not be based on click activity, but rather just the position of the timeline on screen. In the future, when we have a user preferences screen, we can make this configurable based on user choice. So, whatever option we choose here, let's make it easy to change and thus, cleanly encapsulate all the logic into one function.

ZhangHanDong commented 2 weeks ago

I think the process should be like this:

The Timeline component sends appropriate read receipts when the user scrolls. It will send a Read receipt when the message first scrolls to the bottom of the view, send a FullyRead receipt when the message scrolls out of the top of the view, and also send a FullyRead receipt when the message has been displayed on screen for more than 5 seconds.

@kevinaboos Is that correct?

kevinaboos commented 2 weeks ago

Yep! To echo what I said in our chat:

yes, that sounds generally correct!

Note that for a first attempt, we don't even have to make things that complicated -- we can just send only FullyRead receipts once the message is displayed on screen (for any amount of time). Then we can always refine the policy (or later, make it a user-configurable choice) to make the UX even better.

alanpoon commented 2 weeks ago

I think the process should be like this:

The Timeline component sends appropriate read receipts when the user scrolls. It will send a Read receipt when the message first scrolls to the bottom of the view, send a FullyRead receipt when the message scrolls out of the top of the view, and also send a FullyRead receipt when the message has been displayed on screen for more than 5 seconds.

@kevinaboos Is that correct?

I did some fix to add the logic to send FullyRead receipt when the message has been displayed on screen for more than 5 seconds. I removed the click logic for FullyRead receipt .