GNOME Bugzilla – Bug 706897
Add printer dialog - "No printers detected" should be vertically centered
Last modified: 2014-02-12 15:17:11 UTC
Created attachment 253259 [details] screenshot The "No printers detected" string currently sits at the top of the list (see attached screenshot), which looks a bit strange, doesn't attract attention, and leaves a big gap below. It would be better if the string was located in the middle of the list.
Created attachment 257350 [details] [review] Vertically center text for no devices Attached patch moves the text to the center of the treeview. It sets height of rows to 1/3 of the treeview and adds empty row above the one with the text.
Created attachment 257351 [details] corresponding screenshot
Review of attachment 257350 [details] [review]: thats a bit of a cumbersome way to do this. I would suggest to hide the treeview if it is empty, and place a label there instead, with h/valign == center
Created attachment 258002 [details] [review] printers: Vertically center text for no devices (In reply to comment #3) > Review of attachment 257350 [details] [review]: > > thats a bit of a cumbersome way to do this. I would suggest to hide the > treeview if it is empty, and place a label there instead, with h/valign == > center You are right. I somehow forgot that the border is drawn by the GtkScrolledWindow and not by the GtkTreeView so I wanted to preserve the treeview there. I've changed it so the there is a GtkStack inside the GtkScrolledWindow which contains the treeview and the new GtkLabel. I switch betweeen them as needed.
Created attachment 268916 [details] [review] Vertically center text for no devices Updated patch. I forgot to set minimal width of the column with icons to 80 pixels.
Created attachment 268917 [details] updated screenshot
Review of attachment 268916 [details] [review]: Looks fine, but please get the ack from the designers. ::: panels/printers/new-printer-dialog.ui @@ +149,3 @@ + <property name="can_focus">False</property> + <property name="sensitive">False</property> + <property name="label" translatable="yes" comments="Translators: No printers were found">No printers detected.</property> Is this really useful as a comment?
Looks better indeed. Might be better with a symbolic printer icon above and without the bold weight, but don't let that stop you from landing this.
Comment on attachment 268916 [details] [review] Vertically center text for no devices (In reply to comment #7) > Review of attachment 268916 [details] [review]: > ::: panels/printers/new-printer-dialog.ui > @@ +149,3 @@ > + <property name="can_focus">False</property> > + <property name="sensitive">False</property> > + <property name="label" translatable="yes" > comments="Translators: No printers were found">No printers detected.</property> > > Is this really useful as a comment? I guess not because the string is self-explanatory. But since all translated strings should have a comment for translators, I tried to comment it in a little different way. (In reply to comment #8) > Looks better indeed. Might be better with a symbolic printer icon above and > without the bold weight, but don't let that stop you from landing this. Thank you all for the reviews.