From 0c309f2443c2fdff225ff9285e72a4f5824427bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= Date: Sun, 11 May 2025 10:30:23 +0200 Subject: [PATCH] explore: Fix and refactor Use a GListStore for the list of rooms, and RemoteRoom to represent the rooms. Fix cleaning server popover when switching between sessions, and make sure that triggering a search only happens when the view is mapped. --- src/components/avatar/data.rs | 5 - src/session/model/remote/room.rs | 42 +++- src/session/view/content/explore/mod.rs | 127 +++++------ src/session/view/content/explore/mod.ui | 24 +-- .../view/content/explore/public_room.rs | 204 ------------------ .../view/content/explore/public_room_row.rs | 177 +++++++-------- .../{public_room_list.rs => search.rs} | 162 +++++++------- .../view/content/explore/server_list.rs | 8 +- .../view/content/explore/servers_popover.rs | 29 +-- src/session/view/content/mod.rs | 1 - 10 files changed, 277 insertions(+), 502 deletions(-) delete mode 100644 src/session/view/content/explore/public_room.rs rename src/session/view/content/explore/{public_room_list.rs => search.rs} (66%) diff --git a/src/components/avatar/data.rs b/src/components/avatar/data.rs index 1dbb876f..bbcd9fe6 100644 --- a/src/components/avatar/data.rs +++ b/src/components/avatar/data.rs @@ -66,11 +66,6 @@ impl AvatarData { glib::Object::new() } - /// Constructs an `AvatarData` with the given image data. - pub(crate) fn with_image(image: AvatarImage) -> Self { - glib::Object::builder().property("image", image).build() - } - /// Get this avatar as a notification icon. /// /// Returns `None` if an error occurred while generating the icon. diff --git a/src/session/model/remote/room.rs b/src/session/model/remote/room.rs index 8119b504..71280035 100644 --- a/src/session/model/remote/room.rs +++ b/src/session/model/remote/room.rs @@ -7,7 +7,9 @@ use ruma::{ room::get_summary, space::{get_hierarchy, SpaceHierarchyRoomsChunk}, }, - assign, uint, OwnedMxcUri, OwnedRoomAliasId, OwnedRoomId, + assign, + directory::PublicRoomsChunk, + uint, OwnedMxcUri, OwnedRoomAliasId, OwnedRoomId, }; use tracing::{debug, warn}; @@ -413,13 +415,32 @@ glib::wrapper! { } impl RemoteRoom { - pub(super) fn new(session: &Session, uri: MatrixRoomIdUri) -> Self { + /// Construct a new `RemoteRoom` for the given URI, without any data. + fn without_data(session: &Session, uri: MatrixRoomIdUri) -> Self { let obj = glib::Object::builder::() .property("session", session) .build(); - obj.imp().set_uri(uri); + obj + } + + /// Construct a new `RemoteRoom` for the given URI. + /// + /// This method automatically makes a request to load the room's data. + pub(super) fn new(session: &Session, uri: MatrixRoomIdUri) -> Self { + let obj = Self::without_data(session, uri); obj.load_data_if_stale(); + obj + } + + /// Construct a new `RemoteRoom` for the given URI and data. + pub(crate) fn with_data( + session: &Session, + uri: MatrixRoomIdUri, + data: impl Into, + ) -> Self { + let obj = Self::without_data(session, uri); + obj.imp().set_data(data.into()); obj } @@ -464,7 +485,7 @@ impl RemoteRoom { /// The remote room data. #[derive(Debug)] -struct RemoteRoomData { +pub(crate) struct RemoteRoomData { room_id: OwnedRoomId, canonical_alias: Option, name: Option, @@ -498,3 +519,16 @@ impl From for RemoteRoomData { } } } + +impl From for RemoteRoomData { + fn from(value: PublicRoomsChunk) -> Self { + Self { + room_id: value.room_id, + canonical_alias: value.canonical_alias, + name: value.name, + topic: value.topic, + avatar_url: value.avatar_url, + joined_members_count: value.num_joined_members.try_into().unwrap_or(u32::MAX), + } + } +} diff --git a/src/session/view/content/explore/mod.rs b/src/session/view/content/explore/mod.rs index 79f6163e..749c228c 100644 --- a/src/session/view/content/explore/mod.rs +++ b/src/session/view/content/explore/mod.rs @@ -2,24 +2,23 @@ use adw::{prelude::*, subclass::prelude::*}; use gtk::{gio, glib, glib::clone, CompositeTemplate}; use tracing::error; -mod public_room; -mod public_room_list; mod public_room_row; +mod search; mod server; mod server_list; mod server_row; mod servers_popover; -pub use self::{ - public_room::PublicRoom, public_room_list::PublicRoomList, public_room_row::PublicRoomRow, +use self::{ + public_room_row::PublicRoomRow, search::ExploreSearch, server::ExploreServer, + server_list::ExploreServerList, server_row::ExploreServerRow, servers_popover::ExploreServersPopover, }; -use self::{server::ExploreServer, server_list::ExploreServerList, server_row::ExploreServerRow}; use crate::{ components::LoadingRow, prelude::*, - session::model::Session, - utils::{BoundObject, LoadingState, SingleItemListModel}, + session::model::{RemoteRoom, Session}, + utils::{LoadingState, SingleItemListModel}, }; mod imp { @@ -54,8 +53,8 @@ mod imp { /// The current session. #[property(get, set = Self::set_session, explicit_notify)] session: glib::WeakRef, - /// The list of public rooms. - public_room_list: BoundObject, + /// The search of the view. + search: ExploreSearch, /// The items added at the end of the list. end_items: OnceCell, /// The full list model. @@ -69,9 +68,6 @@ mod imp { type ParentType = adw::BreakpointBin; fn class_init(klass: &mut Self::Class) { - PublicRoom::ensure_type(); - PublicRoomRow::ensure_type(); - Self::bind_template(klass); Self::bind_template_callbacks(klass); @@ -88,6 +84,7 @@ mod imp { fn constructed(&self) { self.parent_constructed(); + // Listen to a change of selected server. self.servers_popover.connect_selected_server_notify(clone!( #[weak(rename_to = imp)] self, @@ -96,19 +93,19 @@ mod imp { } )); + // Load more items when scrolling, if needed. let adj = self.scrolled_window.vadjustment(); adj.connect_value_changed(clone!( #[weak(rename_to = imp)] self, move |adj| { if adj.upper() - adj.value() < adj.page_size() * 2.0 { - if let Some(public_room_list) = imp.public_room_list.obj() { - public_room_list.load_more(); - } + imp.search.load_more(); } } )); + // Set up the item factory for the GtkListView. let factory = gtk::SignalListItemFactory::new(); factory.connect_bind(move |_, list_item| { let Some(list_item) = list_item.downcast_ref::() else { @@ -122,9 +119,9 @@ mod imp { return; }; - if let Some(public_room) = item.downcast_ref::() { + if let Some(room) = item.downcast_ref::() { let public_room_row = list_item.child_or_default::(); - public_room_row.set_public_room(public_room); + public_room_row.set_room(room); } else if let Some(loading_row) = item.downcast_ref::() { list_item.set_child(Some(loading_row)); } @@ -134,6 +131,24 @@ mod imp { let flattened_model = gtk::FlattenListModel::new(Some(self.full_model().clone())); self.listview .set_model(Some(>k::NoSelection::new(Some(flattened_model)))); + + // Listen to changes in the search loading state. + self.search.connect_loading_state_notify(clone!( + #[weak(rename_to = imp)] + self, + move |_| { + imp.update_visible_child(); + } + )); + + // Listen to changes in the results. + self.search.list().connect_items_changed(clone!( + #[weak(rename_to = imp)] + self, + move |_, _, _, _| { + imp.update_visible_child(); + } + )); } } @@ -141,6 +156,11 @@ mod imp { fn grab_focus(&self) -> bool { self.search_entry.grab_focus() } + + fn map(&self) { + self.parent_map(); + self.trigger_search(); + } } impl BreakpointBinImpl for Explore {} @@ -153,42 +173,9 @@ mod imp { return; } - self.public_room_list.disconnect_signals(); - - if let Some(session) = session { - let public_room_list = PublicRoomList::new(session); - - let full_model = self.full_model(); - if full_model.n_items() == 2 { - full_model.splice(0, 1, &[public_room_list.clone()]); - } else { - full_model.insert(0, &public_room_list); - } - - let loading_state_handler = public_room_list.connect_loading_state_notify(clone!( - #[weak(rename_to = imp)] - self, - move |_| { - imp.update_visible_child(); - } - )); - - let items_changed_handler = public_room_list.connect_items_changed(clone!( - #[weak(rename_to = imp)] - self, - move |_, _, _, _| { - imp.update_visible_child(); - } - )); - - self.public_room_list.set( - public_room_list, - vec![loading_state_handler, items_changed_handler], - ); - self.update_visible_child(); - } - self.session.set(session); + + self.trigger_search(); self.obj().notify_session(); } @@ -205,6 +192,7 @@ mod imp { fn full_model(&self) -> &gio::ListStore { self.full_model.get_or_init(|| { let model = gio::ListStore::new::(); + model.append(&self.search.list()); model.append(self.end_items()); model }) @@ -256,21 +244,10 @@ mod imp { self.header_bar.pack_end(&*self.servers_button); } - /// Make sure that the view is initialized. - /// - /// If it is already initialized, this is a noop. - pub(super) fn init(&self) { - self.servers_popover.load(); - } - /// Update the visible child according to the current state. fn update_visible_child(&self) { - let Some(public_room_list) = self.public_room_list.obj() else { - return; - }; - - let loading_state = public_room_list.loading_state(); - let is_empty = public_room_list.is_empty(); + let loading_state = self.search.loading_state(); + let is_empty = self.search.is_empty(); // Create or remove the loading row, as needed. let show_loading_row = matches!(loading_state, LoadingState::Loading) && !is_empty; @@ -299,17 +276,24 @@ mod imp { /// Trigger a search with the current term. #[template_callback] - fn trigger_search(&self) { - let Some(public_room_list) = self.public_room_list.obj() else { + pub(super) fn trigger_search(&self) { + if !self.obj().is_mapped() { + // Do not make a search if the view is not mapped. + return; + } + + let Some(session) = self.session.upgrade() else { return; }; + self.servers_popover.set_session(&session); + let text = self.search_entry.text().into(); let server = self .servers_popover .selected_server() .expect("a server should be selected"); - public_room_list.search(Some(text), &server); + self.search.search(&session, Some(text), &server); } /// Handle when the selected server changed. @@ -333,13 +317,6 @@ impl Explore { glib::Object::builder().property("session", session).build() } - /// Make sure that the view is initialized. - /// - /// If it is already initialized, this is a noop. - pub(crate) fn init(&self) { - self.imp().init(); - } - /// The header bar of the explorer. pub(crate) fn header_bar(&self) -> &adw::HeaderBar { &self.imp().header_bar diff --git a/src/session/view/content/explore/mod.ui b/src/session/view/content/explore/mod.ui index 98df11e9..89ebe22b 100644 --- a/src/session/view/content/explore/mod.ui +++ b/src/session/view/content/explore/mod.ui @@ -35,9 +35,7 @@ Switch servers - - - + @@ -102,26 +100,6 @@ 24 24 item - - - - - - - ]]> - - diff --git a/src/session/view/content/explore/public_room.rs b/src/session/view/content/explore/public_room.rs deleted file mode 100644 index 0f3357fc..00000000 --- a/src/session/view/content/explore/public_room.rs +++ /dev/null @@ -1,204 +0,0 @@ -use gtk::{glib, glib::clone, prelude::*, subclass::prelude::*}; -use ruma::{directory::PublicRoomsChunk, OwnedServerName}; - -use crate::{ - components::{AvatarData, AvatarImage, AvatarUriSource}, - session::model::{Room, RoomList}, - utils::BoundConstructOnlyObject, -}; - -mod imp { - use std::cell::{Cell, OnceCell, RefCell}; - - use super::*; - - #[derive(Debug, Default, glib::Properties)] - #[properties(wrapper_type = super::PublicRoom)] - pub struct PublicRoom { - /// The list of rooms in the current session. - #[property(get, set = Self::set_room_list, construct_only)] - room_list: BoundConstructOnlyObject, - /// The server that returned the room. - server: OnceCell, - /// The data for this room. - data: OnceCell, - /// The avatar data for this room. - #[property(get)] - avatar_data: OnceCell, - /// The `Room` object for this room, if the user is already a member of - /// this room. - #[property(get)] - room: RefCell>, - /// Whether the room is pending. - /// - /// A room is pending when the user clicked to join it. - #[property(get)] - is_pending: Cell, - room_added_handler: RefCell>, - } - - #[glib::object_subclass] - impl ObjectSubclass for PublicRoom { - const NAME: &'static str = "PublicRoom"; - type Type = super::PublicRoom; - } - - #[glib::derived_properties] - impl ObjectImpl for PublicRoom { - fn dispose(&self) { - if let Some(handler) = self.room_added_handler.take() { - self.room_list.obj().disconnect(handler); - } - } - } - - impl PublicRoom { - /// Set the list of rooms in the current session. - fn set_room_list(&self, room_list: RoomList) { - let pending_rooms_changed_handler = room_list.connect_joining_rooms_changed(clone!( - #[weak(rename_to = imp)] - self, - move |_| { - imp.update_is_pending(); - } - )); - - self.room_list - .set(room_list, vec![pending_rooms_changed_handler]); - } - - /// Set the data for this room. - pub(super) fn set_server_and_data( - &self, - server: Option, - data: PublicRoomsChunk, - ) { - let room_list = self.room_list.obj(); - let Some(session) = room_list.session() else { - return; - }; - - if let Some(server) = server { - self.server - .set(server) - .expect("server should not be initialized"); - } - - let data = self.data.get_or_init(|| data); - - let avatar_data = AvatarData::with_image(AvatarImage::new( - &session, - AvatarUriSource::Room, - data.avatar_url.clone(), - None, - )); - - if let Some(display_name) = data.name.clone() { - avatar_data.set_display_name(display_name); - } - - self.avatar_data - .set(avatar_data) - .expect("avatar data was not initialized"); - - if let Some(room) = room_list.get(&data.room_id) { - self.set_room(Some(room)); - } else { - let room_id = data.room_id.clone(); - let room_added_handler = room_list.connect_items_changed(clone!( - #[weak(rename_to = imp)] - self, - move |room_list, _, _, _| { - if let Some(room) = room_list.get(&room_id) { - if let Some(handler) = imp.room_added_handler.take() { - imp.set_room(Some(room)); - room_list.disconnect(handler); - } - } - } - )); - - self.room_added_handler.replace(Some(room_added_handler)); - } - - self.update_is_pending(); - } - - /// The server that returned this room. - pub(super) fn server(&self) -> Option<&OwnedServerName> { - self.server.get() - } - - /// The data for this room. - pub(super) fn data(&self) -> &PublicRoomsChunk { - self.data.get().expect("data should be initialized") - } - - /// Set the [`Room`] for this room. - fn set_room(&self, room: Option) { - if *self.room.borrow() == room { - return; - } - - self.room.replace(room); - self.obj().notify_room(); - } - - /// Update whether this room is pending. - fn update_is_pending(&self) { - let identifier = (*self.data().room_id).into(); - let is_pending = self.room_list.obj().is_joining_room(identifier); - - self.set_is_pending(is_pending); - } - - /// Set whether this room is pending. - fn set_is_pending(&self, is_pending: bool) { - if self.is_pending.get() == is_pending { - return; - } - - self.is_pending.set(is_pending); - self.obj().notify_is_pending(); - } - } -} - -glib::wrapper! { - /// A room in a homeserver's public directory. - pub struct PublicRoom(ObjectSubclass); -} - -impl PublicRoom { - pub fn new( - room_list: &RoomList, - server: Option, - data: PublicRoomsChunk, - ) -> Self { - let obj = glib::Object::builder::() - .property("room-list", room_list) - .build(); - obj.imp().set_server_and_data(server, data); - obj - } - - /// The server that returned this room. - pub(crate) fn server(&self) -> Option<&OwnedServerName> { - self.imp().server() - } - - /// The data for this room. - pub(crate) fn data(&self) -> &PublicRoomsChunk { - self.imp().data() - } - - /// The display name for this room. - pub(crate) fn display_name(&self) -> String { - let data = self.imp().data(); - - data.name - .clone() - .or_else(|| data.canonical_alias.as_ref().map(ToString::to_string)) - .unwrap_or_else(|| data.room_id.to_string()) - } -} diff --git a/src/session/view/content/explore/public_room_row.rs b/src/session/view/content/explore/public_room_row.rs index 0108c47f..50a69124 100644 --- a/src/session/view/content/explore/public_room_row.rs +++ b/src/session/view/content/explore/public_room_row.rs @@ -2,17 +2,19 @@ use adw::{prelude::*, subclass::prelude::*}; use gettextrs::gettext; use gtk::{glib, glib::clone, CompositeTemplate}; -use super::PublicRoom; use crate::{ components::{Avatar, LoadingButton}, gettext_f, ngettext_f, prelude::*, - spawn, toast, - utils::{matrix::MatrixIdUri, string::linkify, BoundObject}, + session::model::RemoteRoom, + toast, + utils::{matrix::MatrixIdUri, string::linkify}, Window, }; mod imp { + use std::cell::RefCell; + use glib::subclass::InitializingObject; use super::*; @@ -35,9 +37,10 @@ mod imp { members_count_box: TemplateChild, #[template_child] button: TemplateChild, - /// The public room displayed by this row. - #[property(get, set= Self::set_public_room, explicit_notify)] - public_room: BoundObject, + /// The room displayed by this row. + #[property(get, set= Self::set_room, explicit_notify)] + room: RefCell>, + room_list_info_handlers: RefCell>, } #[glib::object_subclass] @@ -78,6 +81,10 @@ mod imp { } )); } + + fn dispose(&self) { + self.disconnect_signals(); + } } impl WidgetImpl for PublicRoomRow {} @@ -85,54 +92,52 @@ mod imp { #[gtk::template_callbacks] impl PublicRoomRow { - /// Set the public room displayed by this row. - fn set_public_room(&self, public_room: Option) { - if self.public_room.obj() == public_room { + /// Set the room displayed by this row. + fn set_room(&self, room: RemoteRoom) { + if self.room.borrow().as_ref().is_some_and(|r| *r == room) { return; } - self.public_room.disconnect_signals(); + self.disconnect_signals(); - if let Some(public_room) = public_room { - let pending_handler = public_room.connect_is_pending_notify(clone!( - #[weak(rename_to = imp)] - self, - move |_| { - imp.update_button(); - } - )); - let room_handler = public_room.connect_room_notify(clone!( - #[weak(rename_to = imp)] - self, - move |_| { - imp.update_button(); - } - )); + let room_list_info = room.room_list_info(); + let is_joining_handler = room_list_info.connect_is_joining_notify(clone!( + #[weak(rename_to = imp)] + self, + move |_| { + imp.update_button(); + } + )); + let local_room_handler = room_list_info.connect_local_room_notify(clone!( + #[weak(rename_to = imp)] + self, + move |_| { + imp.update_button(); + } + )); - self.public_room - .set(public_room, vec![pending_handler, room_handler]); + self.room_list_info_handlers + .replace(vec![is_joining_handler, local_room_handler]); - self.update_button(); - self.update_row(); - } + self.room.replace(Some(room)); - self.obj().notify_public_room(); + self.update_button(); + self.update_row(); + self.obj().notify_room(); } /// Update this row for the current state. fn update_row(&self) { - let Some(public_room) = self.public_room.obj() else { + let Some(room) = self.room.borrow().clone() else { return; }; - self.avatar.set_data(Some(public_room.avatar_data())); - self.display_name.set_text(&public_room.display_name()); - - let data = public_room.data(); + self.avatar.set_data(Some(room.avatar_data())); + self.display_name.set_text(&room.display_name()); - if let Some(topic) = &data.topic { + if let Some(topic) = room.topic() { // Detect links. - let mut t = linkify(topic); + let mut t = linkify(&topic); // Remove trailing spaces. t.truncate_end_whitespaces(); @@ -142,12 +147,13 @@ mod imp { self.description.set_visible(false); } - if let Some(alias) = &data.canonical_alias { + let canonical_alias = room.canonical_alias(); + if let Some(alias) = &canonical_alias { self.alias.set_text(alias.as_str()); } - self.alias.set_visible(data.canonical_alias.is_some()); + self.alias.set_visible(canonical_alias.is_some()); - let members_count = u32::try_from(data.num_joined_members).unwrap_or(u32::MAX); + let members_count = room.joined_members_count(); self.members_count.set_text(&members_count.to_string()); let members_count_tooltip = ngettext_f( // Translators: Do NOT translate the content between '{' and '}', @@ -163,74 +169,77 @@ mod imp { /// Update the join/view button of this row. fn update_button(&self) { - let Some(public_room) = self.public_room.obj() else { + let Some(room) = self.room.borrow().clone() else { return; }; - let room_joined = public_room.room().is_some(); + let room_list_info = room.room_list_info(); + let room_name = room.display_name(); - let label = if room_joined { - // Translators: This is a verb, as in 'View Room'. - gettext("View") + let (label, accessible_desc) = if room_list_info.local_room().is_some() { + ( + // Translators: This is a verb, as in 'View Room'. + gettext("View"), + gettext_f("View {room_name}", &[("room_name", &room_name)]), + ) } else { - gettext("Join") + ( + gettext("Join"), + gettext_f("Join {room_name}", &[("room_name", &room_name)]), + ) }; - self.button.set_content_label(label); - let room_name = public_room.display_name(); - let accessible_desc = if room_joined { - gettext_f("View {room_name}", &[("room_name", &room_name)]) - } else { - gettext_f("Join {room_name}", &[("room_name", &room_name)]) - }; + self.button.set_content_label(label); self.button .update_property(&[gtk::accessible::Property::Description(&accessible_desc)]); - self.button.set_is_loading(public_room.is_pending()); + self.button.set_is_loading(room_list_info.is_joining()); } /// Join or view the public room. #[template_callback] - fn join_or_view(&self) { - let Some(public_room) = self.public_room.obj() else { + async fn join_or_view(&self) { + let Some(room) = self.room.borrow().clone() else { return; }; - if let Some(room) = public_room.room() { - if let Some(window) = self.obj().root().and_downcast::() { - window.session_view().select_room(room); + let obj = self.obj(); + + if let Some(local_room) = room.room_list_info().local_room() { + if let Some(window) = obj.root().and_downcast::() { + window.session_view().select_room(local_room); } } else { - let data = public_room.data(); - - // Prefer the alias as we are sure the server can find the room that way. - let (room_id, via) = data.canonical_alias.clone().map_or_else( - || { - let id = data.room_id.clone().into(); - let via = public_room.server().cloned().into_iter().collect(); - (id, via) - }, - |id| (id.into(), vec![]), - ); - - let obj = self.obj(); - let room_list = public_room.room_list(); - spawn!(clone!( - #[weak] - obj, - async move { - if let Err(error) = room_list.join_by_id_or_alias(room_id, via).await { - toast!(obj, error); - } - } - )); + let Some(session) = room.session() else { + return; + }; + + let uri = room.uri(); + + if let Err(error) = session + .room_list() + .join_by_id_or_alias(uri.id.clone(), uri.via.clone()) + .await + { + toast!(obj, error); + } + } + } + + /// Disconnect the signal handlers of this row. + fn disconnect_signals(&self) { + if let Some(room) = self.room.borrow().as_ref() { + let room_list_info = room.room_list_info(); + for handler in self.room_list_info_handlers.take() { + room_list_info.disconnect(handler); + } } } } } glib::wrapper! { - /// A row representing a room for a homeserver's public directory. + /// A row representing a room in a homeserver's public directory. pub struct PublicRoomRow(ObjectSubclass) @extends gtk::Widget, adw::Bin, @implements gtk::Accessible; } diff --git a/src/session/view/content/explore/public_room_list.rs b/src/session/view/content/explore/search.rs similarity index 66% rename from src/session/view/content/explore/public_room_list.rs rename to src/session/view/content/explore/search.rs index 46568836..42e786f6 100644 --- a/src/session/view/content/explore/public_room_list.rs +++ b/src/session/view/content/explore/search.rs @@ -8,27 +8,29 @@ use ruma::{ use tokio::task::AbortHandle; use tracing::error; -use super::{ExploreServer, PublicRoom}; -use crate::{session::model::Session, spawn, spawn_tokio, utils::LoadingState}; +use super::ExploreServer; +use crate::{ + session::model::{RemoteRoom, Session}, + spawn, spawn_tokio, + utils::{matrix::MatrixRoomIdUri, LoadingState}, +}; /// The maximum size of a batch of public rooms. const PUBLIC_ROOMS_BATCH_SIZE: u32 = 20; mod imp { - use std::cell::{Cell, RefCell}; + use std::cell::{Cell, OnceCell, RefCell}; use super::*; #[derive(Debug, Default, glib::Properties)] - #[properties(wrapper_type = super::PublicRoomList)] - pub struct PublicRoomList { - /// The current session. - #[property(get, construct_only)] - session: glib::WeakRef, - /// The list of rooms. - list: RefCell>, + #[properties(wrapper_type = super::ExploreSearch)] + pub struct ExploreSearch { + /// The list of public rooms for the current search. + #[property(get = Self::list_owned)] + list: OnceCell, /// The current search. - search: RefCell, + search: RefCell, /// The next batch to continue the search, if any. next_batch: RefCell>, /// The loading state of the list. @@ -39,36 +41,27 @@ mod imp { } #[glib::object_subclass] - impl ObjectSubclass for PublicRoomList { - const NAME: &'static str = "PublicRoomList"; - type Type = super::PublicRoomList; - type Interfaces = (gio::ListModel,); + impl ObjectSubclass for ExploreSearch { + const NAME: &'static str = "ExploreSearch"; + type Type = super::ExploreSearch; } #[glib::derived_properties] - impl ObjectImpl for PublicRoomList {} - - impl ListModelImpl for PublicRoomList { - fn item_type(&self) -> glib::Type { - PublicRoom::static_type() - } + impl ObjectImpl for ExploreSearch {} - fn n_items(&self) -> u32 { - self.list.borrow().len() as u32 + impl ExploreSearch { + /// The list of public rooms for the current search. + fn list(&self) -> &gio::ListStore { + self.list.get_or_init(gio::ListStore::new::) } - fn item(&self, position: u32) -> Option { - self.list - .borrow() - .get(position as usize) - .cloned() - .and_upcast() + /// The owned list of public rooms for the current search. + fn list_owned(&self) -> gio::ListStore { + self.list().clone() } - } - impl PublicRoomList { /// Set the current search. - pub(super) fn set_search(&self, search: PublicRoomsSearch) { + pub(super) fn set_search(&self, search: ExploreSearchData) { if *self.search.borrow() == search { return; } @@ -97,7 +90,7 @@ mod imp { /// Whether the list is empty. pub(super) fn is_empty(&self) -> bool { - self.list.borrow().is_empty() + self.list().n_items() == 0 } /// Whether we can load more rooms with the current search. @@ -110,10 +103,6 @@ mod imp { /// If `clear` is `true`, we start a new search and replace the list of /// rooms, otherwise we use the `next_batch` and add more rooms. pub(super) async fn load(&self, clear: bool) { - let Some(session) = self.session.upgrade() else { - return; - }; - // Only make a request if we can load more items or we want to replace the // current list. if !clear && !self.can_load_more() { @@ -122,22 +111,24 @@ mod imp { if clear { // Clear the list. - let removed = self.list.borrow().len(); - self.list.borrow_mut().clear(); + self.list().remove_all(); self.next_batch.take(); // Abort any ongoing request. if let Some(handle) = self.abort_handle.take() { handle.abort(); } - - self.obj().items_changed(0, removed as u32, 0); } + let search = self.search.borrow().clone(); + + let Some(session) = search.session.upgrade() else { + return; + }; + self.set_loading_state(LoadingState::Loading); let next_batch = self.next_batch.borrow().clone(); - let search = self.search.borrow().clone(); let request = search.as_request(next_batch); let client = session.client(); @@ -159,7 +150,7 @@ mod imp { } match result { - Ok(response) => self.add_rooms(&search, response), + Ok(response) => self.add_rooms(&session, &search, response), Err(error) => { self.set_loading_state(LoadingState::Error); error!("Could not search public rooms: {error}"); @@ -170,49 +161,45 @@ mod imp { /// Add the rooms from the given response to this list. fn add_rooms( &self, - search: &PublicRoomsSearch, + session: &Session, + search: &ExploreSearchData, response: get_public_rooms_filtered::v3::Response, ) { - let Some(session) = self.session.upgrade() else { - return; - }; - - let room_list = session.room_list(); - self.next_batch.replace(response.next_batch); - let (position, added) = { - let mut list = self.list.borrow_mut(); - let position = list.len(); - let added = response.chunk.len(); - - list.extend( - response - .chunk - .into_iter() - .map(|data| PublicRoom::new(&room_list, search.server.clone(), data)), - ); + let new_rooms = response + .chunk + .into_iter() + .map(|data| { + let id = data + .canonical_alias + .clone() + .map_or_else(|| data.room_id.clone().into(), Into::into); + let uri = MatrixRoomIdUri { + id, + via: search.server.clone().into_iter().collect(), + }; + + RemoteRoom::with_data(session, uri, data) + }) + .collect::>(); + + self.list().extend_from_slice(&new_rooms); - (position, added) - }; - - if added > 0 { - self.obj().items_changed(position as u32, 0, added as u32); - } self.set_loading_state(LoadingState::Ready); } } } glib::wrapper! { - /// A list of rooms in a homeserver's public directory. - pub struct PublicRoomList(ObjectSubclass) - @implements gio::ListModel; + /// The search API of the view to explore rooms in the public directory of homeservers. + pub struct ExploreSearch(ObjectSubclass); } -impl PublicRoomList { - pub fn new(session: &Session) -> Self { - glib::Object::builder().property("session", session).build() +impl ExploreSearch { + /// Construct a new empty `ExploreSearch`. + pub fn new() -> Self { + glib::Object::new() } /// Whether the list is empty. @@ -221,8 +208,17 @@ impl PublicRoomList { } /// Search the given term on the given server. - pub(crate) fn search(&self, search_term: Option, server: &ExploreServer) { - let search = PublicRoomsSearch { + pub(crate) fn search( + &self, + session: &Session, + search_term: Option, + server: &ExploreServer, + ) { + let session_weak = glib::WeakRef::new(); + session_weak.set(Some(session)); + + let search = ExploreSearchData { + session: session_weak, search_term, server: server.server().cloned(), third_party_network: server.third_party_network(), @@ -244,9 +240,17 @@ impl PublicRoomList { } } +impl Default for ExploreSearch { + fn default() -> Self { + Self::new() + } +} + /// Data about a search in the public rooms directory. -#[derive(Debug, Clone, Default, PartialEq, Eq)] -struct PublicRoomsSearch { +#[derive(Debug, Clone, Default, PartialEq)] +struct ExploreSearchData { + /// The session to use for performing the search. + session: glib::WeakRef, /// The term to search. search_term: Option, /// The server to search. @@ -255,8 +259,8 @@ struct PublicRoomsSearch { third_party_network: Option, } -impl PublicRoomsSearch { - /// Convert this `PublicRoomsSearch` to a request. +impl ExploreSearchData { + /// Convert this `ExploreSearchData` to a request. fn as_request(&self, next_batch: Option) -> get_public_rooms_filtered::v3::Request { let room_network = if let Some(third_party_network) = &self.third_party_network { RoomNetwork::ThirdParty(third_party_network.clone()) diff --git a/src/session/view/content/explore/server_list.rs b/src/session/view/content/explore/server_list.rs index 601fa5db..93b03233 100644 --- a/src/session/view/content/explore/server_list.rs +++ b/src/session/view/content/explore/server_list.rs @@ -93,12 +93,12 @@ mod imp { fn load_servers(&self) { let removed = self.n_items(); + self.own_server.take(); + self.third_party_networks.borrow_mut().clear(); + self.custom_servers.borrow_mut().clear(); + let Some(session) = self.session.upgrade() else { - self.own_server.take(); - self.third_party_networks.borrow_mut().clear(); - self.custom_servers.borrow_mut().clear(); self.obj().items_changed(0, removed, 0); - return; }; diff --git a/src/session/view/content/explore/servers_popover.rs b/src/session/view/content/explore/servers_popover.rs index 7091b1e7..f362a7c9 100644 --- a/src/session/view/content/explore/servers_popover.rs +++ b/src/session/view/content/explore/servers_popover.rs @@ -93,31 +93,19 @@ mod imp { #[gtk::template_callbacks] impl ExploreServersPopover { /// Set the current session. - fn set_session(&self, session: Option<&Session>) { - if session == self.session.upgrade().as_ref() { + fn set_session(&self, session: &Session) { + if self.session.upgrade().as_ref() == Some(session) { return; } - self.session.set(session); - self.obj().notify_session(); - } - - /// Load the list of servers, if needed. - pub(super) fn load(&self) { - let Some(session) = self.session.upgrade() else { - return; - }; - - if self.server_list.session().is_some_and(|s| s == session) { - // Nothing to do. - return; - } - - self.server_list.set_session(&session); + self.session.set(Some(session)); + self.server_list.set_session(session); // Select the first server by default. self.listbox .select_row(self.listbox.row_at_index(0).as_ref()); + + self.obj().notify_session(); } /// Handle when the selected server has changed. @@ -206,9 +194,4 @@ impl ExploreServersPopover { pub fn new(session: &Session) -> Self { glib::Object::builder().property("session", session).build() } - - /// Load the list of servers, if needed. - pub(crate) fn load(&self) { - self.imp().load(); - } } diff --git a/src/session/view/content/mod.rs b/src/session/view/content/mod.rs index d34a972f..55c74b62 100644 --- a/src/session/view/content/mod.rs +++ b/src/session/view/content/mod.rs @@ -233,7 +233,6 @@ mod imp { .downcast_ref::() .is_some_and(|i| i.item_type() == SidebarIconItemType::Explore) { - self.explore.init(); self.set_visible_page(ContentPage::Explore); } else if let Some(verification) = item.downcast_ref::() { self.identity_verification_widget