From 3f82a5f43529acaee64c1fbacfae9b75e0434f44 Mon Sep 17 00:00:00 2001 From: Sophie Herold Date: Mon, 16 Dec 2024 21:27:20 +0100 Subject: [PATCH] window: Fix arrow key bindings not working The actions for global accels have to be at window level. --- src/widgets/edit_window.rs | 2 +- src/widgets/image_window.rs | 10 ++- src/widgets/image_window/actions.rs | 122 ---------------------------- src/widgets/window.rs | 121 +++++++++++++++++++++++++++ 4 files changed, 128 insertions(+), 127 deletions(-) diff --git a/src/widgets/edit_window.rs b/src/widgets/edit_window.rs index 86def4d..f6a56cd 100644 --- a/src/widgets/edit_window.rs +++ b/src/widgets/edit_window.rs @@ -16,11 +16,11 @@ // SPDX-License-Identifier: GPL-3.0-or-later use std::cell::{OnceCell, RefCell}; +use std::sync::Arc; use adw::prelude::*; use adw::subclass::prelude::*; use glycin::Operations; -use std::sync::Arc; use super::edit::LpEditCrop; use super::{LpImage, LpWindow}; diff --git a/src/widgets/image_window.rs b/src/widgets/image_window.rs index 1a60bae..260d810 100644 --- a/src/widgets/image_window.rs +++ b/src/widgets/image_window.rs @@ -43,6 +43,7 @@ use crate::config; use crate::deps::*; use crate::util::gettext::*; use crate::util::Direction; +use crate::widgets::window::ActionPartGlobal; use crate::widgets::{ LpDragOverlay, LpImage, LpImageView, LpPropertiesView, LpWindow, LpWindowContent, }; @@ -146,8 +147,6 @@ mod imp { ); // Set up actions - - ActionPartGlobal::init_actions_and_bindings(klass); Action::init_actions_and_bindings(klass); } } @@ -355,6 +354,8 @@ mod imp { obj, move |_| obj.update_accel_status() )); + obj.connect_unmap(|win| win.update_accel_status()); + obj.connect_map(|win| win.update_accel_status()); self.status_page .set_icon_name(Some(&format!("{}-symbolic", config::APP_ID))); @@ -476,7 +477,7 @@ impl LpImageWindow { } } - fn pan(&self, direction: >k::PanDirection) { + pub fn pan(&self, direction: >k::PanDirection) { if let Some(image) = self.imp().image_view.current_image() { image.pan(direction); } @@ -905,8 +906,9 @@ impl LpImageWindow { // Only change status if active window if self.window().is_active() { - if self.window().visible_dialog().is_some() { + if self.window().visible_dialog().is_some() || !self.is_mapped() { // If AdwDialog is visible, remove global accels that are for the main window + // Same for this not being visible, so potentially editing view is open ActionPartGlobal::remove_accels(&application); } else { // Add accels if viewing main window diff --git a/src/widgets/image_window/actions.rs b/src/widgets/image_window/actions.rs index 3c57d5d..91eb640 100644 --- a/src/widgets/image_window/actions.rs +++ b/src/widgets/image_window/actions.rs @@ -24,121 +24,6 @@ use strum::IntoEnumIterator; use crate::deps::*; use crate::util::{Direction, Position}; -/// Actions that need some global accels -/// -/// The app level accels are needed for arrow key bindings. Because the arrow -/// keys are usually used for widget navigation, it's not possible to overwrite -/// them on key-binding level. -#[derive(strum::Display, strum::AsRefStr, strum::EnumIter)] -pub enum ActionPartGlobal { - #[strum(to_string = "win.image-left-instant")] - ImageLeftInstant, - #[strum(to_string = "win.image-right-instant")] - ImageRightInstant, - // Pan - #[strum(to_string = "win.pan-up")] - PanUp, - #[strum(to_string = "win.pan-right")] - PanRight, - #[strum(to_string = "win.pan-down")] - PanDown, - #[strum(to_string = "win.pan-left")] - PanLeft, -} - -impl ActionPartGlobal { - pub fn add_accels(application: >k::Application) { - for action in Self::iter() { - match action { - ActionPartGlobal::ImageRightInstant => { - application.set_accels_for_action(&action, &["Right"]) - } - ActionPartGlobal::ImageLeftInstant => { - application.set_accels_for_action(&action, &["Left"]) - } - ActionPartGlobal::PanUp => { - application.set_accels_for_action(&action, &["Up"]) - } - ActionPartGlobal::PanRight => { - application.set_accels_for_action(&action, &["Right"]) - } - ActionPartGlobal::PanDown => { - application.set_accels_for_action(&action, &["Down"]) - } - ActionPartGlobal::PanLeft => { - application.set_accels_for_action(&action, &["Left"]) - } - } - } - } - - pub fn remove_accels(application: >k::Application) { - for action in Self::iter() { - application.set_accels_for_action(&action, &[]); - } - } - - pub fn init_actions_and_bindings( - klass: &mut ::Class, - ) { - for action in Self::iter() { - match action { - ActionPartGlobal::ImageLeftInstant => { - klass.install_action(&action, None, move |win, _, _| { - if win.direction() == gtk::TextDirection::Rtl { - win.imp().image_view.navigate(Direction::Forward, false); - } else { - win.imp().image_view.navigate(Direction::Back, false); - } - }); - klass.add_binding_action( - gdk::Key::Page_Down, - gdk::ModifierType::empty(), - &action, - ); - } - - ActionPartGlobal::ImageRightInstant => { - klass.install_action(&action, None, move |win, _, _| { - if win.direction() == gtk::TextDirection::Rtl { - win.imp().image_view.navigate(Direction::Back, false); - } else { - win.imp().image_view.navigate(Direction::Forward, false); - } - }); - klass.add_binding_action( - gdk::Key::Page_Up, - gdk::ModifierType::empty(), - &action, - ); - } - - // Pan - ActionPartGlobal::PanUp => { - klass.install_action(&action, None, move |win, _, _| { - win.pan(>k::PanDirection::Up); - }); - } - ActionPartGlobal::PanRight => { - klass.install_action(&action, None, move |win, _, _| { - win.pan(>k::PanDirection::Right); - }); - } - ActionPartGlobal::PanDown => { - klass.install_action(&action, None, move |win, _, _| { - win.pan(>k::PanDirection::Down); - }); - } - ActionPartGlobal::PanLeft => { - klass.install_action(&action, None, move |win, _, _| { - win.pan(>k::PanDirection::Left); - }); - } - } - } - } -} - #[derive(strum::Display, strum::AsRefStr, strum::EnumIter)] pub enum Action { // Navigation @@ -521,13 +406,6 @@ impl Action { } } -impl Deref for ActionPartGlobal { - type Target = str; - fn deref(&self) -> &Self::Target { - self.as_ref() - } -} - impl Deref for Action { type Target = str; fn deref(&self) -> &Self::Target { diff --git a/src/widgets/window.rs b/src/widgets/window.rs index a65f2cd..39e59cc 100644 --- a/src/widgets/window.rs +++ b/src/widgets/window.rs @@ -15,13 +15,17 @@ // // SPDX-License-Identifier: GPL-3.0-or-later +use std::ops::Deref; + use adw::prelude::*; use adw::subclass::prelude::*; use gio::glib::VariantTy; use gtk::CompositeTemplate; +use strum::IntoEnumIterator; use crate::config; use crate::deps::*; +use crate::util::Direction; use crate::widgets::{LpEditWindow, LpImageView, LpImageWindow}; /// Show window after X milliseconds even if image dimensions are not known yet @@ -90,6 +94,8 @@ mod imp { } }, ); + + ActionPartGlobal::init_actions_and_bindings(klass); } fn instance_init(obj: &glib::subclass::InitializingObject) { @@ -218,4 +224,119 @@ impl LpWindow { pub fn image_view(&self) -> LpImageView { self.imp().image_window.image_view() } + + pub fn image_window(&self) -> LpImageWindow { + self.imp().image_window.clone() + } +} + +/// Actions that need some global accels +/// +/// The app level accels are needed for arrow key bindings. Because the arrow +/// keys are usually used for widget navigation, it's not possible to overwrite +/// them on key-binding level. +/// +/// These have to be registered at window level to be callable from +/// GtkApplication. +#[derive(strum::Display, strum::AsRefStr, strum::EnumIter)] +pub enum ActionPartGlobal { + #[strum(to_string = "win.image-left-instant")] + ImageLeftInstant, + #[strum(to_string = "win.image-right-instant")] + ImageRightInstant, + // Pan + #[strum(to_string = "win.pan-up")] + PanUp, + #[strum(to_string = "win.pan-right")] + PanRight, + #[strum(to_string = "win.pan-down")] + PanDown, + #[strum(to_string = "win.pan-left")] + PanLeft, +} + +impl ActionPartGlobal { + pub fn add_accels(application: >k::Application) { + for action in Self::iter() { + match action { + Self::ImageRightInstant => application.set_accels_for_action(&action, &["Right"]), + Self::ImageLeftInstant => application.set_accels_for_action(&action, &["Left"]), + Self::PanUp => application.set_accels_for_action(&action, &["Up"]), + Self::PanRight => application.set_accels_for_action(&action, &["Right"]), + Self::PanDown => application.set_accels_for_action(&action, &["Down"]), + Self::PanLeft => application.set_accels_for_action(&action, &["Left"]), + } + } + } + + pub fn remove_accels(application: >k::Application) { + for action in Self::iter() { + application.set_accels_for_action(&action, &[]); + } + } + + pub fn init_actions_and_bindings(klass: &mut ::Class) { + for action in Self::iter() { + match action { + ActionPartGlobal::ImageLeftInstant => { + klass.install_action(&action, None, move |win, _, _| { + if win.direction() == gtk::TextDirection::Rtl { + win.image_view().navigate(Direction::Forward, false); + } else { + win.image_view().navigate(Direction::Back, false); + } + }); + klass.add_binding_action( + gdk::Key::Page_Down, + gdk::ModifierType::empty(), + &action, + ); + } + + ActionPartGlobal::ImageRightInstant => { + klass.install_action(&action, None, move |win, _, _| { + if win.direction() == gtk::TextDirection::Rtl { + win.image_view().navigate(Direction::Back, false); + } else { + win.image_view().navigate(Direction::Forward, false); + } + }); + klass.add_binding_action( + gdk::Key::Page_Up, + gdk::ModifierType::empty(), + &action, + ); + } + + // Pan + ActionPartGlobal::PanUp => { + klass.install_action(&action, None, move |win, _, _| { + win.image_window().pan(>k::PanDirection::Up); + }); + } + ActionPartGlobal::PanRight => { + klass.install_action(&action, None, move |win, _, _| { + win.image_window().pan(>k::PanDirection::Right); + }); + } + ActionPartGlobal::PanDown => { + klass.install_action(&action, None, move |win, _, _| { + win.image_window().pan(>k::PanDirection::Down); + }); + } + ActionPartGlobal::PanLeft => { + klass.install_action(&action, None, move |win, _, _| { + win.image_window().pan(>k::PanDirection::Left); + }); + } + } + } + } +} + +impl Deref for ActionPartGlobal { + type Target = str; + fn deref(&self) -> &Self::Target { + self.as_ref() + } }