GNOME Bugzilla – Bug 687787
Don't always show the message tray in the overview, add a new messages indicator
Last modified: 2013-03-04 21:31:37 UTC
Always showing the message tray in the overview has a few disadvantages: it uses a lot of vertical screen space that could be allocated to the window thumbnails, it distracts from application launching and window switching tasks, and it means that we have to use a custom visual style for when the tray is in the overview (leading to inconsistency). An alternative is to let users manually show and hide the tray as normal, and to add a new messages indicator to the overview. Mockups for these design updates can be found on the wiki: https://live.gnome.org/GnomeShell/Design/Guidelines/MessageTray/#A3.8_Roadmap This ties in with bug 687785.
The new messages indicator was rejected in bug 687785, and the visibility of the message tray was fixed in bug 693987. Closing.
I don't think this bug is dependent on bug 687785. We can have a new messages indicator without indicating updated items in the tray.
Uh, you're right, my bad.
*** Bug 694476 has been marked as a duplicate of this bug. ***
Created attachment 237669 [details] [review] messageTray: emit a signal when we're showing or hiding This will be useful in a future commit.
Created attachment 237670 [details] [review] overview: use a bin for the main overview actor We'll pack the new messages indicator on top of the current overview group, so we need to pack it into a ClutterBinLayout first.
Created attachment 237671 [details] [review] messageTray: add an indicatorCount property This filters out resident and transient notifications in the normal case, but just returns the number of unread messages for the Telepathy implementation.
Created attachment 237672 [details] [review] overviewControls: add a new messages indicator The new messages indicator will be visible when there are non-resident notifications in the tray.
Review of attachment 237670 [details] [review]: I called it "stack" in bug 694261 :-)
(In other words, one of us has to rebase patches ...)
Created attachment 237673 [details] [review] overviewControls: add a new messages indicator The new messages indicator will be visible when there are non-resident notifications in the tray.
Review of attachment 237671 [details] [review]: I'm not sure I like the name "indicatorCount". I'm also not sure I like that it's not the same number as the sum of all the little blue badges in the tray itself... ::: js/ui/messageTray.js @@ +1156,3 @@ + get indicatorCount() { + let notifications = this.notifications.filter( + function(n) { Indentation is a bit screwy. You could probably just put the function on the same line. @@ +1157,3 @@ + let notifications = this.notifications.filter( + function(n) { + return (!n.isTransient && !n.resident); No need for the parens.
Review of attachment 237672 [details] [review]: ::: js/ui/overviewControls.js @@ +381,3 @@ + this.actor.add_actor(this._highlight); + + Main.messageTray.connect('source-added', Lang.bind(this, this._onSourceAdded)); Where do you track existing sources? Shouldn't you need something like: Main.messageTray.getSources().forEach(Lang.bind(this, function(source) { this._onSourceAdded(null, source); }); ? @@ +398,3 @@ + + _onSourceRemoved: function(tray, source) { + this._sources = this._sources.filter(function(s) { return s != source }); We usually use this._sources.splice(this._sources.indexOf(source), 1);
Review of attachment 237669 [details] [review]: I'd prefer this to be done in updateState, where all the magic is, because otherwise, there are cases where the tray *won't* open, even when traySummoned = true;
(In reply to comment #13) > Where do you track existing sources? Shouldn't you need something like: Well ControlsManager is instantiated only once at startup, so I don't think that's really needed - I believe the screen shield code needs it because it's recreated every time the screen is locked.
Ah, I thought ControlsManager was recreated with the rest of the overview.
Created attachment 237675 [details] [review] messageTray: emit a signal when we're showing or hiding -- Okay; showTray and hideTray are only called from updateState so it should be fine to emit the signal just before the tween starts.
Created attachment 237677 [details] [review] messageTray: add an indicatorCount property -- They are some subtle differences for resident notification handling with the badge count, which represents the total notifications for the source; e.g. an app that has a resident notification and then adds another regular one has a (2) badge but only one new message. The alternative is trying to filter out resident notifications inside the indicator, but I didn't like it, since Telepathy notifications are also resident but behave differently and would need special casing.
Created attachment 237678 [details] [review] overviewControls: add a new messages indicator The new messages indicator will be visible when there are non-resident notifications in the tray.
(In reply to comment #10) > (In other words, one of us has to rebase patches ...) Maybe I can just rename it to stack and we get the small patch in first?
(In reply to comment #20) > (In reply to comment #10) > > (In other words, one of us has to rebase patches ...) > > Maybe I can just rename it to stack and we get the small patch in first? I should be able to manage a s/stack/overviewBin/ as well :-) (there's a small problem with StBin and ClutterBinLayout being very different, so the name "bin" is a bit confusing)
Review of attachment 237670 [details] [review]: I like the name "stack" better.
Review of attachment 237675 [details] [review]: OK.
Review of attachment 237677 [details] [review]: If this is the design, go for it, I guess.
Review of attachment 237678 [details] [review]: ::: data/theme/gnome-shell.css @@ +759,3 @@ +.messages-indicator-highlight { + background-gradient-start: transparent; + background-gradient-end: #999999; If we wanted to rip Android off, we could use their color scheme of using #33B5E5 for this thing: http://developer.android.com/design/style/color.html ::: js/ui/overviewControls.js @@ +385,3 @@ + this.actor.add_actor(this._highlight); + + Main.messageTray.connect('source-added', Lang.bind(this, this._onSourceAdded)); Either add a comment about how we're expected to be initialized during startup, and thus we can't possibly have any sources, or just do the getSources iteration and append sources. I'm leaning towards the latter, as it's cleaner and slightly more extension friendly. @@ +416,3 @@ + + this._count = count; + this._label.text = _("%d new messages").format(count); Shouldn't this use ngettext? Otherwise we'll have "1 new messages"
Created attachment 237683 [details] [review] overviewControls: add a new messages indicator -- Fixed to use ngettext, and also adding sources at creation. As for the style, I think it's best to leave that for Jakub or Allan to tweak - right now it's using a simple light gray to transparent gradient.
Review of attachment 237683 [details] [review]: OK.
I asked for an UI freeze break for this now.
Created attachment 237717 [details] screenshot
(In reply to comment #29) > Created an attachment (id=237717) [details] > screenshot Off-topic: looking at your terminal, I feel less alone now ...
Created attachment 237718 [details] [review] overview: use a bin for the main overview actor -- Reattaching the patch using _stack so people can test.
Comment on attachment 237718 [details] [review] overview: use a bin for the main overview actor Restoring patch status
Comment on attachment 237718 [details] [review] overview: use a bin for the main overview actor Attachment 237718 [details] pushed as 43ed66c - overview: use a bin for the main overview actor Getting this out of the way so Florian can rebase his patchset for bug 694261
Attachment 237675 [details] pushed as 8fb2263 - messageTray: emit a signal when we're showing or hiding Attachment 237677 [details] pushed as bdbea24 - messageTray: add an indicatorCount property Attachment 237683 [details] pushed as 8b48560 - overviewControls: add a new messages indicator
Thanks Cosimo, this looks great. One thing: the chat icon was intended to indicate if there are chat notifications waiting - it wasn't supposed to be a permanent presence.
That is now fixed too in git master.