timeline: Clean up TimelineItem API

Remove the `selectable` property which is not used, and move properties
about the header to Event, which is the only child type using it.

src/session/model/room/timeline/event/mod.rs

@@ -134,6 +134,9 @@ mod imp {
         /// Whether this event has any read receipt.
         #[property(get = Self::has_read_receipts)]
         has_read_receipts: PhantomData<bool>,
+        /// Whether this event should show its header in the room history.
+        #[property(get, set = Self::set_show_header, explicit_notify)]
+        show_header: Cell<bool>,
     }
 
     #[glib::object_subclass]
@@ -160,19 +163,7 @@ mod imp {
         }
     }
 
-    impl TimelineItemImpl for Event {
+    impl TimelineItemImpl for Event {}
-        fn can_hide_header(&self) -> bool {
-            self.item().content().can_show_header()
-        }
-
-        fn event_sender_id(&self) -> Option<OwnedUserId> {
-            Some(self.sender_id())
-        }
-
-        fn selectable(&self) -> bool {
-            true
-        }
-    }
 
     impl Event {
         /// Set the underlying SDK timeline item.
@@ -470,6 +461,16 @@ mod imp {
         fn has_read_receipts(&self) -> bool {
             self.read_receipts().n_items() > 0
         }
+
+        /// Set whether this event should show its header in the room history.
+        fn set_show_header(&self, show: bool) {
+            if self.show_header.get() == show {
+                return;
+            }
+
+            self.show_header.set(show);
+            self.obj().notify_show_header();
+        }
     }
 }
 
@@ -638,6 +639,11 @@ impl Event {
         self.item().content().can_contain_at_room()
     }
 
+    /// Whether this event can show a header.
+    pub(crate) fn can_show_header(&self) -> bool {
+        self.item().content().can_show_header()
+    }
+
     /// Get the ID of the event this event replies to, if any.
     pub(crate) fn reply_to_id(&self) -> Option<OwnedEventId> {
         match self.item().content() {

src/session/model/room/timeline/mod.rs

@@ -568,9 +568,9 @@ mod imp {
             let mut previous_sender = if pos > 0 {
                 sdk_items
                     .item(pos - 1)
-                    .and_downcast::<TimelineItem>()
+                    .and_downcast::<Event>()
-                    .filter(TimelineItem::can_hide_header)
+                    .filter(Event::can_show_header)
-                    .and_then(|item| item.event_sender_id())
+                    .map(|event| event.sender_id())
             } else {
                 None
             };
@@ -580,15 +580,22 @@ mod imp {
                 let Some(current) = self.item_at(i) else {
                     break;
                 };
+                let Ok(current) = current.downcast::<Event>() else {
+                    previous_sender = None;
+                    continue;
+                };
 
-                let current_sender = current.event_sender_id();
+                let current_sender = current.sender_id();
 
-                if !current.can_hide_header() {
+                if !current.can_show_header() {
                     current.set_show_header(false);
                     previous_sender = None;
-                } else if current_sender != previous_sender {
+                } else if previous_sender
+                    .as_ref()
+                    .is_none_or(|previous_sender| current_sender != *previous_sender)
+                {
                     current.set_show_header(true);
-                    previous_sender = current_sender;
+                    previous_sender = Some(current_sender);
                 } else {
                     current.set_show_header(false);
                 }

src/session/model/room/timeline/timeline_item.rs

@@ -1,46 +1,24 @@
 use gtk::{glib, prelude::*, subclass::prelude::*};
 use matrix_sdk_ui::timeline::{TimelineItem as SdkTimelineItem, TimelineItemKind};
-use ruma::OwnedUserId;
 use tracing::error;
 
 use super::{Event, Timeline, VirtualItem};
 use crate::session::model::Room;
 
 mod imp {
-    use std::{
+    use std::cell::{OnceCell, RefCell};
-        cell::{Cell, OnceCell, RefCell},
-        marker::PhantomData,
-    };
 
     use super::*;
 
     #[repr(C)]
     pub struct TimelineItemClass {
         parent_class: glib::object::ObjectClass,
-        pub(super) selectable: fn(&super::TimelineItem) -> bool,
-        pub(super) can_hide_header: fn(&super::TimelineItem) -> bool,
-        pub(super) event_sender_id: fn(&super::TimelineItem) -> Option<OwnedUserId>,
     }
 
     unsafe impl ClassStruct for TimelineItemClass {
         type Type = TimelineItem;
     }
 
-    pub(super) fn timeline_item_selectable(this: &super::TimelineItem) -> bool {
-        let klass = this.class();
-        (klass.as_ref().selectable)(this)
-    }
-
-    pub(super) fn timeline_item_can_hide_header(this: &super::TimelineItem) -> bool {
-        let klass = this.class();
-        (klass.as_ref().can_hide_header)(this)
-    }
-
-    pub(super) fn timeline_item_event_sender_id(this: &super::TimelineItem) -> Option<OwnedUserId> {
-        let klass = this.class();
-        (klass.as_ref().event_sender_id)(this)
-    }
-
     #[derive(Debug, Default, glib::Properties)]
     #[properties(wrapper_type = super::TimelineItem)]
     pub struct TimelineItem {
@@ -50,26 +28,6 @@ mod imp {
         /// A unique ID for this `TimelineItem` in the local timeline.
         #[property(get, construct_only)]
         timeline_id: RefCell<String>,
-        /// Whether this `TimelineItem` is selectable.
-        ///
-        /// Defaults to `false`.
-        #[property(get = Self::selectable)]
-        selectable: PhantomData<bool>,
-        /// Whether this `TimelineItem` should show its header.
-        ///
-        /// Defaults to `false`.
-        #[property(get, set = Self::set_show_header, explicit_notify)]
-        show_header: Cell<bool>,
-        /// Whether this `TimelineItem` is allowed to hide its header.
-        ///
-        /// Defaults to `false`.
-        #[property(get = Self::can_hide_header)]
-        can_hide_header: PhantomData<bool>,
-        /// If this is a Matrix event, the sender of the event.
-        ///
-        /// Defaults to `None`.
-        #[property(get = Self::event_sender_id)]
-        event_sender_id: PhantomData<Option<String>>,
     }
 
     #[glib::object_subclass]
@@ -82,39 +40,6 @@ mod imp {
 
     #[glib::derived_properties]
     impl ObjectImpl for TimelineItem {}
-
-    impl TimelineItem {
-        /// Whether this `TimelineItem` is selectable.
-        ///
-        /// Defaults to `false`.
-        fn selectable(&self) -> bool {
-            imp::timeline_item_selectable(&self.obj())
-        }
-
-        /// Set whether this `TimelineItem` should show its header.
-        fn set_show_header(&self, show: bool) {
-            if self.show_header.get() == show {
-                return;
-            }
-
-            self.show_header.set(show);
-            self.obj().notify_show_header();
-        }
-
-        /// Whether this `TimelineItem` is allowed to hide its header.
-        ///
-        /// Defaults to `false`.
-        fn can_hide_header(&self) -> bool {
-            imp::timeline_item_can_hide_header(&self.obj())
-        }
-
-        /// If this is a Matrix event, the sender of the event.
-        ///
-        /// Defaults to `None`.
-        fn event_sender_id(&self) -> Option<String> {
-            imp::timeline_item_event_sender_id(&self.obj()).map(Into::into)
-        }
-    }
 }
 
 glib::wrapper! {
@@ -184,29 +109,6 @@ pub(crate) trait TimelineItemExt: 'static {
 
     /// A unique ID for this `TimelineItem` in the local timeline.
     fn timeline_id(&self) -> String;
-
-    /// Whether this `TimelineItem` is selectable.
-    ///
-    /// Defaults to `false`.
-    fn selectable(&self) -> bool;
-
-    /// Whether this `TimelineItem` should show its header.
-    ///
-    /// Defaults to `false`.
-    fn show_header(&self) -> bool;
-
-    /// Set whether this `TimelineItem` should show its header.
-    fn set_show_header(&self, show: bool);
-
-    /// Whether this `TimelineItem` is allowed to hide its header.
-    ///
-    /// Defaults to `false`.
-    fn can_hide_header(&self) -> bool;
-
-    /// If this is a Matrix event, the sender of the event.
-    ///
-    /// Defaults to `None`.
-    fn event_sender_id(&self) -> Option<OwnedUserId>;
 }
 
 impl<O: IsA<TimelineItem>> TimelineItemExt for O {
@@ -217,26 +119,6 @@ impl<O: IsA<TimelineItem>> TimelineItemExt for O {
     fn timeline_id(&self) -> String {
         self.upcast_ref().timeline_id()
     }
-
-    fn selectable(&self) -> bool {
-        self.upcast_ref().selectable()
-    }
-
-    fn show_header(&self) -> bool {
-        self.upcast_ref().show_header()
-    }
-
-    fn set_show_header(&self, show: bool) {
-        self.upcast_ref().set_show_header(show);
-    }
-
-    fn can_hide_header(&self) -> bool {
-        self.upcast_ref().can_hide_header()
-    }
-
-    fn event_sender_id(&self) -> Option<OwnedUserId> {
-        imp::timeline_item_event_sender_id(self.upcast_ref())
-    }
 }
 
 /// Public trait that must be implemented for everything that derives from
@@ -244,19 +126,7 @@ impl<O: IsA<TimelineItem>> TimelineItemExt for O {
 ///
 /// Overriding a method from this Trait overrides also its behavior in
 /// `TimelineItemExt`.
-pub(crate) trait TimelineItemImpl: ObjectImpl {
+pub(crate) trait TimelineItemImpl: ObjectImpl {}
-    fn selectable(&self) -> bool {
-        false
-    }
-
-    fn can_hide_header(&self) -> bool {
-        false
-    }
-
-    fn event_sender_id(&self) -> Option<OwnedUserId> {
-        None
-    }
-}
 
 // Make `TimelineItem` subclassable.
 unsafe impl<T> IsSubclassable<T> for TimelineItem
@@ -266,39 +136,5 @@ where
 {
     fn class_init(class: &mut glib::Class<Self>) {
         Self::parent_class_init::<T>(class.upcast_ref_mut());
-
-        let klass = class.as_mut();
-
-        klass.selectable = selectable_trampoline::<T>;
-        klass.can_hide_header = can_hide_header_trampoline::<T>;
-        klass.event_sender_id = event_sender_id_trampoline::<T>;
     }
 }
-
-// Virtual method implementation trampolines.
-fn selectable_trampoline<T>(this: &TimelineItem) -> bool
-where
-    T: ObjectSubclass + TimelineItemImpl,
-    T::Type: IsA<TimelineItem>,
-{
-    let this = this.downcast_ref::<T::Type>().unwrap();
-    this.imp().selectable()
-}
-
-fn can_hide_header_trampoline<T>(this: &TimelineItem) -> bool
-where
-    T: ObjectSubclass + TimelineItemImpl,
-    T::Type: IsA<TimelineItem>,
-{
-    let this = this.downcast_ref::<T::Type>().unwrap();
-    this.imp().can_hide_header()
-}
-
-fn event_sender_id_trampoline<T>(this: &TimelineItem) -> Option<OwnedUserId>
-where
-    T: ObjectSubclass + TimelineItemImpl,
-    T::Type: IsA<TimelineItem>,
-{
-    let this = this.downcast_ref::<T::Type>().unwrap();
-    this.imp().event_sender_id()
-}