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 682738 - Don't auto-expand notifications
Don't auto-expand notifications
Status: RESOLVED OBSOLETE
Product: gnome-shell
Classification: Core
Component: message-tray
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2012-08-26 17:53 UTC by Giovanni Campagna
Modified: 2015-02-26 22:51 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
MessageTray: clamp unexpanded notification height (1.57 KB, patch)
2012-08-26 18:29 UTC, Giovanni Campagna
none Details | Review
MessageTray: hide notification contents when not expanded (3.93 KB, patch)
2012-08-26 19:35 UTC, Giovanni Campagna
none Details | Review
MessageTray: clamp unexpanded notification height (2.63 KB, patch)
2012-08-26 19:58 UTC, Giovanni Campagna
committed Details | Review
st-table: Don't add spacing for zero-height rows/columns (2.50 KB, patch)
2012-08-26 20:38 UTC, Jasper St. Pierre (not reading bugmail)
none Details | Review

Description Giovanni Campagna 2012-08-26 17:53:45 UTC
If a notification has widgets (such as actions, images or entries), it's always shown expanded. What's more awkward, there is still the truncated body next to the title, and it disappears when you mouse over.

git bisect tells me it's a regression from 6f8e7f07f3c0ae38acda81eb05d8f28aab3c35d5, which incorrectly assumed that before calling .expand(), the height of a notification would include only the banner.
Comment 1 Jasper St. Pierre (not reading bugmail) 2012-08-26 17:58:02 UTC
(In reply to comment #0)
> If a notification has widgets (such as actions, images or entries), it's always
> shown expanded. What's more awkward, there is still the truncated body next to
> the title, and it disappears when you mouse over.
> 
> git bisect tells me it's a regression from
> 6f8e7f07f3c0ae38acda81eb05d8f28aab3c35d5, which incorrectly assumed that before
> calling .expand(), the height of a notification would include only the banner.

I would like for that to be a correct assumption. Can we make it so?
Comment 2 Giovanni Campagna 2012-08-26 18:29:20 UTC
Created attachment 222489 [details] [review]
MessageTray: clamp unexpanded notification height

The height of an unexpanded notification could include expanded
content if the notification has extra widgets (like actions and images),
so tweening to that cause it to expand visually.
Instead, use the height of the message tray before the restyle
as an upper bound.
Comment 3 Giovanni Campagna 2012-08-26 18:38:31 UTC
(In reply to comment #1)
> (In reply to comment #0)
> > If a notification has widgets (such as actions, images or entries), it's always
> > shown expanded. What's more awkward, there is still the truncated body next to
> > the title, and it disappears when you mouse over.
> > 
> > git bisect tells me it's a regression from
> > 6f8e7f07f3c0ae38acda81eb05d8f28aab3c35d5, which incorrectly assumed that before
> > calling .expand(), the height of a notification would include only the banner.
> 
> I would like for that to be a correct assumption. Can we make it so?

It's not this simple: it means that either you override get_preferred_height() (evil and hackish), or that you delay adding the actors until the notification is shown (hard for libnotify, not really possible for telepathy)
Comment 4 Jasper St. Pierre (not reading bugmail) 2012-08-26 18:41:37 UTC
Couldn't you just hide the buttons/widgets until the notification is expanded?
Comment 5 Giovanni Campagna 2012-08-26 18:47:43 UTC
(In reply to comment #4)
> Couldn't you just hide the buttons/widgets until the notification is expanded?

I'm an idiot...
I'll prepare a patch.
Comment 6 Giovanni Campagna 2012-08-26 19:29:59 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > Couldn't you just hide the buttons/widgets until the notification is expanded?
> 
> I'm an idiot...
> I'll prepare a patch.

No, turns out that you can't: even if the actors inside the table are hidden, you still get the row spacing and extra padding at the bottom.
Comment 7 Giovanni Campagna 2012-08-26 19:35:55 UTC
Created attachment 222493 [details] [review]
MessageTray: hide notification contents when not expanded

Hide the contentArea, actionArea and imageBin when the notification is
not expanded. This way the height of the notification actor corresponds
exactly to the banner.

For future record, not for review. This is the best I got so far.
Comment 8 Giovanni Campagna 2012-08-26 19:58:32 UTC
Created attachment 222497 [details] [review]
MessageTray: clamp unexpanded notification height

The height of an unexpanded notification could include expanded
content if the notification has extra widgets (like actions and images),
so tweening to that cause it to expand visually.
Instead, use the height of the message tray before the restyle
as an upper bound.

This time using CSS, and forcing notificationStackWidget.height to do
the right thing.
Comment 9 Jasper St. Pierre (not reading bugmail) 2012-08-26 20:03:52 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > Couldn't you just hide the buttons/widgets until the notification is expanded?
> > 
> > I'm an idiot...
> > I'll prepare a patch.
> 
> No, turns out that you can't: even if the actors inside the table are hidden,
> you still get the row spacing and extra padding at the bottom.

That seems like an StTable bug.
Comment 10 Jasper St. Pierre (not reading bugmail) 2012-08-26 20:38:25 UTC
Created attachment 222498 [details] [review]
st-table: Don't add spacing for zero-height rows/columns

If we hide an entire row/column, we shouldn't see a blank bit of spacing
surrounding the row/column.



Try this out for size.
Comment 11 Giovanni Campagna 2012-08-28 20:20:56 UTC
No way, rhythmbox notifications are still half expanded.
Comment 12 Jasper St. Pierre (not reading bugmail) 2012-08-28 23:00:49 UTC
Review of attachment 222497 [details] [review]:

This is fine for now if we can't get the other thing done in time.
Comment 13 Giovanni Campagna 2012-08-28 23:06:24 UTC
Comment on attachment 222497 [details] [review]
MessageTray: clamp unexpanded notification height

Attachment 222497 [details] pushed as f96dcac - MessageTray: clamp unexpanded notification height
Have fun getting it right.
Comment 14 Allan Day 2015-02-26 22:51:52 UTC
I'm pretty sure that we dropped actions and images with the new notifications design, and banners with entries aren't auto-expanding for me - closing as obsolete. Feel free to reopen if I got it wrong.