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 779702 - Hide the supply level bar entirely when there's no ink info to show
Hide the supply level bar entirely when there's no ink info to show
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: Printers
git master
Other Linux
: Normal normal
: ---
Assigned To: Marek Kašík
Control-Center Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-03-07 12:48 UTC by Felipe Borges
Modified: 2017-06-15 13:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
printers: Hide supply level bar when there's no inklevel data (6.92 KB, patch)
2017-03-07 13:14 UTC, Felipe Borges
none Details | Review
screenshot (41.93 KB, image/png)
2017-03-09 12:42 UTC, Felipe Borges
  Details
screenshot of missalignment (42.91 KB, image/png)
2017-03-09 12:54 UTC, Marek Kašík
  Details
printers: Hide supply level bar when there's no inklevel data (7.50 KB, patch)
2017-03-21 12:09 UTC, Felipe Borges
none Details | Review
printer-entry-alignment (164.94 KB, image/png)
2017-03-21 12:10 UTC, Felipe Borges
  Details
printers: Hide supply level bar when there's no inklevel data (7.62 KB, patch)
2017-05-31 12:46 UTC, Felipe Borges
none Details | Review
printers: Hide supply level bar when there is no inklevel data (10.01 KB, patch)
2017-06-08 14:24 UTC, Felipe Borges
none Details | Review
printers: Hide supply level bar when there's no inklevel data (11.87 KB, patch)
2017-06-08 14:25 UTC, Felipe Borges
rejected Details | Review
printers: Hide supply level bar when there is no inklevel data (11.04 KB, patch)
2017-06-14 09:12 UTC, Felipe Borges
committed Details | Review

Description Felipe Borges 2017-03-07 12:48:52 UTC
It would save us some vertical space to hide the supply level bar, instead of greying it out, when there's no inklevel data to show.
Comment 1 Allan Day 2017-03-07 12:52:22 UTC
Makes sense to me.
Comment 2 Marek Kašík 2017-03-07 12:54:28 UTC
I was thinking about the same 10 minutes ago when looking at the panel.
Comment 3 Felipe Borges 2017-03-07 13:14:05 UTC
Created attachment 347388 [details] [review]
printers: Hide supply level bar when there's no inklevel data
Comment 4 Felipe Borges 2017-03-07 13:14:30 UTC
be aware that we are going through a freeze.
Comment 5 Marek Kašík 2017-03-09 12:00:59 UTC
Review of attachment 347388 [details] [review]:

Thank you for the patch. When I apply the patch the icons are not aligned if there are printers with/without "Ink Level" indicator or with/without "Location" field. Maybe a GtkSizeGroup could help here.
Comment 6 Felipe Borges 2017-03-09 12:42:19 UTC
Created attachment 347549 [details]
screenshot

(In reply to Marek Kašík from comment #5)
> Review of attachment 347388 [details] [review] [review]:
> 
> Thank you for the patch. When I apply the patch the icons are not aligned if
> there are printers with/without "Ink Level" indicator or with/without
> "Location" field. Maybe a GtkSizeGroup could help here.

Please, can you be more specific? I don't see which misalignment you mean.
Comment 7 Marek Kašík 2017-03-09 12:54:13 UTC
Created attachment 347551 [details]
screenshot of missalignment
Comment 8 Felipe Borges 2017-03-21 12:09:34 UTC
Created attachment 348398 [details] [review]
printers: Hide supply level bar when there's no inklevel data
Comment 9 Felipe Borges 2017-03-21 12:10:41 UTC
Created attachment 348401 [details]
printer-entry-alignment

I think now we got the alignments correctly.
Comment 10 Marek Kašík 2017-03-27 16:38:35 UTC
Review of attachment 348398 [details] [review]:

Not yet. You have to consider the entries which do not have Location field.
Comment 11 Felipe Borges 2017-05-31 12:46:29 UTC
Created attachment 352947 [details] [review]
printers: Hide supply level bar when there's no inklevel data
Comment 12 Marek Kašík 2017-06-06 13:44:42 UTC
Review of attachment 352947 [details] [review]:

Unfortunately, the misalignment is still there. Try to use e.g. ky_KG or pl_PL locale. For this kind of issues it is better to use some systematic solutions (e.g. size group) since a fixed value can be wrong with another locale.
Comment 13 Felipe Borges 2017-06-08 14:24:41 UTC
Created attachment 353384 [details] [review]
printers: Hide supply level bar when there is no inklevel data
Comment 14 Felipe Borges 2017-06-08 14:25:15 UTC
Created attachment 353385 [details] [review]
printers: Hide supply level bar when there's no inklevel data
Comment 15 Felipe Borges 2017-06-08 14:26:08 UTC
attachment 353385 [details] [review] and attachment 353384 [details] [review] are two alternatives for the alignment issue.
Comment 16 Marek Kašík 2017-06-12 14:53:30 UTC
Review of attachment 353384 [details] [review]:

Thank you for the 2 alternatives. I would use this one with some modifications.

Moreover, set xalign to 1.0 for the labels so they are aligned to right.

::: panels/printers/cc-printers-panel.c
@@ +758,3 @@
+  for (; widgets != NULL; widgets = g_slist_next (widgets))
+    gtk_size_group_add_widget (priv->size_group, GTK_WIDGET (widgets->data));
+  g_slist_free (widgets);

This wouldn't free anything since you've overwritten the pointer. Use some dedicated iterator here.

::: panels/printers/pp-printer-entry.c
@@ +680,3 @@
+
+  gtk_size_group_add_widget (group, GTK_WIDGET (self->printer_icon));
+  gtk_size_group_add_widget (group, GTK_WIDGET (self->printer_location_label));

Add self->printer_model_label and self->printer_inklevel_label please.

::: panels/printers/pp-printer-entry.h
@@ +37,3 @@
 void            pp_printer_entry_update_jobs_count (PpPrinterEntry *self);
 
+GtkSizeGroup   *pp_printer_entry_get_size_group (PpPrinterEntry *self);

Lets return just a list of widgets which needs to be grouped and maybe change the name to pp_printer_entry_get_size_group_widgets().
Comment 17 Felipe Borges 2017-06-14 09:12:02 UTC
Created attachment 353735 [details] [review]
printers: Hide supply level bar when there is no inklevel data
Comment 18 Marek Kašík 2017-06-15 13:25:33 UTC
Review of attachment 353735 [details] [review]:

Except the one small issue the patch looks good to me. Thank you for all of them!

::: panels/printers/pp-printer-entry.c
@@ +678,3 @@
+pp_printer_entry_get_size_group_widgets (PpPrinterEntry *self)
+{
+  GSList *widgets;

You need to initialize this to NULL.
Comment 19 Felipe Borges 2017-06-15 13:40:16 UTC
Thanks. Pushed with the change.

Attachment 353735 [details] pushed as 0d0b42e - printers: Hide supply level bar when there is no inklevel data