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 630936 - Have an indication that there are more queued notifications
Have an indication that there are more queued notifications
Status: RESOLVED DUPLICATE of bug 677217
Product: gnome-shell
Classification: Core
Component: message-tray
unspecified
Other Linux
: High normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2010-09-29 19:15 UTC by Marina Zhurakhinskaya
Modified: 2012-08-28 16:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Needs work! Right now,the patch indicates queued notifications. (2.78 KB, patch)
2011-04-06 20:11 UTC, Neha
needs-work Details | Review
Patch indicates queued notifications (5.14 KB, patch)
2011-05-10 17:59 UTC, Neha
none Details | Review
patch indicates queued notifications (3.25 KB, patch)
2011-05-31 14:47 UTC, Neha
none Details | Review
patch indicates queued notifications (3.25 KB, patch)
2011-06-01 06:04 UTC, Neha
reviewed Details | Review
Indicates that there are more queued notifications (3.40 KB, patch)
2011-06-11 06:43 UTC, Neha
none Details | Review

Description Marina Zhurakhinskaya 2010-09-29 19:15:17 UTC
We need some indication that there are more queued notifications when a notification is being shown.
Comment 1 Neha 2011-04-06 20:11:27 UTC
Created attachment 185355 [details] [review]
Needs work! Right now,the patch indicates queued notifications.
Comment 2 Marina Zhurakhinskaya 2011-04-20 17:16:10 UTC
Review of attachment 185355 [details] [review]:

Thank you for taking a stab at this feature!

The two issues I see with that patch are:

1) The style doesn't get updated if a second notification is added to or removed from the queue while a notification is showing. It's better to associate adding and removing the style with places where the set of notifications contained in this._notificationQueue is changed. We have different functions used for that: splice(), push(), shift(), and filter(). You can learn more about what each of these functions does here:
https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Array

Perhaps you can have a single function called _updateNotificationStyle(), which adds or removes the style class depending on the number of notifications in the queue. Then you can call this function after each of the above changes of the set of notifications in the queue.

2) Adding the border makes the notification in the banner mode sink down. If we really wanted the all-around border as a final effect, we would have to update the y value for the notification actor when the border is added/removed. However, as a quick fix, you can just use the right border alone, instead of the all-around border, to indicate that there are more queued notifications.

The other things mentioned below are small issues.

There are many trailing whitespaces in this patch. Please see
http://live.gnome.org/GnomeShell/Development/WorkingWithPatches#How_to_rid_your_patch_of_trailing_whitespaces
for info on how to ensure your patches don't have trailing whitespaces.

::: js/ui/messageTray.js
@@ +1860,2 @@
         this._notificationBin.show();
+       // this._notification.actor.remove_style_class_name('notification-bordering');

Please don't leave commented out lines of code in the patches you attach.

@@ +1910,3 @@
                           };
+        if (!this._notification.expanded) 
+            tweenParams.y = 0;        

This only adds trailing whitespaces.

@@ +1969,3 @@
         this._notificationBin.hide();
+        if (this._notificationStyleAdd)
+            this._notification.actor.remove_style_class_name('notification-bordering'); 

You actually don't need to have this._notificationStyleAdd because it is just fine to call remove_style_class_name() even if the actor doesn't have the style being removed. It just won't do anything in that case.

Also you should use true and false values, rather than 1 and 0 for booleans.
Comment 3 Neha 2011-05-10 17:59:43 UTC
Created attachment 187591 [details] [review]
Patch indicates queued notifications

Update the style of right border when notifications are queued.
Comment 4 Hellyna Ng 2011-05-12 04:34:18 UTC
A few points before marinaz or even me review your patch:

- There are still many trailing whitespaces, please fix that.

"There are many trailing whitespaces in this patch. Please see
http://live.gnome.org/GnomeShell/Development/WorkingWithPatches#How_to_rid_your_patch_of_trailing_whitespaces"

or if you use vim

"http://blog.hellyna.com/2010/12/auto-stripping-of-trailing-whitespace.html"

- Seems like this patch is based on your previous patch, correct me if I am wrong. It should be based on origin/HEAD instead. Check out http://blog.hellyna.com/2011/04/quick-and-easy-development-with-git.html
Comment 5 Neha 2011-05-31 14:47:03 UTC
Created attachment 188944 [details] [review]
patch indicates queued notifications

ready for designers' review
Comment 6 Neha 2011-06-01 06:04:32 UTC
Created attachment 188969 [details] [review]
patch indicates queued notifications

ready for designers' review
Comment 7 Marina Zhurakhinskaya 2011-06-02 19:54:25 UTC
Review of attachment 188969 [details] [review]:

The patch looks very good and works as expected! Let's wait for the designers to come up with the effect for what we want to do if there are notifications queued after the one showing.

Eventually, the patch will need a good commit message. See http://lists.cairographics.org/archives/cairo/2008-September/015092.html and the git log for what we'd like to have in the commit messages.

::: js/ui/messageTray.js
@@ +2037,3 @@
+        }
+        else {
+            this._notification.actor.remove_style_class_name('notification-queue');

You don't need curly braces if both parts of the if-else clause have only one line.

I think 'more-notifications-queued' would be a clearer css class name.
Comment 8 Marina Zhurakhinskaya 2011-06-02 20:15:18 UTC
Review of attachment 188969 [details] [review]:

::: js/ui/messageTray.js
@@ +2037,3 @@
+        }
+        else {
+            this._notification.actor.remove_style_class_name('notification-queue');

Actually, realized that we also need to check if this._notification is not null before doing anything with it. Putting

if (!this._notification)
    return;

in the beginning of this function would work.

You can wait though for the design before making any further changes.
Comment 9 William Jon McCann 2011-06-07 19:01:48 UTC
What does it look like?
Comment 10 Marina Zhurakhinskaya 2011-06-07 20:49:23 UTC
Right now it is showing a right border as a place holder indicator that there are more notifications queued, but we need a design for what it should look like.
Comment 11 Neha 2011-06-11 06:43:17 UTC
Created attachment 189708 [details] [review]
Indicates that there are more queued notifications

It would be helpful if the notifications have an indication that there are
more messages waiting to show up, so that the user can pay heed without
getting distracted more often.
Comment 12 Jasper St. Pierre (not reading bugmail) 2012-06-01 11:25:09 UTC
Do we still want something like this?
Comment 13 Marina Zhurakhinskaya 2012-08-28 16:53:04 UTC
Bug 677217 has the new design for how we should indicate that there are more queued notifications.

*** This bug has been marked as a duplicate of bug 677217 ***