Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

report: larger vertical margins for audit-group summaries #6688

Merged
merged 1 commit into from
Dec 7, 2018

Conversation

brendankenny
Copy link
Member

part of #6395

Widens the vertical margins around group header summaries to match the specced design (see discussion starting with #6395 (comment)).

Before:

screen shot 2018-11-29 at 15 19 05

After:

screen shot 2018-11-29 at 15 21 25

Things are a little spread out for e.g. the a11y groups:

screen shot 2018-11-29 at 15 22 46

but it's not egregious

@patrickhulce
Copy link
Collaborator

Does this go for DevTools too? I guess we're already different enough from the rest of the aesthetic what's a little more spacious layout 😆

margin-bottom: 0;
}

.lh-audit-group__summary {
display: flex;
justify-content: space-between;
padding-right: var(--text-indent);
margin-bottom: var(--lh-audit-vpadding);
margin-top: calc(var(--section-padding) * 1.5);
margin-bottom: var(--section-padding);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--section-padding is admittedly somewhat a pick out of a hat, but it mostly matches what's being done here, and it gets the slightly more crowded style for devtools (24/16px by default, 18/12px in devtools)

(plus @paulirish suggested using it for this :)

@brendankenny
Copy link
Member Author

Does this go for DevTools too?

ha, ninjaed :)

It's possible it's still too spread out for devtools, but maybe we can tweak? It was margin-bottom 4px before, now it's 12px.

@paulirish paulirish merged commit ebc9b2b into master Dec 7, 2018
@paulirish paulirish deleted the reportmargin branch December 7, 2018 20:34
mattzeunert pushed a commit to kdzwinel/lighthouse that referenced this pull request Dec 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants