[geary/wip/765516-gtk-widget-conversation-viewer: 132/142] Replace the conversation list store's model on folder change.
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/wip/765516-gtk-widget-conversation-viewer: 132/142] Replace the conversation list store's model on folder change.
- Date: Fri, 2 Sep 2016 04:36:43 +0000 (UTC)
commit ed92f8068e34a0a101ddc1657462e77ef3abc155
Author: Michael James Gratton <mike vee net>
Date: Fri Aug 19 12:03:40 2016 +1000
Replace the conversation list store's model on folder change.
Rather than keeping a single conversation list model instance around -
attaching and adding conversations then clearing and detaching
conversation viewers, simply remove the old model instance and add a new
one.
This also allows the ConversationListStore::is_clearing property to be
removed, fixing a related critical warning, and simply unhooking the
selection-changed signal handler instead when changing the model.
* src/client/components/main-window.vala (MainWindow): Remove props that
are now dynamically managed. When the conversation monitor changes,
replace the ConversationListView's model, update progress listeners.
* src/client/conversation-list/conversation-list-view.vala
(ConversationListView): Remove list store property since the parent
class already maintains it. Override ::get_model() and ::set_model() to
use ConversationListStore instances, manage signals on model instances
when they are set/unset.
* src/client/conversation-list/conversation-list-store.vala
(ConversationListStore): Replae destructor with an explict destroy
method that ensures the object is cleaned up enough to actualy be
finalised. Remove use of is_clearing in ::on_selection_changed() to fix
critical warning.
src/client/application/geary-controller.vala | 2 +-
src/client/components/main-window.vala | 40 ++++---
.../conversation-list/conversation-list-store.vala | 127 +++++++++----------
.../conversation-list/conversation-list-view.vala | 125 ++++++++++++--------
4 files changed, 156 insertions(+), 138 deletions(-)
---
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index afb2313..79fb581 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -1328,7 +1328,7 @@ public class GearyController : Geary.BaseObject {
// If the folder is being unset, clear the message list and exit here.
if (folder == null) {
current_folder = null;
- main_window.conversation_list_store.clear();
+ main_window.conversation_list_view.set_model(null);
main_window.main_toolbar.folder = null;
folder_selected(null);
diff --git a/src/client/components/main-window.vala b/src/client/components/main-window.vala
index ed006dc..65bdac3 100644
--- a/src/client/components/main-window.vala
+++ b/src/client/components/main-window.vala
@@ -15,10 +15,9 @@ public class MainWindow : Gtk.ApplicationWindow {
public signal void on_shift_key(bool pressed);
public FolderList.Tree folder_list { get; private set; default = new FolderList.Tree(); }
- public ConversationListStore conversation_list_store { get; private set; default = new
ConversationListStore(); }
public MainToolbar main_toolbar { get; private set; }
public SearchBar search_bar { get; private set; default = new SearchBar(); }
- public ConversationListView conversation_list_view { get; private set; }
+ public ConversationListView conversation_list_view { get; private set; default = new
ConversationListView(); }
public ConversationViewer conversation_viewer { get; private set; default = new ConversationViewer(); }
public StatusBar status_bar { get; private set; default = new StatusBar(); }
public Geary.Folder? current_folder { get; private set; default = null; }
@@ -35,15 +34,12 @@ public class MainWindow : Gtk.ApplicationWindow {
private Gtk.Box folder_box;
private Gtk.Box conversation_box;
private Geary.AggregateProgressMonitor progress_monitor = new Geary.AggregateProgressMonitor();
- private Geary.ProgressMonitor? conversation_monitor_progress = null;
private Geary.ProgressMonitor? folder_progress = null;
public MainWindow(GearyApplication application) {
Object(application: application);
set_show_menubar(false);
- conversation_list_view = new ConversationListView(conversation_list_store);
-
add_events(Gdk.EventMask.KEY_PRESS_MASK | Gdk.EventMask.KEY_RELEASE_MASK
| Gdk.EventMask.FOCUS_CHANGE_MASK);
@@ -77,7 +73,6 @@ public class MainWindow : Gtk.ApplicationWindow {
add_accel_group(GearyApplication.instance.ui_manager.get_accel_group());
spinner.set_progress_monitor(progress_monitor);
- progress_monitor.add(conversation_list_store.preview_monitor);
delete_event.connect(on_delete_event);
key_press_event.connect(on_key_press_event);
@@ -249,24 +244,31 @@ public class MainWindow : Gtk.ApplicationWindow {
on_shift_key(false);
return false;
}
-
+
private void on_conversation_monitor_changed() {
- Geary.App.ConversationMonitor? conversation_monitor =
+ ConversationListStore? old_model = this.conversation_list_view.get_model();
+ if (old_model != null) {
+ this.progress_monitor.remove(old_model.preview_monitor);
+ this.progress_monitor.remove(old_model.conversations.progress_monitor);
+ }
+
+ Geary.App.ConversationMonitor? conversations =
GearyApplication.instance.controller.current_conversations;
-
- // Remove existing progress monitor.
- if (conversation_monitor_progress != null) {
- progress_monitor.remove(conversation_monitor_progress);
- conversation_monitor_progress = null;
+
+ if (conversations != null) {
+ ConversationListStore new_model =
+ new ConversationListStore(conversations);
+ this.progress_monitor.add(new_model.preview_monitor);
+ this.progress_monitor.add(conversations.progress_monitor);
+ this.conversation_list_view.set_model(new_model);
}
-
- // Add new one.
- if (conversation_monitor != null) {
- conversation_monitor_progress = conversation_monitor.progress_monitor;
- progress_monitor.add(conversation_monitor_progress);
+
+ if (old_model != null) {
+ // Must be destroyed, but only after it has been replaced.
+ old_model.destroy();
}
}
-
+
private void on_folder_selected(Geary.Folder? folder) {
if (folder_progress != null) {
progress_monitor.remove(folder_progress);
diff --git a/src/client/conversation-list/conversation-list-store.vala
b/src/client/conversation-list/conversation-list-store.vala
index 7e3d8c2..e869591 100644
--- a/src/client/conversation-list/conversation-list-store.vala
+++ b/src/client/conversation-list/conversation-list-store.vala
@@ -67,12 +67,11 @@ public class ConversationListStore : Gtk.ListStore {
return row.get_model().get_iter(out iter, get_path());
}
}
-
- public Geary.ProgressMonitor preview_monitor { get; private set; default =
+
+ public Geary.App.ConversationMonitor conversations { get; set; }
+ public Geary.ProgressMonitor preview_monitor { get; private set; default =
new Geary.SimpleProgressMonitor(Geary.ProgressType.ACTIVITY); }
- public bool is_clearing { get; private set; default = false; }
-
- private Geary.App.ConversationMonitor conversation_monitor;
+
private Gee.HashMap<Geary.App.Conversation, RowWrapper> row_map = new Gee.HashMap<
Geary.App.Conversation, RowWrapper>();
private Geary.App.EmailStore? email_store = null;
@@ -84,65 +83,50 @@ public class ConversationListStore : Gtk.ListStore {
public signal void conversations_added_began();
public signal void conversations_added_finished();
-
- public ConversationListStore() {
+
+ public ConversationListStore(Geary.App.ConversationMonitor conversations) {
set_column_types(Column.get_types());
set_default_sort_func(sort_by_date);
set_sort_column_id(Gtk.SortColumn.DEFAULT, Gtk.SortType.DESCENDING);
-
+
+ this.conversations = conversations;
+ this.update_id = Timeout.add_seconds_full(
+ Priority.LOW, 60, update_date_strings
+ );
+ this.email_store = new Geary.App.EmailStore(
+ conversations.folder.account
+ );
GearyApplication.instance.config.settings.changed[Configuration.DISPLAY_PREVIEW_KEY].connect(
on_display_preview_changed);
- update_id = Timeout.add_seconds_full(Priority.LOW, 60, update_date_strings);
-
- GearyApplication.instance.controller.notify[GearyController.PROP_CURRENT_CONVERSATION].
- connect(on_conversation_monitor_changed);
- }
-
- ~ConversationListStore() {
- if (update_id != 0)
- Source.remove(update_id);
-
- GearyApplication.instance.controller.notify[GearyController.PROP_CURRENT_CONVERSATION].
- disconnect(on_conversation_monitor_changed);
+
+ conversations.scan_completed.connect(on_scan_completed);
+ conversations.conversations_added.connect(on_conversations_added);
+ conversations.conversation_removed.connect(on_conversation_removed);
+ conversations.conversation_appended.connect(on_conversation_appended);
+ conversations.conversation_trimmed.connect(on_conversation_trimmed);
+ conversations.email_flags_changed.connect(on_email_flags_changed);
+
+ // add all existing conversations
+ on_conversations_added(conversations.get_conversations());
}
-
- private void on_conversation_monitor_changed() {
- is_clearing = true;
-
- if (conversation_monitor != null) {
- conversation_monitor.scan_completed.disconnect(on_scan_completed);
- conversation_monitor.conversations_added.disconnect(on_conversations_added);
- conversation_monitor.conversation_removed.disconnect(on_conversation_removed);
- conversation_monitor.conversation_appended.disconnect(on_conversation_appended);
- conversation_monitor.conversation_trimmed.disconnect(on_conversation_trimmed);
- conversation_monitor.email_flags_changed.disconnect(on_email_flags_changed);
- }
-
- cancellable.cancel();
- cancellable = new Cancellable();
-
+
+ public void destroy() {
+ this.cancellable.cancel();
clear();
- row_map.clear();
- conversation_monitor = GearyApplication.instance.controller.current_conversations;
- email_store = null;
-
- if (conversation_monitor != null) {
- email_store = new Geary.App.EmailStore(conversation_monitor.folder.account);
-
- // add all existing conversations
- on_conversations_added(conversation_monitor.get_conversations());
-
- conversation_monitor.scan_completed.connect(on_scan_completed);
- conversation_monitor.conversations_added.connect(on_conversations_added);
- conversation_monitor.conversation_removed.connect(on_conversation_removed);
- conversation_monitor.conversation_appended.connect(on_conversation_appended);
- conversation_monitor.conversation_trimmed.connect(on_conversation_trimmed);
- conversation_monitor.email_flags_changed.connect(on_email_flags_changed);
+
+ // Release circular refs.
+ this.row_map.clear();
+ if (this.update_id != 0) {
+ Source.remove(this.update_id);
+ this.update_id = 0;
}
-
- is_clearing = false;
+ // We need to clear the sort func to release its ref to this
+ // object so it can be finalised, but the API doesn't let a
+ // null sort fun to be set, hence set it to a dummy static
+ // func.
+ set_default_sort_func(ConversationListStore.finaliser_sort_by_date);
}
-
+
public Geary.App.Conversation? get_conversation_at_path(Gtk.TreePath path) {
Gtk.TreeIter iter;
if (!get_iter(out iter, path))
@@ -228,7 +212,7 @@ public class ConversationListStore : Gtk.ListStore {
// the user experience
Gee.TreeSet<Geary.App.Conversation> sorted_conversations = new Gee.TreeSet<Geary.App.Conversation>(
compare_conversation_descending);
- sorted_conversations.add_all(conversation_monitor.get_conversations());
+ sorted_conversations.add_all(this.conversations.get_conversations());
foreach (Geary.App.Conversation conversation in sorted_conversations) {
// find oldest unread message for the preview
Geary.Email? need_preview = null;
@@ -282,12 +266,15 @@ public class ConversationListStore : Gtk.ListStore {
else
debug("Unable to find preview for conversation");
}
-
+
private void set_row(Gtk.TreeIter iter, Geary.App.Conversation conversation, Geary.Email preview) {
- FormattedConversationData conversation_data = new FormattedConversationData(conversation,
- preview, conversation_monitor.folder,
- conversation_monitor.folder.account.information.get_all_mailboxes());
-
+ FormattedConversationData conversation_data = new FormattedConversationData(
+ conversation,
+ preview,
+ this.conversations.folder,
+ this.conversations.folder.account.information.get_all_mailboxes()
+ );
+
Gtk.TreePath? path = get_path(iter);
assert(path != null);
RowWrapper wrapper = new RowWrapper(this, conversation, path);
@@ -452,7 +439,7 @@ public class ConversationListStore : Gtk.ListStore {
}
private void on_display_preview_changed() {
- refresh_previews_async.begin(conversation_monitor);
+ refresh_previews_async.begin(this.conversations);
}
private void on_email_flags_changed(Geary.App.Conversation conversation) {
@@ -461,7 +448,7 @@ public class ConversationListStore : Gtk.ListStore {
// refresh previews because the oldest unread message is displayed as the preview, and if
// that's changed, need to change the preview
// TODO: need support code to load preview for single conversation, not scan all
- refresh_previews_async.begin(conversation_monitor);
+ refresh_previews_async.begin(this.conversations);
}
private int sort_by_date(Gtk.TreeModel model, Gtk.TreeIter aiter, Gtk.TreeIter biter) {
@@ -472,13 +459,12 @@ public class ConversationListStore : Gtk.ListStore {
return compare_conversation_ascending(a, b);
}
-
+
private bool update_date_strings() {
this.foreach(update_date_string);
- // Keep calling this SourceFunc
- return true;
+ return Source.CONTINUE;
}
-
+
private bool update_date_string(Gtk.TreeModel model, Gtk.TreePath path, Gtk.TreeIter iter) {
FormattedConversationData? message_data;
model.get(iter, Column.CONVERSATION_DATA, out message_data);
@@ -489,5 +475,12 @@ public class ConversationListStore : Gtk.ListStore {
// Continue iterating, don't stop
return false;
}
+
+ private static int finaliser_sort_by_date(Gtk.TreeModel m,
+ Gtk.TreeIter a,
+ Gtk.TreeIter b) {
+ return 0;
+ }
+
}
diff --git a/src/client/conversation-list/conversation-list-view.vala
b/src/client/conversation-list/conversation-list-view.vala
index 587b07f..2a14eba 100644
--- a/src/client/conversation-list/conversation-list-view.vala
+++ b/src/client/conversation-list/conversation-list-view.vala
@@ -13,14 +13,13 @@ public class ConversationListView : Gtk.TreeView {
// scroll adjustment seen at the call to load_more().
private double last_upper = -1.0;
private bool reset_adjustment = false;
- private Gee.Set<Geary.App.Conversation> selected = new Gee.HashSet<Geary.App.Conversation>();
- private ConversationListStore conversation_list_store;
private Geary.App.ConversationMonitor? conversation_monitor;
private Gee.Set<Geary.App.Conversation>? current_visible_conversations = null;
private Geary.Scheduler.Scheduled? scheduled_update_visible_conversations = null;
private Gtk.Menu? context_menu = null;
+ private Gee.Set<Geary.App.Conversation> selected = new Gee.HashSet<Geary.App.Conversation>();
private uint selection_changed_id = 0;
-
+
public signal void conversations_selected(Gee.Set<Geary.App.Conversation> selected);
// Signal for when a conversation has been double-clicked, or selected and enter is pressed.
@@ -34,33 +33,21 @@ public class ConversationListView : Gtk.TreeView {
Geary.EmailFlags? flags_to_add, Geary.EmailFlags? flags_to_remove, bool only_mark_preview);
public signal void visible_conversations_changed(Gee.Set<Geary.App.Conversation> visible);
-
- public ConversationListView(ConversationListStore conversation_list_store) {
- this.conversation_list_store = conversation_list_store;
- set_model(conversation_list_store);
-
+
+ public ConversationListView() {
set_show_expanders(false);
set_headers_visible(false);
-
+
append_column(create_column(ConversationListStore.Column.CONVERSATION_DATA,
new ConversationListCellRenderer(), ConversationListStore.Column.CONVERSATION_DATA.to_string(),
0));
-
+
Gtk.TreeSelection selection = get_selection();
- selection.changed.connect(on_selection_changed);
selection.set_mode(Gtk.SelectionMode.MULTIPLE);
style_set.connect(on_style_changed);
show.connect(on_show);
row_activated.connect(on_row_activated);
-
- get_model().row_inserted.connect(on_rows_changed);
- get_model().rows_reordered.connect(on_rows_changed);
- get_model().row_changed.connect(on_rows_changed);
- get_model().row_deleted.connect(on_rows_changed);
- get_model().row_deleted.connect(on_row_deleted);
-
- conversation_list_store.conversations_added_began.connect(on_conversations_added_began);
- conversation_list_store.conversations_added_finished.connect(on_conversations_added_finished);
+
button_press_event.connect(on_button_press);
// Set up drag and drop.
@@ -82,7 +69,50 @@ public class ConversationListView : Gtk.TreeView {
assert(binding_set != null);
Gtk.BindingEntry.remove(binding_set, Gdk.Key.N, Gdk.ModifierType.CONTROL_MASK);
}
-
+
+ public new ConversationListStore? get_model() {
+ return (this as Gtk.TreeView).get_model() as ConversationListStore;
+ }
+
+ public new void set_model(ConversationListStore? new_store) {
+ ConversationListStore? old_store = get_model();
+ if (old_store != null) {
+ old_store.conversations_added_finished.disconnect(
+ on_conversations_added_finished
+ );
+ old_store.conversations_added_began.disconnect(
+ on_conversations_added_began
+ );
+ old_store.row_inserted.disconnect(on_rows_changed);
+ old_store.rows_reordered.disconnect(on_rows_changed);
+ old_store.row_changed.disconnect(on_rows_changed);
+ old_store.row_deleted.disconnect(on_rows_changed);
+ old_store.row_deleted.disconnect(on_row_deleted);
+ }
+
+ if (new_store != null) {
+ new_store.row_inserted.connect(on_rows_changed);
+ new_store.rows_reordered.connect(on_rows_changed);
+ new_store.row_changed.connect(on_rows_changed);
+ new_store.row_deleted.connect(on_rows_changed);
+ new_store.row_deleted.connect(on_row_deleted);
+ new_store.conversations_added_began.connect(
+ on_conversations_added_began
+ );
+ new_store.conversations_added_finished.connect(
+ on_conversations_added_finished
+ );
+ }
+
+ // Disconnect the selection handler since we don't want to
+ // fire selection signals while changing the model.
+ Gtk.TreeSelection selection = get_selection();
+ selection.changed.disconnect(on_selection_changed);
+ (this as Gtk.TreeView).set_model(new_store);
+ this.selected.clear();
+ selection.changed.connect(on_selection_changed);
+ }
+
private void on_conversation_monitor_changed() {
if (conversation_monitor != null) {
conversation_monitor.scan_started.disconnect(on_scan_started);
@@ -178,7 +208,7 @@ public class ConversationListView : Gtk.TreeView {
// Get the current conversation. If it's selected, we'll apply the mark operation to
// all selected conversations; otherwise, it just applies to this one.
- Geary.App.Conversation conversation = conversation_list_store.get_conversation_at_path(path);
+ Geary.App.Conversation conversation = get_model().get_conversation_at_path(path);
Gee.Collection<Geary.App.Conversation> to_mark;
if (GearyApplication.instance.controller.get_selected_conversations().contains(conversation))
to_mark = GearyApplication.instance.controller.get_selected_conversations();
@@ -215,7 +245,7 @@ public class ConversationListView : Gtk.TreeView {
return true;
if (event.button == 3 && event.type == Gdk.EventType.BUTTON_PRESS) {
- Geary.App.Conversation conversation = conversation_list_store.get_conversation_at_path(path);
+ Geary.App.Conversation conversation = get_model().get_conversation_at_path(path);
string?[] action_names = {};
action_names += GearyController.ACTION_DELETE_MESSAGE;
@@ -316,29 +346,22 @@ public class ConversationListView : Gtk.TreeView {
if (this.selection_changed_id != 0)
Source.remove(this.selection_changed_id);
- if (this.conversation_list_store.is_clearing) {
- // The list store is clearing, so the folder has changed
- // and we don't want to notify about the selection
- // changing, so just clear it.
- this.selected.clear();
- } else {
- // Schedule processing selection changes at low idle for
- // two reasons: (a) if a lot of changes come in
- // back-to-back, this allows for all that activity to
- // settle before updating state and firing signals (which
- // results in a lot of I/O), and (b) it means the
- // ConversationMonitor's signals may be processed in any
- // order by this class and the ConversationListView and
- // not result in a lot of screen flashing and (again)
- // unnecessary I/O as both classes update selection state.
- this.selection_changed_id = Idle.add(() => {
- // De-schedule the callback
- this.selection_changed_id = 0;
+ // Schedule processing selection changes at low idle for
+ // two reasons: (a) if a lot of changes come in
+ // back-to-back, this allows for all that activity to
+ // settle before updating state and firing signals (which
+ // results in a lot of I/O), and (b) it means the
+ // ConversationMonitor's signals may be processed in any
+ // order by this class and the ConversationListView and
+ // not result in a lot of screen flashing and (again)
+ // unnecessary I/O as both classes update selection state.
+ this.selection_changed_id = Idle.add(() => {
+ // De-schedule the callback
+ this.selection_changed_id = 0;
- do_selection_changed();
- return false;
- }, Priority.LOW);
- }
+ do_selection_changed();
+ return Source.REMOVE;
+ }, Priority.LOW);
}
// Gtk.TreeSelection can fire its "changed" signal even when
@@ -353,7 +376,7 @@ public class ConversationListView : Gtk.TreeView {
// signal if different
foreach (Gtk.TreePath path in paths) {
Geary.App.Conversation? conversation =
- this.conversation_list_store.get_conversation_at_path(path);
+ get_model().get_conversation_at_path(path);
if (conversation != null)
new_selection.add(conversation);
}
@@ -376,7 +399,7 @@ public class ConversationListView : Gtk.TreeView {
return visible_conversations;
while (start_path.compare(end_path) <= 0) {
- Geary.App.Conversation? conversation =
conversation_list_store.get_conversation_at_path(start_path);
+ Geary.App.Conversation? conversation = get_model().get_conversation_at_path(start_path);
if (conversation != null)
visible_conversations.add(conversation);
@@ -390,7 +413,7 @@ public class ConversationListView : Gtk.TreeView {
Gee.HashSet<Geary.App.Conversation> selected_conversations = new
Gee.HashSet<Geary.App.Conversation>();
foreach (Gtk.TreePath path in get_all_selected_paths()) {
- Geary.App.Conversation? conversation = conversation_list_store.get_conversation_at_path(path);
+ Geary.App.Conversation? conversation = get_model().get_conversation_at_path(path);
if (path != null)
selected_conversations.add(conversation);
}
@@ -427,7 +450,7 @@ public class ConversationListView : Gtk.TreeView {
}
public void select_conversation(Geary.App.Conversation conversation) {
- Gtk.TreePath path = conversation_list_store.get_path_for_conversation(conversation);
+ Gtk.TreePath path = get_model().get_path_for_conversation(conversation);
if (path != null)
set_cursor(path, null, false);
}
@@ -435,7 +458,7 @@ public class ConversationListView : Gtk.TreeView {
public void select_conversations(Gee.Set<Geary.App.Conversation> conversations) {
Gtk.TreeSelection selection = get_selection();
foreach (Geary.App.Conversation conversation in conversations) {
- Gtk.TreePath path = conversation_list_store.get_path_for_conversation(conversation);
+ Gtk.TreePath path = get_model().get_path_for_conversation(conversation);
if (path != null)
selection.select_path(path);
}
@@ -465,7 +488,7 @@ public class ConversationListView : Gtk.TreeView {
}
private void on_row_activated(Gtk.TreePath path) {
- Geary.App.Conversation? c = conversation_list_store.get_conversation_at_path(path);
+ Geary.App.Conversation? c = get_model().get_conversation_at_path(path);
if (c != null)
conversation_activated(c);
}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
