-
Notifications
You must be signed in to change notification settings - Fork 15
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
📏 Fix spacing in themes #450
Conversation
🦋 Changeset detectedLatest commit: 1350d35 The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
on the abstract heading... that probably also applies to any FrontmatterPart not just Abstract and I think we might want to cut the padding but keep other styling. The article-theme is where I really saw the mismatch. |
@@ -16,7 +16,7 @@ export function Abstract({ | |||
if (!content) return null; | |||
return ( | |||
<div className={className}> | |||
<h2 id={id} className="mb-3 text-base font-semibold group"> | |||
<h2 id={id} className="mb-3 text-base font-semibold group not-prose"> |
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.
Making this not-prose will change the font which is not ideal. Can we just add mt-0
?
I think that we already have good alignment without this PR and could close @agoose77, I took care of alignment in with the toc/nav work |
I noticed that there are a couple of spacing problems right now in our themes:
abstract
heading is padded:I think the
abstract
heading should benot-prose
, as we don't want to pad it. Do you agree @stevejpurves?As for
useThemeTop
, it looks like a mistake that it's used in themain
body. So, I've removed it.