From 935e4df9273e2add9e1a5c9c6e7d0dbc8fa2832d Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Mon, 9 Dec 2024 14:15:00 +0200 Subject: [PATCH] feat(ui): make the timeline date separators configurable; have them appear either when the day changes or when the month changes. --- bindings/matrix-sdk-ffi/src/room.rs | 5 +- bindings/matrix-sdk-ffi/src/timeline/mod.rs | 16 +++ crates/matrix-sdk-ui/src/timeline/builder.rs | 9 +- .../src/timeline/controller/mod.rs | 25 +++- .../src/timeline/controller/state.rs | 9 +- .../src/timeline/day_dividers.rs | 128 +++++++++++++----- crates/matrix-sdk-ui/src/timeline/mod.rs | 7 + crates/matrix-sdk-ui/src/timeline/util.rs | 6 + 8 files changed, 162 insertions(+), 43 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/room.rs b/bindings/matrix-sdk-ffi/src/room.rs index 9f0e30149..fd0f6aef3 100644 --- a/bindings/matrix-sdk-ffi/src/room.rs +++ b/bindings/matrix-sdk-ffi/src/room.rs @@ -39,7 +39,7 @@ use crate::{ room_info::RoomInfo, room_member::RoomMember, ruma::{ImageInfo, Mentions, NotifyType}, - timeline::{FocusEventError, ReceiptType, SendHandle, Timeline}, + timeline::{DateDividerMode, FocusEventError, ReceiptType, SendHandle, Timeline}, utils::u64_to_uint, TaskHandle, }; @@ -278,6 +278,7 @@ impl Room { &self, internal_id_prefix: Option, allowed_message_types: Vec, + date_divider_mode: DateDividerMode, ) -> Result, ClientError> { let mut builder = matrix_sdk_ui::timeline::Timeline::builder(&self.inner); @@ -285,6 +286,8 @@ impl Room { builder = builder.with_internal_id_prefix(internal_id_prefix); } + builder = builder.with_date_divider_mode(date_divider_mode.into()); + builder = builder.event_filter(move |event, room_version_id| { default_event_filter(event, room_version_id) && match event { diff --git a/bindings/matrix-sdk-ffi/src/timeline/mod.rs b/bindings/matrix-sdk-ffi/src/timeline/mod.rs index c9075a928..08553bbc0 100644 --- a/bindings/matrix-sdk-ffi/src/timeline/mod.rs +++ b/bindings/matrix-sdk-ffi/src/timeline/mod.rs @@ -1358,3 +1358,19 @@ impl LazyTimelineItemProvider { self.0.local_echo_send_handle().map(|handle| Arc::new(SendHandle::new(handle))) } } + +/// Changes how dividers get inserted, either in between each day or in between each month +#[derive(Debug, Clone, uniffi::Enum)] +pub enum DateDividerMode { + Daily, + Monthly, +} + +impl From for matrix_sdk_ui::timeline::DateDividerMode { + fn from(value: DateDividerMode) -> Self { + match value { + DateDividerMode::Daily => Self::Daily, + DateDividerMode::Monthly => Self::Monthly, + } + } +} diff --git a/crates/matrix-sdk-ui/src/timeline/builder.rs b/crates/matrix-sdk-ui/src/timeline/builder.rs index 8e131e21f..b3e7ec8cd 100644 --- a/crates/matrix-sdk-ui/src/timeline/builder.rs +++ b/crates/matrix-sdk-ui/src/timeline/builder.rs @@ -28,7 +28,7 @@ use tracing::{info, info_span, trace, warn, Instrument, Span}; use super::{ controller::{TimelineController, TimelineSettings}, to_device::{handle_forwarded_room_key_event, handle_room_key_event}, - Error, Timeline, TimelineDropHandle, TimelineFocus, + DateDividerMode, Error, Timeline, TimelineDropHandle, TimelineFocus, }; use crate::{ timeline::{controller::TimelineNewItemPosition, event_item::RemoteEventOrigin}, @@ -89,6 +89,13 @@ impl TimelineBuilder { self } + /// Chose when to insert the date separators, either in between each day + /// or each month. + pub fn with_date_divider_mode(mut self, mode: DateDividerMode) -> Self { + self.settings.date_divider_mode = mode; + self + } + /// Enable tracking of the fully-read marker and the read receipts on the /// timeline. pub fn track_read_marker_and_receipts(mut self) -> Self { diff --git a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs index 6fb796a04..96231a094 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs @@ -69,9 +69,9 @@ use super::{ item::TimelineUniqueId, traits::{Decryptor, RoomDataProvider}, util::{rfind_event_by_id, rfind_event_item, RelativePosition}, - Error, EventSendState, EventTimelineItem, InReplyToDetails, Message, PaginationError, Profile, - ReactionInfo, RepliedToEvent, TimelineDetails, TimelineEventItemId, TimelineFocus, - TimelineItem, TimelineItemContent, TimelineItemKind, + DateDividerMode, Error, EventSendState, EventTimelineItem, InReplyToDetails, Message, + PaginationError, Profile, ReactionInfo, RepliedToEvent, TimelineDetails, TimelineEventItemId, + TimelineFocus, TimelineItem, TimelineItemContent, TimelineItemKind, }; use crate::{ timeline::{ @@ -136,6 +136,8 @@ pub(super) struct TimelineSettings { pub(super) event_filter: Arc, /// Are unparsable events added as timeline items of their own kind? pub(super) add_failed_to_parse: bool, + /// Should the timeline items be grouped by day or month? + pub(super) date_divider_mode: DateDividerMode, } #[cfg(not(tarpaulin_include))] @@ -154,6 +156,7 @@ impl Default for TimelineSettings { track_read_receipts: false, event_filter: Arc::new(default_event_filter), add_failed_to_parse: true, + date_divider_mode: DateDividerMode::Daily, } } } @@ -742,9 +745,19 @@ impl TimelineController

{ // Only add new items if the timeline is live. let should_add_new_items = self.is_live().await; + let date_divider_mode = self.settings.date_divider_mode.clone(); + let mut state = self.state.write().await; state - .handle_local_event(sender, profile, should_add_new_items, txn_id, send_handle, content) + .handle_local_event( + sender, + profile, + should_add_new_items, + date_divider_mode, + txn_id, + send_handle, + content, + ) .await; } @@ -784,7 +797,7 @@ impl TimelineController

{ txn.items.remove(idx); // Adjust the day dividers, if needs be. - let mut adjuster = DayDividerAdjuster::default(); + let mut adjuster = DayDividerAdjuster::new(self.settings.date_divider_mode.clone()); adjuster.run(&mut txn.items, &mut txn.meta); } @@ -883,7 +896,7 @@ impl TimelineController

{ // A read marker or a day divider may have been inserted before the local echo. // Ensure both are up to date. - let mut adjuster = DayDividerAdjuster::default(); + let mut adjuster = DayDividerAdjuster::new(self.settings.date_divider_mode.clone()); adjuster.run(&mut txn.items, &mut txn.meta); txn.meta.update_read_marker(&mut txn.items); diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state.rs b/crates/matrix-sdk-ui/src/timeline/controller/state.rs index 878573af3..aaf502d1d 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state.rs @@ -48,7 +48,7 @@ use super::{ AllRemoteEvents, ObservableItems, ObservableItemsTransaction, ObservableItemsTransactionEntry, }, - HandleManyEventsResult, TimelineFocusKind, TimelineSettings, + DateDividerMode, HandleManyEventsResult, TimelineFocusKind, TimelineSettings, }; use crate::{ events::SyncTimelineEventWithoutContent, @@ -198,6 +198,7 @@ impl TimelineState { own_user_id: OwnedUserId, own_profile: Option, should_add_new_items: bool, + date_divider_mode: DateDividerMode, txn_id: OwnedTransactionId, send_handle: Option, content: TimelineEventKind, @@ -216,7 +217,7 @@ impl TimelineState { let mut txn = self.transaction(); - let mut day_divider_adjuster = DayDividerAdjuster::default(); + let mut day_divider_adjuster = DayDividerAdjuster::new(date_divider_mode); TimelineEventHandler::new(&mut txn, ctx) .handle_event(&mut day_divider_adjuster, content) @@ -239,7 +240,7 @@ impl TimelineState { { let mut txn = self.transaction(); - let mut day_divider_adjuster = DayDividerAdjuster::default(); + let mut day_divider_adjuster = DayDividerAdjuster::new(settings.date_divider_mode.clone()); // Loop through all the indices, in order so we don't decrypt edits // before the event being edited, if both were UTD. Keep track of @@ -381,7 +382,7 @@ impl TimelineStateTransaction<'_> { let position = position.into(); - let mut day_divider_adjuster = DayDividerAdjuster::default(); + let mut day_divider_adjuster = DayDividerAdjuster::new(settings.date_divider_mode.clone()); // Implementation note: when `position` is `TimelineEnd::Front`, events are in // the reverse topological order. Prepending them one by one in the order they diff --git a/crates/matrix-sdk-ui/src/timeline/day_dividers.rs b/crates/matrix-sdk-ui/src/timeline/day_dividers.rs index ba9cd9175..3b78c6ae8 100644 --- a/crates/matrix-sdk-ui/src/timeline/day_dividers.rs +++ b/crates/matrix-sdk-ui/src/timeline/day_dividers.rs @@ -23,7 +23,7 @@ use tracing::{error, event_enabled, instrument, trace, warn, Level}; use super::{ controller::{ObservableItemsTransaction, TimelineMetadata}, util::timestamp_to_date, - TimelineItem, TimelineItemKind, VirtualTimelineItem, + DateDividerMode, TimelineItem, TimelineItemKind, VirtualTimelineItem, }; /// Algorithm ensuring that day dividers are adjusted correctly, according to @@ -36,6 +36,8 @@ pub(super) struct DayDividerAdjuster { /// A boolean indicating whether the struct has been used and thus must be /// mark unused manually by calling [`Self::run`]. consumed: bool, + + mode: DateDividerMode, } impl Drop for DayDividerAdjuster { @@ -47,17 +49,6 @@ impl Drop for DayDividerAdjuster { } } -impl Default for DayDividerAdjuster { - fn default() -> Self { - Self { - ops: Default::default(), - // The adjuster starts as consumed, and it will be marked no consumed iff it's used - // with `mark_used`. - consumed: true, - } - } -} - /// A descriptor for a previous item. struct PrevItemDesc<'a> { /// The index of the item in the `self.items` array. @@ -71,6 +62,16 @@ struct PrevItemDesc<'a> { } impl DayDividerAdjuster { + pub fn new(mode: DateDividerMode) -> Self { + Self { + ops: Default::default(), + // The adjuster starts as consumed, and it will be marked no consumed iff it's used + // with `mark_used`. + consumed: true, + mode: mode, + } + } + /// Marks this [`DayDividerAdjuster`] as used, which means it'll require a /// call to [`DayDividerAdjuster::run`] before getting dropped. pub fn mark_used(&mut self) { @@ -199,7 +200,7 @@ impl DayDividerAdjuster { match prev_item.kind() { TimelineItemKind::Event(event) => { // This day divider is preceded by an event. - if is_same_date_as(event.timestamp(), ts) { + if self.is_same_date_as(event.timestamp(), ts) { // The event has the same date as the day divider: remove the current day // divider. trace!("removing day divider following event with same timestamp @ {i}"); @@ -245,7 +246,7 @@ impl DayDividerAdjuster { // insert a day divider. let prev_ts = prev_event.timestamp(); - if !is_same_date_as(prev_ts, ts) { + if !self.is_same_date_as(prev_ts, ts) { trace!("inserting day divider @ {} between two events with different dates", i); self.ops.push(DayDividerOperation::Insert(i, ts)); } @@ -415,7 +416,7 @@ impl DayDividerAdjuster { // We have the same date as the previous event we've seen. if let Some(prev_ts) = prev_event_ts { - if !is_same_date_as(prev_ts, ts) { + if !self.is_same_date_as(prev_ts, ts) { report.errors.push( DayDividerInsertError::MissingDayDividerBetweenEvents { at: i }, ); @@ -424,7 +425,7 @@ impl DayDividerAdjuster { // There is a day divider before us, and it's the same date as our timestamp. if let Some(prev_ts) = prev_day_divider_ts { - if !is_same_date_as(prev_ts, ts) { + if !self.is_same_date_as(prev_ts, ts) { report.errors.push( DayDividerInsertError::InconsistentDateAfterPreviousDayDivider { at: i, @@ -443,7 +444,7 @@ impl DayDividerAdjuster { { // The previous day divider is for a different date. if let Some(prev_ts) = prev_day_divider_ts { - if is_same_date_as(prev_ts, *ts) { + if self.is_same_date_as(prev_ts, *ts) { report .errors .push(DayDividerInsertError::DuplicateDayDivider { at: i }); @@ -472,6 +473,20 @@ impl DayDividerAdjuster { Some(report) } } + + /// Returns whether the two dates for the given timestamps are the same or not. + fn is_same_date_as( + &self, + lhs: MilliSecondsSinceUnixEpoch, + rhs: MilliSecondsSinceUnixEpoch, + ) -> bool { + match self.mode { + DateDividerMode::Daily => timestamp_to_date(lhs) == timestamp_to_date(rhs), + DateDividerMode::Monthly => { + timestamp_to_date(lhs).is_same_month_as(timestamp_to_date(rhs)) + } + } + } } #[derive(Debug)] @@ -491,12 +506,6 @@ impl DayDividerOperation { } } -/// Returns whether the two dates for the given timestamps are the same or not. -#[inline] -fn is_same_date_as(lhs: MilliSecondsSinceUnixEpoch, rhs: MilliSecondsSinceUnixEpoch) -> bool { - timestamp_to_date(lhs) == timestamp_to_date(rhs) -} - /// A report returned by [`DayDividerAdjuster::check_invariants`]. struct DayDividerInvariantsReport<'a, 'o> { /// Initial state before inserting the items. @@ -607,7 +616,7 @@ mod tests { controller::TimelineMetadata, event_item::{EventTimelineItemKind, RemoteEventTimelineItem}, util::timestamp_to_date, - EventTimelineItem, TimelineItemContent, VirtualTimelineItem, + DateDividerMode, EventTimelineItem, TimelineItemContent, VirtualTimelineItem, }; fn event_with_ts(timestamp: MilliSecondsSinceUnixEpoch) -> EventTimelineItem { @@ -661,7 +670,7 @@ mod tests { ); txn.push_back(meta.new_timeline_item(VirtualTimelineItem::ReadMarker), None); - let mut adjuster = DayDividerAdjuster::default(); + let mut adjuster = DayDividerAdjuster::new(DateDividerMode::Daily); adjuster.run(&mut txn, &mut meta); txn.commit(); @@ -701,7 +710,7 @@ mod tests { txn.push_back(meta.new_timeline_item(VirtualTimelineItem::ReadMarker), None); txn.push_back(meta.new_timeline_item(event), None); - let mut adjuster = DayDividerAdjuster::default(); + let mut adjuster = DayDividerAdjuster::new(DateDividerMode::Daily); adjuster.run(&mut txn, &mut meta); txn.commit(); @@ -734,7 +743,7 @@ mod tests { txn.push_back(meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp)), None); txn.push_back(meta.new_timeline_item(event_with_ts(timestamp_next_day)), None); - let mut adjuster = DayDividerAdjuster::default(); + let mut adjuster = DayDividerAdjuster::new(DateDividerMode::Daily); adjuster.run(&mut txn, &mut meta); txn.commit(); @@ -766,7 +775,7 @@ mod tests { txn.push_back(meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp)), None); txn.push_back(meta.new_timeline_item(event_with_ts(timestamp_next_day)), None); - let mut adjuster = DayDividerAdjuster::default(); + let mut adjuster = DayDividerAdjuster::new(DateDividerMode::Daily); adjuster.run(&mut txn, &mut meta); txn.commit(); @@ -792,7 +801,7 @@ mod tests { txn.push_back(meta.new_timeline_item(VirtualTimelineItem::ReadMarker), None); txn.push_back(meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp)), None); - let mut adjuster = DayDividerAdjuster::default(); + let mut adjuster = DayDividerAdjuster::new(DateDividerMode::Daily); adjuster.run(&mut txn, &mut meta); txn.commit(); @@ -818,7 +827,7 @@ mod tests { txn.push_back(meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp)), None); txn.push_back(meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp)), None); - let mut adjuster = DayDividerAdjuster::default(); + let mut adjuster = DayDividerAdjuster::new(DateDividerMode::Daily); adjuster.run(&mut txn, &mut meta); txn.commit(); @@ -840,7 +849,7 @@ mod tests { txn.push_back(meta.new_timeline_item(VirtualTimelineItem::ReadMarker), None); txn.push_back(meta.new_timeline_item(event_with_ts(timestamp)), None); - let mut adjuster = DayDividerAdjuster::default(); + let mut adjuster = DayDividerAdjuster::new(DateDividerMode::Daily); adjuster.run(&mut txn, &mut meta); txn.commit(); @@ -852,4 +861,61 @@ mod tests { assert!(iter.next().unwrap().is_remote_event()); assert!(iter.next().is_none()); } + + #[test] + fn test_dayly_divider_mode() { + let mut items = ObservableVector::new(); + let mut txn = items.transaction(); + + let mut meta = test_metadata(); + + txn.push_back(meta.new_timeline_item(event_with_ts(MilliSecondsSinceUnixEpoch(uint!(0))))); + txn.push_back( + meta.new_timeline_item(event_with_ts(MilliSecondsSinceUnixEpoch(uint!(100000000)))), + ); + txn.push_back(meta.new_timeline_item(event_with_ts(MilliSecondsSinceUnixEpoch::now()))); + + let mut adjuster = DayDividerAdjuster::new(DateDividerMode::Daily); + adjuster.run(&mut txn, &mut meta); + + txn.commit(); + + let mut iter = items.iter(); + + assert!(iter.next().unwrap().is_day_divider()); + assert!(iter.next().unwrap().is_remote_event()); + assert!(iter.next().unwrap().is_day_divider()); + assert!(iter.next().unwrap().is_remote_event()); + assert!(iter.next().unwrap().is_day_divider()); + assert!(iter.next().unwrap().is_remote_event()); + assert!(iter.next().is_none()); + } + + #[test] + fn test_monthly_divider_mode() { + let mut items = ObservableVector::new(); + let mut txn = items.transaction(); + + let mut meta = test_metadata(); + + txn.push_back(meta.new_timeline_item(event_with_ts(MilliSecondsSinceUnixEpoch(uint!(0))))); + txn.push_back( + meta.new_timeline_item(event_with_ts(MilliSecondsSinceUnixEpoch(uint!(100000000)))), + ); + txn.push_back(meta.new_timeline_item(event_with_ts(MilliSecondsSinceUnixEpoch::now()))); + + let mut adjuster = DayDividerAdjuster::new(DateDividerMode::Monthly); + adjuster.run(&mut txn, &mut meta); + + txn.commit(); + + let mut iter = items.iter(); + + assert!(iter.next().unwrap().is_day_divider()); + assert!(iter.next().unwrap().is_remote_event()); + assert!(iter.next().unwrap().is_remote_event()); + assert!(iter.next().unwrap().is_day_divider()); + assert!(iter.next().unwrap().is_remote_event()); + assert!(iter.next().is_none()); + } } diff --git a/crates/matrix-sdk-ui/src/timeline/mod.rs b/crates/matrix-sdk-ui/src/timeline/mod.rs index da5dcbfef..2461251d4 100644 --- a/crates/matrix-sdk-ui/src/timeline/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/mod.rs @@ -177,6 +177,13 @@ impl TimelineFocus { } } +/// Changes how dividers get inserted, either in between each day or in between each month +#[derive(Debug, Clone)] +pub enum DateDividerMode { + Daily, + Monthly, +} + impl Timeline { /// Create a new [`TimelineBuilder`] for the given room. pub fn builder(room: &Room) -> TimelineBuilder { diff --git a/crates/matrix-sdk-ui/src/timeline/util.rs b/crates/matrix-sdk-ui/src/timeline/util.rs index 9ad3be2b8..2bb9c4494 100644 --- a/crates/matrix-sdk-ui/src/timeline/util.rs +++ b/crates/matrix-sdk-ui/src/timeline/util.rs @@ -132,6 +132,12 @@ pub(super) struct Date { day: u32, } +impl Date { + pub fn is_same_month_as(&self, date: Date) -> bool { + self.year == date.year && self.month == date.month + } +} + /// Converts a timestamp since Unix Epoch to a year, month and day. pub(super) fn timestamp_to_date(ts: MilliSecondsSinceUnixEpoch) -> Date { let datetime = Local