-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Docs separate css blocks by example #5050
Conversation
Build successful! 🎉 |
I think the importing is good. I'm a little worried that we have maybe split things into too many layers though? Like maybe we could only use layers where we actually need them in order to solve an ordering issue and not all the time? I think the original intent was to split the CSS at the top of the page up into more sections further down, and the ordering issue might arise from that. But in many cases that isn't an actual problem so we don't really need layers then. For example we don't really need HCM to be on a separate layer in many of the pages. Maybe we can chat about it before you go too much further. |
So one thing I've noticed is that with layers, the order of imports matters, this makes sense because it determines the order in which layers are declared. However, if we don't have layers, then not only does order matter, but also specificity. I think it's better to only have one of those to worry about. I think we could do a single |
Build successful! 🎉 |
Build successful! 🎉 |
Build successful! 🎉 |
Build successful! 🎉 |
Build successful! 🎉 |
Build successful! 🎉 |
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.
Only took a look at the components starting with T's so far
# Conflicts: # packages/react-aria-components/docs/DropZone.mdx
Build successful! 🎉 |
Build successful! 🎉 |
@import './Checkbox.mdx'; | ||
@import './ListBox.mdx'; | ||
@import './Popover.mdx'; | ||
``` | ||
|
||
```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.
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.
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, this one is just a little confusing because it shares styles with another custom wrapper example, and I think it probably shouldn't
maybe I need some way to not export this one, can do in follow up later
for now I've just done the minimal work to get it back to how it used to look
Build successful! 🎉 |
@import './Checkbox.mdx'; | ||
@import './ListBox.mdx'; | ||
@import './Popover.mdx'; | ||
``` | ||
|
||
```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.
@@ -691,39 +702,6 @@ import {Link} from 'react-aria-components'; | |||
</Router> | |||
``` | |||
|
|||
<details> |
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 do think the links look a bit large...maybe we should change it like you suggested
Build successful! 🎉 |
Build successful! 🎉 |
Build successful! 🎉 |
Build successful! 🎉 |
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.
LGTM, the shared css importing + block separation by variant makes the css more readable, especially for cases like Dialog/DatePicker/etc.
Build successful! 🎉 |
## API Changes
unknown top level export { type: 'any' } |
Closes
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project:
Tracking