-
Notifications
You must be signed in to change notification settings - Fork 91
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
refactor(tables): use pseudo private custom properties #1204
Conversation
✅ Deploy Preview for stacks ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Hi @dancormier I see that you are refactoring table component and there is Issue that I tried to fix. So maybe there is chance now for merging my PR, if you feel it make sense. |
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.
Great work @dancormier!
There are a couple of small fixes to address which I have highlighted in my comments.
Apart from that this looks good to me. I don't think I need to re-review after you have applied the changes - feel free to go ahead and merge. 🙂
Core's been updated. Time to merge! |
This PR refactors
.s-table
component styling to use pseudo-private custom properties. With this refactor, I didn't flatten the cascade for variants/modifiers entirely as I have with many other refactors. Tables include some very targeted styles (targetingfirst
/last
/nth
of nested elements) that would've made a complete* flattening more confusing and hard to maintain than the alternative.* "complete" as consistent, not as in total or done.