GNOME Bugzilla – Bug 710297
undo notification shouldn't timeout if pointer is above it
Last modified: 2015-03-03 13:26:43 UTC
(from Montreal summit session) The undo notification goes away way too fast. In general, notifications should not go away while the pointer is hovering over it.
Created attachment 298102 [details] [review] notification: Use G_SOURCE_CONTINUE/REMOVE for clarity
Created attachment 298103 [details] [review] notification: Keep it visible while the pointer is hovering over it
Created attachment 298104 [details] [review] notification: Use gtk_window_(un)register_window
The 2nd patch is the actual fix, 1st and 3rd are just unrelated cleanups I did while looking at the code.
Review of attachment 298102 [details] [review]: Looks good to me. Thanks, Kalev.
Review of attachment 298104 [details] [review]: Looks good to me.
Review of attachment 298103 [details] [review]: I would have preferred an approach where we queue an auto-hide when the pointer leaves the notification, and unqueue it when it enters. Here is an example of how gnome-documents controls the overlaid chrome while previewing: https://git.gnome.org/browse/gnome-documents/tree/src/preview.js#n644 If I understand correctly, with this patch, it is possible for the notification to get hidden almost immediately after the pointer leaves. eg., if gd_notification_timeout_cb happens to get invoked as soon as the pointer left. This can be a bit jarring for the user.
Review of attachment 298103 [details] [review]: I agree with Debarshi here, that sounds like a better approach.
Attachment 298102 [details] pushed as 0c1d758 - notification: Use G_SOURCE_CONTINUE/REMOVE for clarity Attachment 298104 [details] pushed as 3dbcd6a - notification: Use gtk_window_(un)register_window
(In reply to Debarshi Ray from comment #7) > Review of attachment 298103 [details] [review] [review]: > > I would have preferred an approach where we queue an auto-hide when the > pointer leaves the notification, and unqueue it when it enters. Here is an > example of how gnome-documents controls the overlaid chrome while > previewing: > https://git.gnome.org/browse/gnome-documents/tree/src/preview.js#n644 > > If I understand correctly, with this patch, it is possible for the > notification to get hidden almost immediately after the pointer leaves. eg., > if gd_notification_timeout_cb happens to get invoked as soon as the pointer > left. This can be a bit jarring for the user. Thanks, I've switched this to use the same logic now. I'm a bit unsure how long the timeout should be when an user moves the pointer away from the notification. Perhaps it should be shorter than the regular timeout? Anyway, this can be tuned later. I've used the same timeout length for now.
Created attachment 298218 [details] [review] notification: Break out two functions
Created attachment 298219 [details] [review] notification: Don't timeout if a pointer is above the notification
Review of attachment 298218 [details] [review]: Makes sense. Thanks, Kalev.
Review of attachment 298219 [details] [review]: Looks good to me. ::: libgd/gd-notification.c @@ +452,3 @@ + GdNotificationPrivate *priv = notification->priv; + + if ((event->window == priv->bin_window) && I don't know if this check is required, but Kalev showed me that GtkButton is doing the same. Doesn't hurt, I guess.
Thanks Rishi! Attachment 298218 [details] pushed as a4d5a10 - notification: Break out two functions Attachment 298219 [details] pushed as b33b9a0 - notification: Don't timeout if a pointer is above the notification