GNOME Bugzilla – Bug 739426
Asking for permission to receive files should be a CRITICAL notification
Last modified: 2014-11-01 02:04:25 UTC
The notification that is presented to the user when it's about to receive files over bluetooth is not the easiest one to use, as it: - Expires very early as soon as the mouse starts moving, forcing the user to have to go to the messages tray to see it again and accept/decline the incoming file - It shows up not in its full extension, so the Receive/Cancel buttons are only seen if you hover the notification, assuming you were fast enough to reach it before it hides :) So, I think it would be better to make this notification CRITICAL, so it never expires until the user explicitly accepts or rejects it, and so it's shown in its full extension (showing buttons) right from the start
Created attachment 289695 [details] [review] Patch proposal
Review of attachment 289695 [details] [review]: Ditto about the "obex: " prefix. ::: src/obexpush.c @@ +242,3 @@ "bluetooth"); + notify_notification_set_urgency (notification, NOTIFY_URGENCY_CRITICAL); How does replace the "never expires" timeout fix it? Should have both lines, even if the critical urgency behaves like the "never expires" timeout.
(In reply to comment #2) > Review of attachment 289695 [details] [review]: > > Ditto about the "obex: " prefix. > > ::: src/obexpush.c > @@ +242,3 @@ > "bluetooth"); > > + notify_notification_set_urgency (notification, NOTIFY_URGENCY_CRITICAL); > > How does replace the "never expires" timeout fix it? > Should have both lines, even if the critical urgency behaves like the "never > expires" timeout. The CRITICAL urgency implies that the notification never expires until explicitly dismissed, so the two lines are not needed. From https://developer.gnome.org/notification-spec/: """ Critical notifications should not automatically expire, as they are things that the user will most likely want to know about. They should only be closed when the user dismisses them, for example, by clicking on the notification. """ Also, from gnome-shell:js/ui/messageTray.js: [...] } else if (this._notificationState == State.SHOWN) { let expired = (this._userActiveWhileNotificationShown && this._notificationTimeoutId == 0 && this._notification.urgency != Urgency.CRITICAL && !this._notification.focused && !this._pointerInNotification) || this._notificationExpired; let mustClose = (this._notificationRemoved || !hasNotifications || expired || this._traySummoned); [...] ...it confirms that the notification won't expire if it's marked as CRITICAL, so this is why I think this might be the right fix (although I recognise that initially I had the two lines).
Hmm.. .just realized that my previous comment might look as if I strongly opposed to having both lines, which I don't. I was just trying to explain why it's not needed, but if we prefer to leave it for other reasons (e.g. readability?) I'm fine with it too.
(In reply to comment #4) > Hmm.. .just realized that my previous comment might look as if I strongly > opposed to having both lines, which I don't. I was just trying to explain why > it's not needed, but if we prefer to leave it for other reasons (e.g. > readability?) I'm fine with it too. I'd rather it was there as well, as it helps readability. The chunk from the notification spec should probably also be in the commit message.
Ok, I'll prepare a new patch adding that line then and the chunk from the spec too
Created attachment 289762 [details] [review] Patch proposal
Review of attachment 289762 [details] [review]: Looks good.
Thanks. Just committed it to the master branch