Skip to content

Instantly share code, notes, and snippets.

@msecret
Last active July 22, 2016 20:04
Show Gist options
  • Save msecret/97df675a924fb04bafa7914d5b89cb9a to your computer and use it in GitHub Desktop.
Save msecret/97df675a924fb04bafa7914d5b89cb9a to your computer and use it in GitHub Desktop.
An idea of how we can do margins on headers in WDS
// The problem
/*
In working on the standards, seeing other people's code and seeing our own code, I see a lot of having to
remove the margins on elements like headings. This usually happens when headings are included in elements
that aren't in normal content flow such as in an accordion.
*/
// accordion.scss, example of our own rules to remove margin
.usa-accordion {
h1,
h2,
h3,
h4,
h5,
h6 {
margin: 0;
}
}
// Currently we have some of the following global margin rules in typograhpy.scss
p {
...
margin-bottom: 1em;
margin-top: 1em;
}
h1,
h2,
h3,
h4,
h5,
h6 {
...
margin-bottom: .5em;
margin-top: 1.5em;
}
// Can we be more specific when margins should happen though? What if we don't set margins on headings or
// p and have rules like this instead:
h1, h2, h3, h4, h5, h6 {
...
margin: 0;
& + p {
margin-top: 0.5rem;
}
& + .text_block {
margin-top: 1.5rem;
}
}
p {
margin: 0;
...
& + h2,
& + h3,
& + h4,
& + h5 {
margin-top: 2rem;
}
}
@msecret
Copy link
Author

msecret commented Jul 22, 2016

I've been doing this with great success on cg-style.

@juliaelman
Copy link

Thank you for typing out your thoughts and posting this gist. This is a very smart approach, as it references relationships to add spacing around elements. 👍 from me on moving fwd with this approach, but also recommend we think about documentation if we decide this is the way to go.

@rogeruiz
Copy link

rogeruiz commented Jul 22, 2016

Seems like this is a good example of when to use BEM styling syntax no? Instead of looking for specific elements the CSS could look like this:

.usa-accordion {
  // relevant styles for accordion
  .usa-accordion__title {
    // relevant styles for accordion title
    margin: 0; 
  }
}

Sure it's more classes to maintain, but it would help in cases like this for specificity. More classes aren't always a bad thing.

@juliaelman
Copy link

Agreed that BEM might make more sense on a component level basis, but am not sure how useful it is on a global element stand point. Hope that makes sense!

Agree that more classes aren't always a bad thing, especially if creating these types of classes makes more sense to our user base. 👍 Great point, Roger!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment