-
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
--- | ||
'myst-to-react': patch | ||
'@myst-theme/frontmatter': patch | ||
'@myst-theme/styles': patch | ||
--- | ||
|
||
Improve margins on children and balancing of callouts and equations. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@myst-theme/frontmatter': patch | ||
--- | ||
|
||
Add underline on hover to DOI link. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,8 @@ | |
.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; | ||
} | ||
Comment on lines
9
to
14
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. understood π |
||
.article-grid-gap { | ||
@apply gap-1 md:gap-2 xl:gap-3 2xl:gap-4; | ||
|
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 me2rem
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 explicitmb-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. :)