diff --git a/src/session/view/content/room_history/event_row.rs b/src/session/view/content/room_history/event_row.rs index 953b150e..f3b31e5f 100644 --- a/src/session/view/content/room_history/event_row.rs +++ b/src/session/view/content/room_history/event_row.rs @@ -1,7 +1,6 @@ use adw::{prelude::*, subclass::prelude::*}; use gtk::{gdk, gio, glib, glib::clone}; use matrix_sdk_ui::timeline::TimelineEventItemId; -use tracing::error; use super::{EventActionsGroup, MessageRow, RoomHistory, StateRow}; use crate::{ @@ -41,31 +40,6 @@ mod imp { fn class_init(klass: &mut Self::Class) { klass.set_css_name("event-row"); klass.set_accessible_role(gtk::AccessibleRole::ListItem); - - klass.install_action( - "event-row.enable-copy-image", - Some(&bool::static_variant_type()), - |obj, _, param| { - let enable = param - .and_then(glib::Variant::get::) - .expect("The parameter should be a boolean"); - let imp = obj.imp(); - - let Some(action_group) = imp.action_group.borrow().clone() else { - error!("Could not change state of copy-image action: no action group"); - return; - }; - let Some(action) = action_group.lookup_action("copy-image") else { - error!("Could not change state of copy-image action: action not found"); - return; - }; - let Some(action) = action.downcast_ref::() else { - error!("Could not change state of copy-image action: not a GSimpleAction"); - return; - }; - action.set_enabled(enable); - }, - ); } } @@ -295,7 +269,30 @@ mod imp { let child = obj.child_or_default::(); child.set_event(event); } else { - let child = obj.child_or_default::(); + let child = obj.child_or_else::(|| { + let child = MessageRow::default(); + + child.connect_texture_notify(clone!( + #[weak(rename_to = imp)] + self, + move |child| { + let Some(copy_image_action) = imp + .action_group + .borrow() + .as_ref() + .and_then(|action_group| action_group.lookup_action("copy-image")) + .and_downcast::() + else { + return; + }; + + copy_image_action.set_enabled(child.texture().is_some()); + } + )); + + child + }); + child.set_event(event); } } @@ -333,6 +330,14 @@ mod imp { let action_group = self.event_actions_group(); let has_context_menu = action_group.is_some(); + if let Some(copy_image_action) = action_group + .as_ref() + .and_then(|action_group| action_group.lookup_action("copy-image")) + .and_downcast::() + { + copy_image_action.set_enabled(self.texture().is_some()); + } + obj.insert_action_group("event", action_group.as_ref()); self.action_group.replace(action_group); obj.set_has_context_menu(has_context_menu); diff --git a/src/session/view/content/room_history/message_row/content.rs b/src/session/view/content/room_history/message_row/content.rs index 7f44824c..7beb55ec 100644 --- a/src/session/view/content/room_history/message_row/content.rs +++ b/src/session/view/content/room_history/message_row/content.rs @@ -41,7 +41,7 @@ pub enum ContentFormat { } mod imp { - use std::cell::Cell; + use std::{cell::Cell, marker::PhantomData}; use super::*; @@ -51,6 +51,12 @@ mod imp { /// The displayed format of the message. #[property(get, set = Self::set_format, explicit_notify, builder(ContentFormat::default()))] format: Cell, + /// The texture of the image preview displayed by the descendant of this + /// widget, if any. + #[property(get = Self::texture)] + texture: PhantomData>, + /// The widget with the visual media content of the event, if any. + pub(super) visual_media_widget: glib::WeakRef, } #[glib::object_subclass] @@ -76,6 +82,62 @@ mod imp { self.format.set(format); self.obj().notify_format(); } + + /// The texture of the image preview displayed by the descendant of this + /// widget, if any. + fn texture(&self) -> Option { + self.visual_media_widget.upgrade()?.texture() + } + + /// Update the current visual media widget if necessary. + pub(super) fn update_visual_media_widget(&self) { + let prev_widget = self.visual_media_widget.upgrade(); + let current_widget = self.visual_media_widget(); + + if prev_widget == current_widget { + return; + } + + let obj = self.obj(); + + if let Some(visual_media) = ¤t_widget { + visual_media.connect_texture_notify(clone!( + #[weak] + obj, + move |_| { + obj.notify_texture(); + } + )); + } + self.visual_media_widget.set(current_widget.as_ref()); + + let prev_texture = prev_widget.and_then(|visual_media| visual_media.texture()); + let current_texture = current_widget.and_then(|visual_media| visual_media.texture()); + if prev_texture != current_texture { + obj.notify_texture(); + } + } + + /// The widget with the visual media content of the event, if any. + /// + /// This allows to access the descendant content while discarding the + /// content of a related message, like a replied-to event, or the + /// caption of the event. + fn visual_media_widget(&self) -> Option { + let mut child = self.obj().child()?; + + // If it is a reply, the media is in the main content. + if let Some(reply) = child.downcast_ref::() { + child = reply.content().child()?; + } + + // If it is a caption, the media is the child of the caption. + if let Some(caption) = child.downcast_ref::() { + child = caption.child()?; + } + + child.downcast::().ok() + } } } @@ -90,25 +152,9 @@ impl MessageContent { glib::Object::new() } - /// Access the widget with the visual media content of the event, if any. - /// - /// This allows to access the descendant content while discarding the - /// content of a related message, like a replied-to event, or the caption of - /// the event. + /// The widget with the visual media content of the event, if any. pub(crate) fn visual_media_widget(&self) -> Option { - let mut child = BinExt::child(self)?; - - // If it is a reply, the media is in the main content. - if let Some(reply) = child.downcast_ref::() { - child = BinExt::child(reply.content())?; - } - - // If it is a caption, the media is the child of the caption. - if let Some(caption) = child.downcast_ref::() { - child = caption.child()?; - } - - child.downcast::().ok() + self.imp().visual_media_widget.upgrade() } /// Update this widget to present the given `Event`. @@ -136,7 +182,9 @@ impl MessageContent { TimelineDetails::Error(error) => { error!( "Could not fetch replied to event '{}': {error}", - event.reply_to_id().unwrap() + event + .reply_to_id() + .expect("reply event should have replied-to event ID") ); } TimelineDetails::Ready(replied_to_event) => { @@ -169,7 +217,9 @@ impl MessageContent { event.transaction_id(), event.event_id(), ); - BinExt::set_child(self, Some(&reply)); + self.set_child(Some(&reply)); + + self.imp().update_visual_media_widget(); return; } @@ -186,6 +236,8 @@ impl MessageContent { event.transaction_id(), event.event_id(), ); + + self.imp().update_visual_media_widget(); } /// Update this widget to present the given related event. @@ -209,11 +261,6 @@ impl MessageContent { }, ); } - - /// Get the texture displayed by this widget, if any. - pub(crate) fn texture(&self) -> Option { - self.visual_media_widget()?.texture() - } } impl IsABin for MessageContent {} diff --git a/src/session/view/content/room_history/message_row/mod.rs b/src/session/view/content/room_history/message_row/mod.rs index 8e7aa547..ac510b50 100644 --- a/src/session/view/content/room_history/message_row/mod.rs +++ b/src/session/view/content/room_history/message_row/mod.rs @@ -61,6 +61,10 @@ mod imp { /// This is ignored if this event doesn’t have a header. #[property(get = Self::show_header, set = Self::set_show_header, explicit_notify)] show_header: PhantomData, + /// The texture of the image preview displayed by the descendant of this + /// widget, if any. + #[property(get = Self::texture)] + texture: PhantomData>, } #[glib::object_subclass] @@ -86,6 +90,7 @@ mod imp { impl ObjectImpl for MessageRow { fn constructed(&self) { self.parent_constructed(); + let obj = self.obj(); self.content.connect_format_notify(clone!( #[weak(rename_to = imp)] @@ -97,6 +102,13 @@ mod imp { )); } )); + self.content.connect_texture_notify(clone!( + #[weak] + obj, + move |_| { + obj.notify_texture(); + } + )); let system_settings = Application::default().system_settings(); let system_settings_handler = system_settings.connect_clock_format_notify(clone!( @@ -281,11 +293,6 @@ impl MessageRow { pub fn new() -> Self { glib::Object::new() } - - /// Get the texture displayed by this widget, if any. - pub(crate) fn texture(&self) -> Option { - self.imp().texture() - } } impl Default for MessageRow { diff --git a/src/session/view/content/room_history/message_row/visual_media.rs b/src/session/view/content/room_history/message_row/visual_media.rs index 124b4209..5571d0f1 100644 --- a/src/session/view/content/room_history/message_row/visual_media.rs +++ b/src/session/view/content/room_history/message_row/visual_media.rs @@ -39,7 +39,10 @@ const PLACEHOLDER_PAGE: &str = "placeholder"; const MEDIA_PAGE: &str = "media"; mod imp { - use std::cell::{Cell, RefCell}; + use std::{ + cell::{Cell, RefCell}, + marker::PhantomData, + }; use glib::subclass::InitializingObject; @@ -97,6 +100,9 @@ mod imp { /// The current video file, if any. file: RefCell>, paintable_animation_ref: RefCell>, + /// The texture of the current image preview, if any. + #[property(get = Self::texture)] + texture: PhantomData>, } #[glib::object_subclass] @@ -239,6 +245,8 @@ mod imp { /// /// Removes the previous media child if one was set. fn set_media_child(&self, child: Option<&impl IsA>) { + let prev_texture = self.texture(); + if let Some(prev_child) = self.stack.child_by_name(MEDIA_PAGE) { self.stack.remove(&prev_child); } @@ -246,6 +254,10 @@ mod imp { if let Some(child) = child { self.stack.add_named(child, Some(MEDIA_PAGE)); } + + if self.texture() != prev_texture { + self.obj().notify_texture(); + } } /// Set the state of the media. @@ -409,6 +421,19 @@ mod imp { self.update_visible_page(); } + /// The texture of the current image preview, if any. + fn texture(&self) -> Option { + let paintable = self + .media_child::() + .and_then(|p| p.paintable())?; + + if let Some(paintable) = paintable.downcast_ref::() { + paintable.current_texture() + } else { + paintable.downcast().ok() + } + } + /// Set the visual media message to display. pub(super) fn set_media_message( &self, @@ -627,11 +652,6 @@ mod imp { return; } - // Disable the copy-image action while the image is loading. - if matches!(media_message, VisualMediaMessage::Image(_)) { - self.enable_copy_image_action(false); - } - let scale_factor = self.obj().scale_factor(); let settings = ThumbnailSettings { dimensions: FrameDimensions::thumbnail_max_dimensions(scale_factor), @@ -657,16 +677,18 @@ mod imp { return; } - let child = if let Some(child) = self.media_child::() { - child + let paintable = gdk::Paintable::from(image); + + if let Some(child) = self.media_child::() { + child.set_paintable(Some(&paintable)); + self.obj().notify_texture(); } else { let child = gtk::Picture::builder() .content_fit(gtk::ContentFit::ScaleDown) .build(); + child.set_paintable(Some(&paintable)); self.set_media_child(Some(&child)); - child - }; - child.set_paintable(Some(&gdk::Paintable::from(image))); + } if matches!(&media_message, VisualMediaMessage::Sticker(_)) { self.overlay.remove_css_class("opaque-bg"); @@ -675,27 +697,6 @@ mod imp { } self.set_state(LoadingState::Ready); - - // Enable the copy-image action now that the image is loaded. - if matches!(media_message, VisualMediaMessage::Image(_)) { - self.enable_copy_image_action(true); - } - } - - /// Enable or disable the context menu action to copy the image. - fn enable_copy_image_action(&self, enable: bool) { - if self.compact.get() { - // In its compact form the message does not have actions. - return; - } - - if self - .obj() - .activate_action("event-row.enable-copy-image", Some(&enable.to_variant())) - .is_err() - { - error!("Could not change state of copy-image action: `event-row.enable-copy-image` action not found"); - } } /// Build the content for the video in the given media message. @@ -828,20 +829,6 @@ impl MessageVisualMedia { self.imp() .set_media_message(media_message, room, format, cache_key); } - - /// Get the texture displayed by this widget, if any. - pub(crate) fn texture(&self) -> Option { - let paintable = self - .imp() - .media_child::() - .and_then(|p| p.paintable())?; - - if let Some(paintable) = paintable.downcast_ref::() { - paintable.current_texture() - } else { - paintable.downcast().ok() - } - } } impl Default for MessageVisualMedia {