GNOME Bugzilla – Bug 761269
roomList: Extent headers to the full sidebar width
Last modified: 2016-01-29 11:24:36 UTC
Personal pet peeve of mine, see patch. (If wanted, I do have a version of the patch that uses the darker highlight color and the exact padding from master - it's just that I think the default row highlight goes better with the header border, and using the same padding on the left/right makes for slightly nicer CSS) And of course it may just be me who is weirded out by a room list header that looks like a row and quacks like a button :-)
Created attachment 319993 [details] [review] roomList: Extent headers to the full sidebar width The current room header style is slightly odd: On the one hand, we use the 'row' CSS name to style it like a row (at this time: single background color with no borders), on the other hand we add left and right margins that make the header appear more like a button inside a row. Resolve that inconsistency by removing the margins and slightly tweaking the style to be more consistent with "normal" rows while maintaining the current visuals when not active/hovered.
(In reply to Florian Müllner from comment #1) > Created attachment 319993 [details] [review] [review] hmm does not seem to apply cleanly to master (issue in room-list-header.ui:3 and roomList.js:178)
(In reply to Bastian Ilsø from comment #2) > hmm does not seem to apply cleanly to master Yup, it was easier to make it depend on bug 761057
Review of attachment 319993 [details] [review]: changing the behavior is fine by me. Some nits _not_ related to the changes this patch introduces: - I noticed our current headerbar grey bg color is slightly lighter in color (239,239,239) different from the grey bg color of the rows (235,235,235). - a row's height is 26px while a header's height is 28px. maybe those are separate bugs though.
(In reply to Bastian Ilsø from comment #4) > - a row's height is 26px while a header's height is 28px. sorry, numbers are switched around: - a row's height is 28px while a header's height is only 26px.
(In reply to Bastian Ilsø from comment #4) > Some nits _not_ related to the changes this patch introduces: > - I noticed our current headerbar grey bg color is slightly lighter in color > (239,239,239) different from the grey bg color of the rows (235,235,235). https://git.gnome.org/browse/polari/commit/?id=0de7d317d3 > - a row's height is 26px while a header's height is 28px. Ah yeah: - the row child has 2px margin at bottom/top - the header has 1px spacing + 1px separator at bottom So the row is indeed 2px higher than the header, but I'm not sure what to do about it - adding a 2px top margin to the header child looks unbalanced to me, and I don't think we want a margin under the separator. Maybe use a 1px top margin and bump the spacing to 2px?
(In reply to Florian Müllner from comment #6) > (In reply to Bastian Ilsø from comment #4) > > Some nits _not_ related to the changes this patch introduces: > > - I noticed our current headerbar grey bg color is slightly lighter in color > > (239,239,239) different from the grey bg color of the rows (235,235,235). > > https://git.gnome.org/browse/polari/commit/?id=0de7d317d3 Nice. > So the row is indeed 2px higher than the header, but I'm not sure what to do > about it - adding a 2px top margin to the header child looks unbalanced to > me, and I don't think we want a margin under the separator. > > Maybe use a 1px top margin and bump the spacing to 2px? I think that could work.
Created attachment 320008 [details] pixels (In reply to Bastian Ilsø from comment #7) > > So the row is indeed 2px higher than the header, but I'm not sure what to do > > about it - adding a 2px top margin to the header child looks unbalanced to > > me, and I don't think we want a margin under the separator. > > > > Maybe use a 1px top margin and bump the spacing to 2px? > > I think that could work. I'm posting some measurements that I imagine could work, was that what you had in mind?
Attachment 319993 [details] pushed as 65a5e2a - roomList: Extent headers to the full sidebar width
(In reply to Bastian Ilsø from comment #8) > I'm posting some measurements that I imagine could work, was that what you > had in mind? I don't know, that's not how labels work - and thanks $goddess for that: Image two labels, "G" and "g", in a horizontal box - if labels worked like in the image, it would read "G9" instead of "Gg" ...