GNOME Bugzilla – Bug 754565
listbox styling issues
Last modified: 2016-09-20 08:15:55 UTC
The listbox in the 'new box' dialog has a couple of quirks: - the separator is styled as a raised line, having a highlight line followed by a shadow/dark line. - the corner rounding is inconsistant. The styling is applied on first and last child, yet some pages, like the initial source selection have labels on those positions, losing the border rounding. Possible solutions/worarounds - use a special class for the first and last items or always having a non-interactive label on those positions and have then blank at places, to keep nth-child(2) nth-last-child(2) working.
Created attachment 310657 [details] screenshot illustrating both the separators and completele lack of corner rounding
Created attachment 310658 [details] corner rounding working on another page
nth-child(2) is not needed, the boxes-menu-row's in the scrolled window are actually inside a box, so boxes-menu-row:first-child {} catches the first, we should tweak the overshoots on that scrolled window to make the whole thing less ugly when that overflows, but that's another issue.
Created attachment 311003 [details] [review] rewrite wizard styling The css contained quite a bit of cruft accumulated over the years, so I rewrote the whole wizard styling istead of just fixing this particular issue, letting things we now do in adwaita be inherited istead of adding custom css. Also there I tried to comment things to make the whole thing clearer so hopefully the next time a css change is needed it would be hopefully less painfull than this one...
Review of attachment 311003 [details] [review]: ::: data/gtk-style.css @@ +5,2 @@ .boxes-snapshot-list-row { + border-bottom: 1px solid @borders; Let's not use this patch for changing the coding style of the css please. :)
Well, that's the coding style we use in Adwaita, it's pretty automatic to write like that for me so the new code was formatted that way. Being the old css less than the new one it was faster to refactor that.
Created attachment 311025 [details] [review] css wizard rewrite 4 spaces indent formatted patch. Btw Zeeshan since you clearly don't look at boxes css, why the fsck do you care about the coding style?! :-)
(In reply to Lapo Calamandrei from comment #7) > Created attachment 311025 [details] [review] [review] > css wizard rewrite > > 4 spaces indent formatted patch. > Btw Zeeshan since you clearly don't look at boxes css, I do! And I still don't think that small CSS file is nearly as ugly as you assert. :) > why the fsck do you > care about the coding style?! :-) It was not just about coding-style but also the fact that those coding-style changes were prettty irrelevant to this patch. Please see this nice document from Lasse about why atomic commits are important: http://coala.readthedocs.org/en/latest/Getting_Involved/Writing_Good_Commits/
Review of attachment 311025 [details] [review]: ::: data/gtk-style.css @@ +5,2 @@ .boxes-snapshot-list-row { + border-bottom: 1px solid @borders; Not sure what you changed in the patch but this (and other lines below) are still coding style changes.
(In reply to Zeeshan Ali (Khattak) from comment #9) > Review of attachment 311025 [details] [review] [review]: > > ::: data/gtk-style.css > @@ +5,2 @@ > .boxes-snapshot-list-row { > + border-bottom: 1px solid @borders; > > Not sure what you changed in the patch but this (and other lines below) are > still coding style changes. Where did you go dude? As I said before, if you don't have time to clean-up patches, just let me know and I can try to help with that.
Created attachment 311387 [details] [review] css wizard rewrite Sorry pal, last time I uploaded the wrong patch (read the very same one which was obsoleted...), this is the good one.
Review of attachment 311387 [details] [review]: ::: data/gtk-style.css @@ +116,2 @@ +/* this looks very wrong FIXME by fixing the code + it should just probably be a 'dim-label' */ * FIXME is the comment itself, right? Unless you meant to say "workaround"? Why does it look wrong? * So why are we not using dim-label then? * Why are you moving these lines below?
Review of attachment 311387 [details] [review]: ::: data/gtk-style.css @@ +116,2 @@ +/* this looks very wrong FIXME by fixing the code + it should just probably be a 'dim-label' */ Ah now I get it! You started a new sentence without a '.' at the end of last one so I read it as one. :( I thought you knew enough Vala to `sed s/boxes-property-name-label/dim-label/`. :)
I moved that there since it's in the wizard section, the stuff is organized by order of appearance, when technically possible and when makes sense, to make it easier to grok the code, I didn't change the vala code since I didn't had time for it, it's ugly since it abuses a color we use for unfocused windows completelly out of context.
Created attachment 311475 [details] [review] style: Drop custom 'boxes-property-name-label' class Just use 'dim-label'. Based on a patch from Lapo Calamandrei <calamandrei@gmail.com>.
Created attachment 311476 [details] [review] style: Add a comment to boxes-step-label class Based on a patch from Lapo Calamandrei <calamandrei@gmail.com>.
Created attachment 311477 [details] [review] style: Make use of dim-label class Instead of hardcoding color. Based on a patch from Lapo Calamandrei <calamandrei@gmail.com>.
Created attachment 311478 [details] [review] style: Prettier media subtitles in wizard Based on a patch from Lapo Calamandrei <calamandrei@gmail.com>.
Created attachment 311479 [details] [review] style: Drop redundant theme classes Based on a patch from Lapo Calamandrei <calamandrei@gmail.com>.
Created attachment 311480 [details] [review] style: Prettier logos legal notice Based on a patch from Lapo Calamandrei <calamandrei@gmail.com>.
Created attachment 311481 [details] [review] style: Fix & beautify wizard media menu
Created attachment 311482 [details] [review] style: Group all wizard classes at the end Let's group all wizard classes together at the bottom of the CSS file to clean it up a bit. Based on a patch from Lapo Calamandrei <calamandrei@gmail.com>.
Created attachment 311493 [details] [review] style: Make use of dim-label class Instead of hardcoding color. Based on a patch from Lapo Calamandrei <calamandrei@gmail.com>.
Created attachment 311494 [details] [review] style: Prettier media subtitles in wizard Based on a patch from Lapo Calamandrei <calamandrei@gmail.com>.
Created attachment 311495 [details] [review] style: Drop redundant theme classes Based on a patch from Lapo Calamandrei <calamandrei@gmail.com>.
Created attachment 311496 [details] [review] style: Prettier logos legal notice Based on a patch from Lapo Calamandrei <calamandrei@gmail.com>.
Created attachment 311497 [details] [review] style: Fix & beautify wizard media menu
Created attachment 311498 [details] [review] style: Group all wizard classes at the end Let's group all wizard classes together at the bottom of the CSS file to clean it up a bit. Based on a patch from Lapo Calamandrei <calamandrei@gmail.com>.
Attachment 311475 [details] pushed as ef910f0 - style: Drop custom 'boxes-property-name-label' class Attachment 311476 [details] pushed as a5f6cdc - style: Add a comment to boxes-step-label class Attachment 311493 [details] pushed as 217749f - style: Make use of dim-label class Attachment 311494 [details] pushed as b273f20 - style: Prettier media subtitles in wizard Attachment 311495 [details] pushed as ee0b343 - style: Drop redundant theme classes Attachment 311496 [details] pushed as b689996 - style: Prettier logos legal notice Attachment 311497 [details] pushed as 35ad873 - style: Fix & beautify wizard media menu Attachment 311498 [details] pushed as 2250647 - style: Group all wizard classes at the end