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

Display number of unread messages as a badge on the jump-to-bottom button. Send fully-read receipts more efficiently. #206

Open
wants to merge 51 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
5657931
reduce complexity of send_read_receipt
alanpoon Oct 4, 2024
bb92afb
cleanup
alanpoon Oct 4, 2024
9255365
removed unnecessary import
alanpoon Oct 4, 2024
24be468
added unread_message_badge
alanpoon Oct 10, 2024
c1d1ab6
remove unneccesary changes
alanpoon Oct 11, 2024
35c5977
add jump to top
alanpoon Oct 11, 2024
bc2aa8c
added unread_notification_badge
alanpoon Oct 11, 2024
a9e4945
Merge branch 'main' into jtb_change_in_timeline_item#131
alanpoon Oct 12, 2024
053c8f0
Merge branch 'main' into send_read_receipt_improve
alanpoon Oct 13, 2024
e4c1268
Added scroll past marker
alanpoon Oct 13, 2024
072b9cb
Merge branch 'jtb_change_in_timeline_item#131' into send_read+jtb
alanpoon Oct 13, 2024
3884a66
wip
alanpoon Oct 14, 2024
b7a83a0
send read +jtb
alanpoon Oct 15, 2024
a3b1f10
Merge branch 'main' into send_read+jtb
alanpoon Oct 15, 2024
3824160
added some docs, remove icon_jump_to_top
alanpoon Nov 4, 2024
5633e2b
Merge branch 'main' into send_read+jtb
alanpoon Nov 4, 2024
44f4c25
merge .cargo
alanpoon Nov 4, 2024
9076c53
minor formating
alanpoon Nov 4, 2024
5bf2413
change to start_timeout
alanpoon Nov 4, 2024
1c66730
Merge branch 'main' into send_read+jtb
alanpoon Nov 6, 2024
8bbde58
removed jump_to_top & fix timer issue
alanpoon Nov 6, 2024
b99a57d
set read event into RoomInfo's fully read when sending
alanpoon Nov 6, 2024
99ff1b1
restructure scroll pass read marker logic
alanpoon Nov 6, 2024
0b89757
Merge branch 'main' into send_read+jtb
alanpoon Nov 18, 2024
009077f
Added unread_notification badge
alanpoon Nov 19, 2024
aba7e69
code cleanup
alanpoon Nov 19, 2024
2ce7a20
Merge branch 'main' into send_read+jtb
alanpoon Nov 19, 2024
945d3a1
fix indentation and grammar
alanpoon Nov 19, 2024
170488c
minor comment changes
alanpoon Nov 19, 2024
20982c2
fix not send read receipt when not scrolling
alanpoon Nov 19, 2024
1e5e36c
removed last_display_event
alanpoon Nov 19, 2024
b4b5116
Using fully_read_event's timestamp to determine scrolled_past_fully_read
alanpoon Nov 23, 2024
84faf5e
added doc, use timer.is_empty to check for events
alanpoon Nov 23, 2024
e748196
code cleanup
alanpoon Nov 23, 2024
963a722
Code improvement n log improvement
alanpoon Nov 25, 2024
1566621
changed and_then to map
alanpoon Nov 27, 2024
3627a1f
changed to get_fully_read_event
alanpoon Nov 27, 2024
07df9fd
cap at 99+
alanpoon Nov 27, 2024
b44105a
Merge branch 'send_read+jtb' of https://github.com/alanpoon/robrix in…
alanpoon Nov 27, 2024
c32b7d5
Merge branch 'main' of https://github.com/project-robius/robrix into …
alanpoon Nov 27, 2024
1e7d4b2
change to get_fully_read_event
alanpoon Nov 27, 2024
ced53e5
Move client num_of_unread to async
alanpoon Nov 28, 2024
1caacc1
make oval for 99+
alanpoon Nov 28, 2024
22d8e30
Merge branch 'main' of https://github.com/project-robius/robrix into …
alanpoon Nov 28, 2024
35b8b02
fix clippy warning
alanpoon Nov 28, 2024
906e53e
fix error handling for set_fully_read_event
alanpoon Nov 28, 2024
fe9973b
added subscribe to update read_receipt
alanpoon Dec 2, 2024
3d090fb
Added comments for subscribe to Updates
alanpoon Dec 2, 2024
b12592a
added new lines and fix clippy
alanpoon Dec 3, 2024
eb857c6
added update_subscriber_initialised check
alanpoon Dec 3, 2024
794cd0a
fix clippy
alanpoon Dec 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 107 additions & 62 deletions src/home/room_screen.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! A room screen is the UI page that displays a single Room's timeline of events/messages
//! along with a message input bar at the bottom.

use std::{borrow::Cow, collections::{BTreeMap, HashMap}, ops::{DerefMut, Range}, sync::{Arc, Mutex}, time::{Instant, SystemTime}};
use std::{borrow::Cow, collections::BTreeMap, ops::{DerefMut, Range}, sync::{Arc, Mutex}, time::SystemTime};

use bytesize::ByteSize;
use imbl::Vector;
Expand All @@ -27,7 +27,7 @@ use crate::{
user_profile_cache,
}, shared::{
avatar::{AvatarRef, AvatarWidgetRefExt}, html_or_plaintext::{HtmlOrPlaintextRef, HtmlOrPlaintextWidgetRefExt}, jump_to_bottom_button::JumpToBottomButtonWidgetExt, text_or_image::{TextOrImageRef, TextOrImageWidgetRefExt}, typing_animation::TypingAnimationWidgetExt
}, sliding_sync::{get_client, submit_async_request, take_timeline_endpoints, BackwardsPaginateUntilEventRequest, MatrixRequest, PaginationDirection, TimelineRequestSender}, utils::{self, unix_time_millis_to_datetime, ImageFormat, MediaFormatConst}
}, sliding_sync::{get_client, get_fully_read_event, submit_async_request, take_timeline_endpoints, BackwardsPaginateUntilEventRequest, MatrixRequest, PaginationDirection, TimelineRequestSender}, utils::{self, unix_time_millis_to_datetime, ImageFormat, MediaFormatConst}
};
use rangemap::RangeSet;

Expand All @@ -50,6 +50,7 @@ live_design! {
import crate::shared::avatar::Avatar;
import crate::shared::text_or_image::TextOrImage;
import crate::shared::html_or_plaintext::*;
import crate::shared::icon_button::*;
import crate::profile::user_profile::UserProfileSlidingPane;
import crate::shared::typing_animation::TypingAnimation;
import crate::shared::icon_button::*;
Expand Down Expand Up @@ -969,8 +970,12 @@ pub struct RoomScreen {
#[rust] room_name: String,
/// The persistent UI-relevant states for the room that this widget is currently displaying.
#[rust] tl_state: Option<TimelineUiState>,
/// 5 secs timer when scroll ends
/// Timer to send fully read receipt
///
/// 5 secs timer starts when user scrolled down and scrolled_past_read_marker is true
/// Last displayed event will be sent as fully read receipt
Comment on lines +973 to +976
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pro-tip: doc comments like this get rendered into Html, so you need to format them as full sentences rather than just sentence fragments across line breaks.

Suggested change
/// Timer to send fully read receipt
///
/// 5 secs timer starts when user scrolled down and scrolled_past_read_marker is true
/// Last displayed event will be sent as fully read receipt
/// Timer used to send fully-read receipts for the current room.
///
/// We start a short (5 second) timer when the user scrolls down,
/// if `scrolled_past_read_marker` is true. Then, we send a fully-read receipt
/// for the last displayed event in this room.

#[rust] fully_read_timer: Timer,

}
impl Drop for RoomScreen {
fn drop(&mut self) {
Expand Down Expand Up @@ -1306,15 +1311,18 @@ impl Widget for RoomScreen {
});
}
}

// Mark events as fully read after they have been displayed on screen for 5 seconds.
if self.fully_read_timer.is_event(event).is_some() {
if let (Some(ref mut tl_state), Some(ref _room_id)) = (&mut self.tl_state, &self.room_id) {
for (k, (room, event, start, ref mut moved_to_queue)) in &mut tl_state.read_event_hashmap {
if start.elapsed() > std::time::Duration::new(5, 0) && !*moved_to_queue{
tl_state.marked_fully_read_queue.insert(k.clone(), (room.clone(), event.clone()));
*moved_to_queue = true;
}
// Send fully read receipt for last displayed event 5 Seconds after user scrolled past the read marker
if self.fully_read_timer.is_event(event).is_some() && !self.fully_read_timer.is_empty() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you explain (and add a comment) why it's necessary to check that the fully_read_timer is not empty?

Also, what does it mean for a timer to be non-empty?

if let (Some(ref mut tl_state), Some(ref room_id)) = (&mut self.tl_state, &self.room_id) {
if let Some((event_id, timestamp)) = tl_state
.last_display_event
.take()
{
submit_async_request(MatrixRequest::FullyReadReceipt {
room_id: room_id.clone(),
event_id,
timestamp,
});
}
}
cx.stop_timer(self.fully_read_timer);
Expand Down Expand Up @@ -1511,7 +1519,7 @@ impl RoomScreen {
tl.items = initial_items;
done_loading = true;
}
TimelineUpdate::NewItems { new_items, changed_indices, is_append, clear_cache } => {
TimelineUpdate::NewItems { new_items, changed_indices, is_append, clear_cache, unread_messages_count } => {
if new_items.is_empty() {
if !tl.items.is_empty() {
log!("Timeline::handle_event(): timeline (had {} items) was cleared for room {}", tl.items.len(), tl.room_id);
Expand Down Expand Up @@ -1559,7 +1567,9 @@ impl RoomScreen {
log!("Timeline::handle_event(): jumping view from event index {curr_item_idx} to new index {new_item_idx}, scroll {new_item_scroll}, event ID {_event_id}");
portal_list.set_first_id_and_scroll(new_item_idx, new_item_scroll);
tl.prev_first_index = Some(new_item_idx);
// Reset the fully read timer when we jump to a new event
cx.stop_timer(self.fully_read_timer);
tl.scrolled_past_read_marker = false;
}
}
//
Expand All @@ -1571,10 +1581,15 @@ impl RoomScreen {

// If new items were appended to the end of the timeline, show an unread messages badge on the jump to bottom button.
if is_append && !portal_list.is_at_end() {
// log!("is_append was true, showing unread message badge on the jump to bottom button visible");
jump_to_bottom.show_unread_message_badge(1);
if let Some(room_id) = &self.room_id {
// Set the number of unread messages to unread_notification_badge by async request to avoid locking in the Main UI thread
submit_async_request(MatrixRequest::GetNumOfUnReadMessages{ room_id: room_id.clone() });
}
}
if let Some(unread_messages_count) = unread_messages_count {
jump_to_bottom.show_unread_message_badge(cx, unread_messages_count);
continue;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why continue here? that seems odd, as we typically wouldn't want to skip anything else beneath this point.

}

if clear_cache {
tl.content_drawn_since_last_update.clear();
tl.profile_drawn_since_last_update.clear();
Expand Down Expand Up @@ -1862,8 +1877,8 @@ impl RoomScreen {
message_highlight_animation_state: MessageHighlightAnimationState::default(),
last_scrolled_index: usize::MAX,
prev_first_index: None,
read_event_hashmap: HashMap::new(),
marked_fully_read_queue: HashMap::new(),
scrolled_past_read_marker: false,
last_display_event: None,
};
(new_tl_state, true)
};
Expand All @@ -1877,6 +1892,9 @@ impl RoomScreen {
}
);

// Query for User's fully read event
submit_async_request(MatrixRequest::GetFullyReadEvent { room_id: room_id.clone() });
submit_async_request(MatrixRequest::SubscribeToUpdates { room_id: room_id.clone(), subscribe: true });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and do you ever unsubscribe from these updates?

// Kick off a back pagination request for this room. This is "urgent",
// because we want to show the user some messages as soon as possible
// when they first open the room, and there might not be any messages yet.
Expand Down Expand Up @@ -2019,52 +2037,63 @@ impl RoomScreen {
return;
}
let first_index = portal_list.first_id();

let Some(tl_state) = self.tl_state.as_mut() else { return };
let Some(room_id) = self.room_id.as_ref() else { return };
let (Some(tl_state), Some(room_id)) = (self.tl_state.as_mut(), self.room_id.as_ref()) else {
return;
};
kevinaboos marked this conversation as resolved.
Show resolved Hide resolved
if let Some(ref mut index) = tl_state.prev_first_index {
// to detect change of scroll when scroll ends
if *index != first_index {
// scroll changed
self.fully_read_timer = cx.start_interval(5.0);
let time_now = std::time::Instant::now();
if first_index > *index {
// Store visible event messages with current time into a hashmap
let mut read_receipt_event = None;
for r in first_index .. (first_index + portal_list.visible_items() + 1) {
if let Some(v) = tl_state.items.get(r) {
if let Some(e) = v.as_event().and_then(|f| f.event_id()) {
read_receipt_event = Some(e.to_owned());
tl_state.read_event_hashmap
.entry(e.to_string())
.or_insert_with(|| (room_id.clone(), e.to_owned(), time_now, false));
}
}
}
if let Some(event_id) = read_receipt_event {
submit_async_request(MatrixRequest::ReadReceipt { room_id: room_id.clone(), event_id });
}
let mut fully_read_receipt_event = None;
// Implements sending fully read receipts when message is scrolled out of first row
for r in *index..first_index {
if let Some(v) = tl_state.items.get(r) {
if let Some(e) = v.as_event().and_then(|f| f.event_id()) {
let mut to_remove = vec![];
for (event_id_string, (_, event_id)) in &tl_state.marked_fully_read_queue {
if e == event_id {
fully_read_receipt_event = Some(event_id.clone());
to_remove.push(event_id_string.clone());
}
}
for r in to_remove {
tl_state.marked_fully_read_queue.remove(&r);
}
}
// Any scrolling, resets the fully_read_timer
cx.stop_timer(self.fully_read_timer);
if first_index >= *index {
// Get event_id and timestamp for the last visible event
let Some((last_event_id, last_timestamp)) = tl_state
.items
.get(first_index + portal_list.visible_items())
.and_then(|f| f.as_event())
.and_then(|f| f.event_id().map(|e| (e, f.timestamp())))
else {
*index = first_index;
return;
};
submit_async_request(MatrixRequest::ReadReceipt {
room_id: room_id.clone(),
event_id: last_event_id.to_owned(),
});
// If scrolled_past_read_marker is true, set last_display_event to last_event_id and start the timer
if tl_state.scrolled_past_read_marker {
self.fully_read_timer = cx.start_timeout(5.0);
tl_state.last_display_event = Some((last_event_id.to_owned(), last_timestamp));
} else {
// Get event_id and timestamp for the first visible event
// If scrolled_past_read_marker is false, check if the saved fully read event's timestamp in between the first and last visible event
// If true, set scrolled_past_read_marker to true
// If true, set last_event_id to last_display_event
// If true, start the 5 seconds timer
let Some((_fully_read_event, fully_read_timestamp)) =
get_fully_read_event(room_id)
else {
*index = first_index;
return;
};
let Some((_first_event_id, first_timestamp)) = tl_state
.items
.get(first_index)
.and_then(|f| f.as_event())
.and_then(|f| f.event_id().map(|e| (e, f.timestamp())))
else {
*index = first_index;
return;
};
if fully_read_timestamp >= first_timestamp
&& fully_read_timestamp <= last_timestamp
{
tl_state.scrolled_past_read_marker = true;
tl_state.last_display_event =
Some((last_event_id.to_owned(), last_timestamp));
self.fully_read_timer = cx.start_timeout(5.0);
}
}
if let Some(event_id) = fully_read_receipt_event {
submit_async_request(MatrixRequest::FullyReadReceipt { room_id: room_id.clone(), event_id: event_id.clone()});
}
}
*index = first_index;
}
Expand Down Expand Up @@ -2135,6 +2164,8 @@ pub enum TimelineUpdate {
/// Whether to clear the entire cache of drawn items in the timeline.
/// This supersedes `index_of_first_change` and is used when the entire timeline is being redrawn.
clear_cache: bool,
/// The updated number of unread messages in the room.
unread_messages_count: Option<u64>
},
/// The target event ID was found at the given `index` in the timeline items vector.
///
Expand Down Expand Up @@ -2259,9 +2290,23 @@ struct TimelineUiState {
/// at which point we submit a backwards pagination request to fetch more events.
last_scrolled_index: usize,

/// The index of the first item in the timeline that is currently visible
prev_first_index: Option<usize>,
read_event_hashmap: HashMap<String, (OwnedRoomId, OwnedEventId, Instant, bool)>,
marked_fully_read_queue: HashMap<String, (OwnedRoomId, OwnedEventId)>,

/// Boolean to indicate if the user scrolled pass the read marker
///
/// Used to send fully read receipt after user scrolled pass the read marker
/// Read marker is referred to Red marker in the UI or also known as latest fully read receipt
/// Value is determined by comparing the fully read event's timestamp with the first and last timestamp of displayed events in the timeline
/// When scrolling down, if scrolled_past_read_marker is true, a 5 seconds timer is started.
/// Once the countdown ends, the app will send out last visible event's fully read receipt which will be stored as RoomInfo's fully_read_event
/// In between, any scrolling or new message coming in will reset the timer
scrolled_past_read_marker: bool,

/// EventId and timestamp of the last displayed event in the timeline
///
/// To be send as fully read receipt
last_display_event: Option<(OwnedEventId, MilliSecondsSinceUnixEpoch)>,
}

#[derive(Default, Debug)]
Expand Down Expand Up @@ -3887,4 +3932,4 @@ impl MessageRef {
inner.set_data(can_be_replied_to, item_id, replied_to_event_id, room_screen_widget_uid, mentions_user);
};
}
}
}
Loading
Loading