window: Fix arrow key bindings not working

The actions for global accels have to be at window level.

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};

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: &gtk::PanDirection) {
+    pub fn pan(&self, direction: &gtk::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

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: &gtk::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, &["<Ctrl>Up"])
-                }
-                ActionPartGlobal::PanRight => {
-                    application.set_accels_for_action(&action, &["<Ctrl>Right"])
-                }
-                ActionPartGlobal::PanDown => {
-                    application.set_accels_for_action(&action, &["<Ctrl>Down"])
-                }
-                ActionPartGlobal::PanLeft => {
-                    application.set_accels_for_action(&action, &["<Ctrl>Left"])
-                }
-            }
-        }
-    }
-
-    pub fn remove_accels(application: &gtk::Application) {
-        for action in Self::iter() {
-            application.set_accels_for_action(&action, &[]);
-        }
-    }
-
-    pub fn init_actions_and_bindings(
-        klass: &mut <super::imp::LpImageWindow as ObjectSubclass>::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(&gtk::PanDirection::Up);
-                    });
-                }
-                ActionPartGlobal::PanRight => {
-                    klass.install_action(&action, None, move |win, _, _| {
-                        win.pan(&gtk::PanDirection::Right);
-                    });
-                }
-                ActionPartGlobal::PanDown => {
-                    klass.install_action(&action, None, move |win, _, _| {
-                        win.pan(&gtk::PanDirection::Down);
-                    });
-                }
-                ActionPartGlobal::PanLeft => {
-                    klass.install_action(&action, None, move |win, _, _| {
-                        win.pan(&gtk::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 {

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<Self>) {
@@ -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: &gtk::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, &["<Ctrl>Up"]),
+                Self::PanRight => application.set_accels_for_action(&action, &["<Ctrl>Right"]),
+                Self::PanDown => application.set_accels_for_action(&action, &["<Ctrl>Down"]),
+                Self::PanLeft => application.set_accels_for_action(&action, &["<Ctrl>Left"]),
+            }
+        }
+    }
+
+    pub fn remove_accels(application: &gtk::Application) {
+        for action in Self::iter() {
+            application.set_accels_for_action(&action, &[]);
+        }
+    }
+
+    pub fn init_actions_and_bindings(klass: &mut <imp::LpWindow as ObjectSubclass>::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(&gtk::PanDirection::Up);
+                    });
+                }
+                ActionPartGlobal::PanRight => {
+                    klass.install_action(&action, None, move |win, _, _| {
+                        win.image_window().pan(&gtk::PanDirection::Right);
+                    });
+                }
+                ActionPartGlobal::PanDown => {
+                    klass.install_action(&action, None, move |win, _, _| {
+                        win.image_window().pan(&gtk::PanDirection::Down);
+                    });
+                }
+                ActionPartGlobal::PanLeft => {
+                    klass.install_action(&action, None, move |win, _, _| {
+                        win.image_window().pan(&gtk::PanDirection::Left);
+                    });
+                }
+            }
+        }
+    }
+}
+
+impl Deref for ActionPartGlobal {
+    type Target = str;
+    fn deref(&self) -> &Self::Target {
+        self.as_ref()
+    }
 }