From b04eff6c81850ca648721f39ff02becfb0995aca Mon Sep 17 00:00:00 2001 From: Marco Melorio Date: Wed, 24 Aug 2022 22:07:07 +0200 Subject: [PATCH] room-history: Move item activation code to MessageRow The previous way of tracking message activation was via `GtkListView`'s "activate" signal, which was not ideal since it required a complicated management of each items' "activatable" property. This new implementation is simpler since it just adds a `GtkClickGesture` to the message rows that require activation and it handles the activation in the `MessageRow`. This is also useful for implementing a media viewer opening animation (which is planned in the next commits) because we can retrieve the actual widget of the media widget being pressed, which was not possible previously. --- data/resources/ui/content-message-media.ui | 5 +++ data/resources/ui/content-room-history.ui | 1 - .../content/room_history/message_row/media.rs | 8 +++++ .../content/room_history/message_row/mod.rs | 18 ++++++++++ src/session/content/room_history/mod.rs | 20 ++--------- src/session/room/event/mod.rs | 14 -------- src/session/room/timeline/timeline_item.rs | 33 ------------------- 7 files changed, 33 insertions(+), 66 deletions(-) diff --git a/data/resources/ui/content-message-media.ui b/data/resources/ui/content-message-media.ui index f165fb3c..2788545c 100644 --- a/data/resources/ui/content-message-media.ui +++ b/data/resources/ui/content-message-media.ui @@ -9,6 +9,11 @@ hidden + + + + + center diff --git a/data/resources/ui/content-room-history.ui b/data/resources/ui/content-room-history.ui index c6213c4d..481c0942 100644 --- a/data/resources/ui/content-room-history.ui +++ b/data/resources/ui/content-room-history.ui @@ -202,7 +202,6 @@ - True Room History diff --git a/src/session/content/room_history/message_row/media.rs b/src/session/content/room_history/message_row/media.rs index d6343bd2..c8b3f6d7 100644 --- a/src/session/content/room_history/message_row/media.rs +++ b/src/session/content/room_history/message_row/media.rs @@ -87,6 +87,7 @@ mod imp { fn class_init(klass: &mut Self::Class) { Self::bind_template(klass); + Self::Type::bind_template_callbacks(klass); } fn instance_init(obj: &InitializingObject) { @@ -234,12 +235,19 @@ glib::wrapper! { @extends gtk::Widget, @implements gtk::Accessible; } +#[gtk::template_callbacks] impl MessageMedia { /// Create a new media message. pub fn new() -> Self { glib::Object::new(&[]) } + #[template_callback] + fn handle_release(&self) { + self.activate_action("message-row.show-media", None) + .unwrap(); + } + /// The intended display width of the media. pub fn width(&self) -> i32 { self.imp().width.get() diff --git a/src/session/content/room_history/message_row/mod.rs b/src/session/content/room_history/message_row/mod.rs index 1f560358..d302b477 100644 --- a/src/session/content/room_history/message_row/mod.rs +++ b/src/session/content/room_history/message_row/mod.rs @@ -14,6 +14,7 @@ use gtk::{ glib::{clone, signal::SignalHandlerId}, CompositeTemplate, }; +use matrix_sdk::{room::timeline::TimelineItemContent, ruma::events::room::message::MessageType}; pub use self::content::ContentFormat; use self::{content::MessageContent, reaction_list::MessageReactionList}; @@ -55,6 +56,10 @@ mod imp { fn class_init(klass: &mut Self::Class) { Self::bind_template(klass); + + klass.install_action("message-row.show-media", None, move |obj, _, _| { + obj.show_media(); + }); } fn instance_init(obj: &InitializingObject) { @@ -201,4 +206,17 @@ impl MessageRow { pub fn texture(&self) -> Option { self.imp().content.texture() } + + fn show_media(&self) { + if let Some(event) = self.imp().event.borrow().as_ref() { + if let TimelineItemContent::Message(content) = event.content() { + if matches!( + content.msgtype(), + MessageType::Image(_) | MessageType::Video(_) + ) { + event.room().session().show_media(event); + } + } + } + } } diff --git a/src/session/content/room_history/mod.rs b/src/session/content/room_history/mod.rs index 6103133c..45a0b201 100644 --- a/src/session/content/room_history/mod.rs +++ b/src/session/content/room_history/mod.rs @@ -50,9 +50,7 @@ use crate::{ i18n::gettext_f, session::{ content::{room_details, RoomDetails}, - room::{ - Event, EventKey, Room, RoomAction, RoomType, Timeline, TimelineItem, TimelineState, - }, + room::{Event, EventKey, Room, RoomAction, RoomType, Timeline, TimelineState}, user::UserExt, }, spawn, spawn_tokio, toast, @@ -301,8 +299,8 @@ mod imp { factory.connect_setup(clone!(@weak obj => move |_, item| { let row = ItemRow::new(&obj); item.set_child(Some(&row)); - ItemRow::this_expression("item").chain_property::("activatable").bind(item, "activatable", Some(&row)); item.bind_property("item", &row, "item").build(); + item.set_activatable(false); item.set_selectable(false); })); self.listview.set_factory(Some(&factory)); @@ -311,20 +309,6 @@ mod imp { self.listview .set_vscroll_policy(gtk::ScrollablePolicy::Natural); - self.listview - .connect_activate(clone!(@weak obj => move |listview, pos| { - if let Some(event) = listview - .model() - .and_then(|model| model.item(pos)) - .as_ref() - .and_then(|o| o.downcast_ref::()) - { - if let Some(room) = obj.room() { - room.session().show_media(event); - } - } - })); - obj.set_sticky(true); let adj = self.listview.vadjustment().unwrap(); diff --git a/src/session/room/event/mod.rs b/src/session/room/event/mod.rs index c6f286b4..d1f52ab9 100644 --- a/src/session/room/event/mod.rs +++ b/src/session/room/event/mod.rs @@ -144,19 +144,6 @@ mod imp { } } - fn activatable(&self) -> bool { - match self.obj().content() { - // The event can be activated to open the media viewer if it's an image or a video. - TimelineItemContent::Message(message) => { - matches!( - message.msgtype(), - MessageType::Image(_) | MessageType::Video(_) - ) - } - _ => false, - } - } - fn can_hide_header(&self) -> bool { match self.obj().content() { TimelineItemContent::Message(message) => { @@ -254,7 +241,6 @@ impl Event { ); imp.item.replace(Some(item)); - self.notify("activatable"); self.notify("source"); } diff --git a/src/session/room/timeline/timeline_item.rs b/src/session/room/timeline/timeline_item.rs index 183b344c..71b7b5b0 100644 --- a/src/session/room/timeline/timeline_item.rs +++ b/src/session/room/timeline/timeline_item.rs @@ -19,7 +19,6 @@ mod imp { pub parent_class: glib::object::ObjectClass, pub is_visible: fn(&super::TimelineItem) -> bool, pub selectable: fn(&super::TimelineItem) -> bool, - pub activatable: fn(&super::TimelineItem) -> bool, pub can_hide_header: fn(&super::TimelineItem) -> bool, pub event_sender: fn(&super::TimelineItem) -> Option, } @@ -38,11 +37,6 @@ mod imp { (klass.as_ref().selectable)(this) } - pub(super) fn timeline_item_activatable(this: &super::TimelineItem) -> bool { - let klass = this.class(); - (klass.as_ref().activatable)(this) - } - pub(super) fn timeline_item_can_hide_header(this: &super::TimelineItem) -> bool { let klass = this.class(); (klass.as_ref().can_hide_header)(this) @@ -76,9 +70,6 @@ mod imp { glib::ParamSpecBoolean::builder("selectable") .read_only() .build(), - glib::ParamSpecBoolean::builder("activatable") - .read_only() - .build(), glib::ParamSpecBoolean::builder("show-header") .explicit_notify() .build(), @@ -107,7 +98,6 @@ mod imp { match pspec.name() { "is-visible" => obj.is_visible().to_value(), "selectable" => obj.selectable().to_value(), - "activatable" => obj.activatable().to_value(), "show-header" => obj.show_header().to_value(), "can-hide-header" => obj.can_hide_header().to_value(), "event-sender" => obj.event_sender().to_value(), @@ -181,11 +171,6 @@ pub trait TimelineItemExt: 'static { /// Defaults to `false`. fn selectable(&self) -> bool; - /// Whether this `TimelineItem` is activatable. - /// - /// Defaults to `false`. - fn activatable(&self) -> bool; - /// Whether this `TimelineItem` should show its header. /// /// Defaults to `false`. @@ -214,10 +199,6 @@ impl> TimelineItemExt for O { imp::timeline_item_selectable(self.upcast_ref()) } - fn activatable(&self) -> bool { - imp::timeline_item_activatable(self.upcast_ref()) - } - fn show_header(&self) -> bool { self.upcast_ref().imp().show_header.get() } @@ -256,10 +237,6 @@ pub trait TimelineItemImpl: ObjectImpl { false } - fn activatable(&self) -> bool { - false - } - fn can_hide_header(&self) -> bool { false } @@ -282,7 +259,6 @@ where klass.is_visible = is_visible_trampoline::; klass.selectable = selectable_trampoline::; - klass.activatable = activatable_trampoline::; klass.can_hide_header = can_hide_header_trampoline::; klass.event_sender = event_sender_trampoline::; } @@ -307,15 +283,6 @@ where this.imp().selectable() } -fn activatable_trampoline(this: &TimelineItem) -> bool -where - T: ObjectSubclass + TimelineItemImpl, - T::Type: IsA, -{ - let this = this.downcast_ref::().unwrap(); - this.imp().activatable() -} - fn can_hide_header_trampoline(this: &TimelineItem) -> bool where T: ObjectSubclass + TimelineItemImpl,