-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Move responsive position utilities from marketing to core #459
Conversation
This might not be the best time to ask this, but I'd love to see these utilities broken apart more. While working on the Blog migration awhile back, many of these weren't useful for a basic layout and this one file mixes utilities that affect inline-ish elements ( |
I feel good about this, but I'm on the fence about whether this is a minor or major change. On one hand, moving styles feels major to me because it "breaks" people's assumptions about what CSS will be output when the |
I can't think of any projects where I've used primer-marketing without primer-core. |
Well, I went searching, and here's what I found:
|
f18919b
to
a158835
Compare
1e6990b
to
9d3eb82
Compare
relative, | ||
absolute, | ||
fixed | ||
) !default; |
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.
Heads up @mdo: I added this variable so that responsive position utilities can be customized. For instance, if you only use relative or absolute positioning, you can do something like:
$responsive-positions: (relative, absolute);
@import "primer-core/index.scss";
And instead of 20 position utilities, you'd get 10.
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.
@@ -30,6 +30,7 @@ const exceptions = { | |||
'/packages/primer-product': removed, | |||
'/principles/HTML': moved('/principles/html'), | |||
'/principles/SCSS': moved('/principles/scss'), | |||
'/utilities/marketing-layout': moved('/utilities/layout'), |
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.
This is actually pretty cool: removing primer-marketing-utilities/docs/layout.md
caused the /utilities/layout
page to disappear, which triggered a failing test. I had to mark that URL as having moved in order to get it passing again.
This PR moves the responsive position styles (and their docs) from primer-marketing into primer-core.
This first came up as a need in this GitHub PR, but I suspect that as we're doing more and more responsive work in the product that these will be valuable in primer-core in the future.
/cc @primer/ds-core @jonrohan