After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 606979 - Fittsify notification buttons
Fittsify notification buttons
Status: RESOLVED OBSOLETE
Product: gnome-shell
Classification: Core
Component: message-tray
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2010-01-14 17:09 UTC by Dan Winship
Modified: 2015-02-23 21:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[MessageTray] support for notification actions (7.38 KB, patch)
2010-01-14 17:09 UTC, Dan Winship
none Details | Review
[MessageTray] Merge Notification and NotificationBox (9.56 KB, patch)
2010-02-01 20:43 UTC, Dan Winship
none Details | Review
[MessageTray] add support for notification actions (4.02 KB, patch)
2010-02-01 20:44 UTC, Dan Winship
none Details | Review
[MessageTray] Merge Notification and NotificationBox (9.31 KB, patch)
2010-02-02 21:43 UTC, Dan Winship
committed Details | Review
[MessageTray] add support for notification actions (4.02 KB, patch)
2010-02-02 21:44 UTC, Dan Winship
committed Details | Review

Description Dan Winship 2010-01-14 17:09:03 UTC
work in progress; I know the UI needs work, and people can try this out
and provide suggestions on that now while I'm refactoring the non-UI code

unfortunately I don't think there's any really easy way to generate
notifications-with-actions other than the test programs in the libnotify
source (http://svn.galago-project.org/trunk/libnotify). (The PackageKit
security-updates-are-available notification is another example, but you
can't really make that pop up on demand.)

I assume the UI for accepting/rejecting a voice/video chat will use
the same UI as we implement here.

oh, this depends on bug 606755
Comment 1 Dan Winship 2010-01-14 17:09:06 UTC
Created attachment 151411 [details] [review]
[MessageTray] support for notification actions

work-in-progress. the code needs to be refactored, but it works. the
visual design presumably also needs improvement
Comment 2 Dan Winship 2010-02-01 20:43:42 UTC
Created attachment 152764 [details] [review]
[MessageTray] Merge Notification and NotificationBox

(this will also be important when implementing the new chat notifications)
depends on bug 606755
Comment 3 Dan Winship 2010-02-01 20:44:32 UTC
Created attachment 152765 [details] [review]
[MessageTray] add support for notification actions

The UI is now more like the mockups, though I had to guess on the hover
and active states.

depends on the above patch and bug 606755
Comment 4 William Jon McCann 2010-02-02 01:11:35 UTC
Right, I guess in the mockups I only drew an urgent message with actions.  And in that case it would start expanded.  In the normal case it should just be a simple banner like you have here.  However, I'm wondering if we need an additional indication that there are actions hidden "below the fold".  I'll try to think about what this might look like.

As mentioned in the other bug I think we'll have to tweak the spacing a bit but over it is looking pretty decent already.

I think maybe the buttons should probably have the same size though.  We'll also have to think a bit about how we should handle the case where the requested buttons don't fit... go vertical maybe?  Not sure.  Suggestions welcome.
Comment 5 William Jon McCann 2010-02-02 01:12:58 UTC
Also, maybe we can have the clickable area for the action buttons extend to the bottom edge of the screen?
Comment 6 Dan Winship 2010-02-02 21:43:53 UTC
Created attachment 152880 [details] [review]
[MessageTray] Merge Notification and NotificationBox

rebased
Comment 7 Dan Winship 2010-02-02 21:44:20 UTC
Created attachment 152882 [details] [review]
[MessageTray] add support for notification actions

rebased
Comment 8 Marina Zhurakhinskaya 2010-02-02 23:39:59 UTC
Review of attachment 152880 [details] [review]:

Looks good.

Need a "why" line in the commit message for this patch and others.

::: js/ui/messageTray.js
@@ +463,3 @@
                                this._notificationBin.hide();
+                               this._notificationBin.child = null;
+                               this._notification = null;

Do you know if the garbage collection of the notifications will work fine, destroying all the old notifications, after we do this?
Comment 9 Marina Zhurakhinskaya 2010-02-03 00:24:02 UTC
Review of attachment 152882 [details] [review]:

Looks good. This certainly should be committed as is (with minor fixes), but we'll probably need add checks later for which notifications to show expanded up-front based on the urgency or on application preferences of some sort. For example, I should not have to mouse over to the bottom of the screen to expand each Gwibber update (even though in the case of Gwibber, I care about the full text, not the availability of the Reply action).

Need "why" line for the commit message.

You need to uncomment 'actions' in GetCapabilities in notificationDaemon.js for the impressionable applications that check capabilities first :). (You had that uncommented in one of the previous versions of your patch.)

::: js/ui/messageTray.js
@@ +122,3 @@
+                                     label: label });
+        this._buttonBox.add(button);
+        button.connect('clicked', Lang.bind(this, function() { this.emit('action', id); }));

I think the signal name should be 'action-invoked' rather than 'action'.
Comment 10 Dan Winship 2010-02-03 18:44:11 UTC
Attachment 152880 [details] pushed as d20d815 - [MessageTray] Merge Notification and NotificationBox
Attachment 152882 [details] pushed as e40063f - [MessageTray] add support for notification actions
Comment 11 Marina Zhurakhinskaya 2010-05-04 19:36:43 UTC
This bug remains open for further re-design of the action buttons.

If we want to have the area below the buttons activate the buttons, the existing buttons can be changed so that the actual StClickable was unstyled, and extended to the bottom of the screen, and then it had a (non-reactive) child that was styled to look like a button, and had padding-bottom to keep it aligned correctly (suggested by Dan on IRC).
Comment 12 Dan Winship 2011-02-17 00:49:42 UTC
retitling to reflect what's left
Comment 13 Jean-François Fortin Tam 2012-10-29 21:18:40 UTC
Afaik, this is solved with the notification bar in 3.6, right?
Comment 14 Florian Müllner 2012-10-29 21:22:10 UTC
(In reply to comment #13)
> Afaik, this is solved with the notification bar in 3.6, right?

Nope. The remaining part of the bug is about having action buttons in banner notifications (e.g. at the center of the screen, with the tray hidden) extend their active area to the screen edge (while keeping the current visual appearance).
Comment 15 Allan Day 2015-02-23 21:27:12 UTC
The new notifications design [1] doesn't use action buttons that are alongside the screen edge, so this bug is no longer needed.

[1] https://wiki.gnome.org/Design/OS/Notifications/