Skip to content

feat(timeline): add a virtual start of timeline item #4816

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

Merged
merged 4 commits into from
Mar 26, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions bindings/matrix-sdk-ffi/src/timeline/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -974,6 +974,7 @@ impl TimelineItem {
match self.0.as_virtual()? {
VItem::DateDivider(ts) => Some(VirtualTimelineItem::DateDivider { ts: (*ts).into() }),
VItem::ReadMarker => Some(VirtualTimelineItem::ReadMarker),
VItem::TimelineStart => Some(VirtualTimelineItem::TimelineStart),
}
}

Expand Down Expand Up @@ -1254,6 +1255,9 @@ pub enum VirtualTimelineItem {

/// The user's own read marker.
ReadMarker,

/// The timeline start, that is, the *oldest* event in time for that room.
TimelineStart,
}

/// A [`TimelineItem`](super::TimelineItem) that doesn't correspond to an event.
Expand Down
9 changes: 3 additions & 6 deletions crates/matrix-sdk-ui/src/timeline/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ impl TimelineBuilder {
.unwrap_or_default();

let controller = TimelineController::new(
room,
room.clone(),
focus.clone(),
internal_id_prefix.clone(),
unable_to_decrypt_hook,
Expand All @@ -188,9 +188,6 @@ impl TimelineBuilder {

let has_events = controller.init_focus(&room_event_cache).await?;

let room = controller.room();
let client = room.client();

let pinned_events_join_handle = if is_pinned_events {
Some(spawn(pinned_events_task(room.pinned_event_ids_stream(), controller.clone())))
} else {
Expand Down Expand Up @@ -256,7 +253,7 @@ impl TimelineBuilder {
room.room_id().to_owned(),
));

let handles = vec![room_key_handle, forwarded_room_key_handle];
let event_handlers = vec![room_key_handle, forwarded_room_key_handle];

// Not using room.add_event_handler here because RoomKey events are
// to-device events that are not received in the context of a room.
Expand Down Expand Up @@ -291,7 +288,7 @@ impl TimelineBuilder {
event_cache: room_event_cache,
drop_handle: Arc::new(TimelineDropHandle {
client,
event_handler_handles: handles,
event_handler_handles: event_handlers,
room_update_join_handle,
pinned_events_join_handle,
room_key_from_backups_join_handle,
Expand Down
2 changes: 1 addition & 1 deletion crates/matrix-sdk-ui/src/timeline/controller/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub(in crate::timeline) struct TimelineMetadata {
/// This value is constant over the lifetime of the metadata.
internal_id_prefix: Option<String>,

/// The `count` value for the `Skip higher-order stream used by the
/// The `count` value for the `Skip` higher-order stream used by the
/// `TimelineSubscriber`. See its documentation to learn more.
pub(super) subscriber_skip_count: SkipCount,

Expand Down
14 changes: 13 additions & 1 deletion crates/matrix-sdk-ui/src/timeline/controller/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ use super::{
traits::{Decryptor, RoomDataProvider},
DateDividerMode, Error, EventSendState, EventTimelineItem, InReplyToDetails, Message,
PaginationError, Profile, RepliedToEvent, TimelineDetails, TimelineEventItemId, TimelineFocus,
TimelineItem, TimelineItemContent, TimelineItemKind,
TimelineItem, TimelineItemContent, TimelineItemKind, VirtualTimelineItem,
};
use crate::{
timeline::{
Expand Down Expand Up @@ -1309,6 +1309,18 @@ impl<P: RoomDataProvider, D: Decryptor> TimelineController<P, D> {
}
}
}

/// Insert a timeline start item at the beginning of the room, if it's
/// missing.
pub async fn insert_timeline_start_if_missing(&self) {
let mut state = self.state.write().await;
let mut txn = state.transaction();
if txn.items.get(0).is_some_and(|item| item.is_timeline_start()) {
return;
}
txn.items.push_front(txn.meta.new_timeline_item(VirtualTimelineItem::TimelineStart), None);
txn.commit();
}
}

impl TimelineController {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use super::{
ObservableItems, ObservableItemsTransaction, TimelineFocusKind, TimelineMetadata,
TimelineSettings,
};
use crate::events::SyncTimelineEventWithoutContent;
use crate::{events::SyncTimelineEventWithoutContent, timeline::VirtualTimelineItem};

pub(in crate::timeline) struct TimelineStateTransaction<'a> {
/// A vector transaction over the items themselves. Holds temporary state
Expand Down Expand Up @@ -484,9 +484,16 @@ impl<'a> TimelineStateTransaction<'a> {
// `VectorDiff::Clear` should be much more efficient to process for
// subscribers.
if has_local_echoes {
// Remove all remote events and the read marker
// Remove all remote events and virtual items that aren't date dividers.
self.items.for_each(|entry| {
if entry.is_remote_event() || entry.is_read_marker() {
if entry.is_remote_event()
|| entry.as_virtual().is_some_and(|vitem| match vitem {
VirtualTimelineItem::DateDivider(_) => false,
VirtualTimelineItem::ReadMarker | VirtualTimelineItem::TimelineStart => {
true
}
})
{
ObservableItemsTransactionEntry::remove(entry);
}
});
Expand Down
32 changes: 20 additions & 12 deletions crates/matrix-sdk-ui/src/timeline/date_dividers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ impl DateDividerAdjuster {
latest_event_ts = Some(ts);
}

TimelineItemKind::Virtual(VirtualTimelineItem::ReadMarker) => {
TimelineItemKind::Virtual(VirtualTimelineItem::ReadMarker)
| TimelineItemKind::Virtual(VirtualTimelineItem::TimelineStart) => {
// Nothing to do.
}
}
Expand Down Expand Up @@ -242,8 +243,9 @@ impl DateDividerAdjuster {
return true;
}

TimelineItemKind::Virtual(VirtualTimelineItem::ReadMarker) => {
// Nothing to do for read markers.
TimelineItemKind::Virtual(VirtualTimelineItem::ReadMarker)
| TimelineItemKind::Virtual(VirtualTimelineItem::TimelineStart) => {
// Nothing to do.
}
}

Expand Down Expand Up @@ -304,7 +306,8 @@ impl DateDividerAdjuster {
}
}

TimelineItemKind::Virtual(VirtualTimelineItem::ReadMarker) => {
TimelineItemKind::Virtual(VirtualTimelineItem::ReadMarker)
| TimelineItemKind::Virtual(VirtualTimelineItem::TimelineStart) => {
// Nothing to do.
}
}
Expand Down Expand Up @@ -399,16 +402,21 @@ impl DateDividerAdjuster {
};

// Assert invariants.
// 1. The timeline starts with a date divider.
if let Some(item) = items.get(0) {
if item.is_read_marker() {
if let Some(next_item) = items.get(1) {
if !next_item.is_date_divider() {
report.errors.push(DateDividerInsertError::FirstItemNotDateDivider);
// 1. The timeline starts with a date divider, if it's not only virtual items.
{
let mut i = 0;
while let Some(item) = items.get(i) {
if let Some(virt) = item.as_virtual() {
if matches!(virt, VirtualTimelineItem::DateDivider(_)) {
// We found a date divider among the first virtual items: stop here.
break;
}
} else {
// We found an event, but we didn't have a date divider: report an error.
report.errors.push(DateDividerInsertError::FirstItemNotDateDivider);
break;
}
} else if !item.is_date_divider() {
report.errors.push(DateDividerInsertError::FirstItemNotDateDivider);
i += 1;
}
}

Expand Down
96 changes: 45 additions & 51 deletions crates/matrix-sdk-ui/src/timeline/event_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1050,7 +1050,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {

let is_room_encrypted = self.meta.is_room_encrypted;

let mut item = EventTimelineItem::new(
let item = EventTimelineItem::new(
sender,
sender_profile,
timestamp,
Expand All @@ -1071,13 +1071,13 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
Flow::Remote {
position: TimelineItemPosition::Start { .. }, event_id, txn_id, ..
} => {
let removed_duplicated_timeline_item = Self::deduplicate_local_timeline_item(
let item = Self::recycle_local_or_create_item(
self.items,
&mut item,
Some(event_id),
txn_id.as_ref().map(AsRef::as_ref),
self.meta,
item,
event_id,
txn_id.as_deref(),
);
let item = new_timeline_item(self.meta, item, removed_duplicated_timeline_item);

trace!("Adding new remote timeline item at the start");

Expand All @@ -1090,13 +1090,13 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
txn_id,
..
} => {
let removed_duplicated_timeline_item = Self::deduplicate_local_timeline_item(
let item = Self::recycle_local_or_create_item(
self.items,
&mut item,
Some(event_id),
txn_id.as_ref().map(AsRef::as_ref),
self.meta,
item,
event_id,
txn_id.as_deref(),
);
let item = new_timeline_item(self.meta, item, removed_duplicated_timeline_item);

let all_remote_events = self.items.all_remote_events();
let event_index = *event_index;
Expand Down Expand Up @@ -1128,7 +1128,17 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
(!timeline_item.as_event()?.is_local_echo())
.then_some(timeline_item_index + 1)
})
.unwrap_or(0)
.unwrap_or_else(|| {
// We don't have any local echo, so we could insert at 0. However, in
// the case of an insertion caused by a pagination, we
// may have already pushed the start of the timeline item, so we need
// to check if the first item is that, and insert after it otherwise.
if self.items.get(0).is_some_and(|item| item.is_timeline_start()) {
1
} else {
0
}
})
});

trace!(
Expand All @@ -1143,13 +1153,13 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
Flow::Remote {
position: TimelineItemPosition::End { .. }, event_id, txn_id, ..
} => {
let removed_duplicated_timeline_item = Self::deduplicate_local_timeline_item(
let item = Self::recycle_local_or_create_item(
self.items,
&mut item,
Some(event_id),
txn_id.as_ref().map(AsRef::as_ref),
self.meta,
item,
event_id,
txn_id.as_deref(),
);
let item = new_timeline_item(self.meta, item, removed_duplicated_timeline_item);

// Local events are always at the bottom. Let's find the latest remote event
// and insert after it, otherwise, if there is no remote event, insert at 0.
Expand Down Expand Up @@ -1221,16 +1231,18 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
}
}

/// Remove the local timeline item matching the `event_id` or the
/// `transaction_id` of `new_event_timeline_item` if it exists.
// Note: this method doesn't take `&mut self` to avoid a borrow checker
// conflict with `TimelineEventHandler::add_item`.
fn deduplicate_local_timeline_item(
/// Try to recycle a local timeline item for the same event, or create a new
/// timeline item for it.
///
/// Note: this method doesn't take `&mut self` to avoid a borrow checker
/// conflict with `TimelineEventHandler::add_item`.
fn recycle_local_or_create_item(
items: &mut ObservableItemsTransaction<'_>,
new_event_timeline_item: &mut EventTimelineItem,
event_id: Option<&EventId>,
meta: &mut TimelineMetadata,
mut new_item: EventTimelineItem,
event_id: &EventId,
transaction_id: Option<&TransactionId>,
) -> Option<Arc<TimelineItem>> {
) -> Arc<TimelineItem> {
// Detect a local timeline item that matches `event_id` or `transaction_id`.
if let Some((local_timeline_item_index, local_timeline_item)) = items
.iter()
Expand Down Expand Up @@ -1259,7 +1271,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
return ControlFlow::Break(None);
}

if event_id == event_timeline_item.event_id()
if Some(event_id) == event_timeline_item.event_id()
|| (transaction_id.is_some()
&& transaction_id == event_timeline_item.transaction_id())
{
Expand All @@ -1281,13 +1293,15 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
"Removing local timeline item"
);

transfer_details(new_event_timeline_item, local_timeline_item);
transfer_details(&mut new_item, local_timeline_item);

// Remove the local timeline item.
return Some(items.remove(local_timeline_item_index));
};

None
let recycled = items.remove(local_timeline_item_index);
TimelineItem::new(new_item, recycled.internal_id.clone())
} else {
// We haven't found a matching local item to recycle; create a new item.
meta.new_timeline_item(new_item)
}
}

/// After updating the timeline item `new_item` which id is
Expand Down Expand Up @@ -1355,23 +1369,3 @@ fn transfer_details(new_item: &mut EventTimelineItem, old_item: &EventTimelineIt
in_reply_to.event = old_in_reply_to.event.clone();
}
}

/// Create a new timeline item from an [`EventTimelineItem`].
///
/// It is possible that the new timeline item replaces a duplicated timeline
/// event (see [`TimelineEventHandler::deduplicate_local_timeline_item`]) in
/// case it replaces a local timeline item.
fn new_timeline_item(
metadata: &mut TimelineMetadata,
event_timeline_item: EventTimelineItem,
replaced_timeline_item: Option<Arc<TimelineItem>>,
) -> Arc<TimelineItem> {
match replaced_timeline_item {
// Reuse the internal ID.
Some(to_replace_timeline_item) => {
TimelineItem::new(event_timeline_item, to_replace_timeline_item.internal_id.clone())
}

None => metadata.new_timeline_item(event_timeline_item),
}
}
8 changes: 6 additions & 2 deletions crates/matrix-sdk-ui/src/timeline/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,19 @@ impl TimelineItem {
matches!(&self.kind, TimelineItemKind::Event(_))
}

/// Check whether this item is a date divider.
#[must_use]
/// Check whether this item is a (virtual) date divider.
pub fn is_date_divider(&self) -> bool {
matches!(self.kind, TimelineItemKind::Virtual(VirtualTimelineItem::DateDivider(_)))
}

pub(crate) fn is_read_marker(&self) -> bool {
matches!(self.kind, TimelineItemKind::Virtual(VirtualTimelineItem::ReadMarker))
}

/// Check whether this item is a (virtual) timeline start item.
pub fn is_timeline_start(&self) -> bool {
matches!(self.kind, TimelineItemKind::Virtual(VirtualTimelineItem::TimelineStart))
}
}

impl Deref for TimelineItem {
Expand Down
Loading
Loading