[fractal] context-menu-bin: Create popup only when needed
- From: Julian Sparber <jsparber src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [fractal] context-menu-bin: Create popup only when needed
- Date: Wed, 6 Apr 2022 13:57:34 +0000 (UTC)
commit eae73592858d55134cce833239ea950ff70f7524
Author: Julian Sparber <julian sparber net>
Date: Wed Apr 6 14:35:24 2022 +0200
context-menu-bin: Create popup only when needed
This reduces the time needed to build a Sidebar::RoomRow and
RoomHisotry::MessageRow, by a sinificant amount.
data/resources/ui/sidebar-room-row.ui | 3 +-
src/components/context_menu_bin.rs | 128 ++++++++-------------------
src/session/content/room_history/item_row.rs | 109 +++++++----------------
src/session/sidebar/room_row.rs | 8 +-
4 files changed, 79 insertions(+), 169 deletions(-)
---
diff --git a/data/resources/ui/sidebar-room-row.ui b/data/resources/ui/sidebar-room-row.ui
index ba2c0ce84..d8ab375e8 100644
--- a/data/resources/ui/sidebar-room-row.ui
+++ b/data/resources/ui/sidebar-room-row.ui
@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<interface>
- <menu id="room-row-menu">
+ <menu id="room_row_menu">
<section>
<item>
<attribute name="label" translatable="yes">_Accept</attribute>
@@ -54,7 +54,6 @@
</section>
</menu>
<template class="SidebarRoomRow" parent="ContextMenuBin">
- <property name="context-menu">room-row-menu</property>
<child>
<object class="GtkBox">
<property name="spacing">12</property>
diff --git a/src/components/context_menu_bin.rs b/src/components/context_menu_bin.rs
index e20672c90..758f61b27 100644
--- a/src/components/context_menu_bin.rs
+++ b/src/components/context_menu_bin.rs
@@ -1,35 +1,23 @@
use adw::subclass::prelude::*;
-use gtk::{gdk, gio, glib, glib::clone, prelude::*, subclass::prelude::*, CompositeTemplate};
+use gtk::{gdk, glib, glib::clone, prelude::*, subclass::prelude::*, CompositeTemplate};
use log::debug;
mod imp {
+ use std::cell::RefCell;
+
use glib::subclass::InitializingObject;
use super::*;
+ type FactoryFn = RefCell<Option<Box<dyn Fn(&super::ContextMenuBin, >k::PopoverMenu)>>>;
- #[derive(Debug, CompositeTemplate)]
+ #[derive(Default, CompositeTemplate)]
#[template(resource = "/org/gnome/Fractal/context-menu-bin.ui")]
pub struct ContextMenuBin {
#[template_child]
pub click_gesture: TemplateChild<gtk::GestureClick>,
#[template_child]
pub long_press_gesture: TemplateChild<gtk::GestureLongPress>,
- pub popover: gtk::PopoverMenu,
- }
-
- impl Default for ContextMenuBin {
- fn default() -> Self {
- Self {
- click_gesture: Default::default(),
- long_press_gesture: Default::default(),
- // WORKAROUND: there is some issue with creating the popover from the template
- popover: gtk::PopoverMenu::builder()
- .position(gtk::PositionType::Bottom)
- .has_arrow(false)
- .halign(gtk::Align::Start)
- .build(),
- }
- }
+ pub factory: FactoryFn,
}
#[glib::object_subclass]
@@ -56,10 +44,6 @@ mod imp {
"context-menu.activate",
None,
);
-
- klass.install_action("context-menu.close", None, move |widget, _, _| {
- widget.imp().popover.popdown();
- });
}
fn instance_init(obj: &InitializingObject<Self>) {
@@ -68,45 +52,7 @@ mod imp {
}
impl ObjectImpl for ContextMenuBin {
- fn properties() -> &'static [glib::ParamSpec] {
- use once_cell::sync::Lazy;
- static PROPERTIES: Lazy<Vec<glib::ParamSpec>> = Lazy::new(|| {
- vec![glib::ParamSpecObject::new(
- "context-menu",
- "Context Menu",
- "The context menu",
- gio::MenuModel::static_type(),
- glib::ParamFlags::READWRITE | glib::ParamFlags::EXPLICIT_NOTIFY,
- )]
- });
-
- PROPERTIES.as_ref()
- }
-
- fn set_property(
- &self,
- obj: &Self::Type,
- _id: usize,
- value: &glib::Value,
- pspec: &glib::ParamSpec,
- ) {
- match pspec.name() {
- "context-menu" => {
- obj.set_context_menu(value.get::<Option<gio::MenuModel>>().unwrap().as_ref())
- }
- _ => unimplemented!(),
- }
- }
-
- fn property(&self, obj: &Self::Type, _id: usize, pspec: &glib::ParamSpec) -> glib::Value {
- match pspec.name() {
- "context-menu" => obj.context_menu().to_value(),
- _ => unimplemented!(),
- }
- }
-
fn constructed(&self, obj: &Self::Type) {
- self.popover.set_parent(obj);
self.long_press_gesture
.connect_pressed(clone!(@weak obj => move |gesture, x, y| {
gesture.set_state(gtk::EventSequenceState::Claimed);
@@ -126,10 +72,6 @@ mod imp {
);
self.parent_constructed(obj);
}
-
- fn dispose(&self, _obj: &Self::Type) {
- self.popover.unparent();
- }
}
impl WidgetImpl for ContextMenuBin {}
@@ -149,46 +91,50 @@ impl ContextMenuBin {
}
fn open_menu_at(&self, x: i32, y: i32) {
- let popover = &self.imp().popover;
-
debug!("Context menu was activated");
+ if let Some(factory) = &*self.imp().factory.borrow() {
+ let popover = gtk::PopoverMenu::builder()
+ .position(gtk::PositionType::Bottom)
+ .has_arrow(false)
+ .halign(gtk::Align::Start)
+ .build();
- if popover.menu_model().is_none() {
- return;
- }
+ popover.set_parent(self);
+
+ popover.connect_closed(|popover| {
+ popover.unparent();
+ });
+
+ (factory)(self, &popover);
- popover.set_pointing_to(Some(&gdk::Rectangle::new(x, y, 0, 0)));
- popover.popup();
+ popover.set_pointing_to(Some(&gdk::Rectangle::new(x, y, 0, 0)));
+ popover.popup();
+ }
}
}
pub trait ContextMenuBinExt: 'static {
- /// Set the `MenuModel` used in the context menu.
- fn set_context_menu(&self, menu: Option<&gio::MenuModel>);
-
- /// Get the `MenuModel` used in the context menu.
- fn context_menu(&self) -> Option<gio::MenuModel>;
+ /// Set the closure used to create the content of the `gtk::PopoverMenu`
+ fn set_factory<F>(&self, factory: F)
+ where
+ F: Fn(&Self, >k::PopoverMenu) + 'static;
- /// Get the `PopoverMenu` used in the context menu.
- fn popover(&self) -> >k::PopoverMenu;
+ fn remove_factory(&self);
}
impl<O: IsA<ContextMenuBin>> ContextMenuBinExt for O {
- fn set_context_menu(&self, menu: Option<&gio::MenuModel>) {
- if self.context_menu().as_ref() == menu {
- return;
- }
-
- self.upcast_ref().imp().popover.set_menu_model(menu);
- self.notify("context-menu");
- }
-
- fn context_menu(&self) -> Option<gio::MenuModel> {
- self.upcast_ref().imp().popover.menu_model()
+ fn set_factory<F>(&self, factory: F)
+ where
+ F: Fn(&O, >k::PopoverMenu) + 'static,
+ {
+ let f = move |obj: &ContextMenuBin, popover: >k::PopoverMenu| {
+ factory(obj.downcast_ref::<O>().unwrap(), popover);
+ };
+ self.upcast_ref().imp().factory.replace(Some(Box::new(f)));
}
- fn popover(&self) -> >k::PopoverMenu {
- &self.upcast_ref().imp().popover
+ fn remove_factory(&self) {
+ self.upcast_ref().imp().factory.take();
}
}
diff --git a/src/session/content/room_history/item_row.rs b/src/session/content/room_history/item_row.rs
index 7e3eb30b5..8c253ae11 100644
--- a/src/session/content/room_history/item_row.rs
+++ b/src/session/content/room_history/item_row.rs
@@ -7,7 +7,7 @@ use crate::{
components::{ContextMenuBin, ContextMenuBinExt, ContextMenuBinImpl, ReactionChooser},
session::{
content::room_history::{message_row::MessageRow, DividerRow, StateRow},
- room::{Event, EventActions, Item, ItemType, ReactionList},
+ room::{Event, EventActions, Item, ItemType},
},
};
@@ -73,7 +73,7 @@ mod imp {
}
}
- fn dispose(&self, obj: &Self::Type) {
+ fn dispose(&self, _obj: &Self::Type) {
if let Some(ItemType::Event(event)) =
self.item.borrow().as_ref().map(|item| item.type_())
{
@@ -81,8 +81,6 @@ mod imp {
event.disconnect(handler);
}
}
-
- obj.remove_reaction_chooser();
}
}
@@ -126,21 +124,25 @@ impl ItemRow {
if let Some(ref item) = item {
match item.type_() {
ItemType::Event(event) => {
- let action_group = self.set_event_actions(Some(event));
-
if event.message_content().is_some() {
- self.set_context_menu(Some(Self::event_message_menu_model()));
- self.set_reaction_chooser(event.reactions());
-
- // Open emoji chooser
- let more_reactions = gio::SimpleAction::new("more-reactions", None);
- more_reactions.connect_activate(clone!(@weak self as obj => move |_, _| {
- obj.show_emoji_chooser();
+ let action_group = self.set_event_actions(Some(event)).unwrap();
+ self.set_factory(clone!(@weak event => move |obj, popover| {
+ popover.set_menu_model(Some(Self::event_message_menu_model()));
+ let reaction_chooser = ReactionChooser::new();
+ reaction_chooser.set_reactions(Some(event.reactions().to_owned()));
+ popover.add_child(&reaction_chooser, "reaction-chooser");
+
+ // Open emoji chooser
+ let more_reactions = gio::SimpleAction::new("more-reactions", None);
+ more_reactions.connect_activate(clone!(@weak obj, @weak popover => move |_, _| {
+ obj.show_emoji_chooser(&popover);
+ }));
+ action_group.add_action(&more_reactions);
}));
- action_group.unwrap().add_action(&more_reactions);
} else {
- self.set_context_menu(Some(Self::event_state_menu_model()));
- self.remove_reaction_chooser();
+ self.set_factory(|_, popover| {
+ popover.set_menu_model(Some(Self::event_state_menu_model()));
+ });
}
let event_notify_handler = event.connect_notify_local(
@@ -158,11 +160,8 @@ impl ItemRow {
self.set_event_widget(event);
}
ItemType::DayDivider(date) => {
- if self.context_menu().is_some() {
- self.set_context_menu(None);
- self.set_event_actions(None);
- self.remove_reaction_chooser();
- }
+ self.remove_factory();
+ self.set_event_actions(None);
let fmt = if date.year() == glib::DateTime::now_local().unwrap().year() {
// Translators: This is a date format in the day divider without the year
@@ -181,11 +180,8 @@ impl ItemRow {
};
}
ItemType::NewMessageDivider => {
- if self.context_menu().is_some() {
- self.set_context_menu(None);
- self.set_event_actions(None);
- self.remove_reaction_chooser();
- }
+ self.remove_factory();
+ self.set_event_actions(None);
let label = gettext("New Messages");
@@ -241,59 +237,22 @@ impl ItemRow {
}
}
- /// Set the reaction chooser for the given `reactions`.
- ///
- /// If it doesn't exist, it is created
- fn set_reaction_chooser(&self, reactions: &ReactionList) {
- let priv_ = self.imp();
-
- if priv_.reaction_chooser.borrow().is_none() {
- let reaction_chooser = ReactionChooser::new();
- self.popover()
- .add_child(&reaction_chooser, "reaction-chooser");
- priv_.reaction_chooser.replace(Some(reaction_chooser));
- }
-
- priv_
- .reaction_chooser
- .borrow()
- .as_ref()
- .unwrap()
- .set_reactions(Some(reactions.to_owned()));
- }
-
- /// Remove the reaction chooser and the emoji chooser, if they exist.
- fn remove_reaction_chooser(&self) {
- let priv_ = self.imp();
-
- if let Some(reaction_chooser) = priv_.reaction_chooser.take() {
- reaction_chooser.unparent();
- }
-
- if let Some(emoji_chooser) = priv_.emoji_chooser.take() {
+ fn show_emoji_chooser(&self, popover: >k::PopoverMenu) {
+ let emoji_chooser = gtk::EmojiChooser::builder().has_arrow(false).build();
+ emoji_chooser.connect_emoji_picked(|emoji_chooser, emoji| {
+ emoji_chooser
+ .activate_action("event.toggle-reaction", Some(&emoji.to_variant()))
+ .unwrap();
+ });
+ emoji_chooser.set_parent(self);
+ emoji_chooser.connect_closed(|emoji_chooser| {
emoji_chooser.unparent();
- }
- }
-
- fn show_emoji_chooser(&self) {
- let priv_ = self.imp();
-
- if priv_.emoji_chooser.borrow().is_none() {
- let emoji_chooser = gtk::EmojiChooser::builder().has_arrow(false).build();
- emoji_chooser.connect_emoji_picked(|emoji_chooser, emoji| {
- emoji_chooser
- .activate_action("event.toggle-reaction", Some(&emoji.to_variant()))
- .unwrap();
- });
- emoji_chooser.set_parent(self);
- priv_.emoji_chooser.replace(Some(emoji_chooser));
- }
+ });
- let emoji_chooser = priv_.emoji_chooser.borrow().clone().unwrap();
- let (_, rectangle) = self.popover().pointing_to();
+ let (_, rectangle) = popover.pointing_to();
emoji_chooser.set_pointing_to(Some(&rectangle));
- self.popover().popdown();
+ popover.popdown();
emoji_chooser.popup();
}
}
diff --git a/src/session/sidebar/room_row.rs b/src/session/sidebar/room_row.rs
index f7a93f8ef..1e7d20da0 100644
--- a/src/session/sidebar/room_row.rs
+++ b/src/session/sidebar/room_row.rs
@@ -2,7 +2,7 @@ use adw::subclass::prelude::BinImpl;
use gtk::{gdk, glib, glib::clone, prelude::*, subclass::prelude::*, CompositeTemplate};
use crate::{
- components::{ContextMenuBin, ContextMenuBinImpl},
+ components::{ContextMenuBin, ContextMenuBinExt, ContextMenuBinImpl},
session::room::{HighlightFlags, Room, RoomType},
};
@@ -24,6 +24,8 @@ mod imp {
pub display_name: TemplateChild<gtk::Label>,
#[template_child]
pub notification_count: TemplateChild<gtk::Label>,
+ #[template_child]
+ pub room_row_menu: TemplateChild<gtk::gio::MenuModel>,
}
#[glib::object_subclass]
@@ -109,6 +111,10 @@ mod imp {
fn constructed(&self, obj: &Self::Type) {
self.parent_constructed(obj);
+ obj.set_factory(|obj, popover| {
+ popover.set_menu_model(Some(&obj.imp().room_row_menu.get()));
+ });
+
// Allow to drag rooms
let drag = gtk::DragSource::builder()
.actions(gdk::DragAction::MOVE)
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
