GNOME Bugzilla – Bug 776058
Withdraw notifications when shuting down
Last modified: 2017-01-18 11:41:33 UTC
.
Created attachment 341910 [details] [review] app: Add api to keep track of notifications The HIG[0] says that "Notifications in GNOME 3 persist after they have been initially displayed. It is therefore important to remove notification messages that are no longer relevant to the user." [0] https://developer.gnome.org/hig/stable/notifications.html.en
Created attachment 341911 [details] [review] app: Withdraw notifications when shuting down
See also https://wiki.gnome.org/Design/Whiteboards/NotificationsImprovements
Review of attachment 341910 [details] [review]: There is no API (not api) being added. Just "app: Keep track of notifications" is fine. Nice description but would be good to first clarify what we are talking about (system notifications rather than in-app notifications) before diving into reasons. Actually this is one of those rare moments where I'd recommend to merging the patch with the next one. We are not adding new API and it's very simple change so not worth having it's own patch IMO.
Review of attachment 341910 [details] [review]: ::: src/app.vala @@ +38,3 @@ private HashTable<string,Broker> brokers; private HashTable<string,CollectionSource> sources; + private HashTable<string,GLib.Notification> notifications; Let's name it 'system_notifications' to ensure no confusion with in-app notifications.
Review of attachment 341911 [details] [review]: typo: shutting. ::: src/app.vala @@ +290,3 @@ suspend_machines (); + + // withdraw all the existing notifications nitpick: Capitalize first letter please. @@ +291,3 @@ + + // withdraw all the existing notifications + foreach (var notification_id in notifications.get_keys ()) If we are not gonna make use of access by ID, no point in having this as hashtable. Just use a GLib.List.
Created attachment 343659 [details] [review] app: Withdraw system notifications when shutting down This patch keeps track of the system notifications as they appear and withdraws them when the application quits. The HIG[0] says that "Notifications in GNOME 3 persist after they have been initially displayed. It is therefore important to remove notification messages that are no longer relevant to the user." [0] https://developer.gnome.org/hig/stable/notifications.html.en
Review of attachment 343659 [details] [review]: Boxes deals with VMs so "shutdown" is ambiguous here, let's "when shutting down" -> "on exit". It'll also make it fit 50 chars. ::: src/app.vala @@ +294,3 @@ + + // Withdraw all the existing notifications + foreach (var notification_id in system_notifications) there is no ID involved anymore.
Created attachment 343698 [details] [review] app: Withdraw system notifications on exit This patch keeps track of the system notifications as they appear and withdraws them when the application quits. The HIG[0] says that "Notifications in GNOME 3 persist after they have been initially displayed. It is therefore important to remove notification messages that are no longer relevant to the user." [0] https://developer.gnome.org/hig/stable/notifications.html.en
Review of attachment 343698 [details] [review]: ack
Thank you! Attachment 343698 [details] pushed as d6e5294 - app: Withdraw system notifications on exit