GNOME Bugzilla – Bug 679106
Need drop shadow for notification bar
Last modified: 2016-03-31 13:53:37 UTC
The notification bar has a drop shadow in the mockups. We need to implement that.
I wonder if the right approach would be to use ClutterEffect, and paint a drop shadow around the notificationbar actor. We might want to control the shadow edges of the effect, so that only the bottom part of the notificationbar would have shadows. Eventually, this approach could be reused for the other drop shadows in boxes?
Created attachment 227588 [details] [review] Update to latest libgd to get working notification header
Created attachment 227589 [details] [review] Enable build of notifications in libgd
Created attachment 227590 [details] [review] Use notification widget from libgd This one has shadows and looks like the gnome-documents and the contacts one. Additionally, its fully gtk, so we can later use it in the display page (which does not use clutter).
Created attachment 227591 [details] [review] Drop notification specific themeing
Created attachment 227592 [details] [review] DisplayPage: move toolbar out of overlay This way we can have notifications inside the overlay, under the toolbar.
Created attachment 227593 [details] [review] Use overlapping grids instead of overlays
Created attachment 227594 [details] [review] DisplayPage: Add space for notifications We add a grid where notifications can be packed in the display page. This is below the overlay toolbar (if its visible) and above the spice view.
Created attachment 227595 [details] [review] notificationbar: Move notifications to display when visible We create the notifications in the right tab (display or in the main ui), and we move them there when we switch tabs.
Review of attachment 227588 [details] [review]: ACK! This will also make my latest commit work.
Review of attachment 227589 [details] [review]: ACK
Review of attachment 227590 [details] [review]: Patch looks good. I trust you have tested it to work and look right.
Review of attachment 227591 [details] [review]: ACK
Review of attachment 227592 [details] [review]: Looks right
Review of attachment 227593 [details] [review]: Looks right otherwise ::: src/display-page.vala @@ -98,3 @@ box.pack_start (toolbar, false, false, 0); - overlay = new Overlay (); You want to remove 'overlay' field as well then?
Review of attachment 227594 [details] [review]: ::: src/display-page.vala @@ +30,2 @@ private Overlay overlay; + public Grid notification_grid; I'd keep this private and provide an "add_notification" method to inert notifcations to this grid. @@ +123,3 @@ + + // We need one expanding row, or all will expand + var empty_grid = new Grid (); don't get understand why you need this
Review of attachment 227595 [details] [review]: looks right
Review of attachment 227593 [details] [review]: ::: src/display-page.vala @@ -98,3 @@ box.pack_start (toolbar, false, false, 0); - overlay = new Overlay (); yeah, good spotting.
Review of attachment 227594 [details] [review]: ::: src/display-page.vala @@ +30,2 @@ private Overlay overlay; + public Grid notification_grid; yeah. @@ +123,3 @@ + + // We need one expanding row, or all will expand + var empty_grid = new Grid (); Without it things goes wrong when overlay_toolbar_box is hidden. The first and last row are both "empty" and thus get half the space, so the notification appears in the middle of the view. I'll add some more comments.
Attachment 227588 [details] pushed as e3c4929 - Update to latest libgd to get working notification header Attachment 227589 [details] pushed as 014f554 - Enable build of notifications in libgd Attachment 227590 [details] pushed as bee1432 - Use notification widget from libgd Attachment 227591 [details] pushed as e110586 - Drop notification specific themeing Attachment 227592 [details] pushed as 2273ec0 - DisplayPage: move toolbar out of overlay Attachment 227593 [details] pushed as 53bca57 - Use overlapping grids instead of overlays Attachment 227594 [details] pushed as 82645c1 - DisplayPage: Add space for notifications Attachment 227595 [details] pushed as 9d5ff8c - notificationbar: Move notifications to display when visible
Created attachment 227648 [details] [review] Use vexpand rather than weird empty grid We don't have to use the empty grid to get an expanding item in the notification grid, as we can just make the notification itself expand (and then ensure it doesn't become large by using a Align.START valign.
Review of attachment 227648 [details] [review]: Looks good
Attachment 227648 [details] pushed as b374eda - Use vexpand rather than weird empty grid