From 784e2ac781a7aa0950a4989ccf25215a07c768de Mon Sep 17 00:00:00 2001 From: Kevin Boos Date: Tue, 26 May 2026 17:44:36 -0700 Subject: [PATCH 1/6] Support downloading media attachements, e.g., files, photos, videos For images that have a preview (PNG/JPG), we show the download button within the ImageViewer widget itself. For any message that we cannot show a preview of, we show a download button beneath that message's content. --- resources/icons/download.svg | 5 + src/home/room_screen.rs | 318 +++++++++++++++++++++++++----- src/media_cache.rs | 4 + src/room/room_input_bar.rs | 93 +++++---- src/shared/attachment_download.rs | 133 +++++++++++++ src/shared/image_viewer.rs | 38 +++- src/shared/mod.rs | 1 + src/shared/styles.rs | 1 + src/sliding_sync.rs | 96 +++++++-- 9 files changed, 569 insertions(+), 120 deletions(-) create mode 100644 resources/icons/download.svg create mode 100644 src/shared/attachment_download.rs diff --git a/resources/icons/download.svg b/resources/icons/download.svg new file mode 100644 index 000000000..4d32ae4ba --- /dev/null +++ b/resources/icons/download.svg @@ -0,0 +1,5 @@ + + + + + diff --git a/src/home/room_screen.rs b/src/home/room_screen.rs index 66307ce3b..0b9e70210 100644 --- a/src/home/room_screen.rs +++ b/src/home/room_screen.rs @@ -32,7 +32,7 @@ use crate::{ }, room::{BasicRoomDetails, room_input_bar::{RoomInputBarState, RoomInputBarWidgetRefExt}, typing_notice::TypingNoticeWidgetExt}, shared::{ - avatar::{AvatarState, AvatarWidgetRefExt}, confirmation_modal::ConfirmationModalContent, file_upload_modal::FileUploadAttemptId, html_or_plaintext::{HtmlOrPlaintextRef, HtmlOrPlaintextWidgetRefExt, RobrixHtmlLinkAction}, image_viewer::{ImageViewerAction, ImageViewerMetaData, LoadState}, jump_to_bottom_button::{JumpToBottomButtonWidgetExt, UnreadMessageCount}, popup_list::{PopupKind, enqueue_popup_notification}, restore_status_view::RestoreStatusViewWidgetExt, styles::*, text_or_image::{TextOrImageAction, TextOrImageRef, TextOrImageWidgetRefExt}, timestamp::TimestampWidgetRefExt + attachment_download::{DownloadKind, DownloadableAttachment, start_attachment_download}, avatar::{AvatarState, AvatarWidgetRefExt}, confirmation_modal::ConfirmationModalContent, file_upload_modal::FileUploadAttemptId, html_or_plaintext::{HtmlOrPlaintextRef, HtmlOrPlaintextWidgetRefExt, RobrixHtmlLinkAction}, image_viewer::{ImageViewerAction, ImageViewerMetaData, LoadState}, jump_to_bottom_button::{JumpToBottomButtonWidgetExt, UnreadMessageCount}, popup_list::{PopupKind, enqueue_popup_notification}, restore_status_view::RestoreStatusViewWidgetExt, styles::*, text_or_image::{TextOrImageAction, TextOrImageRef, TextOrImageStatus, TextOrImageWidgetRefExt}, timestamp::TimestampWidgetRefExt }, sliding_sync::{BackwardsPaginateUntilEventRequest, MatrixRequest, PaginationDirection, TimelineEndpoints, TimelineKind, TimelineRequestSender, UserPowerLevels, get_client, submit_async_request, take_timeline_endpoints}, utils::{self, ImageFormat, MEDIA_THUMBNAIL_FORMAT, RoomNameId, unix_time_millis_to_datetime} }; @@ -87,6 +87,49 @@ script_mod! { // An empty view that takes up no space in the portal list. mod.widgets.Empty = View { } + // Shown beneath media messages. The whole section is hidden for non-media + // ones; for media, one of the two children is visible at a time + // (button vs spinner), driven from `Message::set_data`. + mod.widgets.MessageDownloadSection = View { + visible: false, + width: Fit, height: Fit, + flow: Right, + margin: Inset{top: 8, bottom: 2} + + download_button := RobrixIconButton { + height: mod.widgets.SETTINGS_BUTTON_HEIGHT, + padding: Inset{left: 12, right: 12} + margin: 0 + draw_icon.svg: (ICON_DOWNLOAD) + icon_walk: Walk{width: 16, height: 16} + text: "Download" + } + + downloading_view := View { + visible: false, + width: Fit, height: mod.widgets.SETTINGS_BUTTON_HEIGHT + flow: Right, + align: Align{y: 0.5} + spacing: 8, + padding: Inset{left: 12, right: 12} + + spinner := LoadingSpinner { + width: 16, height: 16 + draw_bg.color: (COLOR_ACTIVE_PRIMARY) + } + status_label := Label { + width: Fit, height: Fit, + padding: 0 + margin: 0 + draw_text +: { + text_style: REGULAR_TEXT { font_size: 11 }, + color: (COLOR_ACTIVE_PRIMARY) + } + text: "Downloading…" + } + } + } + // A summary at the bottom of a message that is the root of a thread. mod.widgets.ThreadRootSummary = RoundedView { visible: false @@ -270,6 +313,7 @@ script_mod! { message := HtmlOrPlaintext { } link_preview_view := mod.widgets.LinkPreview {} + download_section := mod.widgets.MessageDownloadSection {} View { width: Fill, height: Fit @@ -315,6 +359,7 @@ script_mod! { message := HtmlOrPlaintext { } link_preview_view := mod.widgets.LinkPreview {} + download_section := mod.widgets.MessageDownloadSection {} View { width: Fill, height: Fit @@ -358,6 +403,7 @@ script_mod! { height: (mod.widgets.IMG_MSG_FIT) } } } + download_section := mod.widgets.MessageDownloadSection {} View { width: Fill, height: Fit, @@ -385,6 +431,7 @@ script_mod! { height: (mod.widgets.IMG_MSG_FIT) } } } + download_section := mod.widgets.MessageDownloadSection {} View { width: Fill, height: Fit, @@ -1166,6 +1213,7 @@ impl Widget for RoomScreen { &mut tl_state.pending_thread_summary_fetches, &tl_state.user_power, &self.pinned_events, + &tl_state.pending_downloads, item_drawn_status, room_screen_widget_uid, ) @@ -1657,6 +1705,10 @@ impl RoomScreen { self.view.room_input_bar(cx, ids!(room_input_bar)) .hide_upload_progress(cx, upload_id); } + TimelineUpdate::AttachmentDownloadFinished(mxc) => { + tl.pending_downloads.retain(|p| p != &mxc); + portal_list.redraw(cx); + } } } @@ -1824,6 +1876,12 @@ impl RoomScreen { let timestamp_millis = event_tl_item.timestamp(); let (image_name, image_file_size) = get_image_name_and_filesize(event_tl_item); + let downloadable = Some(DownloadableAttachment { + media_source: media_source.clone(), + filename: image_name.clone(), + size: (image_file_size > 0).then_some(image_file_size), + kind: DownloadKind::Image, + }); cx.action(ImageViewerAction::Show(LoadState::Loading( texture.clone(), Some(ImageViewerMetaData { @@ -1834,6 +1892,7 @@ impl RoomScreen { tl_state.kind.clone(), event_tl_item.clone(), )), + downloadable, }), ))); @@ -2145,6 +2204,23 @@ impl RoomScreen { // // TODO // } + MessageAction::DownloadAttachment(info) => { + let Some(tl) = self.tl_state.as_mut() else { continue }; + let mxc = match &info.media_source { + MediaSource::Plain(uri) => uri, + MediaSource::Encrypted(file) => &file.url, + }; + // Mark pending synchronously so the next draw hides the + // button before the user can click it again. Skip if it's + // already in flight. + if tl.pending_downloads.iter().any(|p| p == mxc) { + continue; + } + tl.pending_downloads.push(mxc.clone()); + portal_list.redraw(cx); + let update_sender = tl.media_cache.timeline_update_sender().cloned(); + start_attachment_download(info.clone(), update_sender); + } // This is handled within the Message widget itself. MessageAction::HighlightMessage(..) => { } // This is handled by the top-level App itself. @@ -2325,6 +2401,7 @@ impl RoomScreen { scrolled_past_read_marker: false, latest_own_user_receipt: None, tombstone_info, + pending_downloads: SmallVec::new(), }; timeline_state_store::mark_taken(&tl_state.kind, owner); (tl_state, true) @@ -2856,6 +2933,9 @@ pub enum TimelineUpdate { FileUploadComplete { upload_id: FileUploadAttemptId, }, + /// An inline-button download is done (or failed). `RoomScreen` removes + /// the mxc from `pending_downloads` so the spinner reverts to the button. + AttachmentDownloadFinished(OwnedMxcUri), } /// Stores timeline UI state that is not currently owned by a `RoomScreen`. @@ -3065,6 +3145,10 @@ struct TimelineUiState { /// If `Some`, this room has been tombstoned and the details of its successor room /// are contained within. If `None`, the room has not been tombstoned. tombstone_info: Option, + + /// Media/file attachments in this timeline currently being downloaded. + /// the inline button's spinner state. + pending_downloads: SmallVec<[OwnedMxcUri; 1]>, } #[derive(Default, Debug)] @@ -3195,6 +3279,7 @@ fn populate_message_view( pending_thread_summary_fetches: &mut HashSet, user_power_levels: &UserPowerLevels, pinned_events: &[OwnedEventId], + pending_downloads: &[OwnedMxcUri], item_drawn_status: ItemDrawnStatus, room_screen_widget_uid: WidgetUid, ) -> (WidgetRef, ItemDrawnStatus) { @@ -3222,10 +3307,12 @@ fn populate_message_view( let has_html_body: bool; - // Sometimes we need to call this up-front, so we save the result in this variable - // to avoid having to call it twice. + // Sometimes we need to get the username/avatar up-front, + // so we save that here to avoid calling the function twice. let mut set_username_and_get_avatar_retval = None; let mut has_room_mention = false; + let mut download_info: Option = None; + let (item, used_cached_item) = match &msg_like_content.kind { MsgLikeKind::Message(msg) => { let room_mention_room_id = if msg.mentions().is_some_and(|m| m.room) { @@ -3420,22 +3507,34 @@ fn populate_message_view( id!(ImageMessage) }; let (item, existed) = list.item_with_existed(cx, item_id, template); - if existed && item_drawn_status.content_drawn { - (item, true) + let was_cached = existed && item_drawn_status.content_drawn; + let text_or_image_ref = item.text_or_image(cx, ids!(content.message)); + let fallback = if was_cached { + // Cached path re-reads the status the widget already has. + matches!(text_or_image_ref.status(), TextOrImageStatus::Text) + .then(|| DownloadableAttachment { + media_source: image.source.clone(), + filename: image.filename().to_owned(), + size: image.info.as_ref().and_then(|i| i.size).map(u64::from), + kind: DownloadKind::Image, + }) } else { - let image_info = image.info.clone(); - let text_or_image_ref = item.text_or_image(cx, ids!(content.message)); - let is_image_fully_drawn = populate_image_message_content( + let (is_image_fully_drawn, fallback) = populate_image_message_content_with_fallback( cx, &text_or_image_ref, - image_info, + image.info.clone(), image.source.clone(), msg.body(), media_cache, + image.filename().to_owned(), + image.info.as_ref().and_then(|i| i.size).map(u64::from), + DownloadKind::Image, ); new_drawn_status.content_drawn = is_image_fully_drawn; - (item, false) - } + fallback + }; + download_info = fallback; + (item, was_cached) } MessageType::Location(location) => { has_html_body = false; @@ -3461,6 +3560,12 @@ fn populate_message_view( } MessageType::File(file_content) => { has_html_body = file_content.formatted.as_ref().is_some_and(|f| f.format == MessageFormat::Html); + download_info = Some(DownloadableAttachment { + media_source: file_content.source.clone(), + filename: file_content.filename().to_owned(), + size: file_content.info.as_ref().and_then(|i| i.size).map(u64::from), + kind: DownloadKind::File, + }); let template = if use_compact_view { id!(CondensedMessage) } else { @@ -3482,6 +3587,12 @@ fn populate_message_view( } MessageType::Audio(audio) => { has_html_body = audio.formatted.as_ref().is_some_and(|f| f.format == MessageFormat::Html); + download_info = Some(DownloadableAttachment { + media_source: audio.source.clone(), + filename: audio.filename().to_owned(), + size: audio.info.as_ref().and_then(|i| i.size).map(u64::from), + kind: DownloadKind::Audio, + }); let template = if use_compact_view { id!(CondensedMessage) } else { @@ -3503,6 +3614,12 @@ fn populate_message_view( } MessageType::Video(video) => { has_html_body = video.formatted.as_ref().is_some_and(|f| f.format == MessageFormat::Html); + download_info = Some(DownloadableAttachment { + media_source: video.source.clone(), + filename: video.filename().to_owned(), + size: video.info.as_ref().and_then(|i| i.size).map(u64::from), + kind: DownloadKind::Video, + }); let template = if use_compact_view { id!(CondensedMessage) } else { @@ -3580,35 +3697,59 @@ fn populate_message_view( MsgLikeKind::Sticker(sticker) => { has_html_body = false; let StickerEventContent { body, info, source, .. } = sticker.content(); - let template = if use_compact_view { id!(CondensedImageMessage) } else { id!(ImageMessage) }; let (item, existed) = list.item_with_existed(cx, item_id, template); - - if existed && item_drawn_status.content_drawn { - (item, true) - } else { - if let StickerMediaSource::Plain(owned_mxc_url) = source { - let image_info = info; - let text_or_image_ref = item.text_or_image(cx, ids!(content.message)); - let is_image_fully_drawn = populate_image_message_content( - cx, - &text_or_image_ref, - Some(Box::new(image_info.clone())), - MediaSource::Plain(owned_mxc_url.clone()), - body, - media_cache, - ); - new_drawn_status.content_drawn = is_image_fully_drawn; - (item, false) - } else { - (item, true) + let was_cached = existed && item_drawn_status.content_drawn; + + let text_or_image_ref = item.text_or_image(cx, ids!(content.message)); + match source { + StickerMediaSource::Plain(owned_mxc_url) => { + let filename = if body.is_empty() { "sticker".to_owned() } else { body.clone() }; + let size = info.size.map(u64::from); + download_info = if was_cached { + matches!(text_or_image_ref.status(), TextOrImageStatus::Text) + .then(|| DownloadableAttachment { + media_source: MediaSource::Plain(owned_mxc_url.clone()), + filename, + size, + kind: DownloadKind::Image, + }) + } else { + let (is_image_fully_drawn, fallback) = populate_image_message_content_with_fallback( + cx, + &text_or_image_ref, + Some(Box::new(info.clone())), + MediaSource::Plain(owned_mxc_url.clone()), + body, + media_cache, + filename, + size, + DownloadKind::Image, + ); + new_drawn_status.content_drawn = is_image_fully_drawn; + fallback + }; + } + // Encrypted sticker decryption isn't wired up yet; show a + // placeholder so the message doesn't render blank. + _ => { + if !was_cached { + let label = if body.is_empty() { + "[Encrypted sticker]".to_owned() + } else { + format!("[Encrypted sticker: {body}]") + }; + text_or_image_ref.show_text(cx, label); + new_drawn_status.content_drawn = true; + } } } - } + (item, was_cached) + } // Handle messages that have been redacted (deleted). MsgLikeKind::Redacted => { has_html_body = false; @@ -3696,8 +3837,8 @@ fn populate_message_view( } - // We must always re-set the message details, even when re-using a cached portallist item, - // because the item type might be the same but for a different message entirely. + // Re-set even for cached items: the portal list recycles widgets, so + // the same template might now be showing a totally different message. let message_details = MessageDetails { thread_root_event_id: msg_like_content.thread_root.clone().or_else(|| { msg_like_content.thread_summary.as_ref() @@ -3716,7 +3857,14 @@ fn populate_message_view( ), should_be_highlighted: event_tl_item.is_highlighted() || has_room_mention, }; - item.as_message().set_data(message_details); + let is_downloading = download_info.as_ref().is_some_and(|info| { + let mxc = match &info.media_source { + MediaSource::Plain(uri) => uri, + MediaSource::Encrypted(file) => &file.url, + }; + pending_downloads.iter().any(|p| p == mxc) + }); + item.as_message().set_data(cx, message_details, download_info, is_downloading); // If `used_cached_item` is false, we should always redraw the profile, even if profile_drawn is true. @@ -3887,9 +4035,40 @@ fn populate_text_message_content( } } -/// Draws the given image message's content into the `message_content_widget`. +/// Like `populate_image_message_content`, but also returns metadata +/// about how to download the image if we were unable to show a preview of it. +fn populate_image_message_content_with_fallback( + cx: &mut Cx, + text_or_image_ref: &TextOrImageRef, + image_info_source: Option>, + original_source: MediaSource, + body: &str, + media_cache: &mut MediaCache, + filename: String, + size: Option, + kind: DownloadKind, +) -> (bool, Option) { + let fully_drawn = populate_image_message_content( + cx, + text_or_image_ref, + image_info_source, + original_source.clone(), + body, + media_cache, + ); + let fallback = matches!(text_or_image_ref.status(), TextOrImageStatus::Text) + .then(|| DownloadableAttachment { + media_source: original_source, + filename, + size, + kind, + }); + (fully_drawn, fallback) +} + +/// Draws an image into the given `text_or_image_ref`. /// -/// Returns whether the image message content was fully drawn. +/// Returns whether it was fully drawn (meaning its content was fully loaded/available). fn populate_image_message_content( cx: &mut Cx, text_or_image_ref: &TextOrImageRef, @@ -3898,8 +4077,6 @@ fn populate_image_message_content( body: &str, media_cache: &mut MediaCache, ) -> bool { - // We don't use thumbnails, as their resolution is too low to be visually useful. - // We also don't trust the provided mimetype, as it can be incorrect. let (mimetype, _width, _height) = image_info_source.as_ref() .map(|info| (info.mimetype.as_deref(), info.width, info.height)) .unwrap_or_default(); @@ -3910,7 +4087,7 @@ fn populate_image_message_content( if ImageFormat::from_mimetype(mime).is_none() { text_or_image_ref.show_text( cx, - format!("{body}\n\nUnsupported type {mime:?}"), + format!("{body}\nUnsupported type {mime:?}"), ); return true; // consider this as fully drawn } @@ -4037,7 +4214,6 @@ fn populate_file_message_content( message_content_widget: &HtmlOrPlaintextRef, file_content: &FileMessageEventContent, ) -> bool { - // Display the file name, human-readable size, caption, and a button to download it. let filename = htmlize::escape_text(file_content.filename()); let size = file_content .info @@ -4051,11 +4227,9 @@ fn populate_file_message_content( .or_else(|| file_content.caption().map(|c| format!("
{}", htmlize::escape_text(c)))) .unwrap_or_default(); - // TODO: add a button to download the file - message_content_widget.show_html( cx, - format!("{filename}{size}{caption}
→ File download not yet supported."), + format!("File: {caption}
{filename}{size}"), ); true } @@ -4068,7 +4242,6 @@ fn populate_audio_message_content( message_content_widget: &HtmlOrPlaintextRef, audio: &AudioMessageEventContent, ) -> bool { - // Display the file name, human-readable size, caption, and a button to download it. let filename = htmlize::escape_text(audio.filename()); let (duration, mime, size) = audio .info @@ -4096,7 +4269,7 @@ fn populate_audio_message_content( message_content_widget.show_html( cx, - format!("Audio: {filename}{mime}{duration}{size}{caption}
→ Audio playback not yet supported."), + format!("Audio: {caption}
{mime}{duration}{size}
{filename}
→ Audio playback not yet supported."), ); true } @@ -4110,7 +4283,6 @@ fn populate_video_message_content( message_content_widget: &HtmlOrPlaintextRef, video: &VideoMessageEventContent, ) -> bool { - // Display the file name, human-readable size, caption, and a button to download it. let filename = htmlize::escape_text(video.filename()); let (duration, mime, size, dimensions) = video .info @@ -4137,11 +4309,11 @@ fn populate_video_message_content( .or_else(|| video.caption().map(|c| format!("
{}", htmlize::escape_text(c)))) .unwrap_or_default(); - // TODO: add an video to play the video file + // TODO: populate a video widget here, once makepad supports that message_content_widget.show_html( cx, - format!("Video: {filename}{mime}{duration}{size}{dimensions}{caption}
→ Video playback not yet supported."), + format!("Video: {caption}
{mime}{duration}{size}{dimensions}
{filename}
→ Video playback not yet supported."), ); true } @@ -4838,6 +5010,8 @@ pub enum MessageAction { // /// The user clicked the "report" button on a message. // Report(MessageDetails), + /// The user clicked the "Download" button on a media/file message. + DownloadAttachment(DownloadableAttachment), /// The message at the given item index in the timeline should be highlighted. HighlightMessage(usize), /// The user requested that we show a context menu with actions @@ -4876,6 +5050,10 @@ pub struct Message { #[apply_default] animator: Animator, #[rust] details: Option, + /// Set on file/image/audio/video messages so the download button knows + /// what to save when the user clicks it. `None` for plain text messages, + /// which hide the download button entirely. + #[rust] download_info: Option, } impl Widget for Message { @@ -5027,6 +5205,16 @@ impl Widget for Message { _ => {} } } + + // Handle clicks on the "Download" button shown beneath media messages. + if let Some(info) = self.download_info.as_ref() + && self.view.button(cx, ids!(content.download_section.download_button)).clicked(actions) + { + cx.widget_action( + details.room_screen_widget_uid, + MessageAction::DownloadAttachment(info.clone()), + ); + } } } @@ -5045,15 +5233,41 @@ impl Widget for Message { } impl Message { - fn set_data(&mut self, details: MessageDetails) { + /// Called every time `populate_message_view` runs, including on cached + /// items, so all state must be re-set unconditionally. + fn set_data( + &mut self, + cx: &mut Cx, + details: MessageDetails, + download_info: Option, + is_downloading: bool, + ) { self.details = Some(details); + self.download_info = download_info; + + let section_visible = self.download_info.is_some(); + self.view.view(cx, ids!(content.download_section)) + .set_visible(cx, section_visible); + if let Some(info) = self.download_info.as_ref() { + let download_button = self.view.button(cx, ids!(content.download_section.download_button)); + let downloading_view = self.view.view(cx, ids!(content.download_section.downloading_view)); + download_button.set_text(cx, info.kind.button_text()); + download_button.set_visible(cx, !is_downloading); + downloading_view.set_visible(cx, is_downloading); + } } } impl MessageRef { - fn set_data(&self, details: MessageDetails) { + fn set_data( + &self, + cx: &mut Cx, + details: MessageDetails, + download_info: Option, + is_downloading: bool, + ) { let Some(mut inner) = self.borrow_mut() else { return }; - inner.set_data(details); + inner.set_data(cx, details, download_info, is_downloading); } } diff --git a/src/media_cache.rs b/src/media_cache.rs index 672a16cf1..99aac707b 100644 --- a/src/media_cache.rs +++ b/src/media_cache.rs @@ -66,6 +66,10 @@ impl MediaCache { } } + pub fn timeline_update_sender(&self) -> Option<&crossbeam_channel::Sender> { + self.timeline_update_sender.as_ref() + } + /// Tries to get the media from the cache, or submits an async request to fetch it. /// /// This method *does not* block or wait for the media to be fetched, diff --git a/src/room/room_input_bar.rs b/src/room/room_input_bar.rs index 41cb6f94d..3c76ee985 100644 --- a/src/room/room_input_bar.rs +++ b/src/room/room_input_bar.rs @@ -698,14 +698,12 @@ impl RoomInputBar { #[cfg(feature = "tsp")] let sign_with_tsp = self.is_tsp_signing_enabled(cx); - let dialog = rfd::AsyncFileDialog::new() - .set_title("Select file to upload"); let (sender, receiver) = std::sync::mpsc::channel(); self.pending_file_selection = Some(receiver); - let dialog_task = dialog.pick_file(); + let dialog = rfd::AsyncFileDialog::new(); - cx.spawn_thread(move || { - let result = match futures::executor::block_on(dialog_task) { + crate::sliding_sync::spawn_async_task(async move { + let result = match dialog.pick_file().await { Some(selected_file) => { #[cfg(feature = "tsp")] { @@ -714,7 +712,7 @@ impl RoomInputBar { timeline_kind, in_reply_to, sign_with_tsp, - ) + ).await } #[cfg(not(feature = "tsp"))] { @@ -722,7 +720,7 @@ impl RoomInputBar { selected_file.path().to_path_buf(), timeline_kind, in_reply_to, - ) + ).await } } None => PendingFileSelection::Cancelled, @@ -966,53 +964,52 @@ enum ShowEditingPaneBehavior { } #[cfg(not(any(target_os = "ios", target_os = "android")))] -fn load_selected_file( +async fn load_selected_file( selected_file_path: std::path::PathBuf, timeline_kind: TimelineKind, in_reply_to: Option, #[cfg(feature = "tsp")] sign_with_tsp: bool, ) -> PendingFileSelection { - match std::fs::metadata(&selected_file_path) { - Ok(metadata) => { - if !metadata.is_file() { - return PendingFileSelection::Error("Cannot upload directories or special files".to_string()); - } - let file_size = metadata.len(); - if file_size == 0 { - return PendingFileSelection::Error("Cannot upload empty file".to_string()); - } - let mime = mime_guess::from_path(&selected_file_path) - .first_or_octet_stream(); - let preview_data = if crate::image_utils::is_displayable_image(mime.essence_str()) { - match std::fs::read(&selected_file_path) { - Ok(data) => Some(std::sync::Arc::new(data)), - Err(e) => return PendingFileSelection::Error(format!("Unable to read image preview: {e}")), - } - } else { - None - }; - let name = selected_file_path - .file_name() - .map(|n| n.to_string_lossy().to_string()) - .unwrap_or_else(|| "unknown".to_string()); - - PendingFileSelection::Selected { - upload: AttachmentUpload { - timeline_kind, - file_data: crate::shared::file_upload_modal::FileUploadMetadata { - path: selected_file_path, - caption: Some(name), - mime_type: mime.to_string(), - preview_data, - size: file_size, - }, - in_reply_to, - #[cfg(feature = "tsp")] - sign_with_tsp, - }, - } + let metadata = match tokio::fs::metadata(&selected_file_path).await { + Ok(m) => m, + Err(e) => return PendingFileSelection::Error(format!("Unable to access file: {e}")), + }; + if !metadata.is_file() { + return PendingFileSelection::Error("Cannot upload directories or special files".to_string()); + } + let file_size = metadata.len(); + if file_size == 0 { + return PendingFileSelection::Error("Cannot upload empty file".to_string()); + } + let mime = mime_guess::from_path(&selected_file_path) + .first_or_octet_stream(); + let preview_data = if crate::image_utils::is_displayable_image(mime.essence_str()) { + match tokio::fs::read(&selected_file_path).await { + Ok(data) => Some(std::sync::Arc::new(data)), + Err(e) => return PendingFileSelection::Error(format!("Unable to read image preview: {e}")), } - Err(e) => PendingFileSelection::Error(format!("Unable to access file: {e}")), + } else { + None + }; + let name = selected_file_path + .file_name() + .map(|n| n.to_string_lossy().to_string()) + .unwrap_or_else(|| "unknown".to_string()); + + PendingFileSelection::Selected { + upload: AttachmentUpload { + timeline_kind, + file_data: crate::shared::file_upload_modal::FileUploadMetadata { + path: selected_file_path, + caption: Some(name), + mime_type: mime.to_string(), + preview_data, + size: file_size, + }, + in_reply_to, + #[cfg(feature = "tsp")] + sign_with_tsp, + }, } } diff --git a/src/shared/attachment_download.rs b/src/shared/attachment_download.rs new file mode 100644 index 000000000..fc13b66bf --- /dev/null +++ b/src/shared/attachment_download.rs @@ -0,0 +1,133 @@ +//! Save a Matrix media attachment to disk via the rfd save dialog. +//! Used by the inline message button and the image viewer overlay. + +use std::sync::Arc; + +use makepad_widgets::SignalToUI; +use matrix_sdk::ruma::events::room::MediaSource; + +use crate::home::room_screen::TimelineUpdate; +use crate::shared::popup_list::{PopupKind, enqueue_popup_notification}; +use crate::sliding_sync::{MatrixRequest, spawn_async_task, submit_async_request}; + +#[derive(Clone, Debug)] +pub struct DownloadableAttachment { + pub media_source: MediaSource, + pub filename: String, + pub size: Option, + pub kind: DownloadKind, +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum DownloadKind { + File, + Audio, + Video, + Image, +} +impl DownloadKind { + pub fn button_text(&self) -> &'static str { + match self { + Self::File => "Download File", + Self::Audio => "Download Audio", + Self::Video => "Download Video", + Self::Image => "Download Image", + } + } +} + +/// Opens the rfd save dialog with sensible defaults for `info`. +#[cfg(not(any(target_os = "ios", target_os = "android")))] +fn build_save_dialog(info: &DownloadableAttachment) -> rfd::AsyncFileDialog { + let dialog = rfd::AsyncFileDialog::new().set_file_name(&info.filename); + if let Some(user_dirs) = robius_directories::UserDirs::new() { + let dir = user_dirs.download_dir() + .map(|p| p.to_path_buf()) + .unwrap_or_else(|| user_dirs.home_dir().to_path_buf()); + dialog.set_directory(dir) + } else { + dialog + } +} + +/// Opens the save dialog, then submits the actual fetch+write request. +/// Pass `update_sender` if the caller wants spinner updates routed back to +/// a specific timeline; pass `None` from contexts without one (e.g. the +/// image viewer overlay). +#[cfg(not(any(target_os = "ios", target_os = "android")))] +pub fn start_attachment_download( + info: DownloadableAttachment, + update_sender: Option>, +) { + let dialog = build_save_dialog(&info); + spawn_async_task(async move { + match dialog.save_file().await { + Some(handle) => { + submit_async_request(MatrixRequest::DownloadMediaToFile { + media_source: info.media_source, + save_path: handle.path().to_path_buf(), + filename: info.filename, + update_sender, + }); + } + // User cancelled. The action handler already marked this mxc as + // pending, so revert it now or the spinner stays forever. + None => { + if let Some(sender) = update_sender { + let mxc = match info.media_source { + MediaSource::Plain(uri) => uri, + MediaSource::Encrypted(file) => file.url.clone(), + }; + let _ = sender.send(TimelineUpdate::AttachmentDownloadFinished(mxc)); + SignalToUI::set_ui_signal(); + } + } + } + }); +} + +/// Like `start_attachment_download` but for callers that already have the +/// bytes in memory (e.g. the image viewer). Skips the matrix worker +/// round-trip and writes straight to disk. +#[cfg(not(any(target_os = "ios", target_os = "android")))] +pub fn save_loaded_attachment(info: DownloadableAttachment, bytes: Arc<[u8]>) { + let dialog = build_save_dialog(&info); + spawn_async_task(async move { + let Some(handle) = dialog.save_file().await else { return }; + let save_path = handle.path().to_path_buf(); + match tokio::fs::write(&save_path, &bytes[..]).await { + Ok(()) => enqueue_popup_notification( + format!("Saved \"{}\" to {}", info.filename, save_path.display()), + PopupKind::Success, + Some(5.0), + ), + Err(e) => enqueue_popup_notification( + format!("Failed to save \"{}\": {e}", info.filename), + PopupKind::Error, + None, + ), + } + }); +} + +/// Mobile: rfd doesn't have a save dialog there, so just tell the user. +#[cfg(any(target_os = "ios", target_os = "android"))] +pub fn start_attachment_download( + _info: DownloadableAttachment, + _update_sender: Option>, +) { + enqueue_popup_notification( + "Saving attachments is not yet supported on mobile.", + PopupKind::Error, + Some(5.0), + ); +} + +#[cfg(any(target_os = "ios", target_os = "android"))] +pub fn save_loaded_attachment(_info: DownloadableAttachment, _bytes: Arc<[u8]>) { + enqueue_popup_notification( + "Saving attachments is not yet supported on mobile.", + PopupKind::Error, + Some(5.0), + ); +} diff --git a/src/shared/image_viewer.rs b/src/shared/image_viewer.rs index fa70bfa9d..a568db909 100644 --- a/src/shared/image_viewer.rs +++ b/src/shared/image_viewer.rs @@ -15,7 +15,7 @@ use matrix_sdk_ui::timeline::EventTimelineItem; use crate::utils::format_decimal_file_size; use thiserror::Error; use crate::{ - shared::{avatar::AvatarWidgetExt, timestamp::TimestampWidgetRefExt}, + shared::{attachment_download::{DownloadableAttachment, save_loaded_attachment, start_attachment_download}, avatar::AvatarWidgetExt, timestamp::TimestampWidgetRefExt}, sliding_sync::TimelineKind, }; @@ -332,6 +332,11 @@ script_mod! { icon_walk: Walk{width: 25, height: 25, margin: Inset{bottom: 2}} } + download_button := mod.widgets.ImageViewerButton { + draw_icon +: { svg: (ICON_DOWNLOAD) } + icon_walk: Walk{width: 24, height: 24} + } + close_button := mod.widgets.ImageViewerButton { draw_icon +: { svg: (ICON_CLOSE) } icon_walk: Walk{width: 21, height: 21 } @@ -488,6 +493,13 @@ struct ImageViewer { /// from the continuous `FingerHoverOver` events that fire every frame. #[rust] last_mouse_pos: DVec2, #[rust] capped_dimension: DVec2, + /// Drives the overlay download button: visible when `Some`, fed to + /// `start_attachment_download` on click. + #[rust] downloadable: Option, + /// Image bytes from the most recent `LoadState::Loaded`. Lets the + /// download button skip the SDK fetch round-trip and write straight + /// to disk. + #[rust] loaded_bytes: Option>, } impl Widget for ImageViewer { @@ -825,6 +837,19 @@ impl MatchEvent for ImageViewer { } } + if self.view.button(cx, ids!(download_button)).clicked(actions) + && let Some(info) = self.downloadable.clone() + { + was_overlay_button_clicked = true; + // Use the already-loaded bytes if we have them, otherwise fall + // back to the matrix-worker fetch path. + if let Some(bytes) = self.loaded_bytes.clone() { + save_loaded_attachment(info, bytes); + } else { + start_attachment_download(info, None); + } + } + // Restart the auto-hide timer if any overlay button was clicked. If the // mouse is still over the overlay the hover handler keeps the timer // stopped anyway. @@ -845,6 +870,7 @@ impl MatchEvent for ImageViewer { self.show_loading(cx); } LoadState::Loaded(image_bytes) => { + self.loaded_bytes = Some(image_bytes.clone()); self.show_loaded(cx, image_bytes); } LoadState::FinishedBackgroundDecoding => { @@ -897,6 +923,7 @@ impl ImageViewer { self.last_mouse_pos = DVec2::default(); self.receiver = None; self.is_loaded = false; + self.loaded_bytes = None; self.image_container_size = DVec2::new(); self.ui_overlay_visible = true; self.mouse_over_overlay_ui = false; @@ -1132,6 +1159,13 @@ impl ImageViewer { .set_date_time(cx, timestamp); } + // New image starting to load: any cached bytes from a previous one + // are now stale, so a download click can't accidentally save them. + self.loaded_bytes = None; + self.downloadable = metadata.downloadable.clone(); + self.view.button(cx, ids!(download_button)) + .set_visible(cx, self.downloadable.is_some()); + if let Some((timeline_kind, event_timeline_item)) = &metadata.avatar_parameter { let (sender, _) = self.view.avatar(cx, ids!(avatar_timestamp_view.avatar)).set_avatar_and_get_username( cx, @@ -1221,4 +1255,6 @@ pub struct ImageViewerMetaData { pub image_name: String, // Image size in bytes pub image_file_size: u64, + /// When `Some`, the overlay's download button is shown. + pub downloadable: Option, } diff --git a/src/shared/mod.rs b/src/shared/mod.rs index 4f31b80d7..20be2eb97 100644 --- a/src/shared/mod.rs +++ b/src/shared/mod.rs @@ -1,5 +1,6 @@ use makepad_widgets::ScriptVm; +pub mod attachment_download; pub mod avatar; pub mod collapsible_header; pub mod expand_arrow; diff --git a/src/shared/styles.rs b/src/shared/styles.rs index d861ad60a..ee731d81a 100644 --- a/src/shared/styles.rs +++ b/src/shared/styles.rs @@ -17,6 +17,7 @@ script_mod! { mod.widgets.ICON_ROTATE_CW = crate_resource("self://resources/icons/rotate_right_fa.svg") mod.widgets.ICON_ROTATE_CCW = crate_resource("self://resources/icons/rotate_left_fa.svg") mod.widgets.ICON_COPY = crate_resource("self://resources/icons/copy.svg") + mod.widgets.ICON_DOWNLOAD = crate_resource("self://resources/icons/download.svg") mod.widgets.ICON_EDIT = crate_resource("self://resources/icons/edit.svg") mod.widgets.ICON_EXTERNAL_LINK = crate_resource("self://resources/icons/external_link.svg") mod.widgets.ICON_IMPORT = crate_resource("self://resources/icons/import.svg") diff --git a/src/sliding_sync.rs b/src/sliding_sync.rs index 3c1cec83e..568c320f3 100644 --- a/src/sliding_sync.rs +++ b/src/sliding_sync.rs @@ -711,6 +711,16 @@ pub enum MatrixRequest { destination: Arc>, update_sender: Option>, }, + /// Fetches a media attachment in full and writes it to `save_path`. + /// Shows a popup on success/failure; if `update_sender` is set, also + /// emits started/finished updates so the source timeline can show a + /// spinner. + DownloadMediaToFile { + media_source: MediaSource, + save_path: PathBuf, + filename: String, + update_sender: Option>, + }, } /// Submits a request to the worker thread to be executed asynchronously. @@ -721,6 +731,14 @@ pub fn submit_async_request(req: MatrixRequest) { } } +/// Spawns a one-off async task on the backend Tokio runtime. +pub fn spawn_async_task(future: F) +where + F: Future + Send + 'static, +{ + get_or_create_tokio_runtime().spawn(future); +} + /// Details of a login request that get submitted within [`MatrixRequest::Login`]. pub enum LoginRequest{ LoginByPassword(LoginByPassword), @@ -2162,6 +2180,53 @@ async fn matrix_worker_task( SignalToUI::set_ui_signal(); }); } + + MatrixRequest::DownloadMediaToFile { media_source, save_path, filename, update_sender } => { + let Some(client) = get_client() else { continue }; + Handle::current().spawn(async move { + let mxc_uri = match &media_source { + MediaSource::Plain(uri) => uri.clone(), + MediaSource::Encrypted(file) => file.url.clone(), + }; + let media_request = MediaRequestParameters { + source: media_source, + format: matrix_sdk::media::MediaFormat::File, + }; + match client.media().get_media_content(&media_request, true).await { + Ok(bytes) => match tokio::fs::write(&save_path, &bytes).await { + Ok(()) => { + log!("Saved downloaded attachment {filename:?} to {}", save_path.display()); + enqueue_popup_notification( + format!("Saved \"{filename}\" to {}", save_path.display()), + PopupKind::Success, + Some(5.0), + ); + } + Err(e) => { + error!("Failed to write downloaded attachment {filename:?} to {}: {e}", save_path.display()); + enqueue_popup_notification( + format!("Failed to save \"{filename}\": {e}"), + PopupKind::Error, + None, + ); + } + } + Err(e) => { + error!("Failed to fetch media content for attachment {filename:?}: {e}"); + enqueue_popup_notification( + format!("Failed to download \"{filename}\": {e}"), + PopupKind::Error, + None, + ); + } + } + if let Some(sender) = update_sender.as_ref() { + let _ = sender.send(TimelineUpdate::AttachmentDownloadFinished(mxc_uri)); + SignalToUI::set_ui_signal(); + } + }); + } + } } @@ -2170,8 +2235,15 @@ async fn matrix_worker_task( } -/// The single global Tokio runtime that is used by all async tasks. -static TOKIO_RUNTIME: Mutex> = Mutex::new(None); +/// Returns the global Tokio runtime, creating it on first use. +fn get_or_create_tokio_runtime() -> &'static tokio::runtime::Runtime { + /// The single global Tokio runtime that is used by all async tasks. + static TOKIO_RUNTIME: LazyLock = LazyLock::new(|| + tokio::runtime::Runtime::new().expect("Failed to create Tokio runtime") + ); + + &TOKIO_RUNTIME +} /// The sender used by [`submit_async_request`] to send requests to the async worker thread. /// Currently there is only one, but it can be cloned if we need more concurrent senders. @@ -2212,9 +2284,7 @@ pub fn block_on_async_with_timeout( timeout: Option, async_future: impl Future, ) -> Result { - let rt = TOKIO_RUNTIME.lock().unwrap().get_or_insert_with(|| - tokio::runtime::Runtime::new().expect("Failed to create Tokio runtime") - ).handle().clone(); + let rt = get_or_create_tokio_runtime().handle().clone(); if let Some(timeout) = timeout { rt.block_on(async { @@ -2231,10 +2301,7 @@ pub fn block_on_async_with_timeout( /// /// Returns a handle to the Tokio runtime that is used to run async background tasks. pub fn start_matrix_tokio() -> Result { - // Create a Tokio runtime, and save it in a static variable to ensure it isn't dropped. - let rt_handle = TOKIO_RUNTIME.lock().unwrap().get_or_insert_with(|| { - tokio::runtime::Runtime::new().expect("Failed to create Tokio runtime") - }).handle().clone(); + let rt_handle = get_or_create_tokio_runtime().handle().clone(); let rt = rt_handle.clone(); // Spawn the main async task that drives the Matrix client SDK and @@ -2403,16 +2470,7 @@ pub fn set_sync_service_desired_running(running: bool, reason: &'static str) { return; } - let rt_handle = TOKIO_RUNTIME.lock().unwrap().as_ref().map(|rt| rt.handle().clone()); - let Some(rt_handle) = rt_handle else { - log!( - "Stored Matrix sync desired state as {}; Tokio runtime is not running yet ({reason}).", - if running { "running" } else { "stopped" } - ); - return; - }; - - rt_handle.spawn(apply_sync_service_desired_state(reason)); + get_or_create_tokio_runtime().spawn(apply_sync_service_desired_state(reason)); } async fn apply_sync_service_desired_state(reason: &'static str) { From a434ae3a599194465c0d441e92f6d4501d902e2d Mon Sep 17 00:00:00 2001 From: Kevin Boos Date: Tue, 26 May 2026 21:08:39 -0700 Subject: [PATCH 2/6] fix compiler warnings --- src/shared/attachment_download.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/shared/attachment_download.rs b/src/shared/attachment_download.rs index fc13b66bf..7d03c78eb 100644 --- a/src/shared/attachment_download.rs +++ b/src/shared/attachment_download.rs @@ -2,13 +2,10 @@ //! Used by the inline message button and the image viewer overlay. use std::sync::Arc; - -use makepad_widgets::SignalToUI; use matrix_sdk::ruma::events::room::MediaSource; - use crate::home::room_screen::TimelineUpdate; use crate::shared::popup_list::{PopupKind, enqueue_popup_notification}; -use crate::sliding_sync::{MatrixRequest, spawn_async_task, submit_async_request}; + #[derive(Clone, Debug)] pub struct DownloadableAttachment { @@ -59,6 +56,8 @@ pub fn start_attachment_download( info: DownloadableAttachment, update_sender: Option>, ) { + use crate::sliding_sync::{MatrixRequest, spawn_async_task, submit_async_request}; + let dialog = build_save_dialog(&info); spawn_async_task(async move { match dialog.save_file().await { @@ -79,7 +78,7 @@ pub fn start_attachment_download( MediaSource::Encrypted(file) => file.url.clone(), }; let _ = sender.send(TimelineUpdate::AttachmentDownloadFinished(mxc)); - SignalToUI::set_ui_signal(); + makepad_widgets::SignalToUI::set_ui_signal(); } } } From 09958b7997fffe56d223979f64bcd5df5c656f12 Mon Sep 17 00:00:00 2001 From: Kevin Boos Date: Wed, 27 May 2026 10:23:52 -0700 Subject: [PATCH 3/6] a bit of refactoring. Show download state with colored buttons --- src/home/room_screen.rs | 178 +++++++++++++++++++++--------- src/room/room_input_bar.rs | 18 +-- src/shared/attachment_download.rs | 99 ++++++++++++++--- src/shared/image_viewer.rs | 4 +- src/sliding_sync.rs | 69 +++++++++--- 5 files changed, 270 insertions(+), 98 deletions(-) diff --git a/src/home/room_screen.rs b/src/home/room_screen.rs index 0b9e70210..bce737394 100644 --- a/src/home/room_screen.rs +++ b/src/home/room_screen.rs @@ -32,7 +32,7 @@ use crate::{ }, room::{BasicRoomDetails, room_input_bar::{RoomInputBarState, RoomInputBarWidgetRefExt}, typing_notice::TypingNoticeWidgetExt}, shared::{ - attachment_download::{DownloadKind, DownloadableAttachment, start_attachment_download}, avatar::{AvatarState, AvatarWidgetRefExt}, confirmation_modal::ConfirmationModalContent, file_upload_modal::FileUploadAttemptId, html_or_plaintext::{HtmlOrPlaintextRef, HtmlOrPlaintextWidgetRefExt, RobrixHtmlLinkAction}, image_viewer::{ImageViewerAction, ImageViewerMetaData, LoadState}, jump_to_bottom_button::{JumpToBottomButtonWidgetExt, UnreadMessageCount}, popup_list::{PopupKind, enqueue_popup_notification}, restore_status_view::RestoreStatusViewWidgetExt, styles::*, text_or_image::{TextOrImageAction, TextOrImageRef, TextOrImageStatus, TextOrImageWidgetRefExt}, timestamp::TimestampWidgetRefExt + attachment_download::{DownloadDisplayState, DownloadKind, DownloadableAttachment, PendingDownload, PendingDownloadState, media_source_mxc, start_attachment_download}, avatar::{AvatarState, AvatarWidgetRefExt}, confirmation_modal::ConfirmationModalContent, file_upload_modal::FileUploadAttemptId, html_or_plaintext::{HtmlOrPlaintextRef, HtmlOrPlaintextWidgetRefExt, RobrixHtmlLinkAction}, image_viewer::{ImageViewerAction, ImageViewerMetaData, LoadState}, jump_to_bottom_button::{JumpToBottomButtonWidgetExt, UnreadMessageCount}, popup_list::{PopupKind, enqueue_popup_notification}, restore_status_view::RestoreStatusViewWidgetExt, styles::*, text_or_image::{TextOrImageAction, TextOrImageRef, TextOrImageStatus, TextOrImageWidgetRefExt}, timestamp::TimestampWidgetRefExt }, sliding_sync::{BackwardsPaginateUntilEventRequest, MatrixRequest, PaginationDirection, TimelineEndpoints, TimelineKind, TimelineRequestSender, UserPowerLevels, get_client, submit_async_request, take_timeline_endpoints}, utils::{self, ImageFormat, MEDIA_THUMBNAIL_FORMAT, RoomNameId, unix_time_millis_to_datetime} }; @@ -87,9 +87,7 @@ script_mod! { // An empty view that takes up no space in the portal list. mod.widgets.Empty = View { } - // Shown beneath media messages. The whole section is hidden for non-media - // ones; for media, one of the two children is visible at a time - // (button vs spinner), driven from `Message::set_data`. + // A download button or loading spinner shown beneath a message. mod.widgets.MessageDownloadSection = View { visible: false, width: Fit, height: Fit, @@ -111,7 +109,7 @@ script_mod! { flow: Right, align: Align{y: 0.5} spacing: 8, - padding: Inset{left: 12, right: 12} + padding: Inset{left: 12, right: 6} spinner := LoadingSpinner { width: 16, height: 16 @@ -127,6 +125,34 @@ script_mod! { } text: "Downloading…" } + cancel_button := RobrixNegativeIconButton { + height: mod.widgets.SETTINGS_BUTTON_HEIGHT, + padding: Inset{left: 12, right: 12} + margin: 0 + draw_icon.svg: (ICON_CLOSE) + icon_walk: Walk{width: 16, height: 16} + text: "Cancel" + } + } + + success_button := RobrixPositiveIconButton { + visible: false, + height: mod.widgets.SETTINGS_BUTTON_HEIGHT, + padding: Inset{left: 12, right: 12} + margin: 0 + draw_icon.svg: (ICON_CHECKMARK) + icon_walk: Walk{width: 16, height: 16} + text: "Downloaded" + } + + failure_button := RobrixNegativeIconButton { + visible: false, + height: mod.widgets.SETTINGS_BUTTON_HEIGHT, + padding: Inset{left: 12, right: 12} + margin: 0 + draw_icon.svg: (ICON_CLOSE) + icon_walk: Walk{width: 16, height: 16} + text: "Download Failed" } } @@ -890,14 +916,21 @@ impl Widget for RoomScreen { } } - // When transitioning from offline to online, clear stale `Requested`/`Failed` - // entries from per-room caches so they can be re-fetched. + // When transitioning from offline to online, abort all pending downloads + // and clear stale `Requested`/`Failed` entries from per-room caches so they can be re-fetched. if let Some(RoomsListHeaderAction::StateUpdate(new_state)) = action.downcast_ref() { - if !matches!(new_state, State::Offline) { + if matches!(new_state, State::Offline) { if let Some(tl) = self.tl_state.as_mut() { - tl.media_cache.clear_all_pending_and_failed_requests(); - tl.link_preview_cache.clear_all_pending_and_failed_requests(); + // Tell the worker to abort every in-flight download + // for this room before we drop them from local state. + for entry in tl.pending_downloads.drain(..) { + submit_async_request(MatrixRequest::CancelDownload(entry.mxc)); + } + self.view.portal_list(cx, ids!(timeline.list)).redraw(cx); } + } else if let Some(tl) = self.tl_state.as_mut() { + tl.media_cache.clear_all_pending_and_failed_requests(); + tl.link_preview_cache.clear_all_pending_and_failed_requests(); } continue; } @@ -1705,8 +1738,17 @@ impl RoomScreen { self.view.room_input_bar(cx, ids!(room_input_bar)) .hide_upload_progress(cx, upload_id); } - TimelineUpdate::AttachmentDownloadFinished(mxc) => { - tl.pending_downloads.retain(|p| p != &mxc); + TimelineUpdate::AttachmentDownloadFinished(mxc, result) => { + if let Some(entry) = tl.pending_downloads.iter_mut().find(|p| p.mxc == mxc) { + entry.state = match result { + Ok(()) => PendingDownloadState::JustSucceeded, + Err(_) => PendingDownloadState::JustFailed, + }; + } + portal_list.redraw(cx); + } + TimelineUpdate::AttachmentDownloadReset(mxc) => { + tl.pending_downloads.retain(|p| p.mxc != mxc); portal_list.redraw(cx); } } @@ -2206,20 +2248,27 @@ impl RoomScreen { MessageAction::DownloadAttachment(info) => { let Some(tl) = self.tl_state.as_mut() else { continue }; - let mxc = match &info.media_source { - MediaSource::Plain(uri) => uri, - MediaSource::Encrypted(file) => &file.url, - }; - // Mark pending synchronously so the next draw hides the - // button before the user can click it again. Skip if it's - // already in flight. - if tl.pending_downloads.iter().any(|p| p == mxc) { + let mxc = media_source_mxc(&info.media_source); + // Prevent the same attachment from being downloaded more than once at a time. + if tl.pending_downloads.iter().any(|p| &p.mxc == mxc) { continue; } - tl.pending_downloads.push(mxc.clone()); + tl.pending_downloads.push(PendingDownload { + mxc: mxc.clone(), + state: PendingDownloadState::InProgress, + }); portal_list.redraw(cx); let update_sender = tl.media_cache.timeline_update_sender().cloned(); - start_attachment_download(info.clone(), update_sender); + start_attachment_download(cx, info.clone(), update_sender); + } + MessageAction::CancelDownload(mxc) => { + submit_async_request(MatrixRequest::CancelDownload(mxc.clone())); + if let Some(tl) = self.tl_state.as_mut() + && let Some(i) = tl.pending_downloads.iter().position(|p| &p.mxc == mxc) + { + tl.pending_downloads.swap_remove(i); + portal_list.redraw(cx); + } } // This is handled within the Message widget itself. MessageAction::HighlightMessage(..) => { } @@ -2933,9 +2982,12 @@ pub enum TimelineUpdate { FileUploadComplete { upload_id: FileUploadAttemptId, }, - /// An inline-button download is done (or failed). `RoomScreen` removes - /// the mxc from `pending_downloads` so the spinner reverts to the button. - AttachmentDownloadFinished(OwnedMxcUri), + /// Download finished. `Ok` if bytes hit disk, `Err(msg)` otherwise. + /// The inline button briefly shows a success/failure indicator, then + /// `AttachmentDownloadReset` clears the entry from `pending_downloads`. + AttachmentDownloadFinished(OwnedMxcUri, Result<(), String>), + /// Drop the entry so the inline button goes back to its default "Download …" label. + AttachmentDownloadReset(OwnedMxcUri), } /// Stores timeline UI state that is not currently owned by a `RoomScreen`. @@ -3148,7 +3200,7 @@ struct TimelineUiState { /// Media/file attachments in this timeline currently being downloaded. /// the inline button's spinner state. - pending_downloads: SmallVec<[OwnedMxcUri; 1]>, + pending_downloads: SmallVec<[PendingDownload; 1]>, } #[derive(Default, Debug)] @@ -3279,7 +3331,7 @@ fn populate_message_view( pending_thread_summary_fetches: &mut HashSet, user_power_levels: &UserPowerLevels, pinned_events: &[OwnedEventId], - pending_downloads: &[OwnedMxcUri], + pending_downloads: &[PendingDownload], item_drawn_status: ItemDrawnStatus, room_screen_widget_uid: WidgetUid, ) -> (WidgetRef, ItemDrawnStatus) { @@ -3857,14 +3909,15 @@ fn populate_message_view( ), should_be_highlighted: event_tl_item.is_highlighted() || has_room_mention, }; - let is_downloading = download_info.as_ref().is_some_and(|info| { - let mxc = match &info.media_source { - MediaSource::Plain(uri) => uri, - MediaSource::Encrypted(file) => &file.url, - }; - pending_downloads.iter().any(|p| p == mxc) - }); - item.as_message().set_data(cx, message_details, download_info, is_downloading); + let download_state = download_info.as_ref() + .and_then(|info| { + let mxc = media_source_mxc(&info.media_source); + pending_downloads.iter() + .find(|p| &p.mxc == mxc) + .map(|p| p.state.display()) + }) + .unwrap_or_default(); + item.as_message().set_data(cx, message_details, download_info, download_state); // If `used_cached_item` is false, we should always redraw the profile, even if profile_drawn is true. @@ -4223,13 +4276,13 @@ fn populate_file_message_content( .unwrap_or_default(); let caption = file_content.formatted_caption() .filter(|fb| fb.format == MessageFormat::Html) - .map(|fb| format!("
{}", fb.body)) - .or_else(|| file_content.caption().map(|c| format!("
{}", htmlize::escape_text(c)))) + .map(|fb| format!("{}
", fb.body)) + .or_else(|| file_content.caption().map(|c| format!("{}
", htmlize::escape_text(c)))) .unwrap_or_default(); message_content_widget.show_html( cx, - format!("File: {caption}
{filename}{size}"), + format!("File: {caption}{filename}{size}"), ); true } @@ -4248,28 +4301,28 @@ fn populate_audio_message_content( .as_ref() .map(|info| ( info.duration - .map(|d| format!(" {:.2} sec,", d.as_secs_f64())) + .map(|d| format!(", {:.2} sec", d.as_secs_f64())) .unwrap_or_default(), info.mimetype .as_ref() .map(|m| format!(" {},", htmlize::escape_text(m))) .unwrap_or_default(), info.size - .map(|bytes| format!(" ({}),", utils::format_decimal_file_size(bytes.into()))) + .map(|bytes| format!(" ({})", utils::format_decimal_file_size(bytes.into()))) .unwrap_or_default(), )) .unwrap_or_default(); let caption = audio.formatted_caption() .filter(|fb| fb.format == MessageFormat::Html) - .map(|fb| format!("
{}", fb.body)) - .or_else(|| audio.caption().map(|c| format!("
{}", htmlize::escape_text(c)))) + .map(|fb| format!("{}
", fb.body)) + .or_else(|| audio.caption().map(|c| format!("{}
", htmlize::escape_text(c)))) .unwrap_or_default(); // TODO: add an audio to play the audio file message_content_widget.show_html( cx, - format!("Audio: {caption}
{mime}{duration}{size}
{filename}
→ Audio playback not yet supported."), + format!("Audio: {caption}File: {filename}{size}{mime}{duration}
→ Video playback not yet supported."), ); true } @@ -4289,31 +4342,31 @@ fn populate_video_message_content( .as_ref() .map(|info| ( info.duration - .map(|d| format!(" {:.2} sec,", d.as_secs_f64())) + .map(|d| format!(", {:.2} sec", d.as_secs_f64())) .unwrap_or_default(), info.mimetype .as_ref() - .map(|m| format!(" {},", htmlize::escape_text(m))) + .map(|m| format!(", {}", htmlize::escape_text(m))) .unwrap_or_default(), info.size - .map(|bytes| format!(" ({}),", utils::format_decimal_file_size(bytes.into()))) + .map(|bytes| format!(" ({})", utils::format_decimal_file_size(bytes.into()))) .unwrap_or_default(), info.width.and_then(|width| - info.height.map(|height| format!(" {width}x{height},")) + info.height.map(|height| format!(", {width}x{height}")) ).unwrap_or_default(), )) .unwrap_or_default(); let caption = video.formatted_caption() .filter(|fb| fb.format == MessageFormat::Html) - .map(|fb| format!("
{}", fb.body)) - .or_else(|| video.caption().map(|c| format!("
{}", htmlize::escape_text(c)))) + .map(|fb| format!("{}
", fb.body)) + .or_else(|| video.caption().map(|c| format!("{}
", htmlize::escape_text(c)))) .unwrap_or_default(); // TODO: populate a video widget here, once makepad supports that message_content_widget.show_html( cx, - format!("Video: {caption}
{mime}{duration}{size}{dimensions}
{filename}
→ Video playback not yet supported."), + format!("Video: {caption}File: {filename}{size}{mime}{duration}{dimensions}
→ Video playback not yet supported."), ); true } @@ -5012,6 +5065,8 @@ pub enum MessageAction { /// The user clicked the "Download" button on a media/file message. DownloadAttachment(DownloadableAttachment), + /// User clicked the cancel × next to the in-progress spinner. + CancelDownload(OwnedMxcUri), /// The message at the given item index in the timeline should be highlighted. HighlightMessage(usize), /// The user requested that we show a context menu with actions @@ -5215,6 +5270,15 @@ impl Widget for Message { MessageAction::DownloadAttachment(info.clone()), ); } + // Cancel × shown next to the in-progress spinner. + if let Some(info) = self.download_info.as_ref() + && self.view.button(cx, ids!(content.download_section.downloading_view.cancel_button)).clicked(actions) + { + cx.widget_action( + details.room_screen_widget_uid, + MessageAction::CancelDownload(media_source_mxc(&info.media_source).clone()), + ); + } } } @@ -5240,7 +5304,7 @@ impl Message { cx: &mut Cx, details: MessageDetails, download_info: Option, - is_downloading: bool, + download_state: DownloadDisplayState, ) { self.details = Some(details); self.download_info = download_info; @@ -5251,9 +5315,13 @@ impl Message { if let Some(info) = self.download_info.as_ref() { let download_button = self.view.button(cx, ids!(content.download_section.download_button)); let downloading_view = self.view.view(cx, ids!(content.download_section.downloading_view)); + let success_button = self.view.button(cx, ids!(content.download_section.success_button)); + let failure_button = self.view.button(cx, ids!(content.download_section.failure_button)); download_button.set_text(cx, info.kind.button_text()); - download_button.set_visible(cx, !is_downloading); - downloading_view.set_visible(cx, is_downloading); + download_button.set_visible(cx, matches!(download_state, DownloadDisplayState::Idle)); + downloading_view.set_visible(cx, matches!(download_state, DownloadDisplayState::InProgress)); + success_button.set_visible(cx, matches!(download_state, DownloadDisplayState::Succeeded)); + failure_button.set_visible(cx, matches!(download_state, DownloadDisplayState::Failed)); } } } @@ -5264,10 +5332,10 @@ impl MessageRef { cx: &mut Cx, details: MessageDetails, download_info: Option, - is_downloading: bool, + download_state: DownloadDisplayState, ) { let Some(mut inner) = self.borrow_mut() else { return }; - inner.set_data(cx, details, download_info, is_downloading); + inner.set_data(cx, details, download_info, download_state); } } diff --git a/src/room/room_input_bar.rs b/src/room/room_input_bar.rs index 3c76ee985..02184b489 100644 --- a/src/room/room_input_bar.rs +++ b/src/room/room_input_bar.rs @@ -700,10 +700,12 @@ impl RoomInputBar { let (sender, receiver) = std::sync::mpsc::channel(); self.pending_file_selection = Some(receiver); - let dialog = rfd::AsyncFileDialog::new(); + let dialog_task = rfd::AsyncFileDialog::new().pick_file(); - crate::sliding_sync::spawn_async_task(async move { - let result = match dialog.pick_file().await { + // Native thread, not a tokio task: rfd's macOS dialog panics if it + // runs on a tokio worker thread. + cx.spawn_thread(move || { + let result = match futures::executor::block_on(dialog_task) { Some(selected_file) => { #[cfg(feature = "tsp")] { @@ -712,7 +714,7 @@ impl RoomInputBar { timeline_kind, in_reply_to, sign_with_tsp, - ).await + ) } #[cfg(not(feature = "tsp"))] { @@ -720,7 +722,7 @@ impl RoomInputBar { selected_file.path().to_path_buf(), timeline_kind, in_reply_to, - ).await + ) } } None => PendingFileSelection::Cancelled, @@ -964,14 +966,14 @@ enum ShowEditingPaneBehavior { } #[cfg(not(any(target_os = "ios", target_os = "android")))] -async fn load_selected_file( +fn load_selected_file( selected_file_path: std::path::PathBuf, timeline_kind: TimelineKind, in_reply_to: Option, #[cfg(feature = "tsp")] sign_with_tsp: bool, ) -> PendingFileSelection { - let metadata = match tokio::fs::metadata(&selected_file_path).await { + let metadata = match std::fs::metadata(&selected_file_path) { Ok(m) => m, Err(e) => return PendingFileSelection::Error(format!("Unable to access file: {e}")), }; @@ -985,7 +987,7 @@ async fn load_selected_file( let mime = mime_guess::from_path(&selected_file_path) .first_or_octet_stream(); let preview_data = if crate::image_utils::is_displayable_image(mime.essence_str()) { - match tokio::fs::read(&selected_file_path).await { + match std::fs::read(&selected_file_path) { Ok(data) => Some(std::sync::Arc::new(data)), Err(e) => return PendingFileSelection::Error(format!("Unable to read image preview: {e}")), } diff --git a/src/shared/attachment_download.rs b/src/shared/attachment_download.rs index 7d03c78eb..9942b73f2 100644 --- a/src/shared/attachment_download.rs +++ b/src/shared/attachment_download.rs @@ -2,10 +2,68 @@ //! Used by the inline message button and the image viewer overlay. use std::sync::Arc; -use matrix_sdk::ruma::events::room::MediaSource; +use makepad_widgets::Cx; +#[cfg(not(any(target_os = "ios", target_os = "android")))] +use makepad_widgets::CxOsApi; +use matrix_sdk::ruma::{OwnedMxcUri, events::room::MediaSource}; use crate::home::room_screen::TimelineUpdate; use crate::shared::popup_list::{PopupKind, enqueue_popup_notification}; +/// The mxc URI inside any media source, whether plain or encrypted. +pub fn media_source_mxc(source: &MediaSource) -> &OwnedMxcUri { + match source { + MediaSource::Plain(uri) => uri, + MediaSource::Encrypted(file) => &file.url, + } +} + +/// One entry in `TimelineUiState.pending_downloads`. Tracks an attachment +/// download's lifecycle so the inline button can render the right view. +pub struct PendingDownload { + pub mxc: OwnedMxcUri, + pub state: PendingDownloadState, +} + +pub enum PendingDownloadState { + /// Fetch+write in flight. The matrix worker owns the corresponding + /// `AbortHandle`; `MessageAction::CancelDownload` routes through + /// `MatrixRequest::CancelDownload` to abort it. + InProgress, + /// Bytes hit disk. Shown for a few seconds before the entry is cleared. + JustSucceeded, + /// Fetch or write failed. Shown for a few seconds before the entry is cleared. + JustFailed, +} + +impl PendingDownloadState { + pub fn display(&self) -> DownloadDisplayState { + match self { + Self::InProgress => DownloadDisplayState::InProgress, + Self::JustSucceeded => DownloadDisplayState::Succeeded, + Self::JustFailed => DownloadDisplayState::Failed, + } + } +} + +/// What the inline download section should render. Decoupled from +/// `PendingDownloadState` so the `Message` widget doesn't need to know +/// about `AbortHandle`s. +#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] +pub enum DownloadDisplayState { + /// Show the "Download …" button. + #[default] + Idle, + /// Show the spinner + cancel button. + InProgress, + /// Briefly show a green "Saved" indicator. + Succeeded, + /// Briefly show a red "Failed" indicator. + Failed, +} + +/// How long (in seconds) the success/failure state stays visible before resetting the button. +pub const DOWNLOAD_RESULT_DURATION_SECS: f64 = 5.0; + #[derive(Clone, Debug)] pub struct DownloadableAttachment { @@ -51,16 +109,21 @@ fn build_save_dialog(info: &DownloadableAttachment) -> rfd::AsyncFileDialog { /// Pass `update_sender` if the caller wants spinner updates routed back to /// a specific timeline; pass `None` from contexts without one (e.g. the /// image viewer overlay). +/// +/// The dialog runs on a fresh OS thread (not a tokio task) because rfd's +/// macOS backend falls back to a sync dialog and panics when called from +/// a tokio worker thread. #[cfg(not(any(target_os = "ios", target_os = "android")))] pub fn start_attachment_download( + cx: &mut Cx, info: DownloadableAttachment, update_sender: Option>, ) { - use crate::sliding_sync::{MatrixRequest, spawn_async_task, submit_async_request}; + use crate::sliding_sync::{MatrixRequest, submit_async_request}; - let dialog = build_save_dialog(&info); - spawn_async_task(async move { - match dialog.save_file().await { + let dialog_task = build_save_dialog(&info).save_file(); + cx.spawn_thread(move || { + match futures::executor::block_on(dialog_task) { Some(handle) => { submit_async_request(MatrixRequest::DownloadMediaToFile { media_source: info.media_source, @@ -70,14 +133,12 @@ pub fn start_attachment_download( }); } // User cancelled. The action handler already marked this mxc as - // pending, so revert it now or the spinner stays forever. + // pending. Revert directly (skip the success/failure flash, since + // dismissing the dialog isn't really a failure). None => { if let Some(sender) = update_sender { - let mxc = match info.media_source { - MediaSource::Plain(uri) => uri, - MediaSource::Encrypted(file) => file.url.clone(), - }; - let _ = sender.send(TimelineUpdate::AttachmentDownloadFinished(mxc)); + let mxc = media_source_mxc(&info.media_source).clone(); + let _ = sender.send(TimelineUpdate::AttachmentDownloadReset(mxc)); makepad_widgets::SignalToUI::set_ui_signal(); } } @@ -87,14 +148,15 @@ pub fn start_attachment_download( /// Like `start_attachment_download` but for callers that already have the /// bytes in memory (e.g. the image viewer). Skips the matrix worker -/// round-trip and writes straight to disk. +/// round-trip and writes straight to disk on the same OS thread that +/// hosted the save dialog. #[cfg(not(any(target_os = "ios", target_os = "android")))] -pub fn save_loaded_attachment(info: DownloadableAttachment, bytes: Arc<[u8]>) { - let dialog = build_save_dialog(&info); - spawn_async_task(async move { - let Some(handle) = dialog.save_file().await else { return }; +pub fn save_loaded_attachment(cx: &mut Cx, info: DownloadableAttachment, bytes: Arc<[u8]>) { + let dialog_task = build_save_dialog(&info).save_file(); + cx.spawn_thread(move || { + let Some(handle) = futures::executor::block_on(dialog_task) else { return }; let save_path = handle.path().to_path_buf(); - match tokio::fs::write(&save_path, &bytes[..]).await { + match std::fs::write(&save_path, &bytes[..]) { Ok(()) => enqueue_popup_notification( format!("Saved \"{}\" to {}", info.filename, save_path.display()), PopupKind::Success, @@ -112,6 +174,7 @@ pub fn save_loaded_attachment(info: DownloadableAttachment, bytes: Arc<[u8]>) { /// Mobile: rfd doesn't have a save dialog there, so just tell the user. #[cfg(any(target_os = "ios", target_os = "android"))] pub fn start_attachment_download( + _cx: &mut Cx, _info: DownloadableAttachment, _update_sender: Option>, ) { @@ -123,7 +186,7 @@ pub fn start_attachment_download( } #[cfg(any(target_os = "ios", target_os = "android"))] -pub fn save_loaded_attachment(_info: DownloadableAttachment, _bytes: Arc<[u8]>) { +pub fn save_loaded_attachment(_cx: &mut Cx, _info: DownloadableAttachment, _bytes: Arc<[u8]>) { enqueue_popup_notification( "Saving attachments is not yet supported on mobile.", PopupKind::Error, diff --git a/src/shared/image_viewer.rs b/src/shared/image_viewer.rs index a568db909..9920377f0 100644 --- a/src/shared/image_viewer.rs +++ b/src/shared/image_viewer.rs @@ -844,9 +844,9 @@ impl MatchEvent for ImageViewer { // Use the already-loaded bytes if we have them, otherwise fall // back to the matrix-worker fetch path. if let Some(bytes) = self.loaded_bytes.clone() { - save_loaded_attachment(info, bytes); + save_loaded_attachment(cx, info, bytes); } else { - start_attachment_download(info, None); + start_attachment_download(cx, info, None); } } diff --git a/src/sliding_sync.rs b/src/sliding_sync.rs index 568c320f3..6120bccd9 100644 --- a/src/sliding_sync.rs +++ b/src/sliding_sync.rs @@ -711,16 +711,19 @@ pub enum MatrixRequest { destination: Arc>, update_sender: Option>, }, - /// Fetches a media attachment in full and writes it to `save_path`. - /// Shows a popup on success/failure; if `update_sender` is set, also - /// emits started/finished updates so the source timeline can show a - /// spinner. + /// Reqeust to fetch a media attachment/file and save it to the given path. + /// + /// Sends a [`TimelineUpdate::AttachmentDownloadFinished`] upon success or failure. DownloadMediaToFile { media_source: MediaSource, save_path: PathBuf, filename: String, update_sender: Option>, }, + /// Abort an in-flight download started by `DownloadMediaToFile`. + /// No-op if the worker doesn't know about this mxc (already finished, + /// already cancelled, or the original request hasn't been processed yet). + CancelDownload(OwnedMxcUri), } /// Submits a request to the worker thread to be executed asynchronously. @@ -768,6 +771,11 @@ async fn matrix_worker_task( let mut subscribers_own_user_read_receipts: HashMap> = HashMap::new(); // The async tasks that are spawned to subscribe to changes in the pinned events for each room. let mut subscribers_pinned_events: HashMap> = HashMap::new(); + // Abort handles for in-flight attachment-download tasks, keyed by mxc. + // Shared with each spawned task via `Arc` so the task can self-remove + // when it ends. `CancelDownload` looks up the handle and aborts it. + let download_tasks: Arc>> + = Arc::new(Mutex::new(HashMap::new())); while let Some(request) = request_receiver.recv().await { match request { @@ -2183,24 +2191,29 @@ async fn matrix_worker_task( MatrixRequest::DownloadMediaToFile { media_source, save_path, filename, update_sender } => { let Some(client) = get_client() else { continue }; - Handle::current().spawn(async move { - let mxc_uri = match &media_source { - MediaSource::Plain(uri) => uri.clone(), - MediaSource::Encrypted(file) => file.url.clone(), - }; + let mxc_uri = match &media_source { + MediaSource::Plain(uri) => uri.clone(), + MediaSource::Encrypted(file) => file.url.clone(), + }; + let (abort_handle, abort_registration) = futures_util::future::AbortHandle::new_pair(); + let mxc_for_task = mxc_uri.clone(); + let mxc_for_finished = mxc_uri.clone(); + let tasks_for_cleanup = download_tasks.clone(); + let download_future = async move { let media_request = MediaRequestParameters { source: media_source, format: matrix_sdk::media::MediaFormat::File, }; - match client.media().get_media_content(&media_request, true).await { + let result: Result<(), String> = match client.media().get_media_content(&media_request, true).await { Ok(bytes) => match tokio::fs::write(&save_path, &bytes).await { Ok(()) => { log!("Saved downloaded attachment {filename:?} to {}", save_path.display()); enqueue_popup_notification( - format!("Saved \"{filename}\" to {}", save_path.display()), + format!("Downloaded \"{filename}\"."), PopupKind::Success, - Some(5.0), + Some(crate::shared::attachment_download::DOWNLOAD_RESULT_DURATION_SECS), ); + Ok(()) } Err(e) => { error!("Failed to write downloaded attachment {filename:?} to {}: {e}", save_path.display()); @@ -2209,6 +2222,7 @@ async fn matrix_worker_task( PopupKind::Error, None, ); + Err(e.to_string()) } } Err(e) => { @@ -2218,15 +2232,40 @@ async fn matrix_worker_task( PopupKind::Error, None, ); + Err(e.to_string()) } - } - if let Some(sender) = update_sender.as_ref() { - let _ = sender.send(TimelineUpdate::AttachmentDownloadFinished(mxc_uri)); + }; + if let Some(sender) = update_sender { + let _ = sender.send(TimelineUpdate::AttachmentDownloadFinished(mxc_for_finished, result)); SignalToUI::set_ui_signal(); + // Drop the success/failure indicator after a short delay. + let reset_sender = sender.clone(); + Handle::current().spawn(async move { + tokio::time::sleep(std::time::Duration::from_secs_f64( + crate::shared::attachment_download::DOWNLOAD_RESULT_DURATION_SECS, + )).await; + let _ = reset_sender.send(TimelineUpdate::AttachmentDownloadReset(mxc_for_task)); + SignalToUI::set_ui_signal(); + }); } + }; + // Wrap in Abortable so `CancelDownload` and the offline path + // can cleanly stop the fetch+write. + let mxc_for_cleanup = mxc_uri.clone(); + download_tasks.lock().unwrap().insert(mxc_uri, abort_handle); + Handle::current().spawn(async move { + let _ = futures_util::future::Abortable::new(download_future, abort_registration).await; + // Self-clean: drop our handle entry. (No-op if `CancelDownload` already removed us.) + tasks_for_cleanup.lock().unwrap().remove(&mxc_for_cleanup); }); } + MatrixRequest::CancelDownload(mxc) => { + if let Some(abort) = download_tasks.lock().unwrap().remove(&mxc) { + abort.abort(); + } + } + } } From 7a144782f810bd301090f4eb191092765b96fafb Mon Sep 17 00:00:00 2001 From: Kevin Boos Date: Wed, 27 May 2026 10:48:22 -0700 Subject: [PATCH 4/6] fix button hover, improve download icon a bit --- resources/icons/download.svg | 4 ++-- src/home/room_screen.rs | 21 +++++++++++++++++++-- src/shared/attachment_download.rs | 2 +- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/resources/icons/download.svg b/resources/icons/download.svg index 4d32ae4ba..46f9e6022 100644 --- a/resources/icons/download.svg +++ b/resources/icons/download.svg @@ -1,5 +1,5 @@ - - + + diff --git a/src/home/room_screen.rs b/src/home/room_screen.rs index bce737394..55086ba08 100644 --- a/src/home/room_screen.rs +++ b/src/home/room_screen.rs @@ -5109,6 +5109,9 @@ pub struct Message { /// what to save when the user clicks it. `None` for plain text messages, /// which hide the download button entirely. #[rust] download_info: Option, + /// Cached so `set_data` can reset_hover only on the button that just + /// transitioned into visibility, not on every redraw. + #[rust] download_state: DownloadDisplayState, } impl Widget for Message { @@ -5306,15 +5309,18 @@ impl Message { download_info: Option, download_state: DownloadDisplayState, ) { + let prev_section_visible = self.download_info.is_some(); + let prev_state = self.download_state; + self.details = Some(details); self.download_info = download_info; let section_visible = self.download_info.is_some(); - self.view.view(cx, ids!(content.download_section)) - .set_visible(cx, section_visible); + self.view.view(cx, ids!(content.download_section)).set_visible(cx, section_visible); if let Some(info) = self.download_info.as_ref() { let download_button = self.view.button(cx, ids!(content.download_section.download_button)); let downloading_view = self.view.view(cx, ids!(content.download_section.downloading_view)); + let cancel_button = self.view.button(cx, ids!(content.download_section.downloading_view.cancel_button)); let success_button = self.view.button(cx, ids!(content.download_section.success_button)); let failure_button = self.view.button(cx, ids!(content.download_section.failure_button)); download_button.set_text(cx, info.kind.button_text()); @@ -5322,7 +5328,18 @@ impl Message { downloading_view.set_visible(cx, matches!(download_state, DownloadDisplayState::InProgress)); success_button.set_visible(cx, matches!(download_state, DownloadDisplayState::Succeeded)); failure_button.set_visible(cx, matches!(download_state, DownloadDisplayState::Failed)); + // Only reset hover for the button that is just now becoming visible. + let newly_visible = !prev_section_visible || prev_state != download_state; + if newly_visible { + match download_state { + DownloadDisplayState::Idle => download_button.reset_hover(cx), + DownloadDisplayState::InProgress => cancel_button.reset_hover(cx), + DownloadDisplayState::Succeeded => success_button.reset_hover(cx), + DownloadDisplayState::Failed => failure_button.reset_hover(cx), + } + } } + self.download_state = download_state; } } diff --git a/src/shared/attachment_download.rs b/src/shared/attachment_download.rs index 9942b73f2..a259c92ea 100644 --- a/src/shared/attachment_download.rs +++ b/src/shared/attachment_download.rs @@ -158,7 +158,7 @@ pub fn save_loaded_attachment(cx: &mut Cx, info: DownloadableAttachment, bytes: let save_path = handle.path().to_path_buf(); match std::fs::write(&save_path, &bytes[..]) { Ok(()) => enqueue_popup_notification( - format!("Saved \"{}\" to {}", info.filename, save_path.display()), + format!("Downloaded \"{}\".", info.filename), PopupKind::Success, Some(5.0), ), From 78a76e24944b221a447547c4b01eeaeb014af71c Mon Sep 17 00:00:00 2001 From: Kevin Boos Date: Wed, 27 May 2026 10:52:43 -0700 Subject: [PATCH 5/6] fix typo --- src/sliding_sync.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sliding_sync.rs b/src/sliding_sync.rs index 6120bccd9..6c471909b 100644 --- a/src/sliding_sync.rs +++ b/src/sliding_sync.rs @@ -711,7 +711,7 @@ pub enum MatrixRequest { destination: Arc>, update_sender: Option>, }, - /// Reqeust to fetch a media attachment/file and save it to the given path. + /// Request to fetch a media attachment/file and save it to the given path. /// /// Sends a [`TimelineUpdate::AttachmentDownloadFinished`] upon success or failure. DownloadMediaToFile { From cac3641505fc7f54e2bd8440960ce4e3d87536aa Mon Sep 17 00:00:00 2001 From: Kevin Boos Date: Wed, 27 May 2026 11:20:00 -0700 Subject: [PATCH 6/6] more cleanup, code simplification --- src/shared/attachment_download.rs | 53 ++++++++++++------------------- src/shared/image_viewer.rs | 15 ++------- src/sliding_sync.rs | 12 ++----- 3 files changed, 26 insertions(+), 54 deletions(-) diff --git a/src/shared/attachment_download.rs b/src/shared/attachment_download.rs index a259c92ea..61fe27fa9 100644 --- a/src/shared/attachment_download.rs +++ b/src/shared/attachment_download.rs @@ -1,5 +1,4 @@ -//! Save a Matrix media attachment to disk via the rfd save dialog. -//! Used by the inline message button and the image viewer overlay. +//! Download a Matrix media attachment and save it to storage. use std::sync::Arc; use makepad_widgets::Cx; @@ -17,24 +16,21 @@ pub fn media_source_mxc(source: &MediaSource) -> &OwnedMxcUri { } } -/// One entry in `TimelineUiState.pending_downloads`. Tracks an attachment -/// download's lifecycle so the inline button can render the right view. +/// Info about a download that has begun or recently completed. pub struct PendingDownload { pub mxc: OwnedMxcUri, pub state: PendingDownloadState, } pub enum PendingDownloadState { - /// Fetch+write in flight. The matrix worker owns the corresponding - /// `AbortHandle`; `MessageAction::CancelDownload` routes through - /// `MatrixRequest::CancelDownload` to abort it. + /// The download request has been submitted to and is being handled by + /// the backend worker task. InProgress, - /// Bytes hit disk. Shown for a few seconds before the entry is cleared. + /// The download was successful, and will show a success indicator for a few seconds. JustSucceeded, - /// Fetch or write failed. Shown for a few seconds before the entry is cleared. + /// The download failed, and will show an error indicator for a few seconds. JustFailed, } - impl PendingDownloadState { pub fn display(&self) -> DownloadDisplayState { match self { @@ -45,26 +41,24 @@ impl PendingDownloadState { } } -/// What the inline download section should render. Decoupled from -/// `PendingDownloadState` so the `Message` widget doesn't need to know -/// about `AbortHandle`s. +/// What the download section below a message should show. #[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] pub enum DownloadDisplayState { - /// Show the "Download …" button. + /// Default: show the download button. #[default] Idle, - /// Show the spinner + cancel button. + /// Show a loading spinner and cancel button. InProgress, - /// Briefly show a green "Saved" indicator. + /// Briefly show a green success button. Succeeded, - /// Briefly show a red "Failed" indicator. + /// Briefly show a red failed button. Failed, } /// How long (in seconds) the success/failure state stays visible before resetting the button. pub const DOWNLOAD_RESULT_DURATION_SECS: f64 = 5.0; - +/// Metadata describing an attachment/media file to be downloaded. #[derive(Clone, Debug)] pub struct DownloadableAttachment { pub media_source: MediaSource, @@ -105,14 +99,12 @@ fn build_save_dialog(info: &DownloadableAttachment) -> rfd::AsyncFileDialog { } } -/// Opens the save dialog, then submits the actual fetch+write request. -/// Pass `update_sender` if the caller wants spinner updates routed back to -/// a specific timeline; pass `None` from contexts without one (e.g. the -/// image viewer overlay). +/// Opens the save dialog, then submits a request to fetch and write the file. +/// +/// If `update_sender` is provided, it will be used to send progress updates to a timeline. /// -/// The dialog runs on a fresh OS thread (not a tokio task) because rfd's -/// macOS backend falls back to a sync dialog and panics when called from -/// a tokio worker thread. +/// The save dialog runs on a newly-spawned OS-native thread (not a tokio task) +/// because `rfd` requires it, at least on macOS. #[cfg(not(any(target_os = "ios", target_os = "android")))] pub fn start_attachment_download( cx: &mut Cx, @@ -124,6 +116,7 @@ pub fn start_attachment_download( let dialog_task = build_save_dialog(&info).save_file(); cx.spawn_thread(move || { match futures::executor::block_on(dialog_task) { + // If Some, the user chose a valid location from the file dialog. Some(handle) => { submit_async_request(MatrixRequest::DownloadMediaToFile { media_source: info.media_source, @@ -132,9 +125,7 @@ pub fn start_attachment_download( update_sender, }); } - // User cancelled. The action handler already marked this mxc as - // pending. Revert directly (skip the success/failure flash, since - // dismissing the dialog isn't really a failure). + // If None, the user cancelled the file dialog. None => { if let Some(sender) = update_sender { let mxc = media_source_mxc(&info.media_source).clone(); @@ -146,10 +137,7 @@ pub fn start_attachment_download( }); } -/// Like `start_attachment_download` but for callers that already have the -/// bytes in memory (e.g. the image viewer). Skips the matrix worker -/// round-trip and writes straight to disk on the same OS thread that -/// hosted the save dialog. +/// Saves an attachment already in memory directly to storage. #[cfg(not(any(target_os = "ios", target_os = "android")))] pub fn save_loaded_attachment(cx: &mut Cx, info: DownloadableAttachment, bytes: Arc<[u8]>) { let dialog_task = build_save_dialog(&info).save_file(); @@ -171,7 +159,6 @@ pub fn save_loaded_attachment(cx: &mut Cx, info: DownloadableAttachment, bytes: }); } -/// Mobile: rfd doesn't have a save dialog there, so just tell the user. #[cfg(any(target_os = "ios", target_os = "android"))] pub fn start_attachment_download( _cx: &mut Cx, diff --git a/src/shared/image_viewer.rs b/src/shared/image_viewer.rs index 9920377f0..0b8ab139f 100644 --- a/src/shared/image_viewer.rs +++ b/src/shared/image_viewer.rs @@ -493,12 +493,9 @@ struct ImageViewer { /// from the continuous `FingerHoverOver` events that fire every frame. #[rust] last_mouse_pos: DVec2, #[rust] capped_dimension: DVec2, - /// Drives the overlay download button: visible when `Some`, fed to - /// `start_attachment_download` on click. + /// Info about how to download the image being shown. #[rust] downloadable: Option, - /// Image bytes from the most recent `LoadState::Loaded`. Lets the - /// download button skip the SDK fetch round-trip and write straight - /// to disk. + /// A reference to the image being shown so we can easily save it to storage. #[rust] loaded_bytes: Option>, } @@ -841,8 +838,6 @@ impl MatchEvent for ImageViewer { && let Some(info) = self.downloadable.clone() { was_overlay_button_clicked = true; - // Use the already-loaded bytes if we have them, otherwise fall - // back to the matrix-worker fetch path. if let Some(bytes) = self.loaded_bytes.clone() { save_loaded_attachment(cx, info, bytes); } else { @@ -850,9 +845,7 @@ impl MatchEvent for ImageViewer { } } - // Restart the auto-hide timer if any overlay button was clicked. If the - // mouse is still over the overlay the hover handler keeps the timer - // stopped anyway. + // Restart the auto-hide timer if any overlay button was clicked. if was_overlay_button_clicked && !self.mouse_over_overlay_ui { cx.stop_timer(self.hide_ui_timer); self.hide_ui_timer = cx.start_timeout(SHOW_UI_DURATION); @@ -1159,8 +1152,6 @@ impl ImageViewer { .set_date_time(cx, timestamp); } - // New image starting to load: any cached bytes from a previous one - // are now stale, so a download click can't accidentally save them. self.loaded_bytes = None; self.downloadable = metadata.downloadable.clone(); self.view.button(cx, ids!(download_button)) diff --git a/src/sliding_sync.rs b/src/sliding_sync.rs index 6c471909b..52b9bcb1f 100644 --- a/src/sliding_sync.rs +++ b/src/sliding_sync.rs @@ -720,9 +720,7 @@ pub enum MatrixRequest { filename: String, update_sender: Option>, }, - /// Abort an in-flight download started by `DownloadMediaToFile`. - /// No-op if the worker doesn't know about this mxc (already finished, - /// already cancelled, or the original request hasn't been processed yet). + /// Request to cancel an in-progress download. CancelDownload(OwnedMxcUri), } @@ -771,9 +769,7 @@ async fn matrix_worker_task( let mut subscribers_own_user_read_receipts: HashMap> = HashMap::new(); // The async tasks that are spawned to subscribe to changes in the pinned events for each room. let mut subscribers_pinned_events: HashMap> = HashMap::new(); - // Abort handles for in-flight attachment-download tasks, keyed by mxc. - // Shared with each spawned task via `Arc` so the task can self-remove - // when it ends. `CancelDownload` looks up the handle and aborts it. + // Abort handles for in-progress download tasks, keyed by MxcUri. let download_tasks: Arc>> = Arc::new(Mutex::new(HashMap::new())); @@ -2249,13 +2245,11 @@ async fn matrix_worker_task( }); } }; - // Wrap in Abortable so `CancelDownload` and the offline path - // can cleanly stop the fetch+write. + let mxc_for_cleanup = mxc_uri.clone(); download_tasks.lock().unwrap().insert(mxc_uri, abort_handle); Handle::current().spawn(async move { let _ = futures_util::future::Abortable::new(download_future, abort_registration).await; - // Self-clean: drop our handle entry. (No-op if `CancelDownload` already removed us.) tasks_for_cleanup.lock().unwrap().remove(&mxc_for_cleanup); }); }