[geary/mjog/493-undo-send: 4/6] Don't prompt when closing in-main-window composers



commit ed847dbad35fd9261c5c0fb62d23c95e34dd474d
Author: Michael Gratton <mike vee net>
Date:   Tue Nov 12 22:42:26 2019 +1100

    Don't prompt when closing in-main-window composers
    
    Now we have undo support for sending, saving and discarding composers,
    don't prompt when closing in-main-window composers in most cases.
    
    It retains prompting for detached composers, since it may not be obvious
    how to undo that (although it is supported), also when closing the main
    window completely (for the same reason), and when quitting the app.

 src/client/application/application-controller.vala |   4 +-
 src/client/components/main-window.vala             |   6 +-
 src/client/composer/composer-widget.vala           | 167 +++++++++------------
 src/client/composer/composer-window.vala           |  10 +-
 .../conversation-list/conversation-list-view.vala  |   2 +-
 src/client/folder-list/folder-list-tree.vala       |   2 +-
 ui/composer-headerbar.ui                           |   4 +-
 7 files changed, 88 insertions(+), 107 deletions(-)
---
diff --git a/src/client/application/application-controller.vala 
b/src/client/application/application-controller.vala
index 10668435..66c24bc7 100644
--- a/src/client/application/application-controller.vala
+++ b/src/client/application/application-controller.vala
@@ -1550,7 +1550,7 @@ public class Application.Controller : Geary.BaseObject {
                 this.waiting_to_close.add(composer);
                 composer.close.begin();
             } else {
-                switch (composer.confirm_close()) {
+                switch (composer.conditional_close(true)) {
                 case Composer.Widget.CloseStatus.PENDING:
                     this.waiting_to_close.add(composer);
                     break;
@@ -1637,7 +1637,7 @@ public class Application.Controller : Geary.BaseObject {
             // 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()) {
+            if (!this.main_window.close_composer(true)) {
                 return;
             }
         }
diff --git a/src/client/components/main-window.vala b/src/client/components/main-window.vala
index 7faf45c8..ba380800 100644
--- a/src/client/components/main-window.vala
+++ b/src/client/components/main-window.vala
@@ -623,11 +623,11 @@ public class MainWindow : Gtk.ApplicationWindow, Geary.BaseInterface {
      * Returns true if none were open or the user approved closing
      * them.
      */
-    public bool close_composer() {
+    public bool close_composer(bool should_prompt) {
         bool closed = true;
         Composer.Widget? composer = this.conversation_viewer.current_composer;
         if (composer != null &&
-            composer.confirm_close() == CANCELLED) {
+            composer.conditional_close(should_prompt) == CANCELLED) {
             closed = false;
         }
         return closed;
@@ -1598,7 +1598,7 @@ public class MainWindow : Gtk.ApplicationWindow, Geary.BaseInterface {
     [GtkCallback]
     private bool on_delete_event() {
         if (this.application.config.startup_notifications) {
-            if (close_composer()) {
+            if (close_composer(true)) {
                 hide();
             }
         } else {
diff --git a/src/client/composer/composer-widget.vala b/src/client/composer/composer-widget.vala
index a1cc3fa2..4848838c 100644
--- a/src/client/composer/composer-widget.vala
+++ b/src/client/composer/composer-widget.vala
@@ -119,8 +119,7 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
     private const string ACTION_INSERT_LINK = "insert-link";
     private const string ACTION_COMPOSE_AS_HTML = "compose-as-html";
     private const string ACTION_SHOW_EXTENDED_HEADERS = "show-extended-headers";
-    private const string ACTION_CLOSE_AND_SAVE = "close-and-save";
-    private const string ACTION_CLOSE_AND_DISCARD = "close-and-discard";
+    private const string ACTION_DISCARD = "discard";
     private const string ACTION_DETACH = "detach";
     private const string ACTION_SEND = "send";
     private const string ACTION_ADD_ATTACHMENT = "add-attachment";
@@ -168,8 +167,7 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
         { ACTION_ADD_ATTACHMENT,           on_add_attachment                                          },
         { ACTION_ADD_ORIGINAL_ATTACHMENTS, on_pending_attachments                                     },
         { ACTION_CLOSE,                    on_close                                                   },
-        { ACTION_CLOSE_AND_DISCARD,        on_close_and_discard                                       },
-        { ACTION_CLOSE_AND_SAVE,           on_close_and_save                                          },
+        { ACTION_DISCARD,                  on_discard                                       },
         { ACTION_COMPOSE_AS_HTML,          on_toggle_action, null, "true", on_compose_as_html_toggled },
         { ACTION_DETACH,                   on_detach                                                  },
         { ACTION_OPEN_INSPECTOR,           on_open_inspector                  },
@@ -179,7 +177,7 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
     };
 
     public static void add_accelerators(GearyApplication application) {
-        application.add_window_accelerators(ACTION_CLOSE, { "Escape" } );
+        application.add_window_accelerators(ACTION_DISCARD, { "Escape" } );
         application.add_window_accelerators(ACTION_ADD_ATTACHMENT, { "<Ctrl>t" } );
         application.add_window_accelerators(ACTION_DETACH, { "<Ctrl>d" } );
 
@@ -749,63 +747,62 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
      * closed, if it was already being closed, or the prompt was
      * cancelled by a human.
      */
-    public CloseStatus confirm_close() {
+    public CloseStatus conditional_close(bool should_prompt) {
         CloseStatus status = PENDING;
 
-        if (this.is_blank) {
-            this.close.begin();
-        }
-
-        if (!this.is_closing && !this.is_blank) {
-            present();
-
-            if (this.can_save) {
-                var dialog = new TernaryConfirmationDialog(
-                    this.container.top_window,
-                    // Translators: This dialog text is displayed to the
-                    // user when closing a composer where the options are
-                    // Keep, Discard or Cancel.
-                    _("Do you want to keep or discard this draft message?"),
-                    null,
-                    Stock._KEEP,
-                    Stock._DISCARD, Gtk.ResponseType.CLOSE,
-                    "",
-                    "destructive-action",
-                    Gtk.ResponseType.OK // Default == Keep
-                );
-                Gtk.ResponseType response = dialog.run();
-                if (response == CANCEL ||
-                    response == DELETE_EVENT) {
-                    // Cancel
-                    status = CANCELLED;
-                } else if (response == OK) {
-                    // Keep
-                    if (!this.is_draft_saved) {
-                        this.save_and_exit_async.begin();
+        if (!this.is_closing) {
+            if (this.is_blank) {
+                this.close.begin();
+            } else if (should_prompt) {
+                present();
+                if (this.can_save) {
+                    var dialog = new TernaryConfirmationDialog(
+                        this.container.top_window,
+                        // Translators: This dialog text is displayed to the
+                        // user when closing a composer where the options are
+                        // Keep, Discard or Cancel.
+                        _("Do you want to keep or discard this draft message?"),
+                        null,
+                        Stock._KEEP,
+                        Stock._DISCARD, Gtk.ResponseType.CLOSE,
+                        "",
+                        "destructive-",
+                        Gtk.ResponseType.OK // Default == Keep
+                    );
+                    Gtk.ResponseType response = dialog.run();
+                    if (response == CANCEL ||
+                        response == DELETE_EVENT) {
+                        // Cancel
+                        status = CANCELLED;
+                    } else if (response == OK) {
+                        // Keep
+                        this.save_and_exit.begin();
                     } else {
-                        this.close.begin();
+                        // Discard
+                        this.discard_and_exit.begin();
                     }
                 } else {
-                    // Discard
-                    this.discard_and_exit_async.begin();
+                    AlertDialog dialog = new ConfirmationDialog(
+                        container.top_window,
+                        // Translators: This dialog text is displayed to the
+                        // user when closing a composer where the options are
+                        // only Discard or Cancel.
+                        _("Do you want to discard this draft message?"),
+                        null,
+                        Stock._DISCARD,
+                        ""
+                    );
+                    Gtk.ResponseType response = dialog.run();
+                    if (response == OK) {
+                        this.discard_and_exit.begin();
+                    } else {
+                        status = CANCELLED;
+                    }
                 }
+            } else if (!this.can_save) {
+                this.discard_and_exit.begin();
             } else {
-                AlertDialog dialog = new ConfirmationDialog(
-                    container.top_window,
-                    // Translators: This dialog text is displayed to the
-                    // user when closing a composer where the options are
-                    // only Discard or Cancel.
-                    _("Do you want to discard this draft message?"),
-                    null,
-                    Stock._DISCARD,
-                    "destructive-action"
-                );
-                Gtk.ResponseType response = dialog.run();
-                if (response == OK) {
-                    this.discard_and_exit_async.begin();
-                } else {
-                    status = CANCELLED;
-                }
+                this.save_and_exit.begin();
             }
         }
 
@@ -1091,7 +1088,6 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
             );
         }
 
-        get_action(ACTION_CLOSE_AND_SAVE).set_enabled(false);
         get_action(Action.Edit.UNDO).set_enabled(false);
         get_action(Action.Edit.REDO).set_enabled(false);
 
@@ -1442,6 +1438,7 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
 
     // Used internally by on_send()
     private async void on_send_async() {
+        this.is_closing = true;
         set_enabled(false);
 
         try {
@@ -1509,7 +1506,6 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
         this.draft_manager = new_manager;
 
         update_draft_state();
-        get_action(ACTION_CLOSE_AND_SAVE).set_enabled(true);
         this.header.show_save_and_close = true;
     }
 
@@ -1530,12 +1526,9 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
 
     private async void close_draft_manager(GLib.Cancellable? cancellable)
         throws GLib.Error {
-        this.draft_status_text = "";
-
-        get_action(ACTION_CLOSE_AND_SAVE).set_enabled(false);
-
         Geary.App.DraftManager old_manager = this.draft_manager;
         this.draft_manager = null;
+        this.draft_status_text = "";
 
         old_manager.notify[Geary.App.DraftManager.PROP_DRAFT_STATE]
             .disconnect(on_draft_state_changed);
@@ -1610,11 +1603,11 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
         yield this.draft_manager.discard(null);
     }
 
-    private async void save_and_exit_async() {
+    private async void save_and_exit() {
         this.is_closing = true;
         set_enabled(false);
 
-        if (!is_blank) {
+        if (this.should_save) {
             try {
                 yield save_draft();
             } catch (GLib.Error error) {
@@ -1624,20 +1617,17 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
                     )
                 );
             }
+        }
 
-            // Pass on to the controller so the draft can be re-opened
-            // on undo
-            if (this.container != null) {
-                this.container.close();
-            }
-            yield this.application.controller.save_composed_email(this);
-        } else {
-            // The composer is blank, so drop the mic and walk away
-            yield close();
+        // Pass on to the controller so the draft can be re-opened
+        // on undo
+        if (this.container != null) {
+            this.container.close();
         }
+        yield this.application.controller.save_composed_email(this);
     }
 
-    private async void discard_and_exit_async() {
+    private async void discard_and_exit() {
         this.is_closing = true;
         set_enabled(false);
 
@@ -1654,17 +1644,12 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
             }
         }
 
-        if (!is_blank) {
-            // Pass on to the controller so the discarded email can be
-            // re-opened on undo
-            if (this.container != null) {
-                this.container.close();
-            }
-            yield this.application.controller.discard_composed_email(this);
-        } else {
-            // The composer is blank, so drop the mic and walk away
-            yield close();
+        // Pass on to the controller so the discarded email can be
+        // re-opened on undo
+        if (this.container != null) {
+            this.container.close();
         }
+        yield this.application.controller.discard_composed_email(this);
     }
 
     private void update_attachments_view() {
@@ -2641,22 +2626,18 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
         update_cursor_actions();
     }
 
-    private void on_close(SimpleAction action, Variant? param) {
-        confirm_close();
+    private void on_close() {
+        conditional_close(this.container is Window);
     }
 
-    private void on_close_and_save(SimpleAction action, Variant? param) {
-        if (this.should_save) {
-            save_and_exit_async.begin();
+    private void on_discard() {
+        if (this.container is Window) {
+            conditional_close(true);
         } else {
-            this.close.begin();
+            this.discard_and_exit.begin();
         }
     }
 
-    private void on_close_and_discard(SimpleAction action, Variant? param) {
-        discard_and_exit_async.begin();
-    }
-
     private void on_draft_timeout() {
         var current_account = this.account;
         this.save_draft.begin(
diff --git a/src/client/composer/composer-window.vala b/src/client/composer/composer-window.vala
index da2fd72c..9ca8699b 100644
--- a/src/client/composer/composer-window.vala
+++ b/src/client/composer/composer-window.vala
@@ -115,13 +115,13 @@ public class Composer.Window : Gtk.ApplicationWindow, Container {
     }
 
     public override bool delete_event(Gdk.EventAny event) {
-        bool ret = Gdk.EVENT_PROPAGATE;
-        // Use the child instead of the `composer` property so we don't check
-        // with the composer if it has already been removed from the
-        // container.
+        // Use the child instead of the `composer` property so we
+        // don't check with the composer if it has already been
+        // removed from the container.
         Widget? child = get_child() as Widget;
+        bool ret = Gdk.EVENT_PROPAGATE;
         if (child != null &&
-            child.confirm_close() == CANCELLED) {
+            child.conditional_close(true) == CANCELLED) {
             ret = Gdk.EVENT_STOP;
         }
         return ret;
diff --git a/src/client/conversation-list/conversation-list-view.vala 
b/src/client/conversation-list/conversation-list-view.vala
index d489429f..7eb3dcb7 100644
--- a/src/client/conversation-list/conversation-list-view.vala
+++ b/src/client/conversation-list/conversation-list-view.vala
@@ -297,7 +297,7 @@ public class ConversationListView : Gtk.TreeView, Geary.BaseInterface {
         if (event.type == Gdk.EventType.BUTTON_PRESS &&
             !get_selection().path_is_selected(path)) {
             MainWindow? parent = get_toplevel() as MainWindow;
-            if (parent != null && !parent.close_composer()) {
+            if (parent != null && !parent.close_composer(false)) {
                 return true;
             }
         }
diff --git a/src/client/folder-list/folder-list-tree.vala b/src/client/folder-list/folder-list-tree.vala
index 80b31c81..69e05d50 100644
--- a/src/client/folder-list/folder-list-tree.vala
+++ b/src/client/folder-list/folder-list-tree.vala
@@ -59,7 +59,7 @@ public class FolderList.Tree : Sidebar.Tree, Geary.BaseInterface {
         bool can_switch = true;
         MainWindow? parent = get_toplevel() as MainWindow;
         if (parent != null) {
-            can_switch = parent.close_composer();
+            can_switch = parent.close_composer(false);
         }
         return can_switch;
     }
diff --git a/ui/composer-headerbar.ui b/ui/composer-headerbar.ui
index 52ec320c..fb672ae1 100644
--- a/ui/composer-headerbar.ui
+++ b/ui/composer-headerbar.ui
@@ -229,7 +229,7 @@
             <property name="focus_on_click">False</property>
             <property name="receives_default">False</property>
             <property name="tooltip_text" translatable="yes">Discard and Close</property>
-            <property name="action_name">cmh.close-and-discard</property>
+            <property name="action_name">cmh.discard</property>
             <property name="always_show_image">True</property>
             <child>
               <object class="GtkImage" id="discard_and_close_image">
@@ -253,7 +253,7 @@
             <property name="focus_on_click">False</property>
             <property name="receives_default">False</property>
             <property name="tooltip_text" translatable="yes">Save and Close</property>
-            <property name="action_name">cmh.close-and-save</property>
+            <property name="action_name">cmh.close</property>
             <property name="always_show_image">True</property>
             <child>
               <object class="GtkImage" id="save_and_close_image">


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