Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 99 additions & 25 deletions src/home/room_screen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,15 @@ use matrix_sdk_ui::timeline::{
};

use crate::{
app::RoomsPanelRestoreAction, avatar_cache, event_preview::{plaintext_body_of_timeline_item, text_preview_of_encrypted_message, text_preview_of_member_profile_change, text_preview_of_other_state, text_preview_of_redacted_message, text_preview_of_room_membership_change, text_preview_of_timeline_item}, home::{edited_indicator::EditedIndicatorWidgetRefExt, editing_pane::EditingPaneState, loading_pane::{LoadingPaneState, LoadingPaneWidgetExt}, rooms_list::RoomsListRef}, location::init_location_subscriber, media_cache::{MediaCache, MediaCacheEntry}, profile::{
app::{RoomsPanelRestoreAction, SelectedRoom}, avatar_cache, event_preview::{plaintext_body_of_timeline_item, text_preview_of_encrypted_message, text_preview_of_member_profile_change, text_preview_of_other_state, text_preview_of_redacted_message, text_preview_of_room_membership_change, text_preview_of_timeline_item}, home::{edited_indicator::EditedIndicatorWidgetRefExt, editing_pane::EditingPaneState, loading_pane::{LoadingPaneState, LoadingPaneWidgetExt}, rooms_list::{RoomsListAction, RoomsListRef}}, location::init_location_subscriber, media_cache::{MediaCache, MediaCacheEntry}, profile::{
user_profile::{AvatarState, ShowUserProfileAction, UserProfile, UserProfileAndRoomId, UserProfilePaneInfo, UserProfileSlidingPaneRef, UserProfileSlidingPaneWidgetExt},
user_profile_cache,
}, shared::{
avatar::AvatarWidgetRefExt, callout_tooltip::TooltipAction, html_or_plaintext::{HtmlOrPlaintextRef, HtmlOrPlaintextWidgetRefExt, RobrixHtmlLinkAction}, jump_to_bottom_button::{JumpToBottomButtonWidgetExt, UnreadMessageCount}, popup_list::{enqueue_popup_notification, PopupItem}, styles::COLOR_DANGER_RED, text_or_image::{TextOrImageRef, TextOrImageWidgetRefExt}, timestamp::TimestampWidgetRefExt, typing_animation::TypingAnimationWidgetExt
}, sliding_sync::{get_client, submit_async_request, take_timeline_endpoints, BackwardsPaginateUntilEventRequest, MatrixRequest, PaginationDirection, TimelineRequestSender, UserPowerLevels}, utils::{self, room_name_or_id, unix_time_millis_to_datetime, ImageFormat, MEDIA_THUMBNAIL_FORMAT}
};
use crate::home::event_reaction_list::ReactionListWidgetRefExt;
use crate::room::ResolveRoomAliasAction;
use crate::home::room_read_receipt::AvatarRowWidgetRefExt;
use crate::room::room_input_bar::RoomInputBarWidgetExt;
use crate::shared::mentionable_text_input::MentionableTextInputWidgetRefExt;
Expand Down Expand Up @@ -995,6 +996,43 @@ impl Widget for RoomScreen {
);
}
}

// Handle resolved room alias actions - only for requests from this widget
if let Some(ResolveRoomAliasAction::Resolved { requester_uid, room_alias: _ , room_id, servers: _ }) = action.downcast_ref() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should use a match statement for all ResolveRoomAliasActions, not multiple separate if let blocks.

// Only handle this action if it was requested by this widget
if *requester_uid == room_screen_widget_uid {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while it seems like a good idea to use the RoomScreen widget UID for this, we should use the Room ID instead. The reason for this is because if the user changes from Desktop to Mobile view (or simply closes the RoomScreen tab and opens it in a new tab), then the widget UID will change, so this condition will never occur.

We want any RoomScreen that is showing this room's Room ID to be able to handle this resolved room alias action.

if let Some(known_room) = get_client().and_then(|c| c.get_room(room_id)) {
if known_room.is_space() {
enqueue_popup_notification(PopupItem {
message: format!("Found space {} but it is a space, not a regular room.", room_id),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine, but the real reason that we can't do anything here is because we don't yet support showing a preview of a Space itself.

Suggested change
message: format!("Found space {} but it is a space, not a regular room.", room_id),
message: String::from("Showing a space's home page is not yet supported."),

auto_dismissal_duration: Some(3.0)
});
} else {
cx.widget_action(room_screen_widget_uid, &scope.path, RoomsListAction::Selected(
SelectedRoom::JoinedRoom {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are you assuming this is a joined room?

room_id: room_id.clone().into(),
room_name: known_room.name()
}
));
}
} else {
enqueue_popup_notification(PopupItem {
message: format!("Found room {} but you are not joined to it yet.", room_id),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i dont understand this error condition. Why is it a problem that the room hasn't been joined yet? Can't you request a preview of non-joined rooms?

(Note that my comment that you removed also mentioned this. I believe that Client::get_room_preview() will handle all of the cases here.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only conditions we'd want to notify the user of are:

  • an error if we cannot fetch the room preview
  • a notice if the room exists but cannot be previewed and must be joined instead

auto_dismissal_duration: Some(3.0)
});
}
}
}

if let Some(ResolveRoomAliasAction::Failed { requester_uid, room_alias, error }) = action.downcast_ref() {
if *requester_uid == room_screen_widget_uid {
error!("Failed to resolve room alias {}: {:?}", room_alias, error);
enqueue_popup_notification(PopupItem {
message: format!("Could not find room with alias: {}", room_alias),
auto_dismissal_duration: None
});
}
}
}

/*
Expand Down Expand Up @@ -1731,6 +1769,7 @@ impl RoomScreen {
action: &Action,
pane: &UserProfileSlidingPaneRef,
) -> bool {
let uid = self.widget_uid();
// A closure that handles both MatrixToUri and MatrixUri links,
// and returns whether the link was handled.
let mut handle_matrix_link = |id: &MatrixId, _via: &[OwnedServerName]| -> bool {
Expand Down Expand Up @@ -1763,26 +1802,59 @@ impl RoomScreen {
}
MatrixId::Room(room_id) => {
if self.room_id.as_ref() == Some(room_id) {
enqueue_popup_notification(PopupItem {
message: "You are already viewing that room.".into(),
auto_dismissal_duration: None
enqueue_popup_notification(PopupItem {
message: "You are already viewing that room.".into(),
auto_dismissal_duration: Some(3.0)
});
return true;
}
if let Some(_known_room) = get_client().and_then(|c| c.get_room(room_id)) {
log!("TODO: jump to known room {}", room_id);
if let Some(known_room) = get_client().and_then(|c| c.get_room(room_id)) {
cx.widget_action(uid, &Scope::empty().path, RoomsListAction::Selected(
SelectedRoom::JoinedRoom {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how do you know the room is a Joined room here? what if it's an invite?

room_id: room_id.clone().into(),
room_name: known_room.name()
}
));
} else {
log!("TODO: fetch and display room preview for room {}", room_id);
// TODO: fetch and display a room preview for the given room ID.
enqueue_popup_notification(PopupItem {
message: "You are not joined to this room yet.".into(),
auto_dismissal_duration: Some(3.0)
});
}
false
true
}
MatrixId::RoomAlias(room_alias) => {
log!("TODO: open room alias {}", room_alias);
// TODO: open a room loading screen that shows a spinner
// while our background async task calls Client::resolve_room_alias()
// and then either jumps to the room if known, or fetches and displays
// a room preview for that room.
false
// Check if we already have this room alias in the client
if let Some(known_room) = get_client().and_then(|c| {
c.rooms().into_iter().find(|room| {
room.canonical_alias().as_ref() == Some(room_alias) ||
room.alt_aliases().contains(room_alias)
})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is very expensive --- we cannot iterate over all rooms in the client on the main UI thread.

You deleted my comments, but those comments clearly stated what we needed to do. We do need to show a loading screen (like how I use the LoadingPane widget when looking for an old replied-to message) while waiting on a background task to resolve the room alias. This is not optional, and I left the same comment on your previous PR on this topic.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recall that in general, we want to avoid doing as much work as possible on the main UI thread, and move as many things as possible to a background task/thread.

}) {
let room_id = known_room.room_id();
if self.room_id.as_ref() == Some(&room_id.to_owned()) {
enqueue_popup_notification(PopupItem {
message: "You are already viewing that room.".into(),
auto_dismissal_duration: Some(3.0)
});
} else {
cx.widget_action(uid, &Scope::empty().path, RoomsListAction::Selected(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're assuming this room has been joined again.

SelectedRoom::JoinedRoom {
room_id: room_id.to_owned().into(),
room_name: known_room.name()
}
));
}
return true;
}
// TODO: open a room loading screen that shows a spinner.
// Submit async request to resolve the room alias
submit_async_request(MatrixRequest::ResolveRoomAlias {
room_alias: room_alias.clone(),
requester_uid: uid,
});
true
}
MatrixId::Event(room_id, event_id) => {
log!("TODO: open event {} in room {}", event_id, room_id);
Expand All @@ -1795,15 +1867,10 @@ impl RoomScreen {
}
};

if let HtmlLinkAction::Clicked { url, .. } = action.as_widget_action().cast() {
let mut link_was_handled = false;
if let Ok(matrix_to_uri) = MatrixToUri::parse(&url) {
link_was_handled |= handle_matrix_link(matrix_to_uri.id(), matrix_to_uri.via());
}
else if let Ok(matrix_uri) = MatrixUri::parse(&url) {
link_was_handled |= handle_matrix_link(matrix_uri.id(), matrix_uri.via());
}

// Prioritize handling RobrixHtmlLinkAction::ClickedMatrixLink over HtmlLinkAction::Clicked
// to avoid duplicate processing of the same Matrix link
if let RobrixHtmlLinkAction::ClickedMatrixLink { url, matrix_id, via, .. } = action.as_widget_action().cast() {
let link_was_handled = handle_matrix_link(&matrix_id, &via);
if !link_was_handled {
log!("Opening URL \"{}\"", url);
if let Err(e) = robius_open::Uri::new(&url).open() {
Expand All @@ -1816,8 +1883,15 @@ impl RoomScreen {
}
true
}
else if let RobrixHtmlLinkAction::ClickedMatrixLink { url, matrix_id, via, .. } = action.as_widget_action().cast() {
let link_was_handled = handle_matrix_link(&matrix_id, &via);
else if let HtmlLinkAction::Clicked { url, .. } = action.as_widget_action().cast() {
let mut link_was_handled = false;
if let Ok(matrix_to_uri) = MatrixToUri::parse(&url) {
link_was_handled |= handle_matrix_link(matrix_to_uri.id(), matrix_to_uri.via());
}
else if let Ok(matrix_uri) = MatrixUri::parse(&url) {
link_was_handled |= handle_matrix_link(matrix_uri.id(), matrix_uri.via());
}

if !link_was_handled {
log!("Opening URL \"{}\"", url);
if let Err(e) = robius_open::Uri::new(&url).open() {
Expand Down
20 changes: 18 additions & 2 deletions src/room/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::sync::Arc;
use makepad_widgets::Cx;
use matrix_sdk::ruma::OwnedRoomId;
use makepad_widgets::{Cx, WidgetUid};
use matrix_sdk::{ruma::{OwnedRoomAliasId, OwnedRoomId}, OwnedServerName};

pub mod room_input_bar;
pub mod room_member_manager;
Expand All @@ -10,6 +10,22 @@ pub fn live_design(cx: &mut Cx) {
room_input_bar::live_design(cx)
}

/// Actions sent from the backend task as a result of a [`MatrixRequest::ResolveRoomAlias`].
#[derive(Debug)]
pub enum ResolveRoomAliasAction {
Resolved {
requester_uid: WidgetUid,
room_alias: OwnedRoomAliasId,
room_id: OwnedRoomId,
servers: Vec<OwnedServerName>,
},
Failed {
requester_uid: WidgetUid,
room_alias: OwnedRoomAliasId,
error: matrix_sdk::Error,
}
}

/// Basic details about a room, used for displaying a preview of it.
#[derive(Clone, Debug)]
pub struct BasicRoomDetails {
Expand Down
32 changes: 25 additions & 7 deletions src/sliding_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use eyeball::Subscriber;
use eyeball_im::VectorDiff;
use futures_util::{pin_mut, StreamExt};
use imbl::Vector;
use makepad_widgets::{error, log, warning, Cx, SignalToUI};
use makepad_widgets::{error, log, warning, Cx, SignalToUI, WidgetUid};
use matrix_sdk::{
config::RequestConfig, encryption::EncryptionSettings, event_handler::EventHandlerDropGuard, media::MediaRequestParameters, room::{edit::EditedContent, reply::Reply, RoomMember}, ruma::{
api::client::receipt::create_receipt::v3::ReceiptType, events::{
Expand Down Expand Up @@ -33,7 +33,7 @@ use crate::{
}, login::login_screen::LoginAction, media_cache::{MediaCacheEntry, MediaCacheEntryRef}, persistent_state::{self, load_rooms_panel_state, ClientSessionPersisted}, profile::{
user_profile::{AvatarState, UserProfile},
user_profile_cache::{enqueue_user_profile_update, UserProfileUpdate},
}, room::RoomPreviewAvatar, shared::{html_or_plaintext::MatrixLinkPillState, jump_to_bottom_button::UnreadMessageCount, popup_list::{enqueue_popup_notification, PopupItem}}, utils::{self, AVATAR_THUMBNAIL_FORMAT}, verification::add_verification_event_handlers_and_sync_client
}, room::{ResolveRoomAliasAction, RoomPreviewAvatar}, shared::{html_or_plaintext::MatrixLinkPillState, jump_to_bottom_button::UnreadMessageCount, popup_list::{enqueue_popup_notification, PopupItem}}, utils::{self, AVATAR_THUMBNAIL_FORMAT}, verification::add_verification_event_handlers_and_sync_client
};

#[derive(Parser, Debug, Default)]
Expand Down Expand Up @@ -295,7 +295,10 @@ pub enum MatrixRequest {
room_id: OwnedRoomId,
},
/// Request to resolve a room alias into a room ID and the servers that know about that room.
ResolveRoomAlias(OwnedRoomAliasId),
ResolveRoomAlias {
requester_uid: WidgetUid,
room_alias: OwnedRoomAliasId,
},
/// Request to fetch an Avatar image from the server.
/// Upon completion of the async media request, the `on_fetched` function
/// will be invoked with the content of an `AvatarUpdate`.
Expand Down Expand Up @@ -898,13 +901,28 @@ async fn async_worker(
MatrixRequest::SpawnSSOServer { brand, homeserver_url, identity_provider_id} => {
spawn_sso_server(brand, homeserver_url, identity_provider_id, login_sender.clone()).await;
}
MatrixRequest::ResolveRoomAlias(room_alias) => {
MatrixRequest::ResolveRoomAlias { room_alias, requester_uid } => {
let Some(client) = CLIENT.get() else { continue };
let _resolve_task = Handle::current().spawn(async move {
log!("Sending resolve room alias request for {room_alias}...");
let res = client.resolve_room_alias(&room_alias).await;
log!("Resolved room alias {room_alias} to: {res:?}");
todo!("Send the resolved room alias back to the UI thread somehow.");
let result_action = match client.resolve_room_alias(&room_alias).await {
Ok(response) => {
ResolveRoomAliasAction::Resolved {
requester_uid,
room_alias,
room_id: response.room_id,
servers: response.servers
}
}
Err(e) => {
ResolveRoomAliasAction::Failed {
requester_uid,
room_alias,
error: e.into()
}
}
};
Cx::post_action(result_action);
});
}
MatrixRequest::FetchAvatar { mxc_uri, on_fetched } => {
Expand Down