-
Notifications
You must be signed in to change notification settings - Fork 10
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
π©π»βπ¨ Improve margins and spacing in grids #77
Conversation
.article-grid > * { | ||
/* The default is spanning the body for any child component */ | ||
@apply col-body; | ||
/* Grids do not have margin-collapse, so each direct child needs to be addressed */ | ||
margin-top: 0 !important; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tavin, this is basically the fix. The rest is making sure that callouts/equations are balanced and have the same margins as paragraphs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
understood π
@@ -289,7 +289,7 @@ export function FrontmatterBlock({ | |||
subject || github || venue || biblio || open_access || license || hasExports || isJupyter; | |||
const hasDateOrDoi = doi || date; | |||
return ( | |||
<> | |||
<div className="mb-8"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May I just ask about this mb-8
here? It's giving me 2rem
extra space that wasn't there before -- between the title and content, or else just pushing the content down when there's no title.
If I remove the mb-8
class everything looks perfect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yes, if you only have a title this isn't going to be good. I was testing with a full frontmatter, and that should be on conditionally (or something). Good catch!
We want these conditionally if there is frontmatter. Or maybe better is to override the h1
here and have an explicit mb-0
and then have some margin around the stuff that comes after it which is conditionally rendered.
Are you in a place to open a PR for this? If not, I can fix my bug shortly!
The two green things stack, and they should collapse if there isn't anything in between.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm I guess I need to think about it a bit. You may get there faster than me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I am not quite sure which solution is best. I will try and get to it by the end of today and tag you! Thanks again for pointing it out. :)
Originally created by @tavin in #76
This tightens up margins and simulates margin collapse in a grid by eating the top margins. This works inside and outside of the grid, while also allowing us to position child content in the margins.
Thanks @tavin for getting the ball rolling here, sorry to step on your PR. I will get this out in the next day or so as a theme-release!
Examples / Comparisons