-
Notifications
You must be signed in to change notification settings - Fork 608
docs: add css layer names adr #6030
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
base: main
Are you sure you want to change the base?
Conversation
|
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.
Pull Request Overview
This PR introduces an ADR to define the named CSS layers and their application order within Primer.
- Specifies five core layers (
base
,theme
,icons
,components
,utilities
) and their ordering - Establishes naming conventions (camelCase for layers, PascalCase for component layers) and namespace usage
- Details the impact on Primer libraries and poses an unresolved question about where ordering should be defined
Comments suppressed due to low confidence (1)
contributor-docs/adrs/adr-022-css-layer-names.md:8
- [nitpick] The status table shows 'Approved' with 🚧 and comments out 'Adopted'. Update the table to reflect the current stage or uncomment the Adopted status when appropriate, and use consistent labels to avoid confusion.
| Adopted | <!-- ✅ --> |
size-limit report 📦
|
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.
✨
- `components`: styles that are applied to components | ||
- `utilities`: an optional layer for utility classes in order to take precedence over earlier styles | ||
|
||
All layers should exist within the `primer` namespace. For example: |
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.
Should we differentiate PVC css, PCSS css, and PRC css?
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.
Ooo good point, Maybe primer.components.<library>.<component>
? Or primer.<library>.components.<component>
?
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.
I like primer.<library>.components.<component>
cause then we can order layers @layer primer.css, primer.react
|
||
## Unresolved questions | ||
|
||
- Where should the order of CSS layers be defined? |
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.
Feel like we should include a recommended order, but leave it up to the consumer.
|
||
## Unresolved questions | ||
|
||
- Where should the order of CSS layers be defined? |
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.
Where should the order of CSS layers be defined?
Just to confirm, is the ask here where we should have the suggestions documented?
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! Exactly, either documented or could be a file that we author the order in like:
/* base.css */
@layer primer.base, primer.reset, primer.components, primer.utilities;
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.
Looks good!
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.
🚀
|
||
- primer/css: styles in this library should be wrapped in either the utilities | ||
or icons layer. | ||
- primer/primitives: styles in this library should be wrapped in the theme layer |
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.
Are you thinking this would be directly in our dist CSS files?
- `base`: base styles that are applied to the document and the root element. This includes things like resets, typography, and | ||
global styles. | ||
- `theme`: styles that are applied for theming. This layer includes `@primer/primitives` | ||
- `icons`: styles that are applied to octicons | ||
- `components`: styles that are applied to components | ||
- `utilities`: an optional layer for utility classes in order to take precedence over earlier styles |
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.
Non-blocking nitpick: I see the library mentioned (and used) below, but not in this list. Might be helpful to add it here after base
or theme
.
View Rendered ADR
Adds an ADR in order to define our CSS Layers and the order that they should be applied in. I wanted to take a stab at defining this for us as we start moving more into CSS Layers world. Defining the layers and their order should help address issues like: https://github.com/github/primer/issues/5181 since we will have a deterministic order in which styles are applied.
This was a quick draft so let me know if you have any questions or feedback! In particular, would love to see any resources you all might have that talk about this in other systems 👀 I linked a couple but I'm sure there are more out there.