[geary/wip/draft-management: 4/7] Rework how managing composers works when switching conversations



commit 032825c300edd069d589653fb2c12000db88ca8d
Author: Michael Gratton <mike vee net>
Date:   Fri Aug 2 09:58:04 2019 +1000

    Rework how managing composers works when switching conversations
    
    Move responsibility for managing composers when switching conversations
    from the controller to the main window.
    
    Removes Application.Controller::should_create_new_composer and
    ::can_switch_conversation_view, moving half the code to
    ::create_compose_widget_async since it was only used there, and moving
    the rest to a new MainWindow::close_composer method, where it belongs.
    Add new MainWindow::has_composers property, and convert call sites of
    removed methods to use these instead.
    
    Update ConversationViewer to clean up composer management code a bit and
    add a ::current_composer property, and add a new ComposerWidget:close
    method to support the above changes.

 src/client/application/application-controller.vala | 108 +++++++--------------
 src/client/components/main-window.vala             |  65 +++++++++----
 src/client/composer/composer-widget.vala           |  12 ++-
 .../conversation-list/conversation-list-view.vala  |  20 ++--
 .../conversation-viewer/conversation-viewer.vala   |  62 ++++++++----
 src/client/folder-list/folder-list-tree.vala       |   7 +-
 6 files changed, 153 insertions(+), 121 deletions(-)
---
diff --git a/src/client/application/application-controller.vala 
b/src/client/application/application-controller.vala
index 62352cf0..acd62f75 100644
--- a/src/client/application/application-controller.vala
+++ b/src/client/application/application-controller.vala
@@ -1184,7 +1184,7 @@ public class Application.Controller : Geary.BaseObject {
         this.selected_conversations = selected;
         get_window_action(ACTION_FIND_IN_CONVERSATION).set_enabled(false);
         ConversationViewer viewer = this.main_window.conversation_viewer;
-        if (this.current_folder != null && !viewer.is_composer_visible) {
+        if (this.current_folder != null && !this.main_window.has_composer) {
             switch(selected.size) {
             case 0:
                 enable_message_buttons(false);
@@ -1249,6 +1249,8 @@ public class Application.Controller : Geary.BaseObject {
             Geary.App.Conversation.Location.IN_FOLDER
         );
 
+        // Check all known composers since the draft may be open in a
+        // detached composer
         bool already_open = false;
         foreach (ComposerWidget composer in this.composer_widgets) {
             if (composer.draft_id != null &&
@@ -2038,8 +2040,39 @@ public class Application.Controller : Geary.BaseObject {
         if (current_account == null)
             return;
 
-        if (!should_create_new_composer(compose_type, referred, quote, is_draft))
-            return;
+        // There's a few situations where we can re-use an existing
+        // composer, check for these first.
+
+        if (compose_type == NEW_MESSAGE && !is_draft) {
+            // We're creating a new message that isn't a draft, if
+            // there's already a composer open, just use that
+            foreach (ComposerWidget composer in this.composer_widgets) {
+                if (composer.state == PANED && composer.is_blank) {
+                    composer.present();
+                    composer.set_focus();
+                    return;
+                }
+            }
+        } else if (compose_type != NEW_MESSAGE) {
+            // We're replying, see whether we already have a reply for
+            // that message and if so, insert a quote into that.
+            foreach (ComposerWidget composer in this.composer_widgets) {
+                if (composer.state != DETACHED &&
+                    ((referred != null && composer.referred_ids.contains(referred.id)) ||
+                     quote != null)) {
+                    composer.change_compose_type(compose_type, referred, quote);
+                    return;
+                }
+            }
+
+            // Can't re-use an existing composer, so need to create a
+            // new one. Replies must open inline in the main window,
+            // so we need to ensure there are no composers open there
+            // first.
+            if (!this.main_window.close_composer()) {
+                return;
+            }
+        }
 
         ComposerWidget widget;
         if (mailto != null) {
@@ -2089,72 +2122,6 @@ public class Application.Controller : Geary.BaseObject {
         widget.set_focus();
     }
 
-    private bool should_create_new_composer(ComposerWidget.ComposeType? compose_type,
-                                            Geary.Email? referred,
-                                            string? quote,
-                                            bool is_draft) {
-        // In we're replying, see whether we already have a reply for that message.
-        if (compose_type != null && compose_type != ComposerWidget.ComposeType.NEW_MESSAGE) {
-            foreach (ComposerWidget cw in composer_widgets) {
-                if (cw.state != ComposerWidget.ComposerState.DETACHED &&
-                    ((referred != null && cw.referred_ids.contains(referred.id)) ||
-                     quote != null)) {
-                    cw.change_compose_type(compose_type, referred, quote);
-                    return false;
-                }
-            }
-            return true;
-        }
-
-        // If there are no inline composers, go ahead!
-        if (!any_inline_composers())
-            return true;
-
-        // If we're resuming a draft with open composers, open in a new window.
-        if (is_draft) {
-            return true;
-        }
-
-        // If we're creating a new message, and there's already a new message open, focus on
-        // it if it hasn't been modified; otherwise open a new composer in a new window.
-        if (compose_type == ComposerWidget.ComposeType.NEW_MESSAGE) {
-            foreach (ComposerWidget cw in composer_widgets) {
-                if (cw.state == ComposerWidget.ComposerState.PANED) {
-                    if (!cw.is_blank) {
-                        return true;
-                    } else {
-                        cw.change_compose_type(compose_type);  // To refocus
-                        return false;
-                    }
-                }
-            }
-        }
-
-        // Find out what to do with the inline composers.
-        // TODO: Remove this in favor of automatically saving drafts
-        this.main_window.present();
-        bool create_okay = true;
-        foreach (ComposerWidget cw in composer_widgets) {
-            if (cw.state != ComposerWidget.ComposerState.DETACHED &&
-                cw.should_close() == ComposerWidget.CloseStatus.CANCEL_CLOSE) {
-                create_okay = false;
-                break;
-            }
-        }
-        return create_okay;
-    }
-
-    public bool can_switch_conversation_view() {
-        return should_create_new_composer(null, null, null, false);
-    }
-
-    public bool any_inline_composers() {
-        foreach (ComposerWidget cw in composer_widgets)
-            if (cw.state != ComposerWidget.ComposerState.DETACHED)
-                return true;
-        return false;
-    }
-
     private void on_composer_widget_destroy(Gtk.Widget sender) {
         composer_widgets.remove((ComposerWidget) sender);
         debug(@"Destroying composer of type $(((ComposerWidget) sender).compose_type); "
@@ -2346,9 +2313,6 @@ public class Application.Controller : Geary.BaseObject {
 
     private async void archive_or_delete_selection_async(bool archive, bool trash,
         Cancellable? cancellable) throws Error {
-        if (!can_switch_conversation_view())
-            return;
-
         ConversationListBox list_view =
             main_window.conversation_viewer.current_list;
         if (list_view != null &&
diff --git a/src/client/components/main-window.vala b/src/client/components/main-window.vala
index ad46db79..c38e1b97 100644
--- a/src/client/components/main-window.vala
+++ b/src/client/components/main-window.vala
@@ -28,6 +28,13 @@ public class MainWindow : Gtk.ApplicationWindow, Geary.BaseInterface {
         get; private set; default = null;
     }
 
+    /** Determines if a composer is currently open in this window. */
+    public bool has_composer {
+        get {
+            return (this.conversation_viewer.current_composer != null);
+        }
+    }
+
     private Geary.AggregateProgressMonitor progress_monitor = new Geary.AggregateProgressMonitor();
 
     // Used to save/load the window state between sessions.
@@ -122,17 +129,6 @@ public class MainWindow : Gtk.ApplicationWindow, Geary.BaseInterface {
         base_unref();
     }
 
-    public void open_composer_for_mailbox(Geary.RFC822.MailboxAddress to) {
-        Application.Controller controller = this.application.controller;
-        ComposerWidget composer = new ComposerWidget(
-            this.application, this.current_folder.account, null, NEW_MESSAGE
-        );
-        composer.to = to.to_full_display();
-        controller.add_composer(composer);
-        show_composer(composer);
-        composer.load.begin(null, null, false);
-    }
-
     /** Updates the window's account status info bars. */
     public void update_account_status(Geary.Account.Status status,
                                       bool has_auth_error,
@@ -218,15 +214,21 @@ public class MainWindow : Gtk.ApplicationWindow, Geary.BaseInterface {
         this.info_bar_frame.show();
     }
 
-    /** Displays a composer in the window if possible, else in a new window. */
-    public void show_composer(ComposerWidget composer) {
-        bool has_composer = (
-            this.conversation_viewer.is_composer_visible ||
-            (this.conversation_viewer.current_list != null &&
-             this.conversation_viewer.current_list.has_composer)
+    /** Displays a composer addressed to a specific email address. */
+    public void open_composer_for_mailbox(Geary.RFC822.MailboxAddress to) {
+        Application.Controller controller = this.application.controller;
+        ComposerWidget composer = new ComposerWidget(
+            this.application, this.current_folder.account, null, NEW_MESSAGE
         );
+        composer.to = to.to_full_display();
+        controller.add_composer(composer);
+        show_composer(composer);
+        composer.load.begin(null, null, false);
+    }
 
-        if (has_composer) {
+    /** Displays a composer in the window if possible, else in a new window. */
+    public void show_composer(ComposerWidget composer) {
+        if (this.has_composer) {
             composer.state = ComposerWidget.ComposerState.DETACHED;
             new ComposerWindow(composer, this.application);
         } else {
@@ -235,6 +237,29 @@ public class MainWindow : Gtk.ApplicationWindow, Geary.BaseInterface {
         }
     }
 
+    /**
+     * Closes any open composers after prompting the user.
+     *
+     * Returns true if none were open or the user approved closing
+     * them.
+     */
+    public bool close_composer() {
+        bool closed = true;
+        ComposerWidget? composer = this.conversation_viewer.current_composer;
+        if (composer != null) {
+            switch (composer.should_close()) {
+            case DO_CLOSE:
+                composer.close();
+                break;
+
+            case CANCEL_CLOSE:
+                closed = false;
+                break;
+            }
+        }
+        return closed;
+    }
+
     private void load_config(Configuration config) {
         // This code both loads AND saves the pane positions with live updating. This is more
         // resilient against crashes because the value in dconf changes *immediately*, and
@@ -619,8 +644,8 @@ public class MainWindow : Gtk.ApplicationWindow, Geary.BaseInterface {
             // there isn't already a selection or a composer to avoid
             // interrupting those.
             if (!this.application.config.autoselect &&
-                this.conversation_list_view.get_selection().count_selected_rows() == 0 &&
-                !this.conversation_viewer.is_composer_visible) {
+                !this.has_composer &&
+                this.conversation_list_view.get_selection().count_selected_rows() == 0) {
                 this.conversation_viewer.show_none_selected();
                 this.application.controller.enable_message_buttons(false);
             }
diff --git a/src/client/composer/composer-widget.vala b/src/client/composer/composer-widget.vala
index e3e331d7..de225573 100644
--- a/src/client/composer/composer-widget.vala
+++ b/src/client/composer/composer-widget.vala
@@ -623,6 +623,11 @@ public class ComposerWidget : Gtk.EventBox, Geary.BaseInterface {
         }
     }
 
+    /** Closes the composer unconditionally. */
+    public void close() {
+        this.container.close_container();
+    }
+
     /**
      * Loads the message into the composer editor.
      */
@@ -779,7 +784,7 @@ public class ComposerWidget : Gtk.EventBox, Geary.BaseInterface {
                 if (referred.references != null)
                     this.references = referred.references.to_rfc822_string();
                 if (referred.subject != null)
-                    this.subject = referred.subject.value;
+                    this.subject = referred.subject.value ?? "";
                 try {
                     Geary.RFC822.Message message = referred.get_message();
                     if (message.has_html_body()) {
@@ -1213,8 +1218,9 @@ public class ComposerWidget : Gtk.EventBox, Geary.BaseInterface {
     }
 
     private void on_close(SimpleAction action, Variant? param) {
-        if (should_close() == CloseStatus.DO_CLOSE)
-            this.container.close_container();
+        if (should_close() == CloseStatus.DO_CLOSE) {
+            close();
+        }
     }
 
     private void on_close_and_save(SimpleAction action, Variant? param) {
diff --git a/src/client/conversation-list/conversation-list-view.vala 
b/src/client/conversation-list/conversation-list-view.vala
index 9a18ee7f..c72974c8 100644
--- a/src/client/conversation-list/conversation-list-view.vala
+++ b/src/client/conversation-list/conversation-list-view.vala
@@ -201,11 +201,14 @@ public class ConversationListView : Gtk.TreeView, Geary.BaseInterface {
         check_load_more();
 
         // Select the first conversation, if autoselect is enabled,
-        // nothing has been selected yet and we're not composing.
+        // nothing has been selected yet and we're not showing a
+        // composer.
         if (GearyApplication.instance.config.autoselect &&
-            get_selection().count_selected_rows() == 0 &&
-            !GearyApplication.instance.controller.any_inline_composers()) {
-            set_cursor(new Gtk.TreePath.from_indices(0, -1), null, false);
+            get_selection().count_selected_rows() == 0) {
+            MainWindow? parent = get_toplevel() as MainWindow;
+            if (parent != null && !parent.has_composer) {
+                set_cursor(new Gtk.TreePath.from_indices(0, -1), null, false);
+            }
         }
     }
 
@@ -311,9 +314,12 @@ public class ConversationListView : Gtk.TreeView, Geary.BaseInterface {
             }
         }
 
-        if (!get_selection().path_is_selected(path) &&
-            !GearyApplication.instance.controller.can_switch_conversation_view())
-            return true;
+        if (!get_selection().path_is_selected(path)) {
+            MainWindow? parent = get_toplevel() as MainWindow;
+            if (parent != null && !parent.close_composer()) {
+                return true;
+            }
+        }
 
         if (event.button == 3 && event.type == Gdk.EventType.BUTTON_PRESS) {
             Geary.App.Conversation conversation = get_model().get_conversation_at_path(path);
diff --git a/src/client/conversation-viewer/conversation-viewer.vala 
b/src/client/conversation-viewer/conversation-viewer.vala
index 1e4cf178..cce1b14d 100644
--- a/src/client/conversation-viewer/conversation-viewer.vala
+++ b/src/client/conversation-viewer/conversation-viewer.vala
@@ -19,15 +19,16 @@ public class ConversationViewer : Gtk.Stack, Geary.BaseInterface {
         get; private set; default = null;
     }
 
-    /**
-     * Specifies if a full-height composer is currently shown.
-     */
-    public bool is_composer_visible {
-        get { return (get_visible_child() == this.composer_page); }
+    /** Returns the currently displayed composer if any. */
+    public ComposerWidget? current_composer {
+        get; private set; default = null;
     }
 
     private Configuration config;
 
+    private Gee.Set<Geary.App.Conversation>? selection_while_composing = null;
+
+
     // Stack pages
     [GtkChild]
     private Gtk.Spinner loading_page;
@@ -141,21 +142,18 @@ public class ConversationViewer : Gtk.Stack, Geary.BaseInterface {
      */
     public void do_compose(ComposerWidget composer) {
         ComposerBox box = new ComposerBox(composer);
+        this.current_composer = composer;
 
         // XXX move the ConversationListView management code into
-        // GearyController or somewhere more appropriate
-        ConversationListView conversation_list_view =
-            ((MainWindow) GearyApplication.instance.controller.main_window).conversation_list_view;
-        Gee.Set<Geary.App.Conversation>? prev_selection = 
conversation_list_view.get_selected_conversations();
-        conversation_list_view.get_selection().unselect_all();
-        box.vanished.connect((box) => {
-                set_visible_child(this.conversation_page);
-                if (prev_selection.is_empty) {
-                    conversation_list_view.conversations_selected(prev_selection);
-                } else {
-                    conversation_list_view.select_conversations(prev_selection);
-                }
-            });
+        // MainWindow or somewhere more appropriate
+        MainWindow? main_window = get_toplevel() as MainWindow;
+        if (main_window != null) {
+            ConversationListView conversation_list = main_window.conversation_list_view;
+            this.selection_while_composing = conversation_list.get_selected_conversations();
+            conversation_list.get_selection().unselect_all();
+        }
+
+        box.vanished.connect(on_composer_closed);
         this.composer_page.add(box);
         set_visible_child(this.composer_page);
     }
@@ -166,11 +164,13 @@ public class ConversationViewer : Gtk.Stack, Geary.BaseInterface {
     public void do_compose_embedded(ComposerWidget composer,
                                     Geary.Email? referred,
                                     bool is_draft) {
+        this.current_composer = composer;
         ComposerEmbed embed = new ComposerEmbed(
             referred,
             composer,
             this.conversation_scroller
         );
+        embed.vanished.connect(on_composer_closed);
 
         // We need to disable kinetic scrolling so that if it still
         // has some momentum when the composer is inserted and
@@ -441,4 +441,30 @@ public class ConversationViewer : Gtk.Stack, Geary.BaseInterface {
         return Gdk.EVENT_PROPAGATE;
     }
 
+    private void on_composer_closed() {
+        this.current_composer = null;
+        if (get_visible_child() == this.composer_page) {
+            set_visible_child(this.conversation_page);
+
+            // Restore the old selection
+            MainWindow? main_window = get_toplevel() as MainWindow;
+            if (main_window != null &&
+                this.selection_while_composing != null) {
+                ConversationListView conversation_list =
+                    main_window.conversation_list_view;
+                if (this.selection_while_composing.is_empty) {
+                    conversation_list.conversations_selected(
+                        this.selection_while_composing
+                    );
+                } else {
+                    conversation_list.select_conversations(
+                        this.selection_while_composing
+                    );
+                }
+
+                this.selection_while_composing = null;
+            }
+        }
+    }
+
 }
diff --git a/src/client/folder-list/folder-list-tree.vala b/src/client/folder-list/folder-list-tree.vala
index 0fac3770..d7dc2419 100644
--- a/src/client/folder-list/folder-list-tree.vala
+++ b/src/client/folder-list/folder-list-tree.vala
@@ -54,7 +54,12 @@ public class FolderList.Tree : Sidebar.Tree, Geary.BaseInterface {
     }
 
     public override bool accept_cursor_changed() {
-        return GearyApplication.instance.controller.can_switch_conversation_view();
+        bool can_switch = true;
+        MainWindow? parent = get_toplevel() as MainWindow;
+        if (parent != null) {
+            can_switch = parent.close_composer();
+        }
+        return can_switch;
     }
 
     private void on_entry_selected(Sidebar.SelectableEntry selectable) {


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