Slightly refactor Rhythm, support Jetpack Markdown Block #1351
Conversation
🦋 Changeset detectedLatest commit: 4c5cb8d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Paul-Hebert
left a comment
Paul-Hebert
left a comment
There was a problem hiding this comment.
This looks great! The use of custom properties to make the markdown block inherit the rhythm spacing is clever!
I left a few doc-related comments inline. My only other critique is I found the mixin name a little confusing at first. Something about "adjacent" had me confused about where the rhythm would be applied. A few other ideas. Feel free to ignore if they don't jibe with you:
nested.rhythmchild.rhythminner.rhythmspacing.rhythmrhythm.apply
| @@ -0,0 +1,29 @@ | |||
| @use "../compiled/tokens/scss/size"; | |||
There was a problem hiding this comment.
Should this be included in the changeset? Or is that not necessary since this is just an internal change? 🤔
It's probably fine as-is, but figured I'd ask since I wasn't sure.
There was a problem hiding this comment.
I don't think so since it's just an internal change.
Co-authored-by: Paul Hebert <paul@cloudfour.com>
|
@Paul-Hebert I renamed the mixin from |
Paul-Hebert
left a comment
Paul-Hebert
left a comment
There was a problem hiding this comment.
I like the new name! Looks great! 🚢
Overview
See changelog and docs for details.
Screenshots
Testing
On the deploy preview: