Rework modal actions (#2739)

lenemter · web-flow · commit b9b9dde03aa2 · 2026-01-28T11:42:42.000+03:00
We currently have duplicate keybinding filters across multiple components. This PR removes keybinding filter capabilities from modal proxies and instead introduces ModalActions flags. Then WindowManager filters keybindings based on these flags
diff --git a/lib/Gestures/Gesture.vala b/lib/Gestures/Gesture.vala
@@ -41,7 +41,22 @@ namespace Gala {
         MULTITASKING_VIEW,
         ZOOM,
         CUSTOM,
-        N_ACTIONS
+        N_ACTIONS;
+
+        public ModalActions to_modal_action () {
+            switch (this) {
+                case SWITCH_WORKSPACE:
+                    return ModalActions.SWITCH_WORKSPACE;
+                case SWITCH_WINDOWS:
+                    return ModalActions.SWITCH_WINDOWS;
+                case MULTITASKING_VIEW:
+                    return ModalActions.MULTITASKING_VIEW;
+                case ZOOM:
+                    return ModalActions.ZOOM;
+                default:
+                    return ModalActions.NONE;
+            }
+        }
     }
 
     private class Gesture {
diff --git a/lib/Gestures/GestureController.vala b/lib/Gestures/GestureController.vala
@@ -156,8 +156,10 @@ public class Gala.GestureController : Object {
         }
 
         var recognized_action = GestureSettings.get_action (gesture, out _action_info);
-        recognizing = !wm.filter_action (recognized_action) && recognized_action == action ||
-            backend == scroll_backend && recognized_action == NONE;
+        recognizing = (
+            recognized_action == action && !wm.filter_action (recognized_action.to_modal_action ()) ||
+            backend == scroll_backend && recognized_action == NONE
+        );
 
         if (recognizing) {
             if (gesture.direction == UP || gesture.direction == RIGHT || gesture.direction == OUT) {
diff --git a/lib/WindowManager.vala b/lib/WindowManager.vala
@@ -23,10 +23,20 @@ namespace Gala {
         public const string TOGGLE_RECORDING_ACTION = "toggle-recording-action";
     }
 
-    /**
-     * Function that should return true if the given shortcut should be blocked.
-     */
-    public delegate bool KeybindingFilter (Meta.KeyBinding binding);
+    [Flags]
+    public enum ModalActions {
+        NONE = 0,
+        SWITCH_WORKSPACE,
+        SWITCH_WINDOWS,
+        MULTITASKING_VIEW,
+        WINDOW_OVERVIEW,
+        ZOOM,
+        LOCATE_POINTER,
+        SCREENSHOT,
+        SCREENSHOT_AREA,
+        SCREENSHOT_WINDOW,
+        MEDIA_KEYS
+    }
 
     /**
      * A minimal class mostly used to identify your call to {@link WindowManager.push_modal} and used
@@ -35,38 +45,16 @@ namespace Gala {
     public class ModalProxy : Object {
         public Clutter.Grab? grab { get; set; }
 
-        private GestureAction[] allowed_actions;
-
-        /**
-         * A function which is called whenever a keybinding is pressed. If you supply a custom
-         * one you can filter out those that'd you like to be passed through and block all others.
-         * Defaults to blocking all.
-         * @see KeybindingFilter
-         */
-        private KeybindingFilter? _keybinding_filter = () => true;
-        public unowned KeybindingFilter? get_keybinding_filter () {
-            return _keybinding_filter;
-        }
-
-        public void set_keybinding_filter (KeybindingFilter? filter) {
-            _keybinding_filter = filter;
-        }
+        private ModalActions allowed_actions;
 
         public ModalProxy () {
         }
 
-        /**
-         * Small utility to allow all keybindings
-         */
-        public void allow_all_keybindings () {
-            _keybinding_filter = null;
-        }
-
-        public void allow_actions (GestureAction[] actions) {
+        public void allow_actions (ModalActions actions) {
             allowed_actions = actions;
         }
 
-        public bool filter_action (GestureAction action) {
+        public bool filter_action (ModalActions action) {
             return !(action in allowed_actions);
         }
     }
@@ -172,6 +160,6 @@ namespace Gala {
          * Checks whether the action should currently be prohibited.
          * @return true if the action should be prohibited, false otherwise
          */
-        public abstract bool filter_action (GestureAction action);
+        public abstract bool filter_action (ModalActions action);
     }
 }
diff --git a/src/Widgets/ModalGroup.vala b/src/Widgets/ModalGroup.vala
@@ -70,7 +70,7 @@ public class Gala.ModalGroup : Clutter.Actor {
 
             visible = true;
             modal_proxy = wm.push_modal (this, false);
-            modal_proxy.allow_actions ({ ZOOM });
+            modal_proxy.allow_actions (ZOOM | LOCATE_POINTER | SCREENSHOT | SCREENSHOT_AREA | SCREENSHOT_WINDOW);
         }
 
         if (dimmed.size == 1) {
diff --git a/src/Widgets/MultitaskingView/MultitaskingView.vala b/src/Widgets/MultitaskingView/MultitaskingView.vala
@@ -249,8 +249,7 @@ public class Gala.MultitaskingView : ActorTarget, RootTarget, ActivatableCompone
             grab_key_focus ();
 
             modal_proxy = wm.push_modal (get_stage (), false);
-            modal_proxy.set_keybinding_filter (keybinding_filter);
-            modal_proxy.allow_actions ({ MULTITASKING_VIEW, SWITCH_WORKSPACE, ZOOM });
+            modal_proxy.allow_actions (MULTITASKING_VIEW | SWITCH_WORKSPACE | ZOOM | LOCATE_POINTER | MEDIA_KEYS | SCREENSHOT | SCREENSHOT_AREA);
         } else if (action == MULTITASKING_VIEW) {
             DragDropAction.cancel_all_by_id ("multitaskingview-window");
         }
@@ -432,28 +431,6 @@ public class Gala.MultitaskingView : ActorTarget, RootTarget, ActivatableCompone
         multitasking_gesture_controller.goto (0);
     }
 
-    private bool keybinding_filter (Meta.KeyBinding binding) {
-        var action = Meta.Prefs.get_keybinding_action (binding.get_name ());
-
-        switch (action) {
-            case Meta.KeyBindingAction.NONE:
-            case Meta.KeyBindingAction.LOCATE_POINTER_KEY:
-                return false;
-            default:
-                break;
-        }
-
-        switch (binding.get_name ()) {
-            case "screenshot":
-            case "screenshot-clip":
-                return false;
-            default:
-                break;
-        }
-
-        return true;
-    }
-
     public override bool captured_event (Clutter.Event event) {
         /* If we aren't open but receive events this means we are animating closed
          * or we are finishing a workspace switch animation. In any case we want to
diff --git a/src/Widgets/WindowOverview.vala b/src/Widgets/WindowOverview.vala
@@ -106,8 +106,7 @@ public class Gala.WindowOverview : ActorTarget, RootTarget, ActivatableComponent
         grab_key_focus ();
 
         modal_proxy = wm.push_modal (this, true);
-        modal_proxy.set_keybinding_filter (keybinding_filter);
-        modal_proxy.allow_actions ({ ZOOM });
+        modal_proxy.allow_actions (WINDOW_OVERVIEW | ZOOM | LOCATE_POINTER | MEDIA_KEYS | SCREENSHOT | SCREENSHOT_AREA);
 
         unowned var display = wm.get_display ();
 
@@ -147,27 +146,6 @@ public class Gala.WindowOverview : ActorTarget, RootTarget, ActivatableComponent
         gesture_controller.goto (1);
     }
 
-    private bool keybinding_filter (Meta.KeyBinding binding) {
-        var action = Meta.Prefs.get_keybinding_action (binding.get_name ());
-
-        switch (action) {
-            case Meta.KeyBindingAction.NONE:
-            case Meta.KeyBindingAction.LOCATE_POINTER_KEY:
-                return false;
-            default:
-                break;
-        }
-
-        switch (binding.get_name ()) {
-            case "expose-all-windows":
-                return false;
-            default:
-                break;
-        }
-
-        return true;
-    }
-
     private bool window_filter_func (Object obj) requires (obj is Meta.Window) {
         var window = (Meta.Window) obj;
         return window_ids == null || (window.get_id () in window_ids);
diff --git a/src/Widgets/WindowSwitcher/WindowSwitcher.vala b/src/Widgets/WindowSwitcher/WindowSwitcher.vala
@@ -443,21 +443,7 @@ public class Gala.WindowSwitcher : CanvasActor, GestureTarget, RootTarget {
 
     private void push_modal () {
         modal_proxy = wm.push_modal (get_stage (), true);
-        modal_proxy.allow_actions ({ SWITCH_WINDOWS });
-        modal_proxy.set_keybinding_filter ((binding) => {
-            var action = Meta.Prefs.get_keybinding_action (binding.get_name ());
-
-            switch (action) {
-                case Meta.KeyBindingAction.NONE:
-                case Meta.KeyBindingAction.LOCATE_POINTER_KEY:
-                    return false;
-                default:
-                    break;
-            }
-
-            return true;
-        });
-
+        modal_proxy.allow_actions (SWITCH_WINDOWS | LOCATE_POINTER | MEDIA_KEYS);
     }
 
     private void close_switcher (uint32 time, bool cancel = false) {
diff --git a/src/WindowManager.vala b/src/WindowManager.vala
@@ -1650,6 +1650,10 @@ namespace Gala {
                 case Meta.KeyBindingAction.SWITCH_GROUP:
                 case Meta.KeyBindingAction.SWITCH_GROUP_BACKWARD:
                     return filter_action (SWITCH_WINDOWS);
+                case Meta.KeyBindingAction.LOCATE_POINTER_KEY:
+                    return filter_action (LOCATE_POINTER);
+                case Meta.KeyBindingAction.NONE:
+                    return filter_action (MEDIA_KEYS);
                 default:
                     break;
             }
@@ -1665,24 +1669,26 @@ namespace Gala {
                     return filter_action (ZOOM);
                 case "toggle-multitasking-view":
                     return filter_action (MULTITASKING_VIEW);
+                case "expose-all-windows":
+                    return filter_action (WINDOW_OVERVIEW);
+                case "screenshot":
+                case "screenshot-clip":
+                case "interactive-screenshot":
+                    return filter_action (SCREENSHOT);
+                case "area-screenshot":
+                case "area-screenshot-clip":
+                    return filter_action (SCREENSHOT_AREA);
+                case "window-screenshot":
+                case "window-screenshot-clip":
+                    return filter_action (SCREENSHOT_WINDOW);
                 default:
                     break;
             }
 
-            var modal_proxy = modal_stack.peek_head ();
-            if (modal_proxy == null) {
-                return false;
-            }
-
-            unowned var filter = modal_proxy.get_keybinding_filter ();
-            if (filter == null) {
-                return false;
-            }
-
-            return filter (binding);
+            return false;
         }
 
-        public bool filter_action (GestureAction action) {
+        public bool filter_action (ModalActions action) {
             if (!is_modal ()) {
                 return false;
             }
diff --git a/tests/lib/meson.build b/tests/lib/meson.build
@@ -1,4 +1,6 @@
 lib_test_sources = files(
+    meson.project_source_root() / 'lib' / 'CommonEnums.vala',
+    meson.project_source_root() / 'lib' / 'WindowManager.vala',
     meson.project_source_root() / 'lib' / 'Gestures' / 'Gesture.vala',
     meson.project_source_root() / 'lib' / 'Gestures' / 'GestureTarget.vala',
     meson.project_source_root() / 'lib' / 'Gestures' / 'PropertyTarget.vala',

Original file line number	Diff line number	Diff line change
`@@ -70,7 +70,7 @@ public class Gala.ModalGroup : Clutter.Actor {`
`70`	`70`
`71`	`71`	`visible = true;`
`72`	`72`	`modal_proxy = wm.push_modal (this, false);`
`73`		`- modal_proxy.allow_actions ({ ZOOM });`
	`73`	`+ modal_proxy.allow_actions (ZOOM \| LOCATE_POINTER \| SCREENSHOT \| SCREENSHOT_AREA \| SCREENSHOT_WINDOW);`
`74`	`74`	`}`
`75`	`75`
`76`	`76`	`if (dimmed.size == 1) {`