[geary/wip/794700-lazy-load-conversations: 15/19] Handle long-loading indication for conversations better
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/wip/794700-lazy-load-conversations: 15/19] Handle long-loading indication for conversations better
- Date: Mon, 21 Jan 2019 00:42:48 +0000 (UTC)
commit 2f35f586107d46229c6bf70c99a7e5729524b719
Author: Michael Gratton <mike vee net>
Date: Mon Jan 21 10:19:31 2019 +1030
Handle long-loading indication for conversations better
Moving the loading placeholder from ConversationListBox to
ConversationEmail allows a more fine-grained indication of what is
happening - only show the loading indicator when the remote actually
needs to get hit, display the email's details and load the rest of the
conversation while waiting for the remote body load. Also lets us pass
errors loading the initial email locally all the way up to the
controller.
src/client/application/geary-controller.vala | 2 +
.../conversation-viewer/conversation-email.vala | 173 +++++++++++++--------
.../conversation-viewer/conversation-list-box.vala | 27 ----
.../conversation-viewer/conversation-message.vala | 23 +++
.../conversation-viewer/conversation-viewer.vala | 21 +--
ui/conversation-message.ui | 1 +
6 files changed, 139 insertions(+), 108 deletions(-)
---
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index 2908c169..6091a2a1 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -1274,6 +1274,8 @@ public class GearyController : Geary.BaseObject {
get_window_action(
ACTION_FIND_IN_CONVERSATION
).set_enabled(true);
+ } catch (GLib.IOError.CANCELLED err) {
+ // All good
} catch (Error err) {
debug("Unable to load conversation: %s",
err.message);
diff --git a/src/client/conversation-viewer/conversation-email.vala
b/src/client/conversation-viewer/conversation-email.vala
index 71d3b5fb..702a0ab5 100644
--- a/src/client/conversation-viewer/conversation-email.vala
+++ b/src/client/conversation-viewer/conversation-email.vala
@@ -274,10 +274,6 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
/** Determines if all message have had loaded their bodies. */
public bool message_bodies_loaded { get; private set; default = false; }
- /** Determines if all message's web views have finished loading. */
- private Geary.Nonblocking.Spinlock message_bodies_loaded_lock =
- new Geary.Nonblocking.Spinlock();
-
// Cancellable to use when loading message content
private GLib.Cancellable load_cancellable;
@@ -286,6 +282,10 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
private Configuration config;
+ /** Determines if all message's web views have finished loading. */
+ private Geary.Nonblocking.Spinlock message_bodies_loaded_lock =
+ new Geary.Nonblocking.Spinlock();
+
// Message view with selected text, if any
private ConversationMessage? body_selection_message = null;
@@ -338,6 +338,7 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
private Menu email_menu_delete;
private bool shift_key_down;
+
/** Fired when the user clicks "reply" in the message menu. */
public signal void reply_to_message();
@@ -542,70 +543,30 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
// Ensure we have required data to load the message
- Geary.Email email = this.email;
- if (!this.email.fields.fulfills(REQUIRED_FOR_LOAD)) {
- email = yield email_store.fetch_email_async(
- email.id,
- Geary.Email.REQUIRED_FOR_MESSAGE,
- Geary.Folder.ListFlags.NONE,
- this.load_cancellable
- );
- }
- Geary.RFC822.Message message = email.get_message();
-
- // Load all mime parts and construct CID resources from them
-
- Gee.Map<string,Geary.Memory.Buffer> cid_resources =
- new Gee.HashMap<string,Geary.Memory.Buffer>();
- foreach (Geary.Attachment attachment in email.attachments) {
- // Assume all parts are attachments. As the primary and
- // secondary message bodies are loaded, any displayed
- // inline will be removed from the list.
- this.displayed_attachments.add(attachment);
-
- if (attachment.content_id != null) {
- try {
- cid_resources[attachment.content_id] =
- new Geary.Memory.FileBuffer(attachment.file, true);
- } catch (Error err) {
- debug("Could not open attachment: %s", err.message);
- }
+ Geary.Email? email = null;
+ if (this.email.fields.fulfills(REQUIRED_FOR_LOAD)) {
+ email = this.email;
+ } else {
+ try {
+ email = yield email_store.fetch_email_async(
+ this.email.id,
+ Geary.Email.REQUIRED_FOR_MESSAGE,
+ LOCAL_ONLY, // Throw an error if not downloaded
+ this.load_cancellable
+ );
+ } catch (Geary.EngineError.INCOMPLETE_MESSAGE err) {
+ // Don't have the complete message at the moment, so
+ // download it in the background.
+ this.fetch_body_remote.begin(email_store, contact_store);
}
}
- this.attachments_button.set_visible(!this.displayed_attachments.is_empty);
-
- // Load all messages
- this.primary_message.web_view.add_internal_resources(cid_resources);
- yield this.primary_message.load_message_body(
- message, this.load_cancellable
- );
-
- Gee.List<Geary.RFC822.Message> sub_messages = message.get_sub_messages();
- if (sub_messages.size > 0) {
- this.primary_message.body_container.add(this.sub_messages);
- }
- foreach (Geary.RFC822.Message sub_message in sub_messages) {
- ConversationMessage attached_message =
- new ConversationMessage.from_message(
- sub_message, false, this.config
- );
- connect_message_view_signals(attached_message);
- attached_message.web_view.add_internal_resources(cid_resources);
- this.sub_messages.add(attached_message);
- this._attached_messages.add(attached_message);
- attached_message.load_avatar.begin(
- contact_store, this.load_cancellable
- );
- yield attached_message.load_message_body(
- sub_message, this.load_cancellable
+ if (email != null) {
+ yield update_body(email, contact_store);
+ yield this.message_bodies_loaded_lock.wait_async(
+ this.load_cancellable
);
- if (!this.is_collapsed) {
- attached_message.show_message_body(false);
- }
}
-
- yield this.message_bodies_loaded_lock.wait_async(this.load_cancellable);
}
/**
@@ -735,6 +696,92 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
});
}
+ private async void fetch_body_remote(Geary.App.EmailStore email_store,
+ Application.AvatarStore contact_store)
+ throws GLib.Error {
+ // XXX Need proper progress reporting here, rather than just
+ // doing a pulse
+ this.primary_message.start_progress_pulse();
+ Geary.Email? email = null;
+ try {
+ email = yield email_store.fetch_email_async(
+ this.email.id,
+ Geary.Email.REQUIRED_FOR_MESSAGE,
+ FORCE_UPDATE,
+ this.load_cancellable
+ );
+ } catch (GLib.Error err) {
+ debug("Remote message download failed: %s", err.message);
+ }
+
+ if (email != null) {
+ this.primary_message.stop_progress_pulse();
+ try {
+ yield update_body(email, contact_store);
+ } catch (GLib.Error err) {
+ debug("Remote message update failed: %s", err.message);
+ }
+ }
+ }
+
+ private async void update_body(Geary.Email email,
+ Application.AvatarStore contact_store)
+ throws GLib.Error {
+ Geary.RFC822.Message message = email.get_message();
+
+ // Load all mime parts and construct CID resources from them
+
+ Gee.Map<string,Geary.Memory.Buffer> cid_resources =
+ new Gee.HashMap<string,Geary.Memory.Buffer>();
+ foreach (Geary.Attachment attachment in email.attachments) {
+ // Assume all parts are attachments. As the primary and
+ // secondary message bodies are loaded, any displayed
+ // inline will be removed from the list.
+ this.displayed_attachments.add(attachment);
+
+ if (attachment.content_id != null) {
+ try {
+ cid_resources[attachment.content_id] =
+ new Geary.Memory.FileBuffer(attachment.file, true);
+ } catch (Error err) {
+ debug("Could not open attachment: %s", err.message);
+ }
+ }
+ }
+ this.attachments_button.set_visible(!this.displayed_attachments.is_empty);
+
+ // Load all messages
+
+ this.primary_message.web_view.add_internal_resources(cid_resources);
+ yield this.primary_message.load_message_body(
+ message, this.load_cancellable
+ );
+
+ Gee.List<Geary.RFC822.Message> sub_messages = message.get_sub_messages();
+ if (sub_messages.size > 0) {
+ this.primary_message.body_container.add(this.sub_messages);
+ }
+ foreach (Geary.RFC822.Message sub_message in sub_messages) {
+ ConversationMessage attached_message =
+ new ConversationMessage.from_message(
+ sub_message, false, this.config
+ );
+ connect_message_view_signals(attached_message);
+ attached_message.web_view.add_internal_resources(cid_resources);
+ this.sub_messages.add(attached_message);
+ this._attached_messages.add(attached_message);
+ attached_message.load_avatar.begin(
+ contact_store, this.load_cancellable
+ );
+ yield attached_message.load_message_body(
+ sub_message, this.load_cancellable
+ );
+ if (!this.is_collapsed) {
+ attached_message.show_message_body(false);
+ }
+ }
+ }
+
private void update_email_state() {
Geary.EmailFlags? flags = this.email.email_flags;
Gtk.StyleContext style = get_style_context();
diff --git a/src/client/conversation-viewer/conversation-list-box.vala
b/src/client/conversation-viewer/conversation-list-box.vala
index b7f19425..3b26ba22 100644
--- a/src/client/conversation-viewer/conversation-list-box.vala
+++ b/src/client/conversation-viewer/conversation-list-box.vala
@@ -38,9 +38,6 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
// account.
private const int EMAIL_TOP_OFFSET = 32;
- // Loading spinner timeout
- private const int LOADING_TIMEOUT_MSEC = 150;
-
// Base class for list rows it the list box
private abstract class ConversationRow : Gtk.ListBoxRow, Geary.BaseInterface {
@@ -312,8 +309,6 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
// Total number of search matches found
private uint search_matches_found = 0;
- private Geary.TimeoutManager loading_timeout;
-
/** Keyboard action to scroll the conversation. */
[Signal (action=true)]
@@ -399,11 +394,6 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
this.conversation.appended.connect(on_conversation_appended);
this.conversation.trimmed.connect(on_conversation_trimmed);
this.conversation.email_flags_changed.connect(on_update_flags);
-
- // If the load is taking too long, display a spinner
- this.loading_timeout = new Geary.TimeoutManager.milliseconds(
- LOADING_TIMEOUT_MSEC, show_loading
- );
}
~ConversationListBox() {
@@ -411,7 +401,6 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
}
public override void destroy() {
- this.loading_timeout.reset();
this.cancellable.cancel();
this.email_rows.clear();
base.destroy();
@@ -425,12 +414,6 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
Geary.App.Conversation.Ordering.SENT_DATE_ASCENDING
);
- // Now have the full set of email and a UI update is
- // imminent. So cancel the spinner timeout if still running,
- // and remove the spinner it may have set in any case.
- this.loading_timeout.reset();
- set_placeholder(null);
-
// Work out what the first interesting email is, and load it
// before all of the email before and after that so we can
// load them in an optimal order.
@@ -477,7 +460,6 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
/** Cancels loading the current conversation, if still in progress */
public void cancel_conversation_load() {
- this.loading_timeout.reset();
this.cancellable.cancel();
}
@@ -830,15 +812,6 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
}
}
- private void show_loading() {
- Gtk.Spinner spinner = new Gtk.Spinner();
- spinner.set_size_request(32, 32);
- spinner.halign = spinner.valign = Gtk.Align.CENTER;
- spinner.start();
- spinner.show();
- set_placeholder(spinner);
- }
-
private void scroll_to(ConversationRow row) {
Gtk.Allocation? alloc = null;
row.get_allocation(out alloc);
diff --git a/src/client/conversation-viewer/conversation-message.vala
b/src/client/conversation-viewer/conversation-message.vala
index ee97c671..56aa810f 100644
--- a/src/client/conversation-viewer/conversation-message.vala
+++ b/src/client/conversation-viewer/conversation-message.vala
@@ -25,6 +25,8 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
private const int MAX_PREVIEW_BYTES = Geary.Email.MAX_PREVIEW_BYTES;
+ private const int PULSE_TIMEOUT_MSEC = 250;
+
// Widget used to display sender/recipient email addresses in
// message header Gtk.FlowBox instances.
@@ -248,6 +250,9 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
private Geary.TimeoutManager show_progress_timeout = null;
private Geary.TimeoutManager hide_progress_timeout = null;
+ // Timer for pulsing progress bar
+ private Geary.TimeoutManager progress_pulse;
+
/** Fired when the user clicks a link in the email. */
public signal void link_activated(string link);
@@ -460,6 +465,11 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
this.hide_progress_timeout = new Geary.TimeoutManager.seconds(
1, () => { this.body_progress.hide(); }
);
+
+ this.progress_pulse = new Geary.TimeoutManager.milliseconds(
+ PULSE_TIMEOUT_MSEC, this.body_progress.pulse
+ );
+ this.progress_pulse.repetition = FOREVER;
}
~ConversationMessage() {
@@ -469,6 +479,7 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
public override void destroy() {
this.show_progress_timeout.reset();
this.hide_progress_timeout.reset();
+ this.progress_pulse.reset();
this.resources.clear();
this.searchable_addresses.clear();
base.destroy();
@@ -492,6 +503,18 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
body_revealer.set_reveal_child(false);
}
+ /** Shows and starts pulsing the progress meter. */
+ public void start_progress_pulse() {
+ this.body_progress.show();
+ this.progress_pulse.start();
+ }
+
+ /** Hides and stops pulsing the progress meter. */
+ public void stop_progress_pulse() {
+ this.body_progress.hide();
+ this.progress_pulse.reset();
+ }
+
/**
* Starts loading the avatar for the message's sender.
*/
diff --git a/src/client/conversation-viewer/conversation-viewer.vala
b/src/client/conversation-viewer/conversation-viewer.vala
index 7ded5f14..cfa7de06 100644
--- a/src/client/conversation-viewer/conversation-viewer.vala
+++ b/src/client/conversation-viewer/conversation-viewer.vala
@@ -221,7 +221,8 @@ public class ConversationViewer : Gtk.Stack, Geary.BaseInterface {
*/
public async void load_conversation(Geary.App.Conversation conversation,
Geary.App.EmailStore email_store,
- Application.AvatarStore avatar_store) {
+ Application.AvatarStore avatar_store)
+ throws GLib.Error {
remove_current_list();
ConversationListBox new_list = new ConversationListBox(
@@ -267,23 +268,7 @@ public class ConversationViewer : Gtk.Stack, Geary.BaseInterface {
}
}
- // Launch this as a background task so that additional
- // conversation selection events can get processed before
- // loading this one has completed.
- //
- // XXX we really should be yielding until the first
- // interesting email has been loaded, and the rest should be
- // loaded in he background.
- new_list.load_conversation.begin(
- query,
- (obj, res) => {
- try {
- new_list.load_conversation.end(res);
- } catch (GLib.Error err) {
- debug("Error loading conversation: %s", err.message);
- }
- }
- );
+ yield new_list.load_conversation(query);
}
// Add a new conversation list to the UI
diff --git a/ui/conversation-message.ui b/ui/conversation-message.ui
index f61f73c8..58db14cf 100644
--- a/ui/conversation-message.ui
+++ b/ui/conversation-message.ui
@@ -595,6 +595,7 @@
</child>
<child>
<object class="GtkOverlay">
+ <property name="height_request">6</property>
<property name="visible">True</property>
<property name="can_focus">False</property>
<child>
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
