[geary/mjog/493-undo-send: 11/20] Rework Composer.Widget close management
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/mjog/493-undo-send: 11/20] Rework Composer.Widget close management
- Date: Tue, 12 Nov 2019 21:42:46 +0000 (UTC)
commit b4361f4258016532b86c3ec0d9e2200ec2b11cb0
Author: Michael Gratton <mike vee net>
Date: Tue Nov 12 12:28:24 2019 +1100
Rework Composer.Widget close management
Rename Composer.Widget::should_close to ::confirm_close and actually
start closing the composer in all cases if not cancelled, so that call
sites don't need to do so manually. Simplify CloseStatus enum and
Application.Controller's management as a result.
src/client/application/application-controller.vala | 60 ++++----
src/client/components/main-window.vala | 15 +-
src/client/composer/composer-widget.vala | 154 ++++++++++++---------
src/client/composer/composer-window.vala | 8 +-
4 files changed, 121 insertions(+), 116 deletions(-)
---
diff --git a/src/client/application/application-controller.vala
b/src/client/application/application-controller.vala
index 2abb151a..549f83d4 100644
--- a/src/client/application/application-controller.vala
+++ b/src/client/application/application-controller.vala
@@ -427,7 +427,7 @@ public class Application.Controller : Geary.BaseObject {
/** Adds a new composer to be kept track of. */
public void add_composer(Composer.Widget widget) {
debug(@"Added composer of type $(widget.compose_type); $(this.composer_widgets.size) composers
total");
- widget.destroy.connect(this.on_composer_widget_destroy);
+ widget.destroy.connect_after(this.on_composer_widget_destroy);
this.composer_widgets.add(widget);
}
@@ -1478,56 +1478,46 @@ public class Application.Controller : Geary.BaseObject {
composer.set_focus();
}
- internal bool close_composition_windows(bool main_window_only = false) {
- Gee.List<Composer.Widget> composers_to_destroy = new Gee.ArrayList<Composer.Widget>();
+ internal bool close_composition_windows() {
+ // Take a copy of the collection of composers since closing
+ // any will cause the underlying collection to change.
+ var composers = new Gee.LinkedList<Composer.Widget>();
+ composers.add_all(this.composer_widgets);
bool quit_cancelled = false;
- // If there's composer windows open, give the user a chance to
- // save or cancel.
- foreach(Composer.Widget cw in composer_widgets) {
- if (!main_window_only ||
- cw.state != Composer.Widget.ComposerState.DETACHED) {
- // Check if we should close the window immediately, or
- // if we need to wait.
- Composer.Widget.CloseStatus status = cw.should_close();
- if (status == Composer.Widget.CloseStatus.PENDING_CLOSE) {
- // Window is currently busy saving.
- waiting_to_close.add(cw);
- } else if (status == Composer.Widget.CloseStatus.CANCEL_CLOSE) {
- // User cancelled operation.
+ foreach (var composer in composers) {
+ if (composer.current_mode == NONE) {
+ // Composer currently isn't being presented at all
+ // (it's probably in the undo stack), so just close it
+ this.waiting_to_close.add(composer);
+ composer.close.begin();
+ } else {
+ switch (composer.confirm_close()) {
+ case Composer.Widget.CloseStatus.PENDING:
+ this.waiting_to_close.add(composer);
+ break;
+
+ case Composer.Widget.CloseStatus.CANCELLED:
quit_cancelled = true;
break;
- } else if (status == Composer.Widget.CloseStatus.DO_CLOSE) {
- // Hide any existing composer windows for the
- // moment; actually deleting the windows will
- // result in their removal from composer_windows,
- // which could crash this loop.
- composers_to_destroy.add(cw);
- cw.set_enabled(false);
}
}
}
- // Safely destroy windows.
- foreach (Composer.Widget cw in composers_to_destroy) {
- cw.close.begin();
- }
-
// If we cancelled the quit we can bail here.
if (quit_cancelled) {
- waiting_to_close.clear();
-
+ this.waiting_to_close.clear();
return false;
}
- // If there's still windows saving, we can't exit just yet. Hide the main window and wait.
- if (waiting_to_close.size > 0) {
- main_window.hide();
-
+ // If there's still windows saving, we can't exit just yet.
+ if (this.waiting_to_close.size > 0) {
+ this.main_window.set_sensitive(false);
return false;
}
- // If we deleted all composer windows without the user cancelling, we can exit.
+ // If we deleted all composer windows without the user
+ // cancelling, we can exit.
return true;
}
diff --git a/src/client/components/main-window.vala b/src/client/components/main-window.vala
index b64cc6e5..7faf45c8 100644
--- a/src/client/components/main-window.vala
+++ b/src/client/components/main-window.vala
@@ -626,16 +626,9 @@ public class MainWindow : Gtk.ApplicationWindow, Geary.BaseInterface {
public bool close_composer() {
bool closed = true;
Composer.Widget? composer = this.conversation_viewer.current_composer;
- if (composer != null) {
- switch (composer.should_close()) {
- case DO_CLOSE:
- composer.close.begin();
- break;
-
- case CANCEL_CLOSE:
- closed = false;
- break;
- }
+ if (composer != null &&
+ composer.confirm_close() == CANCELLED) {
+ closed = false;
}
return closed;
}
@@ -1605,7 +1598,7 @@ public class MainWindow : Gtk.ApplicationWindow, Geary.BaseInterface {
[GtkCallback]
private bool on_delete_event() {
if (this.application.config.startup_notifications) {
- if (this.application.controller.close_composition_windows(true)) {
+ if (close_composer()) {
hide();
}
} else {
diff --git a/src/client/composer/composer-widget.vala b/src/client/composer/composer-widget.vala
index 100ee314..aff254fa 100644
--- a/src/client/composer/composer-widget.vala
+++ b/src/client/composer/composer-widget.vala
@@ -31,10 +31,18 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
FORWARD
}
+ /**
+ * Determines the result of prompting whether to close the composer.
+ *
+ * @see confirm_close
+ */
public enum CloseStatus {
- DO_CLOSE,
- PENDING_CLOSE,
- CANCEL_CLOSE
+
+ /** The composer is being closed. */
+ PENDING,
+
+ /** Closing the composer was not confirmed by a human. */
+ CANCELLED;
}
/** Defines different supported user interface modes. */
@@ -737,8 +745,79 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
}
}
+ /**
+ * Prompts to close the composer if needed, before closing it.
+ *
+ * The return value specifies whether the composer is being
+ * closed, if it was already being closed, or the prompt was
+ * cancelled by a human.
+ */
+ public CloseStatus confirm_close() {
+ 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();
+ } else {
+ this.close.begin();
+ }
+ } else {
+ // Discard
+ this.discard_and_exit_async.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;
+ }
+ }
+ }
+
+ return status;
+ }
+
/** Closes the composer unconditionally. */
public async void close() {
+ this.is_closing = true;
set_enabled(false);
if (this.draft_manager_opening != null) {
@@ -780,7 +859,6 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
*/
public void set_enabled(bool enabled) {
this.current_mode = NONE;
- this.is_closing = !enabled;
this.set_sensitive(enabled);
// Need to update this separately since it may be detached
@@ -788,6 +866,7 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
this.header.set_sensitive(enabled);
if (enabled) {
+ this.is_closing = false;
this.open_draft_manager_async.begin(null, null);
} else {
if (this.container != null) {
@@ -1270,67 +1349,6 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
this.referred_ids.add(referred.id);
}
- public CloseStatus should_close() {
- if (this.is_closing)
- return CloseStatus.PENDING_CLOSE;
- if (this.is_blank)
- return CloseStatus.DO_CLOSE;
-
- present();
-
- CloseStatus status = CloseStatus.PENDING_CLOSE;
- if (this.can_save) {
- AlertDialog dialog = new TernaryConfirmationDialog(
- 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 == Gtk.ResponseType.CANCEL ||
- response == Gtk.ResponseType.DELETE_EVENT) {
- // Cancel
- status = CloseStatus.CANCEL_CLOSE;
- } else if (response == Gtk.ResponseType.OK) {
- // Keep
- if (!this.is_draft_saved) {
- save_and_exit_async.begin();
- } else {
- status = CloseStatus.DO_CLOSE;
- }
- } else {
- // Discard
- discard_and_exit_async.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 == Gtk.ResponseType.OK) {
- discard_and_exit_async.begin();
- } else {
- status = CloseStatus.CANCEL_CLOSE;
- }
- }
-
- return status;
- }
-
public override bool key_press_event(Gdk.EventKey event) {
// Override the method since key-press-event is run last, and
// we want this behaviour to take precedence over the default
@@ -1610,12 +1628,14 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
}
private async void save_and_exit_async() {
+ this.is_closing = true;
set_enabled(false);
yield save_draft();
yield close();
}
private async void discard_and_exit_async() {
+ this.is_closing = true;
set_enabled(false);
if (this.draft_manager != null) {
discard_draft();
@@ -2584,9 +2604,7 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
}
private void on_close(SimpleAction action, Variant? param) {
- if (should_close() == CloseStatus.DO_CLOSE) {
- this.close.begin();
- }
+ confirm_close();
}
private void on_close_and_save(SimpleAction action, Variant? param) {
diff --git a/src/client/composer/composer-window.vala b/src/client/composer/composer-window.vala
index cd4deee8..da2fd72c 100644
--- a/src/client/composer/composer-window.vala
+++ b/src/client/composer/composer-window.vala
@@ -116,8 +116,12 @@ public class Composer.Window : Gtk.ApplicationWindow, Container {
public override bool delete_event(Gdk.EventAny event) {
bool ret = Gdk.EVENT_PROPAGATE;
- Widget? composer = get_child() as Widget;
- if (composer != null && composer.should_close() == CANCEL_CLOSE) {
+ // 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;
+ if (child != null &&
+ child.confirm_close() == CANCELLED) {
ret = Gdk.EVENT_STOP;
}
return ret;
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]