GNOME Bugzilla – Bug 760427
Adwaita: Separators in popovers shouldn't always have margins
Last modified: 2016-01-11 22:42:47 UTC
Commit 0b96b8a1 set margins via css, but accidentally changed the semantics of margins for separators in popovers so that any separator in a gtkpopover had a margin. This meant that the separators in GtkListBoxes in popovers also had a margin around their separators, and this is not what we want because it doesn't match separators in listboxes not in popovers.
Created attachment 318694 [details] [review] Adwaita: fix separator margin in popovers
before/broken: http://i.imgur.com/gjLlLDD.png after/fixed: http://i.imgur.com/82jOeKZ.png
Review of attachment 318694 [details] [review]: The CSS is generated by sass from the scss source files, so this needs to be done in _common.scss
Good catch, thanks for spotting.
Created attachment 318758 [details] [review] Adwaita: fix separator margin in popovers Commit 0b96b8a1 set margins via css, but accidentally changed the semantics of margins for separators in popovers so that any separator in a gtkpopover had a margin. This meant that the separators in GtkListBoxes in popovers also had a margin around their separators, and this is not what we want because it doesn't match separators in listboxes not in popovers.
On a second look this won't work, the popover > separator selector selects nothing most of the time (since separators are rarelly a direct child of the separator), so the patch fixes the specific issue by removing separator margins generally even when needed. Probably the correct solution here is to special case separators in lists in popovers, with something like: popover list separator { margin: 0; }
Created attachment 318821 [details] [review] Adwaita: fix separator margin in popovers Commit 0b96b8a1 set margins via css, but accidentally changed the semantics of margins for separators in popovers so that any separator in a gtkpopover had a margin. This meant that the separators in GtkListBoxes in popovers also had a margin around their separators, and this is not what we want because it doesn't match separators in listboxes not in popovers.
Review of attachment 318821 [details] [review]: Looks good