[geary] Re-enable standard keyboard scrolling for conversations.



commit 1806065496192e263dde6a41d7192d549c922259
Author: Michael James Gratton <mike vee net>
Date:   Thu Feb 16 18:25:28 2017 +1100

    Re-enable standard keyboard scrolling for conversations.
    
    This does the following:
    
    - Implements using conventional controls
      (Up/Down/PageUp/PageDown/Home/End) to control the
      conversation's vertical scrollbar.
    - Uses Spacebar/Shift+Spacebar (but not backspace, since that's also
      mapped to trash/delete) to advance the focus forward/back through
      individual messages.
    
    Bug 772748.
    
    *  src/client/conversation-viewer/conversation-list-box.vala
       (ConversationListBox): Add action signals and keyboard bindings to
       implement the scheme above. Remove the old hacky approach to making
       the spacebar do something useful.
    
    * src/client/conversation-viewer/conversation-web-view.vala
      (ConversationWebView): Blacklist forwarding key press events for the
      keys above, so the ConversationListBox bindings actually work.

 .../conversation-viewer/conversation-list-box.vala |  107 +++++++++++++++-----
 .../conversation-viewer/conversation-web-view.vala |   26 +++++
 2 files changed, 109 insertions(+), 24 deletions(-)
---
diff --git a/src/client/conversation-viewer/conversation-list-box.vala 
b/src/client/conversation-viewer/conversation-list-box.vala
index c481ef1..2024c2e 100644
--- a/src/client/conversation-viewer/conversation-list-box.vala
+++ b/src/client/conversation-viewer/conversation-list-box.vala
@@ -248,6 +248,49 @@ public class ConversationListBox : Gtk.ListBox {
 
     }
 
+    static construct {
+        // Set up custom keybindings
+        unowned Gtk.BindingSet bindings = Gtk.BindingSet.by_class(
+            (ObjectClass) typeof(ConversationListBox).class_ref()
+        );
+        Gtk.BindingEntry.add_signal(
+            bindings, Gdk.Key.space, 0, "focus-next", 0
+        );
+        Gtk.BindingEntry.add_signal(
+            bindings, Gdk.Key.KP_Space, 0, "focus-next", 0
+        );
+        Gtk.BindingEntry.add_signal(
+            bindings, Gdk.Key.space, Gdk.ModifierType.SHIFT_MASK, "focus-prev", 0
+        );
+        Gtk.BindingEntry.add_signal(
+            bindings, Gdk.Key.KP_Space, Gdk.ModifierType.SHIFT_MASK, "focus-prev", 0
+        );
+
+        Gtk.BindingEntry.add_signal(
+            bindings, Gdk.Key.Up, 0, "scroll", 1,
+            typeof(Gtk.ScrollType), Gtk.ScrollType.STEP_UP
+        );
+        Gtk.BindingEntry.add_signal(
+            bindings, Gdk.Key.Down, 0, "scroll", 1,
+            typeof(Gtk.ScrollType), Gtk.ScrollType.STEP_DOWN
+        );
+        Gtk.BindingEntry.add_signal(
+            bindings, Gdk.Key.Page_Up, 0, "scroll", 1,
+            typeof(Gtk.ScrollType), Gtk.ScrollType.PAGE_UP
+        );
+        Gtk.BindingEntry.add_signal(
+            bindings, Gdk.Key.Page_Down, 0, "scroll", 1,
+            typeof(Gtk.ScrollType), Gtk.ScrollType.PAGE_DOWN
+        );
+        Gtk.BindingEntry.add_signal(
+            bindings, Gdk.Key.Home, 0, "scroll", 1,
+            typeof(Gtk.ScrollType), Gtk.ScrollType.START
+        );
+        Gtk.BindingEntry.add_signal(
+            bindings, Gdk.Key.End, 0, "scroll", 1,
+            typeof(Gtk.ScrollType), Gtk.ScrollType.END
+        );
+    }
 
     private static int on_sort(Gtk.ListBoxRow row1, Gtk.ListBoxRow row2) {
         Geary.Email? email1 = ((ConversationRow) row1).email;
@@ -307,6 +350,46 @@ public class ConversationListBox : Gtk.ListBox {
     private uint loading_timeout_id = 0;
 
 
+    /** Keyboard action to scroll the conversation. */
+    [Signal (action=true)]
+    public virtual signal void scroll(Gtk.ScrollType type) {
+        Gtk.Adjustment adj = get_adjustment();
+        double value = adj.get_value();
+        switch (type) {
+        case Gtk.ScrollType.STEP_UP:
+            value -= adj.get_step_increment();
+            break;
+        case Gtk.ScrollType.STEP_DOWN:
+            value += adj.get_step_increment();
+            break;
+        case Gtk.ScrollType.PAGE_UP:
+            value -= adj.get_page_increment();
+            break;
+        case Gtk.ScrollType.PAGE_DOWN:
+            value += adj.get_page_increment();
+            break;
+        case Gtk.ScrollType.START:
+            value = 0.0;
+            break;
+        case Gtk.ScrollType.END:
+            value = adj.get_upper();
+            break;
+        }
+        adj.set_value(value);
+    }
+
+    /** Keyboard action to shift focus to the next message, if any. */
+    [Signal (action=true)]
+    public virtual signal void focus_next() {
+        this.move_cursor(Gtk.MovementStep.DISPLAY_LINES, 1);
+    }
+
+    /** Keyboard action to shift focus to the prev message, if any. */
+    [Signal (action=true)]
+    public virtual signal void focus_prev() {
+        this.move_cursor(Gtk.MovementStep.DISPLAY_LINES, -1);
+    }
+
     /** Fired when an email view is added to the conversation list. */
     public signal void email_added(ConversationEmail email);
 
@@ -351,7 +434,6 @@ public class ConversationListBox : Gtk.ListBox {
         set_selection_mode(Gtk.SelectionMode.NONE);
         set_sort_func(ConversationListBox.on_sort);
 
-        this.key_press_event.connect(on_key_press);
         this.realize.connect(() => {
                 adjustment.value_changed.connect(() => { check_mark_read(); });
             });
@@ -731,14 +813,6 @@ public class ConversationListBox : Gtk.ListBox {
                 return true;
             });
 
-        // Capture key events on the email's web views to allow
-        // scrolling on Space, etc. need to do this after loading so
-        // attached messages are present
-        view.message_view_iterator().foreach((msg_view) => {
-                msg_view.web_view.key_press_event.connect(on_key_press);
-                return true;
-            });
-
         EmailRow row = new EmailRow(view);
         this.email_rows.set(email.id, row);
 
@@ -999,21 +1073,6 @@ public class ConversationListBox : Gtk.ListBox {
         return flags;
     }
 
-    private bool on_key_press(Gtk.Widget widget, Gdk.EventKey event) {
-        // Override some key bindings to get something that works more
-        // like a browser page.
-        if (event.keyval == Gdk.Key.space) {
-            Gtk.ScrollType dir = Gtk.ScrollType.PAGE_DOWN;
-            if ((event.state & Gdk.ModifierType.SHIFT_MASK) ==
-                Gdk.ModifierType.SHIFT_MASK) {
-                dir = Gtk.ScrollType.PAGE_UP;
-            }
-            this.move_cursor(Gtk.MovementStep.PAGES, 1);
-            return true;
-        }
-        return false;
-    }
-
     private void on_row_activated(Gtk.ListBoxRow widget) {
         EmailRow? row = widget as EmailRow;
         if (row != null) {
diff --git a/src/client/conversation-viewer/conversation-web-view.vala 
b/src/client/conversation-viewer/conversation-web-view.vala
index 6d927a7..5ebfb61 100644
--- a/src/client/conversation-viewer/conversation-web-view.vala
+++ b/src/client/conversation-viewer/conversation-web-view.vala
@@ -12,6 +12,20 @@ public class ConversationWebView : ClientWebView {
 
     private const string DECEPTIVE_LINK_CLICKED = "deceptiveLinkClicked";
 
+    // Key codes we don't forward on to the super class on key press
+    // since we want to override them elsewhere, especially
+    // ConversationListBox.
+    private const int[] BLACKLISTED_KEY_CODES = {
+        Gdk.Key.space,
+        Gdk.Key.KP_Space,
+        Gdk.Key.Up,
+        Gdk.Key.Down,
+        Gdk.Key.Page_Up,
+        Gdk.Key.Page_Down,
+        Gdk.Key.Home,
+        Gdk.Key.End
+    };
+
     /** Specifies the type of deceptive link text when clicked. */
     public enum DeceptiveText {
         // Keep this in sync with JS ConversationPageState
@@ -80,6 +94,18 @@ public class ConversationWebView : ClientWebView {
         return WebKitUtil.to_string(result);
     }
 
+    public override bool key_press_event(Gdk.EventKey event) {
+        // WebView consumes a number of key presses for scrolling
+        // itself internally, but we want them to navigate around in
+        // ConversationListBox, so don't forward any on.
+        bool ret = Gdk.EVENT_PROPAGATE;
+        if (!(((int) event.keyval) in BLACKLISTED_KEY_CODES)) {
+            ret = base.key_press_event(event);
+        }
+        return ret;
+    }
+
+
     // XXX Surely since we are doing height-for-width, we should be
     // overriding get_preferred_height_for_width here, but that
     // doesn't seem to work.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]