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 679106 - Need drop shadow for notification bar
Need drop shadow for notification bar
Status: RESOLVED FIXED
Product: gnome-boxes
Classification: Applications
Component: general
unspecified
Other Linux
: Normal normal
: --
Assigned To: GNOME Boxes maintainer(s)
GNOME Boxes maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2012-06-29 08:02 UTC by Alexander Larsson
Modified: 2016-03-31 13:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Update to latest libgd to get working notification header (569 bytes, patch)
2012-10-29 21:15 UTC, Alexander Larsson
committed Details | Review
Enable build of notifications in libgd (574 bytes, patch)
2012-10-29 21:15 UTC, Alexander Larsson
committed Details | Review
Use notification widget from libgd (6.91 KB, patch)
2012-10-29 21:15 UTC, Alexander Larsson
committed Details | Review
Drop notification specific themeing (1.23 KB, patch)
2012-10-29 21:15 UTC, Alexander Larsson
committed Details | Review
DisplayPage: move toolbar out of overlay (1.69 KB, patch)
2012-10-29 21:16 UTC, Alexander Larsson
committed Details | Review
Use overlapping grids instead of overlays (2.17 KB, patch)
2012-10-29 21:16 UTC, Alexander Larsson
committed Details | Review
DisplayPage: Add space for notifications (1.59 KB, patch)
2012-10-29 21:16 UTC, Alexander Larsson
committed Details | Review
notificationbar: Move notifications to display when visible (1.59 KB, patch)
2012-10-29 21:16 UTC, Alexander Larsson
committed Details | Review
Use vexpand rather than weird empty grid (1.61 KB, patch)
2012-10-30 15:52 UTC, Alexander Larsson
committed Details | Review

Description Alexander Larsson 2012-06-29 08:02:00 UTC
The notification bar has a drop shadow in the mockups.
We need to implement that.
Comment 1 Marc-Andre Lureau 2012-07-23 18:26:51 UTC
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?
Comment 2 Alexander Larsson 2012-10-29 21:15:50 UTC
Created attachment 227588 [details] [review]
Update to latest libgd to get working notification header
Comment 3 Alexander Larsson 2012-10-29 21:15:53 UTC
Created attachment 227589 [details] [review]
Enable build of notifications in libgd
Comment 4 Alexander Larsson 2012-10-29 21:15:56 UTC
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).
Comment 5 Alexander Larsson 2012-10-29 21:15:59 UTC
Created attachment 227591 [details] [review]
Drop notification specific themeing
Comment 6 Alexander Larsson 2012-10-29 21:16:02 UTC
Created attachment 227592 [details] [review]
DisplayPage: move toolbar out of overlay

This way we can have notifications inside the overlay, under
the toolbar.
Comment 7 Alexander Larsson 2012-10-29 21:16:04 UTC
Created attachment 227593 [details] [review]
Use overlapping grids instead of overlays
Comment 8 Alexander Larsson 2012-10-29 21:16:07 UTC
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.
Comment 9 Alexander Larsson 2012-10-29 21:16:10 UTC
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.
Comment 10 Zeeshan Ali 2012-10-29 21:36:12 UTC
Review of attachment 227588 [details] [review]:

ACK! This will also make my latest commit work.
Comment 11 Zeeshan Ali 2012-10-29 21:44:15 UTC
Review of attachment 227589 [details] [review]:

ACK
Comment 12 Zeeshan Ali 2012-10-29 21:49:03 UTC
Review of attachment 227590 [details] [review]:

Patch looks good. I trust you have tested it to work and look right.
Comment 13 Zeeshan Ali 2012-10-29 21:52:04 UTC
Review of attachment 227591 [details] [review]:

ACK
Comment 14 Zeeshan Ali 2012-10-29 22:04:48 UTC
Review of attachment 227592 [details] [review]:

Looks right
Comment 15 Zeeshan Ali 2012-10-30 01:29:00 UTC
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?
Comment 16 Zeeshan Ali 2012-10-30 01:53:25 UTC
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
Comment 17 Zeeshan Ali 2012-10-30 01:54:25 UTC
Review of attachment 227595 [details] [review]:

looks right
Comment 18 Alexander Larsson 2012-10-30 07:13:29 UTC
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.
Comment 19 Alexander Larsson 2012-10-30 07:16:18 UTC
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.
Comment 20 Alexander Larsson 2012-10-30 07:31:09 UTC
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
Comment 21 Alexander Larsson 2012-10-30 15:52:34 UTC
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.
Comment 22 Zeeshan Ali 2012-10-30 17:35:49 UTC
Review of attachment 227648 [details] [review]:

Looks good
Comment 23 Alexander Larsson 2012-10-30 19:18:42 UTC
Attachment 227648 [details] pushed as b374eda - Use vexpand rather than weird empty grid