Commit 4b3c1676 authored by Michael Gratton's avatar Michael Gratton 🤞

Merge branch 'wip/conversation-polish' into 'master'

More conversation polish

Closes #80

See merge request !92
parents afd929b1 8d173547
Pipeline #55460 passed with stages
in 11 minutes and 55 seconds
......@@ -688,15 +688,23 @@ public class GearyController : Geary.BaseObject {
}
private void update_account_status() {
Geary.Account.Status effective_status = 0;
// Start off assuming all accounts are online and error free
// (i.e. no status issues to indicate) and proceed until
// proven incorrect.
Geary.Account.Status effective_status = ONLINE;
bool has_auth_error = false;
bool has_cert_error = false;
Geary.Account? service_problem_source = null;
foreach (AccountContext context in this.accounts.values) {
effective_status |= context.get_effective_status();
if (effective_status.has_service_problem() &&
service_problem_source == null) {
service_problem_source = context.account;
Geary.Account.Status status = context.get_effective_status();
if (!status.is_online()) {
effective_status &= ~Geary.Account.Status.ONLINE;
}
if (status.has_service_problem()) {
effective_status |= SERVICE_PROBLEM;
if (service_problem_source == null) {
service_problem_source = context.account;
}
}
has_auth_error |= context.authentication_failed;
has_cert_error |= context.tls_validation_failed;
......@@ -898,6 +906,7 @@ public class GearyController : Geary.BaseObject {
main_window.folder_list.set_user_folders_root_name(account, _("Labels"));
display_main_window_if_ready();
update_account_status();
}
// Returns true if the caller should try opening the account again
......@@ -2570,9 +2579,10 @@ public class GearyController : Geary.BaseObject {
private void on_conversation_viewer_email_added(ConversationEmail view) {
view.attachments_activated.connect(on_attachments_activated);
view.forward_message.connect(on_forward_message);
view.load_error.connect(on_email_load_error);
view.reply_to_message.connect(on_reply_to_message);
view.reply_all_message.connect(on_reply_all_message);
view.forward_message.connect(on_forward_message);
Geary.App.Conversation conversation = main_window.conversation_viewer.current_list.conversation;
bool in_current_folder = (conversation.is_in_base_folder(view.email.id) &&
......@@ -3011,6 +3021,18 @@ public class GearyController : Geary.BaseObject {
);
}
private void on_email_load_error(ConversationEmail view, GLib.Error err) {
// XXX determine the problem better here
report_problem(
new Geary.ServiceProblemReport(
Geary.ProblemType.GENERIC_ERROR,
this.current_account.information,
this.current_account.information.incoming,
err
)
);
}
private void on_save_attachments(Gee.Collection<Geary.Attachment> attachments) {
GLib.Cancellable? cancellable = null;
if (this.current_account != null) {
......
......@@ -202,9 +202,6 @@ public class ClientWebView : WebKit.WebView, Geary.BaseInterface {
/** Determines if the view has any selected text */
public bool has_selection { get; private set; default = false; }
/** Determines if the view has started rendering the HTML */
public bool has_valid_height { get; private set; default = false; }
/** The HTML content's current preferred height in window pixels. */
public int preferred_height {
get {
......@@ -603,7 +600,6 @@ public class ClientWebView : WebKit.WebView, Geary.BaseInterface {
double height = this.webkit_reported_height;
try {
height = WebKitUtil.to_number(result);
this.has_valid_height = true;
} catch (Geary.JS.Error err) {
debug("Could not get preferred height: %s", err.message);
}
......
......@@ -11,6 +11,10 @@
[GtkTemplate (ui = "/org/gnome/Geary/components-placeholder-pane.ui")]
public class Components.PlaceholderPane : Gtk.Grid {
public const string CLASS_HAS_TEXT = "geary-has-text";
/** The icon name of the pane's image. */
public string icon_name {
owned get { return this.placeholder_image.icon_name; }
......@@ -20,13 +24,19 @@ public class Components.PlaceholderPane : Gtk.Grid {
/** The text of the pane's title label. */
public string title {
get { return this.title_label.get_text(); }
set { this.title_label.set_text(value); }
set {
this.title_label.set_text(value);
update();
}
}
/** The text of the pane's sub-title label. */
public string subtitle {
get { return this.subtitle_label.get_text(); }
set { this.subtitle_label.set_text(value); }
set {
this.subtitle_label.set_text(value);
update();
}
}
[GtkChild]
......@@ -38,4 +48,17 @@ public class Components.PlaceholderPane : Gtk.Grid {
[GtkChild]
private Gtk.Label subtitle_label;
private void update() {
if (Geary.String.is_empty_or_whitespace(this.title_label.get_text())) {
this.title_label.hide();
}
if (Geary.String.is_empty_or_whitespace(this.subtitle_label.get_text())) {
this.subtitle_label.hide();
}
if (this.title_label.visible || this.subtitle_label.visible) {
get_style_context().add_class(CLASS_HAS_TEXT);
}
}
}
......@@ -14,13 +14,19 @@
*/
public class ConversationListStore : Gtk.ListStore {
public const Geary.Email.Field REQUIRED_FIELDS =
Geary.Email.Field.ENVELOPE | Geary.Email.Field.FLAGS | Geary.Email.Field.PROPERTIES;
// XXX Remove ALL and NONE when PREVIEW has been fixed. See Bug 714317.
public const Geary.Email.Field WITH_PREVIEW_FIELDS =
Geary.Email.Field.ENVELOPE | Geary.Email.Field.FLAGS | Geary.Email.Field.PROPERTIES | Geary.Email.Field.PREVIEW |
Geary.Email.Field.ALL | Geary.Email.Field.NONE;
public const Geary.Email.Field REQUIRED_FIELDS = (
Geary.Email.Field.ENVELOPE |
Geary.Email.Field.FLAGS |
Geary.Email.Field.PROPERTIES
);
// XXX Remove REQUIRED_FOR_BODY when PREVIEW has been fixed. See Bug 714317.
public const Geary.Email.Field WITH_PREVIEW_FIELDS = (
REQUIRED_FIELDS |
Geary.Email.Field.PREVIEW |
Geary.Email.REQUIRED_FOR_MESSAGE
);
public enum Column {
CONVERSATION_DATA,
......
......@@ -30,9 +30,33 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
/** Fields that must be available for loading the body. */
internal const Geary.Email.Field REQUIRED_FOR_LOAD = (
// Include those needed by the constructor since we'll replace
// the ctor's email arg value once the body has been fully
// loaded
REQUIRED_FOR_CONSTRUCT |
Geary.Email.REQUIRED_FOR_MESSAGE
);
// Time to wait loading the body before showing the progress meter
private const int BODY_LOAD_TIMEOUT_MSEC = 250;
/** Specifies the loading state for a message part. */
public enum LoadState {
/** Loading has not started. */
NOT_STARTED,
/** Loading has started, but not completed. */
STARTED,
/** Loading has started and completed. */
COMPLETED,
/** Loading has started but encountered an error. */
FAILED;
}
/**
* Iterator that returns all message views in an email view.
......@@ -275,11 +299,8 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
private Gee.List<ConversationMessage> _attached_messages =
new Gee.LinkedList<ConversationMessage>();
/** Determines if message bodies have started loading. */
public bool message_body_load_started { get; private set; default = false; }
/** Determines if all message have had loaded their bodies. */
public bool message_bodies_loaded { get; private set; default = false; }
/** Determines the message body loading state. */
public LoadState message_body_state { get; private set; default = NOT_STARTED; }
// Store from which to load message content, if needed
private Geary.App.EmailStore email_store;
......@@ -295,6 +316,9 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
private Configuration config;
private Geary.TimeoutManager body_loading_timeout;
/** Determines if all message's web views have finished loading. */
private Geary.Nonblocking.Spinlock message_bodies_loaded_lock =
new Geary.Nonblocking.Spinlock();
......@@ -352,6 +376,9 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
private bool shift_key_down;
/** Fired when an error occurs loading the message body. */
public signal void load_error(GLib.Error err);
/** Fired when the user clicks "reply" in the message menu. */
public signal void reply_to_message();
......@@ -519,6 +546,14 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
this.primary_message.infobars.add(this.not_saved_infobar);
email_store.account.incoming.notify["current-status"].connect(
on_service_status_change
);
this.body_loading_timeout = new Geary.TimeoutManager.milliseconds(
BODY_LOAD_TIMEOUT_MSEC, this.on_body_loading_timeout
);
pack_start(this.primary_message, true, true, 0);
update_email_state();
}
......@@ -554,31 +589,42 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
*/
public async void load_body()
throws GLib.Error {
this.message_body_load_started = true;
this.message_body_state = STARTED;
// Ensure we have required data to load the message
Geary.Email? email = null;
if (this.email.fields.fulfills(REQUIRED_FOR_LOAD)) {
email = this.email;
} else {
bool loaded = this.email.fields.fulfills(REQUIRED_FOR_LOAD);
if (!loaded) {
this.body_loading_timeout.start();
try {
email = yield this.email_store.fetch_email_async(
this.email = yield this.email_store.fetch_email_async(
this.email.id,
Geary.Email.REQUIRED_FOR_MESSAGE,
LOCAL_ONLY, // Throw an error if not downloaded
REQUIRED_FOR_LOAD,
LOCAL_ONLY, // Throws an error if not downloaded
this.load_cancellable
);
loaded = true;
this.body_loading_timeout.reset();
} 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();
// download it in the background. Don't reset the body
// load timeout here since this will attempt to fetch
// from the remote
this.fetch_remote_body.begin();
} catch (GLib.Error err) {
this.body_loading_timeout.reset();
handle_load_failure(err);
throw err;
}
}
if (email != null) {
this.email = email;
yield update_body();
if (loaded) {
try {
yield update_body();
} catch (GLib.Error err) {
handle_load_failure(err);
throw err;
}
yield this.message_bodies_loaded_lock.wait_async(
this.load_cancellable
);
......@@ -712,37 +758,44 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
});
}
private async void fetch_body_remote()
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 this.email_store.fetch_email_async(
this.email.id,
Geary.Email.REQUIRED_FOR_MESSAGE,
FORCE_UPDATE,
this.load_cancellable
);
} catch (GLib.IOError.CANCELLED err) {
// Don't stop message progress pulse here since if
// cancelled, this could be well after the widgets have
// been removed and destroyed
} catch (GLib.Error err) {
// XXX Notify user of a problem here
debug("Remote message download failed: %s", err.message);
this.primary_message.stop_progress_pulse();
}
if (email != null) {
this.primary_message.stop_progress_pulse();
private async void fetch_remote_body() {
if (is_online()) {
// XXX Need proper progress reporting here, rather than just
// doing a pulse
if (!this.body_loading_timeout.is_running) {
this.body_loading_timeout.start();
}
Geary.Email? loaded = null;
try {
this.email = email;
yield update_body();
debug("Downloading remote message: %s", this.email.to_string());
loaded = yield this.email_store.fetch_email_async(
this.email.id,
REQUIRED_FOR_LOAD,
FORCE_UPDATE,
this.load_cancellable
);
} catch (GLib.IOError.CANCELLED err) {
// All good
} catch (GLib.Error err) {
debug("Remote message update failed: %s", err.message);
debug("Remote message download failed: %s", err.message);
handle_load_failure(err);
}
this.body_loading_timeout.reset();
if (loaded != null) {
try {
this.email = loaded;
yield update_body();
} catch (GLib.Error err) {
debug("Remote message update failed: %s", err.message);
handle_load_failure(err);
}
}
} else {
this.body_loading_timeout.reset();
handle_load_offline();
}
}
......@@ -864,6 +917,21 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
return selected;
}
private void handle_load_failure(GLib.Error err) {
load_error(err);
this.message_body_state = FAILED;
this.primary_message.show_load_error_pane();
}
private void handle_load_offline() {
this.message_body_state = FAILED;
this.primary_message.show_offline_pane();
}
private inline bool is_online() {
return (this.email_store.account.incoming.current_status == CONNECTED);
}
/**
* Updates the email menu if it is open.
*/
......@@ -938,6 +1006,10 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
op.run_dialog(window);
}
private void on_body_loading_timeout() {
this.primary_message.show_loading_pane();
}
private void on_flag_remote_images(ConversationMessage view) {
// XXX check we aren't already auto loading the image
mark_email(Geary.EmailFlags.LOAD_REMOTE_IMAGES, null);
......@@ -961,13 +1033,12 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
private void on_resource_loaded(string id) {
Gee.Iterator<Geary.Attachment> displayed =
this.displayed_attachments.iterator();
displayed.next();
while (displayed.has_next()) {
displayed.next();
Geary.Attachment? attachment = displayed.get();
if (attachment.content_id == id) {
displayed.remove();
}
displayed.next();
}
}
......@@ -979,8 +1050,8 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
break;
}
}
if (all_loaded && !this.message_bodies_loaded) {
this.message_bodies_loaded = true;
if (all_loaded && this.message_body_state != COMPLETED) {
this.message_body_state = COMPLETED;
this.message_bodies_loaded_lock.blind_notify();
// Update attachments once the web views have finished
......@@ -1012,4 +1083,12 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
len < this.displayed_attachments.size);
}
private void on_service_status_change() {
if (this.message_body_state == FAILED &&
!this.load_cancellable.is_cancelled() &&
is_online()) {
this.fetch_remote_body.begin();
}
}
}
......@@ -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 {
......@@ -147,14 +156,9 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
throws GLib.Error {
this.is_expanded = true;
update_row_expansion();
if (!this.view.message_body_load_started) {
if (this.view.message_body_state == NOT_STARTED) {
yield this.view.load_body();
}
foreach (ConversationMessage message in this.view) {
if (!message.web_view.has_valid_height) {
message.web_view.queue_resize();
}
};
}
public override void collapse() {
......@@ -305,6 +309,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 +338,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 +383,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 +409,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 +463,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 +544,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.
*/
......@@ -727,14 +747,14 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
// Loads full version of an email, adds it to the listbox
private async void load_full_email(Geary.EmailIdentifier id)
throws Error {
throws GLib.Error {
// Even though it would save a around-trip, don't load the
// full email here so that ConverationEmail can handle it if
// the full email isn't actually available in the same way as
// any other.
Geary.Email full_email = yield this.email_store.fetch_email_async(
id,
(
REQUIRED_FIELDS |
ConversationEmail.REQUIRED_FOR_CONSTRUCT |
ConversationEmail.REQUIRED_FOR_LOAD
),
REQUIRED_FIELDS | ConversationEmail.REQUIRED_FOR_CONSTRUCT,
Geary.Folder.ListFlags.NONE,
this.cancellable
);
......@@ -773,6 +793,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 +849,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_bodies_loaded &&
!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 +883,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);
......@@ -881,10 +902,10 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
}
private void apply_search_terms(EmailRow row) {
if (row.view.message_bodies_loaded) {
if (row.view.message_body_state == COMPLETED) {
this.apply_search_terms_impl.begin(row);
} else {
row.view.notify["message-bodies-loaded"].connect(() => {
row.view.notify["message-body-state"].connect(() => {
this.apply_search_terms_impl.begin(row);
});
}
......@@ -1016,6 +1037,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) {
......
......@@ -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 SHOW_PROGRESS_TIMEOUT_MSEC = 1000;
private const int HIDE_PROGRESS_TIMEOUT_MSEC = 1000;
private const int PULSE_TIMEOUT_MSEC = 250;
......@@ -201,7 +203,7 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
[GtkChild]
public Gtk.Grid body_container;
[GtkChild]
public Gtk.ProgressBar body_progress;
private Gtk.ProgressBar body_progress;
[GtkChild]
private Gtk.Popover link_popover;
......@@ -213,6 +215,8 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
[GtkChild]
private Gtk.InfoBar remote_images_infobar;
private Gtk.Widget? body_placeholder = null;