diff --git a/src/home/rooms_list.rs b/src/home/rooms_list.rs index c3915970..8b345609 100644 --- a/src/home/rooms_list.rs +++ b/src/home/rooms_list.rs @@ -1,3 +1,21 @@ +//! The `RoomsList` widget displays a filterable list of rooms +//! that can be clicked on to open the room timeline (`RoomScreen`). +//! +//! It is responsible for receiving room-related updates from the background task +//! that runs the room list service. +//! It also receives space-related updates from the background task that runs +//! the space sync service(s). +//! +//! Generally, it maintains several key states: +//! * The set of all joined rooms, which is displayed separately as "direct" rooms +//! and non-direct regular rooms. +//! * The set of invited rooms, which have not yet been joined. +//! * The map of spaces and their child rooms and nested subspaces. +//! +//! This widget is a global singleton and is thus accessible via `Cx::get_global()`, +//! so you can use it from other widgets or functions on the main UI thread +//! that need to query basic info about a particular room or space. + use std::{cell::RefCell, collections::{HashMap, HashSet, hash_map::Entry}, rc::Rc, sync::Arc}; use crossbeam_queue::SegQueue; use makepad_widgets::*; @@ -393,34 +411,25 @@ pub struct RoomsList { /// The currently-active filter function for the list of rooms. /// - /// Note: for performance reasons, this does not get automatically applied - /// when its value changes. Instead, you must manually invoke it on the set of `all_joined_rooms` - /// in order to update the set of `displayed_rooms` accordingly. + /// ## Important Notes + /// 1. Do not use this directly. Instead, use the `should_display_room!()` macro. + /// 2. This does *not* get auto-applied when it changes, for performance reasons. #[rust] display_filter: RoomDisplayFilter, - /// The latest keywords entered into the `RoomFilterInputBar`. + /// The latest keywords (trimmed) entered into the `RoomFilterInputBar`. /// /// If empty, there are no filter keywords in use, and all rooms/spaces should be shown. #[rust] filter_keywords: String, - /// The list of invited rooms currently displayed in the UI, in order from top to bottom. - /// This is a strict subset of the rooms in `all_invited_rooms`, and should be determined - /// by applying the `display_filter` to the set of `all_invited_rooms`. + /// The list of invited rooms currently displayed in the UI. #[rust] displayed_invited_rooms: Vec, #[rust(false)] is_invited_rooms_header_expanded: bool, - /// The list of direct rooms currently displayed in the UI, in order from top to bottom. - /// This is a strict subset of the rooms present in `all_joined_rooms`, - /// and should be determined by applying the `display_filter && is_direct` - /// to the set of `all_joined_rooms`. + /// The list of direct rooms currently displayed in the UI. #[rust] displayed_direct_rooms: Vec, #[rust(false)] is_direct_rooms_header_expanded: bool, - /// The list of regular (non-direct) joined rooms currently displayed in the UI, - /// in order from top to bottom. - /// This is a strict subset of the rooms in `all_joined_rooms`, - /// and should be determined by applying the `display_filter && !is_direct` - /// to the set of `all_joined_rooms`. + /// The list of regular (non-direct) joined rooms currently displayed in the UI. /// /// **Direct rooms are excluded** from this; they are in `displayed_direct_rooms`. #[rust] displayed_regular_rooms: Vec, @@ -428,8 +437,10 @@ pub struct RoomsList { /// The latest status message that should be displayed in the bottom status label. #[rust] status: String, + /// The ID of the currently-selected room. #[rust] current_active_room: Option, + /// The maximum number of rooms that will ever be loaded. /// /// This should not be used to determine whether all requested rooms have been loaded, @@ -446,6 +457,24 @@ impl LiveHook for RoomsList { } } +/// A macro that returns whether a given Room should be displayed in the RoomsList. +/// This is only intended for usage within RoomsList methods. +/// +/// ## Arguments +/// 1. `self: &RoomsList`: an immutable reference to the `RoomsList` widget struct. +/// 2. `room_id: &OwnedRoomId`: an immutable reference to the room's ID. +/// 3. `room: &dyn impl FilterableRoom`: an immutable reference to the room info, +/// which must implement the [`FilterableRoom`] trait. +macro_rules! should_display_room { + ($self:expr, $room_id:expr, $room:expr) => { + !$self.hidden_rooms.contains($room_id) + && ($self.display_filter)($room) + && $self.selected_space.as_ref() + .is_none_or(|space| $self.is_room_indirectly_in_space(space.room_id(), $room_id)) + }; +} + + impl RoomsList { /// Returns whether the homeserver has finished syncing all of the rooms /// that should be synced to our client based on the currently-specified room list filter. @@ -470,7 +499,7 @@ impl RoomsList { match update { RoomsListUpdate::AddInvitedRoom(invited_room) => { let room_id = invited_room.room_name_id.room_id().clone(); - let should_display = (self.display_filter)(&invited_room); + let should_display = should_display_room!(self, &room_id, &invited_room); let _replaced = self.invited_rooms.borrow_mut().insert(room_id.clone(), invited_room); if let Some(_old_room) = _replaced { error!("BUG: Added invited room {room_id} that already existed"); @@ -486,7 +515,7 @@ impl RoomsList { RoomsListUpdate::AddJoinedRoom(joined_room) => { let room_id = joined_room.room_name_id.room_id().clone(); let is_direct = joined_room.is_direct; - let should_display = (self.display_filter)(&joined_room); + let should_display = should_display_room!(self, &room_id, &joined_room); let replaced = self.all_joined_rooms.insert(room_id.clone(), joined_room); if replaced.is_none() { if should_display { @@ -550,64 +579,60 @@ impl RoomsList { } } RoomsListUpdate::UpdateRoomName { new_room_name } => { + + // TODO: broadcast a new AppState action to ensure that this room's or space's new name + // gets updated in all of the `SelectedRoom` instances throughout Robrix, + // e.g., the name of the room in the Dock Tab or the StackNav header. + let room_id = new_room_name.room_id().clone(); // Try to update joined room first if let Some(room) = self.all_joined_rooms.get_mut(&room_id) { - let was_displayed = (self.display_filter)(room); - // Update with the new RoomName (preserves EmptyWas semantics) - room.room_name_id = new_room_name.clone(); - let should_display = (self.display_filter)(room); - match (was_displayed, should_display) { - // No need to update the displayed rooms list. - (true, true) | (false, false) => { } - // Room was displayed but should no longer be displayed. - (true, false) => { - if room.is_direct { - self.displayed_direct_rooms.iter() - .position(|r| r == &room_id) - .map(|index| self.displayed_direct_rooms.remove(index)); - } else { - self.displayed_regular_rooms.iter() - .position(|r| r == &room_id) - .map(|index| self.displayed_regular_rooms.remove(index)); - } - } - // Room was not displayed but should now be displayed. - (false, true) => { - if room.is_direct { - self.displayed_direct_rooms.push(room_id); - } else { - self.displayed_regular_rooms.push(room_id); - } + room.room_name_id = new_room_name; + let is_direct = room.is_direct; + let should_display = should_display_room!(self, &room_id, room); + let (pos_in_list, displayed_list) = if is_direct { + ( + self.displayed_direct_rooms.iter().position(|r| r == &room_id), + &mut self.displayed_direct_rooms, + ) + } else { + ( + self.displayed_regular_rooms.iter().position(|r| r == &room_id), + &mut self.displayed_regular_rooms, + ) + }; + if should_display { + if pos_in_list.is_none() { + displayed_list.push(room_id); } + } else { + pos_in_list.map(|i| displayed_list.remove(i)); } } // If not a joined room, try to update invited room else { let mut invited_rooms = self.invited_rooms.borrow_mut(); if let Some(invited_room) = invited_rooms.get_mut(&room_id) { - let was_displayed = (self.display_filter)(invited_room); invited_room.room_name_id = new_room_name; - let should_display = (self.display_filter)(invited_room); - match (was_displayed, should_display) { - (true, true) | (false, false) => { } - (true, false) => { - self.displayed_invited_rooms.iter() - .position(|r| r == &room_id) - .map(|index| self.displayed_invited_rooms.remove(index)); - } - (false, true) => { - self.displayed_invited_rooms.push(room_id.clone()); + let should_display = should_display_room!(self, &room_id, invited_room); + let pos_in_list = self.displayed_invited_rooms.iter() + .position(|r| r == &room_id); + if should_display { + if pos_in_list.is_none() { + self.displayed_invited_rooms.push(room_id); } + } else { + pos_in_list.map(|i| self.displayed_invited_rooms.remove(i)); } } else { - warning!("Warning: couldn't find invited room {} to update room name", room_id); + warning!("Warning: couldn't find room {new_room_name} to update its name."); } } } RoomsListUpdate::UpdateIsDirect { room_id, is_direct } => { if let Some(room) = self.all_joined_rooms.get_mut(&room_id) { - if room.is_direct == is_direct { + let was_direct = room.is_direct; + if was_direct == is_direct { continue; } enqueue_popup_notification(PopupItem { @@ -616,23 +641,23 @@ impl RoomsList { if room.is_direct { "direct" } else { "regular" }, if is_direct { "direct" } else { "regular" } ), - auto_dismissal_duration: None, + auto_dismissal_duration: Some(5.0), kind: PopupKind::Info, }); - // If the room was currently displayed, remove it from the proper list. - if (self.display_filter)(room) { - let list_to_remove_from = if room.is_direct { - &mut self.displayed_direct_rooms - } else { - &mut self.displayed_regular_rooms - }; - list_to_remove_from.iter() - .position(|r| r == &room_id) - .map(|index| list_to_remove_from.remove(index)); - } - // Update the room. If it should now be displayed, add it to the correct list. + + // Remove the room from the previous list (direct or regular). + let list_to_remove_from = if was_direct { + &mut self.displayed_direct_rooms + } else { + &mut self.displayed_regular_rooms + }; + list_to_remove_from.iter() + .position(|r| r == &room_id) + .map(|index| list_to_remove_from.remove(index)); + + // Update the room. If it should be displayed, add it to the proper list. room.is_direct = is_direct; - if (self.display_filter)(room) { + if should_display_room!(self, &room_id, room) { if is_direct { self.displayed_direct_rooms.push(room_id); } else { @@ -646,15 +671,14 @@ impl RoomsList { RoomsListUpdate::RemoveRoom { room_id, new_state: _ } => { if let Some(removed) = self.all_joined_rooms.remove(&room_id) { log!("Removed room {room_id} from the list of all joined rooms"); - if removed.is_direct { - self.displayed_direct_rooms.iter() - .position(|r| r == &room_id) - .map(|index| self.displayed_direct_rooms.remove(index)); + let list_to_remove_from = if removed.is_direct { + &mut self.displayed_direct_rooms } else { - self.displayed_regular_rooms.iter() - .position(|r| r == &room_id) - .map(|index| self.displayed_regular_rooms.remove(index)); - } + &mut self.displayed_regular_rooms + }; + list_to_remove_from.iter() + .position(|r| r == &room_id) + .map(|index| list_to_remove_from.remove(index)); } else if let Some(_removed) = self.invited_rooms.borrow_mut().remove(&room_id) { log!("Removed room {room_id} from the list of all invited rooms"); @@ -694,32 +718,26 @@ impl RoomsList { } RoomsListUpdate::TombstonedRoom { room_id } => { if let Some(room) = self.all_joined_rooms.get_mut(&room_id) { - let was_displayed = (self.display_filter)(room); room.is_tombstoned = true; - let should_display = (self.display_filter)(room); - match (was_displayed, should_display) { - // No need to update the displayed rooms list. - (true, true) | (false, false) => { } - // Room was displayed but should no longer be displayed. - (true, false) => { - if room.is_direct { - self.displayed_direct_rooms.iter() - .position(|r| r == &room_id) - .map(|index| self.displayed_direct_rooms.remove(index)); - } else { - self.displayed_regular_rooms.iter() - .position(|r| r == &room_id) - .map(|index| self.displayed_regular_rooms.remove(index)); - } - } - // Room was not displayed but should now be displayed. - (false, true) => { - if room.is_direct { - self.displayed_direct_rooms.push(room_id); - } else { - self.displayed_regular_rooms.push(room_id); - } + let is_direct = room.is_direct; + let should_display = should_display_room!(self, &room_id, room); + let (pos_in_list, displayed_list) = if is_direct { + ( + self.displayed_direct_rooms.iter().position(|r| r == &room_id), + &mut self.displayed_direct_rooms, + ) + } else { + ( + self.displayed_regular_rooms.iter().position(|r| r == &room_id), + &mut self.displayed_regular_rooms, + ) + }; + if should_display { + if pos_in_list.is_none() { + displayed_list.push(room_id); } + } else { + pos_in_list.map(|i| displayed_list.remove(i)); } } else { warning!("Warning: couldn't find room {room_id} to update the tombstone status"); @@ -727,15 +745,15 @@ impl RoomsList { } RoomsListUpdate::HideRoom { room_id } => { self.hidden_rooms.insert(room_id.clone()); + // Hiding a regular room is the most common case (e.g., after its successor is joined), + // so we check that list first. if let Some(i) = self.displayed_regular_rooms.iter().position(|r| r == &room_id) { self.displayed_regular_rooms.remove(i); - continue; } - if let Some(i) = self.displayed_direct_rooms.iter().position(|r| r == &room_id) { + else if let Some(i) = self.displayed_direct_rooms.iter().position(|r| r == &room_id) { self.displayed_direct_rooms.remove(i); - continue; } - if let Some(i) = self.displayed_invited_rooms.iter().position(|r| r == &room_id) { + else if let Some(i) = self.displayed_invited_rooms.iter().position(|r| r == &room_id) { self.displayed_invited_rooms.remove(i); continue; } @@ -791,69 +809,30 @@ impl RoomsList { self.status = text; } - /// Returns true if the given room is contained in any of the displayed room sets, - /// i.e., either the invited rooms or the joined rooms. - fn is_room_displayable(&self, room: &OwnedRoomId) -> bool { - self.displayed_invited_rooms.contains(room) - || self.displayed_direct_rooms.contains(room) - || self.displayed_regular_rooms.contains(room) - } - /// Updates and redraws the lists of displayed rooms in the RoomsList. /// /// This will update the display filter based on the current filter keywords /// and the currently-selected space (if any). fn update_displayed_rooms(&mut self, cx: &mut Cx) { - let portal_list = self.view.portal_list(ids!(list)); - if self.filter_keywords.is_empty() { - // Reset each of the displayed_* lists to show all rooms. - self.display_filter = RoomDisplayFilter::default(); - self.displayed_invited_rooms = self.invited_rooms.borrow() - .keys() - .filter(|&room_id| !self.hidden_rooms.contains(room_id) - && self.selected_space.as_ref() - .is_none_or(|space| self.is_room_indirectly_in_space(space.room_id(), room_id)) - ) - .cloned() - .collect(); - - self.displayed_direct_rooms.clear(); - self.displayed_regular_rooms.clear(); - for (room_id, jr) in &self.all_joined_rooms { - if !self.hidden_rooms.contains(room_id) - // If we have a selected space, only display rooms that are in that space. - && self.selected_space.as_ref() - .is_none_or(|space| self.is_room_indirectly_in_space(space.room_id(), room_id)) - { - if jr.is_direct { - self.displayed_direct_rooms.push(room_id.clone()); - } else { - self.displayed_regular_rooms.push(room_id.clone()); - } - } - } - } - // Generate the displayed rooms with a keyword filter applied. - else { - // Create a new filter function based on the given keywords - // and store it in this RoomsList such that we can apply it to newly-added rooms. - let (filter, sort_fn) = RoomDisplayFilterBuilder::new() + // Determine and set the filter function and sort function. + let (display_fn, sort_fn) = if self.filter_keywords.is_empty() { + (RoomDisplayFilter::default(), None) + } else { + // Create a new filter function based on the given keywords. + RoomDisplayFilterBuilder::new() .set_keywords(self.filter_keywords.clone()) .set_filter_criteria(RoomFilterCriteria::All) - .build(); - self.display_filter = filter; - - self.displayed_invited_rooms = self.generate_displayed_invited_rooms(sort_fn.as_deref()); - - let (new_displayed_regular_rooms, new_displayed_direct_rooms) = - self.generate_displayed_joined_rooms(sort_fn.as_deref()); + .build() + }; + self.display_filter = display_fn; - self.displayed_regular_rooms = new_displayed_regular_rooms; - self.displayed_direct_rooms = new_displayed_direct_rooms; - } + self.displayed_invited_rooms = self.generate_displayed_invited_rooms(sort_fn.as_deref()); + let (regular, direct) = self.generate_displayed_joined_rooms(sort_fn.as_deref()); + self.displayed_regular_rooms = regular; + self.displayed_direct_rooms = direct; self.update_status(); - portal_list.set_first_id_and_scroll(0, 0.0); + self.view.portal_list(ids!(list)).set_first_id_and_scroll(0, 0.0); self.redraw(cx); } @@ -863,13 +842,7 @@ impl RoomsList { let invited_rooms_ref = self.invited_rooms.borrow(); let filtered_invited_rooms_iter = invited_rooms_ref .iter() - .filter(|&(room_id, room)| - !self.hidden_rooms.contains(room_id) - && (self.display_filter)(room) - // If we have a selected space, only display rooms that are in that space. - && self.selected_space.as_ref() - .is_none_or(|space| self.is_room_indirectly_in_space(space.room_id(), room_id)) - ); + .filter(|&(room_id, room)| should_display_room!(self, room_id, room)); if let Some(sort_fn) = sort_fn { let mut filtered_invited_rooms = filtered_invited_rooms_iter @@ -877,14 +850,17 @@ impl RoomsList { filtered_invited_rooms.sort_by(|(_, room_a), (_, room_b)| sort_fn(*room_a, *room_b)); filtered_invited_rooms .into_iter() - .map(|(room_id, _)| room_id.clone()).collect() + .map(|(room_id, _)| room_id.clone()) + .collect() } else { - filtered_invited_rooms_iter.map(|(room_id, _)| room_id.clone()).collect() + filtered_invited_rooms_iter + .map(|(room_id, _)| room_id.clone()) + .collect() } } - /// Generates the lists of displayed direct rooms and displayed regular rooms - /// based on the current filter and the given sort function. + /// Generates a tuple of (displayed regular rooms, displayed direct rooms) + /// based on the current `display_filter` function and the given sort function. fn generate_displayed_joined_rooms(&self, sort_fn: Option<&SortFn>) -> (Vec, Vec) { let mut new_displayed_regular_rooms = Vec::new(); let mut new_displayed_direct_rooms = Vec::new(); @@ -899,13 +875,7 @@ impl RoomsList { let filtered_joined_rooms_iter = self.all_joined_rooms .iter() - .filter(|&(room_id, room)| - !self.hidden_rooms.contains(room_id) - && (self.display_filter)(room) - // If we have a selected space, only display rooms that are in that space. - && self.selected_space.as_ref() - .is_none_or(|space| self.is_room_indirectly_in_space(space.room_id(), room_id)) - ); + .filter(|&(room_id, room)| should_display_room!(self, room_id, room)); if let Some(sort_fn) = sort_fn { let mut filtered_rooms = filtered_joined_rooms_iter.collect::>(); @@ -1232,8 +1202,7 @@ impl Widget for RoomsList { let app_state = scope.data.get_mut::().unwrap(); // Update the currently-selected room from the AppState data. self.current_active_room = app_state.selected_room.as_ref() - .map(|sel_room| sel_room.room_id().clone()) - .filter(|room_id| self.is_room_displayable(room_id)); + .map(|sel_room| sel_room.room_id().clone()); // Based on the various displayed room lists and is_expanded state of each room header, // calculate the indices in the PortalList where the headers and rooms should be drawn. diff --git a/src/room/room_display_filter.rs b/src/room/room_display_filter.rs index 67b67beb..7c0b13e9 100644 --- a/src/room/room_display_filter.rs +++ b/src/room/room_display_filter.rs @@ -134,8 +134,11 @@ impl FilterableRoom for JoinedSpaceInfo { pub type RoomFilterFn = dyn Fn(&dyn FilterableRoom) -> bool; pub type SortFn = dyn Fn(&dyn FilterableRoom, &dyn FilterableRoom) -> Ordering; +fn default_room_filter_fn(_: &dyn FilterableRoom) -> bool { + true +} -/// A filter function that is called for each room to determine whether it should be displayed. +/// A filter function that determines whether a given room should be displayed. /// /// If the function returns `true`, the room is displayed; otherwise, it is not shown. /// The default value is a filter function that always returns `true`. @@ -152,16 +155,16 @@ pub type SortFn = dyn Fn(&dyn FilterableRoom, &dyn FilterableRoom) -> Ordering; /// .collect(); /// // Then redraw the rooms_list widget. /// ``` -pub struct RoomDisplayFilter(Box); -impl Default for RoomDisplayFilter { - fn default() -> Self { - RoomDisplayFilter(Box::new(|_| true)) - } -} +#[derive(Default)] +pub struct RoomDisplayFilter(Option>); impl Deref for RoomDisplayFilter { - type Target = Box; + type Target = RoomFilterFn; fn deref(&self) -> &Self::Target { - &self.0 + if let Some(rdf) = &self.0 { + rdf.deref() + } else { + &default_room_filter_fn + } } } @@ -347,14 +350,16 @@ impl RoomDisplayFilterBuilder { let keywords = self.keywords; let filter_criteria = self.filter_criteria; - let filter = RoomDisplayFilter(Box::new(move |room| { - if keywords.is_empty() || filter_criteria.is_empty() { - return true; - } - let keywords = keywords.trim().to_lowercase(); - Self::matches_filter(room, &keywords, self.filter_criteria) - })); - + let filter = if keywords.is_empty() || filter_criteria.is_empty() { + RoomDisplayFilter::default() + } else { + RoomDisplayFilter(Some(Box::new( + move |room| { + let keywords = keywords.trim().to_lowercase(); + Self::matches_filter(room, &keywords, self.filter_criteria) + } + ))) + }; (filter, self.sort_fn) } } diff --git a/src/shared/room_filter_input_bar.rs b/src/shared/room_filter_input_bar.rs index 0c64e618..c47c145f 100644 --- a/src/shared/room_filter_input_bar.rs +++ b/src/shared/room_filter_input_bar.rs @@ -105,6 +105,13 @@ impl WidgetMatchEvent for RoomFilterInputBar { // Handle user changing the input text if let Some(keywords) = input.changed(actions) { + // Trim whitespace, and only alloc a new string if it was trimmed. + let keywords_trimmed = keywords.trim(); + let keywords = if keywords_trimmed.len() < keywords.len() { + keywords_trimmed.to_string() + } else { + keywords + }; clear_button.set_visible(cx, !keywords.is_empty()); clear_button.reset_hover(cx); cx.widget_action(