report(redesign): various whitespace adjustments (DRAFT) #8591
Conversation
|
we don't get anymore top margin now though between the category header should we bump that a tad?
|
|
Ok, I tried to adjust that @patrickhulce, and some other things that didn't match re: section spacing with the report, and it looks closer now ? Super hard to split up changes that affect spacing (padding, margin of various components, font sizes, etc). I want to pull up the design and the impl side by side, and make pixel-perfect adjustments. but if the font size goes in one PR, and spacing for gauges in one, and margins for sections in another ... well, my brain goes kaploot. so maybe I use this PR to make tons of spacing adjustments in one go? |
Why don't we just do that then? :) That's always the strategy that makes more sense to me personally, sit down with a designer and make all the adjustments at once. |
wunderbar |
connorjclark
changed the title
connorjclark
changed the title
| .lh-audit { | ||
| border-top: 1px solid var(--report-secondary-border-color); | ||
| } | ||
| .lh-audit ~ .lh-audit { |
There was a problem hiding this comment.
this is a somewhat expensive selector. can we just do border-top on the .lh-audit:nth-child(1) ?
There was a problem hiding this comment.
done. had to be 2.
There was a problem hiding this comment.
ah. i see. i was looking at best practices where the DOM is a little different.
hmm. if we can still keep to something simple like nth-child and support both these cases, i'd like to. but if its a PITA then perhaps we have to go back to the tilde.
There was a problem hiding this comment.
I'd prefer the tilde. this dom changes too much, and this would easily get out of date
| @@ -500,6 +500,9 @@ | |||
|
|
|||
| .lh-gauge__svg-wrapper { | |||
| position: relative; | |||
| /* The wrapper adds some height to the element. display: inline-block doesn't | |||
| work for some reason, so instead just shave off the extra height. */ | |||
| margin-bottom: -7px; | |||
There was a problem hiding this comment.
hmmm i'm only seeing 5 extra pixels, not 7.
also i'd much rather determine where the phantom height is coming from and address that rather than go with negative margin hacks.
There was a problem hiding this comment.
I tried setting the svg wrapper to inline-block (which should not consume those extra 7 pixels):
But it gets ignored.
...After typing this, I realized I needed to set the height: height: var(--gauge-circle-size).
| /* | ||
| * Apply border-top to just the first audit. First child is the header. | ||
| */ | ||
| .lh-audit:nth-child(2) { |
There was a problem hiding this comment.
we can go back to the tilde since this doesn't work as consistently.
#8185