[geary/wip/conversation-polish: 1/3] Refine auto-mark-read to only happen as a direct result of user action

[Michael Gratton <mjog src gnome org>]

commit d8f58441cc3da21bee8b0b80f9c0724753115250
Author: Michael Gratton <mike vee net>
Date:   Sat Jan 26 14:57:23 2019 +1100

    Refine auto-mark-read to only happen as a direct result of user action
    
    This makes email automatically marked as read if the user performs an
    action (select or scroll), and not if e.g. a message is simply appended
    to the visible conversation.
    
    Fixes #80

 .../conversation-viewer/conversation-list-box.vala | 80 +++++++++++++++-------
 .../conversation-viewer/conversation-viewer.vala   | 15 +++-
 2 files changed, 71 insertions(+), 24 deletions(-)
---
diff --git a/src/client/conversation-viewer/conversation-list-box.vala 
b/src/client/conversation-viewer/conversation-list-box.vala
index e1f29e56..3c443154 100644
--- a/src/client/conversation-viewer/conversation-list-box.vala
+++ b/src/client/conversation-viewer/conversation-list-box.vala
@@ -38,6 +38,15 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
     // account.
     private const int EMAIL_TOP_OFFSET = 32;
 
+    // Amount of time to wait after the user took some action that may
+    // be interpreted as marking the email as read before actually
+    // checking
+    private const int MARK_READ_TIMEOUT_MSEC = 250;
+
+    // Amount of pixels that need to be shown of an email's body to
+    // mark it as read
+    private const int MARK_READ_PADDING = 50;
+
 
     // Base class for list rows it the list box
     private abstract class ConversationRow : Gtk.ListBoxRow, Geary.BaseInterface {
@@ -305,6 +314,8 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
     // Total number of search matches found
     private uint search_matches_found = 0;
 
+    private Geary.TimeoutManager mark_read_timer;
+
 
     /** Keyboard action to scroll the conversation. */
     [Signal (action=true)]
@@ -332,18 +343,21 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
             break;
         }
         adj.set_value(value);
+        this.mark_read_timer.start();
     }
 
     /** 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);
+        this.mark_read_timer.start();
     }
 
     /** 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);
+        this.mark_read_timer.start();
     }
 
     /** Fired when an email view is added to the conversation list. */
@@ -374,18 +388,19 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
         this.avatar_store = avatar_store;
         this.config = config;
 
+        this.mark_read_timer = new Geary.TimeoutManager.milliseconds(
+            MARK_READ_TIMEOUT_MSEC, this.check_mark_read
+        );
+
+        this.selection_mode = NONE;
+
         get_style_context().add_class("background");
         get_style_context().add_class("conversation-listbox");
 
         set_adjustment(adjustment);
-        set_selection_mode(Gtk.SelectionMode.NONE);
         set_sort_func(ConversationListBox.on_sort);
 
-        this.realize.connect(() => {
-                adjustment.value_changed.connect(() => { check_mark_read(); });
-            });
         this.row_activated.connect(on_row_activated);
-        this.size_allocate.connect(() => { check_mark_read(); });
 
         this.conversation.appended.connect(on_conversation_appended);
         this.conversation.trimmed.connect(on_conversation_trimmed);
@@ -399,6 +414,7 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
     public override void destroy() {
         this.cancellable.cancel();
         this.email_rows.clear();
+        this.mark_read_timer.reset();
         base.destroy();
     }
 
@@ -452,6 +468,8 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
         this.finish_loading.begin(
             query, uninteresting, post_interesting
         );
+
+        this.mark_read_timer.start();
     }
 
     /** Cancels loading the current conversation, if still in progress */
@@ -531,6 +549,13 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
             });
     }
 
+    /**
+     * Marks all email with a visible body read.
+     */
+    public void mark_visible_read() {
+        this.mark_read_timer.start();
+    }
+
     /**
      * Displays an email as being read, regardless of its actual flags.
      */
@@ -773,6 +798,9 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
         view.body_selection_changed.connect((email, has_selection) => {
                 this.body_selected_view = has_selection ? email : null;
             });
+        view.notify["message-body-state"].connect(
+            on_message_body_state_notify
+        );
 
         ConversationMessage conversation_message = view.primary_message;
         conversation_message.body_container.button_release_event.connect_after((event) => {
@@ -826,22 +854,25 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
      * Finds any currently visible messages, marks them as being read.
      */
     private void check_mark_read() {
-        Gee.ArrayList<Geary.EmailIdentifier> email_ids =
-            new Gee.ArrayList<Geary.EmailIdentifier>();
-
+        Gee.List<Geary.EmailIdentifier> email_ids =
+            new Gee.LinkedList<Geary.EmailIdentifier>();
         Gtk.Adjustment adj = get_adjustment();
         int top_bound = (int) adj.value;
         int bottom_bound = top_bound + (int) adj.page_size;
 
-        email_view_iterator().foreach((email_view) => {
-            const int TEXT_PADDING = 50;
-            ConversationMessage conversation_message = email_view.primary_message;
+        this.foreach((child) => {
             // Don't bother with not-yet-loaded emails since the
             // size of the body will be off, affecting the visibility
             // of emails further down the conversation.
-            if (email_view.email.is_unread().is_certain() &&
-                email_view.message_body_state == COMPLETED &&
-                !email_view.is_manually_read) {
+            EmailRow? row = child as EmailRow;
+            ConversationEmail? view = (row != null) ? row.view : null;
+            Geary.Email? email = (view != null) ? view.email : null;
+            if (row != null &&
+                row.is_expanded &&
+                view.message_body_state == COMPLETED &&
+                !view.is_manually_read &&
+                email.is_unread().is_certain()) {
+                ConversationMessage conversation_message = view.primary_message;
                  int body_top = 0;
                  int body_left = 0;
                  ConversationWebView web_view = conversation_message.web_view;
@@ -857,23 +888,18 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
                  // Only mark the email as read if it's actually visible
                  if (body_height > 0 &&
                      body_bottom > top_bound &&
-                     body_top + TEXT_PADDING < bottom_bound) {
-                     email_ids.add(email_view.email.id);
+                     body_top + MARK_READ_PADDING < bottom_bound) {
+                     email_ids.add(view.email.id);
 
                      // Since it can take some time for the new flags
                      // to round-trip back to our signal handlers,
                      // mark as manually read here
-                     email_view.is_manually_read = true;
+                     view.is_manually_read = true;
                  }
              }
-            return true;
         });
 
-        // Only-automark if the window is currently focused
-        Gtk.Window? top_level = get_toplevel() as Gtk.Window;
-        if (top_level != null &&
-            top_level.is_active &&
-            email_ids.size > 0) {
+        if (email_ids.size > 0) {
             Geary.EmailFlags flags = new Geary.EmailFlags();
             flags.add(Geary.EmailFlags.UNREAD);
             mark_emails(email_ids, null, flags);
@@ -1016,6 +1042,14 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
         mark_emails(ids, flag_to_flags(to_add), flag_to_flags(to_remove));
     }
 
+    private void on_message_body_state_notify(GLib.Object obj,
+                                              GLib.ParamSpec param) {
+        ConversationEmail? view = obj as ConversationEmail;
+        if (view != null && view.message_body_state == COMPLETED) {
+            this.mark_read_timer.start();
+        }
+    }
+
     private Geary.EmailFlags? flag_to_flags(Geary.NamedFlag? flag) {
         Geary.EmailFlags flags = null;
         if (flag != null) {
diff --git a/src/client/conversation-viewer/conversation-viewer.vala 
b/src/client/conversation-viewer/conversation-viewer.vala
index a6b17326..89e1662b 100644
--- a/src/client/conversation-viewer/conversation-viewer.vala
+++ b/src/client/conversation-viewer/conversation-viewer.vala
@@ -316,8 +316,15 @@ public class ConversationViewer : Gtk.Stack, Geary.BaseInterface {
         scroller.set_hexpand(true);
         scroller.set_vexpand(true);
         scroller.show();
+        scroller.scroll_event.connect(
+            on_conversation_scroll
+        );
+        scroller.get_vscrollbar().button_release_event.connect(
+            on_conversation_scroll
+        );
         this.conversation_scroller = scroller;
         this.conversation_page.add(scroller);
+
     }
 
     /**
@@ -421,5 +428,11 @@ public class ConversationViewer : Gtk.Stack, Geary.BaseInterface {
         }
     }
 
-}
+    private bool on_conversation_scroll() {
+        if (this.current_list != null) {
+            this.current_list.mark_visible_read();
+        }
+        return Gdk.EVENT_PROPAGATE;
+    }
 
+}