-
Notifications
You must be signed in to change notification settings - Fork 19
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
base: main
Are you sure you want to change the base?
Changes from 35 commits
5657931
bb92afb
9255365
24be468
c1d1ab6
35c5977
bc2aa8c
a9e4945
053c8f0
e4c1268
072b9cb
3884a66
b7a83a0
a3b1f10
3824160
5633e2b
44f4c25
9076c53
5bf2413
1c66730
8bbde58
b99a57d
99ff1b1
0b89757
009077f
aba7e69
2ce7a20
945d3a1
170488c
20982c2
1e5e36c
b4b5116
84faf5e
e748196
963a722
1566621
3627a1f
07df9fd
b44105a
c32b7d5
1e7d4b2
ced53e5
1caacc1
22d8e30
35b8b02
906e53e
fe9973b
3d090fb
b12592a
eb857c6
794cd0a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 imbl::Vector; | ||
use makepad_widgets::*; | ||
|
@@ -29,7 +29,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, MediaFormatConst} | ||
}, sliding_sync::{get_client, submit_async_request, take_fully_read_event, take_timeline_endpoints, BackwardsPaginateUntilEventRequest, MatrixRequest, PaginationDirection, TimelineRequestSender}, utils::{self, unix_time_millis_to_datetime, MediaFormatConst} | ||
}; | ||
use rangemap::RangeSet; | ||
|
||
|
@@ -48,6 +48,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::*; | ||
|
@@ -967,8 +968,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 | ||
#[rust] fully_read_timer: Timer, | ||
|
||
} | ||
impl Drop for RoomScreen { | ||
fn drop(&mut self) { | ||
|
@@ -1295,15 +1300,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() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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); | ||
|
@@ -1533,7 +1541,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; | ||
} | ||
} | ||
// | ||
|
@@ -1545,8 +1555,26 @@ 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); | ||
// Set the number of unread messages to unread_notification_badge | ||
if let Some(room_id) = &self.room_id { | ||
if let Some(num_unread) = get_client() | ||
.and_then(|c| c.get_room(room_id)) | ||
.map(|room| room.num_unread_messages()) | ||
alanpoon marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
jump_to_bottom.show_unread_message_badge(num_unread as usize); | ||
} | ||
} | ||
} | ||
// When new messages are added to the end of the timeline, | ||
// we want to send the read receipt when the user sees them. | ||
if is_append && portal_list.is_at_end() { | ||
let last_displayed_event = new_items.last().and_then(|f| f.as_event()).and_then(|f| f.event_id()); | ||
if let (Some(event_id), Some(room_id)) = (last_displayed_event, &self.room_id) { | ||
submit_async_request(MatrixRequest::ReadReceipt { | ||
room_id: room_id.clone(), | ||
event_id: event_id.to_owned(), | ||
}); | ||
} | ||
kevinaboos marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
if clear_cache { | ||
|
@@ -1837,8 +1865,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) | ||
}; | ||
|
@@ -1852,6 +1880,9 @@ impl RoomScreen { | |
} | ||
); | ||
|
||
// Query for User's fully read event | ||
submit_async_request(MatrixRequest::GetFullyReadEvent { room_id: room_id.clone() }); | ||
|
||
// 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. | ||
|
@@ -1989,55 +2020,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()); | ||
if !tl_state.read_event_hashmap.contains_key(&e.to_string()) { | ||
tl_state.read_event_hashmap.insert( | ||
e.to_string(), | ||
(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).clone() { | ||
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)) = | ||
take_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; | ||
} | ||
|
@@ -2227,9 +2266,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 with the event id of read receipt being sent out | ||
/// 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)] | ||
|
@@ -3383,4 +3436,4 @@ impl MessageRef { | |
inner.set_data(can_be_replied_to, item_id, replied_to_event_id, room_screen_widget_uid, mentions_user); | ||
}; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,8 +41,12 @@ live_design! { | |
|
||
// A badge overlay on the jump to bottom button showing unread messages | ||
unread_message_badge = <View> { | ||
width: 12, height: 12, | ||
margin: {right: 33.0, bottom: 11.0}, | ||
width: 20, height: 20, | ||
margin: {right: 28.0, bottom: 8.0}, | ||
align: { | ||
x: 0.5, | ||
y: 0.5 | ||
} | ||
visible: false, | ||
|
||
show_bg: true, | ||
|
@@ -57,17 +61,16 @@ live_design! { | |
} | ||
} | ||
|
||
// // Label that displays the unread message count | ||
// unread_messages_count = <Label> { | ||
// width: Fill, | ||
// height: Fill, | ||
// text: "", | ||
// align: {x: 0.5, y: 0.5}, | ||
// draw_text: { | ||
// color: #ffffff, | ||
// text_style: {font_size: 8.0}, | ||
// } | ||
// } | ||
// Label that displays the unread message count | ||
unread_messages_count = <Label> { | ||
width: Fit, | ||
height: Fit, | ||
text: "", | ||
draw_text: { | ||
color: #ffffff, | ||
text_style: {font_size: 8.0}, | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
@@ -109,11 +112,17 @@ impl JumpToBottomButton { | |
/// Sets both the jump to bottom view and its unread message badge to be visible. | ||
/// | ||
/// This does not automatically redraw any views. | ||
/// | ||
/// TODO: in the future, we'll display the actual count of unread messages in the badge. | ||
pub fn show_unread_message_badge(&mut self, _unread_message_count: usize) { | ||
self.visible = true; | ||
self.view(id!(unread_message_badge)).set_visible(true); | ||
/// If unread_message_count is `0`, the unread message badge is hidden. | ||
pub fn show_unread_message_badge(&mut self, unread_message_count: usize) { | ||
if unread_message_count > 0 { | ||
self.visible = true; | ||
self.view(id!(unread_message_badge)).set_visible(true); | ||
self.label(id!(unread_messages_count)).set_text(&format!("{}", unread_message_count)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should probably limit it to a certain number of events such that the label doesn't look ridiculous if there are 10000000+ unread events. We could do something like up to two digits (a max value of 99) or three digits (a max value of 999) until we just show Use your best judgment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice, looks pretty good. The circular background might need to be an oval/ellipse, since we need more padding on the left and right of the I think if you set the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
} else { | ||
self.visible = false; | ||
self.view(id!(unread_message_badge)).set_visible(false); | ||
self.label(id!(unread_messages_count)).set_text(&format!("{}", "")); | ||
} | ||
} | ||
|
||
/// Updates the visibility of the jump to bottom button and the unread message badge | ||
|
There was a problem hiding this comment.
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.