pill: Refactor and fix visibility

src/components/pill/at_room.rs

@@ -12,7 +12,7 @@ mod imp {
     #[derive(Debug, Default)]
     pub struct AtRoom {
         /// The ID of the room currently represented.
-        pub room_id: OnceCell<OwnedRoomId>,
+        room_id: OnceCell<OwnedRoomId>,
     }
 
     #[glib::object_subclass]
@@ -29,6 +29,18 @@ mod imp {
             gettext("Notify the whole room")
         }
     }
+
+    impl AtRoom {
+        /// Set the ID of the room currently represented.
+        pub(super) fn set_room_id(&self, room_id: OwnedRoomId) {
+            self.room_id.set(room_id).expect("room ID is uninitialized");
+        }
+
+        /// The ID of the room currently represented.
+        pub(super) fn room_id(&self) -> &OwnedRoomId {
+            self.room_id.get().expect("room ID is initialized")
+        }
+    }
 }
 
 glib::wrapper! {
@@ -43,13 +55,13 @@ impl AtRoom {
             .property("display-name", "@room")
             .build();
 
-        obj.imp().room_id.set(room_id).unwrap();
+        obj.imp().set_room_id(room_id);
 
         obj
     }
 
     /// The ID of the room currently represented.
     pub fn room_id(&self) -> &OwnedRoomId {
-        self.imp().room_id.get().unwrap()
+        self.imp().room_id()
     }
 }

src/components/pill/mod.rs

@@ -34,17 +34,17 @@ mod imp {
     #[properties(wrapper_type = super::Pill)]
     pub struct Pill {
         #[template_child]
-        pub content: TemplateChild<gtk::Box>,
+        content: TemplateChild<gtk::Box>,
         #[template_child]
-        pub display_name: TemplateChild<gtk::Label>,
+        display_name: TemplateChild<gtk::Label>,
         #[template_child]
-        pub avatar: TemplateChild<Avatar>,
+        avatar: TemplateChild<Avatar>,
         /// The source of the data displayed by this widget.
         #[property(get, set = Self::set_source, explicit_notify, nullable)]
-        pub source: BoundObject<PillSource>,
+        source: BoundObject<PillSource>,
         /// Whether the pill can be activated.
         #[property(get, set = Self::set_activatable, explicit_notify)]
-        pub activatable: Cell<bool>,
+        activatable: Cell<bool>,
         gesture_click: RefCell<Option<gtk::GestureClick>>,
     }
 
@@ -61,7 +61,7 @@ mod imp {
             klass.set_css_name("inline-pill");
 
             klass.install_action("pill.activate", None, |obj, _, _| {
-                obj.activate();
+                obj.imp().activate();
             });
 
             add_activate_binding_action(klass, "pill.activate");
@@ -178,6 +178,40 @@ mod imp {
 
             self.display_name.set_label(&maybe_ellipsized);
         }
+
+        /// Activate the pill.
+        ///
+        /// This opens a known room or opens the profile of a user or unknown
+        /// room.
+        fn activate(&self) {
+            let Some(source) = self.source.obj() else {
+                return;
+            };
+            let obj = self.obj();
+
+            if let Some(member) = source.downcast_ref::<Member>() {
+                let dialog = UserProfileDialog::new();
+                dialog.set_room_member(member.clone());
+                dialog.present(Some(&*obj));
+            } else if let Some(room) = source.downcast_ref::<Room>() {
+                let Some(session_view) = obj
+                    .ancestor(SessionView::static_type())
+                    .and_downcast::<SessionView>()
+                else {
+                    return;
+                };
+
+                session_view.select_room(Some(room.clone()));
+            } else if let Ok(room) = source.downcast::<RemoteRoom>() {
+                let Some(session) = room.session() else {
+                    return;
+                };
+
+                let dialog = JoinRoomDialog::new(&session);
+                dialog.set_room(room);
+                dialog.present(Some(&*obj));
+            }
+        }
     }
 }
 
@@ -192,36 +226,4 @@ impl Pill {
     pub fn new(source: &impl IsA<PillSource>) -> Self {
         glib::Object::builder().property("source", source).build()
     }
-
-    /// Activate the pill.
-    ///
-    /// This opens a known room or opens the profile of a user or unknown room.
-    fn activate(&self) {
-        let Some(source) = self.source() else {
-            return;
-        };
-
-        if let Some(member) = source.downcast_ref::<Member>() {
-            let dialog = UserProfileDialog::new();
-            dialog.set_room_member(member.clone());
-            dialog.present(Some(self));
-        } else if let Some(room) = source.downcast_ref::<Room>() {
-            let Some(session_view) = self
-                .ancestor(SessionView::static_type())
-                .and_downcast::<SessionView>()
-            else {
-                return;
-            };
-
-            session_view.select_room(Some(room.clone()));
-        } else if let Ok(room) = source.downcast::<RemoteRoom>() {
-            let Some(session) = room.session() else {
-                return;
-            };
-
-            let dialog = JoinRoomDialog::new(&session);
-            dialog.set_room(room);
-            dialog.present(Some(self));
-        }
-    }
 }

src/components/pill/search_entry.rs

@@ -5,10 +5,7 @@ use gtk::{
     CompositeTemplate,
 };
 
-use crate::{
-    components::{Pill, PillSource},
-    prelude::*,
-};
+use crate::components::{Pill, PillSource};
 
 mod imp {
     use std::{cell::RefCell, collections::HashMap, marker::PhantomData, sync::LazyLock};
@@ -22,9 +19,9 @@ mod imp {
     #[properties(wrapper_type = super::PillSearchEntry)]
     pub struct PillSearchEntry {
         #[template_child]
-        pub text_view: TemplateChild<gtk::TextView>,
+        text_view: TemplateChild<gtk::TextView>,
         #[template_child]
-        pub text_buffer: TemplateChild<gtk::TextBuffer>,
+        text_buffer: TemplateChild<gtk::TextBuffer>,
         /// The text of the entry.
         #[property(get = Self::text)]
         text: PhantomData<glib::GString>,
@@ -34,7 +31,7 @@ mod imp {
         /// The pills in the text view.
         ///
         /// A map of pill identifier to anchor of the pill in the text view.
-        pub pills: RefCell<HashMap<String, gtk::TextChildAnchor>>,
+        pills: RefCell<HashMap<String, gtk::TextChildAnchor>>,
     }
 
     #[glib::object_subclass]
@@ -76,6 +73,7 @@ mod imp {
                         return;
                     }
 
+                    // If a pill was removed, emit the corresponding signal.
                     let mut current = *start;
                     loop {
                         if let Some(source) = current
@@ -109,7 +107,7 @@ mod imp {
                 .connect_insert_text(|text_buffer, location, text| {
                     let mut changed = false;
 
-                    // We don't allow adding chars before and between pills
+                    // We do not allow adding chars before and between pills.
                     loop {
                         if location.child_anchor().is_some() {
                             changed = true;
@@ -162,6 +160,61 @@ mod imp {
             self.text_view.set_editable(editable);
             self.obj().notify_editable();
         }
+
+        /// Add a pill for the given source to the entry.
+        pub(super) fn add_pill(&self, source: &PillSource) {
+            let identifier = source.identifier();
+
+            // If the pill already exists, do not insert it again.
+            if self.pills.borrow().contains_key(&identifier) {
+                return;
+            }
+
+            let pill = Pill::new(source);
+            pill.set_margin_start(3);
+            pill.set_margin_end(3);
+
+            let (mut start_iter, mut end_iter) = self.text_buffer.bounds();
+
+            // We don't allow adding chars before and between pills
+            loop {
+                if start_iter.child_anchor().is_some() {
+                    start_iter.forward_char();
+                } else {
+                    break;
+                }
+            }
+
+            self.text_buffer.delete(&mut start_iter, &mut end_iter);
+            let anchor = self.text_buffer.create_child_anchor(&mut start_iter);
+            self.text_view.add_child_at_anchor(&pill, &anchor);
+            self.pills.borrow_mut().insert(identifier, anchor);
+
+            self.text_view.grab_focus();
+        }
+
+        /// Remove the pill with the given identifier.
+        pub(super) fn remove_pill(&self, identifier: &str) {
+            let Some(anchor) = self.pills.borrow_mut().remove(identifier) else {
+                return;
+            };
+
+            if anchor.is_deleted() {
+                // Nothing to do.
+                return;
+            }
+
+            let mut start_iter = self.text_buffer.iter_at_child_anchor(&anchor);
+            let mut end_iter = start_iter;
+            end_iter.forward_char();
+            self.text_buffer.delete(&mut start_iter, &mut end_iter);
+        }
+
+        /// Clear this entry.
+        pub(super) fn clear(&self) {
+            let (mut start, mut end) = self.text_buffer.bounds();
+            self.text_buffer.delete(&mut start, &mut end);
+        }
     }
 }
 
@@ -177,63 +230,18 @@ impl PillSearchEntry {
     }
 
     /// Add a pill for the given source to the entry.
-    pub fn add_pill(&self, source: &impl IsA<PillSource>) {
-        let imp = self.imp();
-        let identifier = source.identifier();
-
-        // If the pill already exists, don't insert it again.
-        if imp.pills.borrow().contains_key(&identifier) {
-            return;
-        }
-
-        let pill = Pill::new(source);
-        pill.set_margin_start(3);
-        pill.set_margin_end(3);
-
-        let (mut start_iter, mut end_iter) = imp.text_buffer.bounds();
-
-        // We don't allow adding chars before and between pills
-        loop {
-            if start_iter.child_anchor().is_some() {
-                start_iter.forward_char();
-            } else {
-                break;
-            }
-        }
-
-        imp.text_buffer.delete(&mut start_iter, &mut end_iter);
-        let anchor = imp.text_buffer.create_child_anchor(&mut start_iter);
-        imp.text_view.add_child_at_anchor(&pill, &anchor);
-        imp.pills.borrow_mut().insert(identifier, anchor);
-
-        imp.text_view.grab_focus();
+    pub(crate) fn add_pill(&self, source: &impl IsA<PillSource>) {
+        self.imp().add_pill(source.upcast_ref());
     }
 
     /// Remove the pill with the given identifier.
-    pub fn remove_pill(&self, identifier: &str) {
-        let imp = self.imp();
-
-        let Some(anchor) = imp.pills.borrow_mut().remove(identifier) else {
-            return;
-        };
-
-        if anchor.is_deleted() {
-            // Nothing to do.
-            return;
-        }
-
-        let text_buffer = &self.imp().text_buffer;
-        let mut start_iter = text_buffer.iter_at_child_anchor(&anchor);
-        let mut end_iter = start_iter;
-        end_iter.forward_char();
-        text_buffer.delete(&mut start_iter, &mut end_iter);
+    pub(crate) fn remove_pill(&self, identifier: &str) {
+        self.imp().remove_pill(identifier);
     }
 
     /// Clear this entry.
-    pub fn clear(&self) {
-        let text_buffer = &self.imp().text_buffer;
-        let (mut start, mut end) = text_buffer.bounds();
-        text_buffer.delete(&mut start, &mut end);
+    pub(crate) fn clear(&self) {
+        self.imp().clear();
     }
 
     /// Connect to the signal emitted when a pill is removed from the entry.

src/components/pill/source.rs

@@ -10,8 +10,8 @@ mod imp {
 
     #[repr(C)]
     pub struct PillSourceClass {
-        pub parent_class: glib::object::ObjectClass,
-        pub identifier: fn(&super::PillSource) -> String,
+        parent_class: glib::object::ObjectClass,
+        pub(super) identifier: fn(&super::PillSource) -> String,
     }
 
     unsafe impl ClassStruct for PillSourceClass {
@@ -28,22 +28,22 @@ mod imp {
     pub struct PillSource {
         /// A unique identifier for this source.
         #[property(get = Self::identifier)]
-        pub identifier: PhantomData<String>,
+        identifier: PhantomData<String>,
         /// The display name of this source.
         #[property(get = Self::display_name, set = Self::set_display_name, explicit_notify)]
-        pub display_name: PhantomData<String>,
+        display_name: PhantomData<String>,
         /// Whether the display name of this source is ambiguous.
         #[property(get, set = Self::set_name_ambiguous, explicit_notify)]
-        pub is_name_ambiguous: Cell<bool>,
+        is_name_ambiguous: Cell<bool>,
         /// The disambiguated display name of this source.
         ///
         /// This is the name to display in case the identifier does not appear
         /// next to it.
         #[property(get = Self::disambiguated_name)]
-        pub disambiguated_name: PhantomData<String>,
+        disambiguated_name: PhantomData<String>,
         /// The avatar data of this source.
         #[property(get)]
-        pub avatar_data: AvatarData,
+        avatar_data: AvatarData,
     }
 
     #[glib::object_subclass]

src/components/pill/source_row.rs

@@ -14,14 +14,14 @@ mod imp {
     #[properties(wrapper_type = super::PillSourceRow)]
     pub struct PillSourceRow {
         #[template_child]
-        pub avatar: TemplateChild<Avatar>,
+        avatar: TemplateChild<Avatar>,
         #[template_child]
-        pub display_name: TemplateChild<gtk::Label>,
+        display_name: TemplateChild<gtk::Label>,
         #[template_child]
-        pub id: TemplateChild<gtk::Label>,
+        id: TemplateChild<gtk::Label>,
         /// The source of the data displayed by this row.
         #[property(get, set = Self::set_source, explicit_notify, nullable)]
-        pub source: RefCell<Option<PillSource>>,
+        source: RefCell<Option<PillSource>>,
     }
 
     #[glib::object_subclass]