room-history: Fix room completion

src/components/mod.rs

@@ -42,7 +42,7 @@ pub use self::{
     location_viewer::LocationViewer,
     media_content_viewer::{ContentType, MediaContentViewer},
     overlapping_avatars::OverlappingAvatars,
-    pill::Pill,
+    pill::{Pill, PillSource},
     power_level_badge::PowerLevelBadge,
     reaction_chooser::ReactionChooser,
     room_title::RoomTitle,

src/components/pill.rs

@@ -3,7 +3,7 @@ use gtk::{glib, prelude::*, CompositeTemplate};
 
 use crate::{
     components::Avatar,
-    session::model::{RemoteRoom, Room, User},
+    session::model::{AvatarData, RemoteRoom, Room, User},
 };
 
 /// The source of the pill's data.
@@ -17,6 +17,52 @@ pub enum PillSource {
     RemoteRoom(RemoteRoom),
 }
 
+impl PillSource {
+    /// The avatar data of this source.
+    pub fn avatar_data(&self) -> AvatarData {
+        match self {
+            PillSource::User(user) => user.avatar_data(),
+            PillSource::Room(room) => room.avatar_data(),
+            PillSource::RemoteRoom(room) => room.avatar_data(),
+        }
+    }
+
+    /// Bind the `display-name` property of the source to the given target.
+    pub fn bind_display_name<O: glib::ObjectType>(
+        &self,
+        target: &O,
+        target_property: &str,
+    ) -> glib::Binding {
+        let obj: &glib::Object = match self {
+            PillSource::User(user) => user.upcast_ref(),
+            PillSource::Room(room) => room.upcast_ref(),
+            PillSource::RemoteRoom(room) => room.upcast_ref(),
+        };
+
+        obj.bind_property("display-name", target, target_property)
+            .sync_create()
+            .build()
+    }
+
+    /// Bind the property matching the identifier of the source to the given
+    /// target.
+    pub fn bind_identifier<O: glib::ObjectType>(
+        &self,
+        target: &O,
+        target_property: &str,
+    ) -> glib::Binding {
+        let (source_property, obj): (&str, &glib::Object) = match self {
+            PillSource::User(user) => ("user-id-string", user.upcast_ref()),
+            PillSource::Room(room) => ("identifier-string", room.upcast_ref()),
+            PillSource::RemoteRoom(room) => ("identifier-string", room.upcast_ref()),
+        };
+
+        obj.bind_property(source_property, target, target_property)
+            .sync_create()
+            .build()
+    }
+}
+
 mod imp {
     use std::{cell::RefCell, marker::PhantomData};
 
@@ -75,7 +121,18 @@ mod imp {
 
     impl Pill {
         /// Set the source of the data displayed by this widget.
-        fn set_source(&self, source: Option<PillSource>) {
+        pub(super) fn set_source(&self, source: Option<PillSource>) {
+            if let Some(binding) = self.binding.take() {
+                binding.unbind();
+            }
+
+            if let Some(source) = &source {
+                let display_name_binding = source.bind_display_name(&*self.display_name, "label");
+                self.binding.replace(Some(display_name_binding));
+            }
+
+            self.avatar
+                .set_data(source.as_ref().map(|s| s.avatar_data()));
             self.source.replace(source);
 
             let obj = self.obj();
@@ -94,20 +151,6 @@ mod imp {
 
         /// Set the user displayed by this widget.
         fn set_user(&self, user: Option<User>) {
-            if let Some(binding) = self.binding.take() {
-                binding.unbind();
-            }
-
-            if let Some(user) = &user {
-                let display_name_binding = user
-                    .bind_property("display-name", &*self.display_name, "label")
-                    .sync_create()
-                    .build();
-                self.binding.replace(Some(display_name_binding));
-            }
-
-            self.avatar.set_data(user.as_ref().map(|u| u.avatar_data()));
-
             self.set_source(user.map(PillSource::User));
         }
 
@@ -121,20 +164,6 @@ mod imp {
 
         /// Set the room displayed by this widget.
         fn set_room(&self, room: Option<Room>) {
-            if let Some(binding) = self.binding.take() {
-                binding.unbind();
-            }
-
-            if let Some(room) = &room {
-                let display_name_binding = room
-                    .bind_property("display-name", &*self.display_name, "label")
-                    .sync_create()
-                    .build();
-                self.binding.replace(Some(display_name_binding));
-            }
-
-            self.avatar.set_data(room.as_ref().map(|r| r.avatar_data()));
-
             self.set_source(room.map(PillSource::Room));
         }
 
@@ -148,20 +177,6 @@ mod imp {
 
         /// Set the remote room displayed by this widget.
         fn set_remote_room(&self, room: Option<RemoteRoom>) {
-            if let Some(binding) = self.binding.take() {
-                binding.unbind();
-            }
-
-            if let Some(room) = &room {
-                let display_name_binding = room
-                    .bind_property("display-name", &*self.display_name, "label")
-                    .sync_create()
-                    .build();
-                self.binding.replace(Some(display_name_binding));
-            }
-
-            self.avatar.set_data(room.as_ref().map(|r| r.avatar_data()));
-
             self.set_source(room.map(PillSource::RemoteRoom));
         }
     }
@@ -174,6 +189,13 @@ glib::wrapper! {
 }
 
 impl Pill {
+    /// Create a pill with the given source.
+    pub fn new(source: PillSource) -> Self {
+        let obj = glib::Object::new::<Self>();
+        obj.imp().set_source(Some(source));
+        obj
+    }
+
     /// Create a pill for the given user.
     pub fn for_user(user: impl IsA<User>) -> Self {
         glib::Object::builder()

src/session/model/room/mod.rs

@@ -89,6 +89,11 @@ mod imp {
         /// The alias of this room, as a string.
         #[property(get = Self::alias_string)]
         pub alias_string: PhantomData<Option<String>>,
+        /// The unique identifier to display for this room, as a string.
+        ///
+        /// Prefers the alias over the room ID.
+        #[property(get = Self::identifier_string)]
+        pub identifier_string: PhantomData<String>,
         /// The version of this room.
         #[property(get = Self::version)]
         pub version: PhantomData<String>,
@@ -261,6 +266,11 @@ mod imp {
             self.alias().map(Into::into)
         }
 
+        /// The unique identifier to display for this room, as a string.
+        fn identifier_string(&self) -> String {
+            self.alias_string().unwrap_or_else(|| self.room_id_string())
+        }
+
         /// The version of this room.
         fn version(&self) -> String {
             self.matrix_room()

src/session/view/content/room_history/message_toolbar/completion/completion_popover.rs

@@ -568,28 +568,32 @@ impl CompletionPopover {
         }
     }
 
+    /// Handle a row being activated.
     fn row_activated(&self, row: &CompletionRow) {
-        if let Some(member) = row.member() {
-            let imp = self.imp();
+        let Some(source) = row.source() else {
+            return;
+        };
+        let imp = self.imp();
+
+        let Some((mut start, mut end, _)) = imp.current_word.take() else {
+            return;
+        };
 
-            if let Some((mut start, mut end, _)) = imp.current_word.take() {
-                let view = self.view();
-                let buffer = view.buffer();
+        let view = self.view();
+        let buffer = view.buffer();
 
-                buffer.delete(&mut start, &mut end);
+        buffer.delete(&mut start, &mut end);
 
-                let anchor = match start.child_anchor() {
-                    Some(anchor) => anchor,
-                    None => buffer.create_child_anchor(&mut start),
-                };
-                let pill = Pill::for_user(member);
-                view.add_child_at_anchor(&pill, &anchor);
+        let anchor = match start.child_anchor() {
+            Some(anchor) => anchor,
+            None => buffer.create_child_anchor(&mut start),
+        };
+        let pill = Pill::new(source);
+        view.add_child_at_anchor(&pill, &anchor);
 
-                self.popdown();
-                self.select_row_at_index(None);
-                view.grab_focus();
-            }
-        }
+        self.popdown();
+        self.select_row_at_index(None);
+        view.grab_focus();
     }
 
     fn is_inhibited(&self) -> bool {

src/session/view/content/room_history/message_toolbar/completion/completion_row.rs

@@ -1,13 +1,12 @@
 use gtk::{glib, prelude::*, subclass::prelude::*, CompositeTemplate};
 
 use crate::{
-    components::Avatar,
-    prelude::*,
+    components::{Avatar, PillSource},
     session::model::{Member, Room},
 };
 
 mod imp {
-    use std::cell::RefCell;
+    use std::{cell::RefCell, marker::PhantomData};
 
     use glib::subclass::InitializingObject;
 
@@ -25,12 +24,15 @@ mod imp {
         pub display_name: TemplateChild<gtk::Label>,
         #[template_child]
         pub id: TemplateChild<gtk::Label>,
+        /// The source of the data displayed by this row.
+        pub source: RefCell<Option<PillSource>>,
         /// The room member presented by this row.
-        #[property(get, set = Self::set_member, explicit_notify, nullable)]
-        pub member: RefCell<Option<Member>>,
+        #[property(get = Self::member, set = Self::set_member, explicit_notify, nullable)]
+        pub member: PhantomData<Option<Member>>,
         /// The room presented by this row.
-        #[property(get, set = Self::set_room, explicit_notify, nullable)]
-        pub room: RefCell<Option<Room>>,
+        #[property(get = Self::room, set = Self::set_room, explicit_notify, nullable)]
+        pub room: PhantomData<Option<Room>>,
+        bindings: RefCell<Option<[glib::Binding; 2]>>,
     }
 
     #[glib::object_subclass]
@@ -49,55 +51,64 @@ mod imp {
     }
 
     #[glib::derived_properties]
-    impl ObjectImpl for CompletionRow {}
+    impl ObjectImpl for CompletionRow {
+        fn dispose(&self) {
+            for binding in self.bindings.take().iter().flatten() {
+                binding.unbind();
+            }
+        }
+    }
 
     impl WidgetImpl for CompletionRow {}
     impl ListBoxRowImpl for CompletionRow {}
 
     impl CompletionRow {
-        /// Set the room member displayed by this row.
-        fn set_member(&self, member: Option<Member>) {
-            if *self.member.borrow() == member {
-                return;
+        /// Set the source of the data displayed by this row.
+        pub(super) fn set_source(&self, source: Option<PillSource>) {
+            for binding in self.bindings.take().iter().flatten() {
+                binding.unbind();
             }
 
-            if let Some(member) = &member {
-                self.avatar.set_data(Some(member.avatar_data()));
-                self.display_name.set_label(&member.display_name());
-                self.id.set_label(member.user_id().as_str());
+            if let Some(source) = &source {
+                let display_name_binding = source.bind_display_name(&*self.display_name, "label");
+                let id_binding = source.bind_identifier(&*self.id, "label");
+                self.bindings
+                    .replace(Some([display_name_binding, id_binding]));
             }
 
-            self.member.replace(member);
-            self.room.replace(None);
+            self.avatar
+                .set_data(source.as_ref().map(|s| s.avatar_data()));
+            self.source.replace(source);
 
             let obj = self.obj();
             obj.notify_member();
             obj.notify_room();
         }
 
-        /// Set the room displayed by this row.
-        fn set_room(&self, room: Option<Room>) {
-            if *self.room.borrow() == room {
-                return;
+        /// The room member displayed by this row.
+        fn member(&self) -> Option<Member> {
+            match self.source.borrow().as_ref()? {
+                PillSource::User(user) => user.clone().downcast().ok(),
+                _ => None,
             }
+        }
 
-            if let Some(room) = &room {
-                self.avatar.set_data(Some(room.avatar_data()));
-                self.display_name.set_label(&room.display_name());
-                self.id.set_label(
-                    room.alias()
-                        .as_ref()
-                        .map(|a| a.as_str())
-                        .unwrap_or_else(|| room.room_id().as_str()),
-                );
-            }
+        /// Set the room member displayed by this row.
+        fn set_member(&self, member: Option<Member>) {
+            self.set_source(member.and_upcast().map(PillSource::User))
+        }
 
-            self.room.replace(room);
-            self.member.replace(None);
+        /// The room displayed by this row.
+        fn room(&self) -> Option<Room> {
+            match self.source.borrow().as_ref()? {
+                PillSource::Room(room) => Some(room.clone()),
+                _ => None,
+            }
+        }
 
-            let obj = self.obj();
-            obj.notify_member();
-            obj.notify_room();
+        /// Set the room displayed by this row.
+        fn set_room(&self, room: Option<Room>) {
+            self.set_source(room.map(PillSource::Room))
         }
     }
 }
@@ -112,6 +123,16 @@ impl CompletionRow {
     pub fn new() -> Self {
         glib::Object::new()
     }
+
+    /// The source of the data displayed by this row
+    pub fn source(&self) -> Option<PillSource> {
+        self.imp().source.borrow().clone()
+    }
+
+    /// Set the source of the data displayed by this row.
+    pub fn set_source(&self, source: Option<PillSource>) {
+        self.imp().set_source(source);
+    }
 }
 
 impl Default for CompletionRow {