Move TabNav styles to PVC
#1627
Conversation
🦋 Changeset detectedLatest commit: 404d165 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 |
… so it can be included in the build
| @@ -0,0 +1,61 @@ | |||
| :root { | |||
There was a problem hiding this comment.
Hey @mxriverlynn! I'm not sure we want to move SCSS variables to PVC. I think they will mostly be reserved for utility CSS at this point, and the CSS that we move over from PCSS should use the new set of design tokens that are already being used in tab_nav. If there's an issue or discussion you can point me towards that is suggesting we should move all our existing variables to PVC, please let me know!
There was a problem hiding this comment.
Migrating the SCSS variables to the CSS variables from Primitives is definitely the hardest part. 😆
There isn't always a clear 1:1 replacement and it depends on the use case. Especially for margin/padding. I started a .scss to .pcss migration guide. It's not really complete and I've just been adding to it while going along. Also might not be 100% accurate. Let me know in case something is unclear.
|
Sorry.. 😩 The To avoid moving the same thing twice, should we add our The ones with
|
| @@ -77,6 +77,7 @@ | |||
| "postcss-cli": "^10.0.0", | |||
| "postcss-import": "^14.1.0", | |||
| "postcss-mixins": "^9.0.3", | |||
| "postcss-nested": "^6.0.0", | |||
There was a problem hiding this comment.
So far we haven been adding a & anywhere a nested class is used:
.some-class {
- .octicon {
+ & .octicon {
...
}
}but I assume postcss-nested would allow us to keep using the "SCSS nesting" without the & ? @jonrohan also mentioned that it might will become a CSS standard. So using postcss-nested might be fine? 🤔
There was a problem hiding this comment.
We're trying to write closer to CSS spec. That said they are talking about changing the specification for nesting and there's an experimental plugin we could use to try it. https://twitter.com/css_tools_/status/1587392707206561792?s=46&t=OsB8hQIe4u2xhsK3WD3iYw
https://twitter.com/css_tools_/status/1587392697857413120?s=46&t=OsB8hQIe4u2xhsK3WD3iYw
|
closed as duplicate |
Description
Describe your changes here.
Closes # (type the issue number after # if applicable; otherwise remove this line)
Integration
Merge checklist