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 763292 - Improve the alignment of notifications
Improve the alignment of notifications
Status: RESOLVED FIXED
Product: gnome-photos
Classification: Applications
Component: general
3.19.x
Other All
: Normal normal
: ---
Assigned To: GNOME photos maintainer(s)
GNOME photos maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2016-03-08 08:04 UTC by Debarshi Ray
Modified: 2016-04-06 10:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Improve the alignment of notifications (3.10 KB, patch)
2016-03-24 14:19 UTC, Jonathan Kang
none Details | Review
Improve the alignment of notifications (3.38 KB, patch)
2016-04-05 13:15 UTC, Jonathan Kang
committed Details | Review

Description Debarshi Ray 2016-03-08 08:04:55 UTC
When you have multiple in-app notifications of different widths, you will notice a blank area on the right edge of the shorter rows. We should horizontally expand each notification (hexpand = TRUE), and, probably, set hexpand to FALSE on the parent GtkGrid so that the notification grid itself doesn't expand any further.

To reproduce:
 * Go to the selection mode
 * Export an item
 * Delete some item(s)
Comment 1 Jonathan Kang 2016-03-24 14:19:02 UTC
Created attachment 324679 [details] [review]
Improve the alignment of notifications

Set hexpand property of several labels in notification to TRUE.
Comment 2 Debarshi Ray 2016-04-05 11:39:50 UTC
Review of attachment 324679 [details] [review]:

Thanks for the patch, Jonathan. Looks good except one comment:

::: src/photos-indexing-notification.c
@@ +271,2 @@
   labels = gtk_grid_new ();
+  gtk_widget_set_hexpand (labels, TRUE);

I wonder if it would be better to set hexpand on self->primary_label and self->secondary_label. It is a pity that testing this notification is not so easy.

As far as I know, the expand flags propagate upwards from a child to its container, not the other way round. So setting hexpand on the grid won't result in expanding the child labels. Also, halign is set on the children, so it makes sense to add the hexpand there.
Comment 3 Jonathan Kang 2016-04-05 13:15:16 UTC
Created attachment 325428 [details] [review]
Improve the alignment of notifications

Patch updated.
Comment 4 Debarshi Ray 2016-04-06 10:22:00 UTC
Review of attachment 325428 [details] [review]:

Thanks, Jonathan. Looks good to me.

Tweaked the commit message and pushed.