-
Notifications
You must be signed in to change notification settings - Fork 21
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
Proposal/RFC scoped styles #539
base: develop
Are you sure you want to change the base?
Conversation
Move them as inline styles to element's shadow root. Add `useShadowStyles` hook, to separate style encapsulation logic. Addresses #65
So I've took at the code in this PR and I'm not really sure / kind of torn about it. 💅 (Personal opinion ahead)
Also, I found a third party library that seem to do similar thing: https://github.com/Wildhoney/ReactShadow, with 961 GitHub stars. It mentions SSR too. At the bottom of the page, it has a link to @tomalec's https://tomalec.github.io/declarative-shadow-dom/ 😄 Similar to #538, the ultimate question: Do we want to merge this in and release this? Are we comfortable with this in production? If there is some sort of prior art within WooCommerce / WordPress, then that would give us more confidence on this. (Personally, because of the opinions above, I feel more reluctant on this PR than #538.) |
Thanks, for the review and your opinions :)
Have you tried fix/65-encapsulated-stylesheets? What makes this PR not be a CSS-in-JS solution, or at least not CSS-in-JS-compatible? I believe it's one, but just native, forever-backward-compatible, with 0 dependencies and framework agnostic. With this PR, you can do, things like:
|
Again, have you checked also the fix/65-encapsulated-stylesheets? Probably you're not used to working with pages that use native Shadow DOM (ones that could disregard IE, or even other browsers before 2018). Try setting your Chrome dev tools preferences: And you will see it everywhere 😈 Speaking on the noise you will see reduced noise when it comes to the classes:
|
❤️ BTW, I'm not 100% happy with
In short, by using In my proposal here, I suggest using a tiny fraction of Shadow DOM features: style scoping, and even a fraction of that: host element style scoping. To solve a serious problem we have with class name conflicts, and providing CSS-in-JS ergonomics, with minimal effort and price to pay. |
Speaking of using it on production as is. I'd say the IE support is the only blocker here. Given we are about to use the relatively young feature for our standards - ~10 years since started, ~3 years since supported in all evergreen browsers, we should probably need to reach a broader audience for approval. I was trying to reach out to our team to find whether the idea is appealing and give value at least in the scale of our plugin. I plan to address any feedback here, and prepare a P2 post over the weekend if I find some spare time. If the idea looks interesting enough, I think we could experiment with it in a calmer, cooldown period, or once we face a serious problem that could be solved using it (and we will not support IE). |
We no longer need to support IE, so maybe it's a time to revisit this idea :) |
This is an experiment/proposal/RFC for a style scoping solution to address #65
(Could be considered a native alternative to libraries like styled-components / emotion #71)
This change plays well with #538, but could be totally separated.
Live demo
https://codesandbox.io/s/scoped-shadow-styles-for-react-82wrh?file=/src/components/QuoteCard.js
Changes proposed in this Pull Request:
Encapsulate
ContentButtonLayout
's styles.Move them as inline styles to element's shadow root.
Add
useShadowStyles
hook, to separate style encapsulation logic.I'm using inline styles here to simplify the PR, we can use
CSSStyleSheets
but making them work in all browsers https://caniuse.com/mdn-api_cssstylesheet_replacesync would require a little bit more code that could blur the picture. I guess with our heavy cannon of webpack, generating stylesheet objects, adding css imports, adding them as chunked styles, should be a big deal.See styleSheet version at fix/65-encapsulated-stylesheets
Please note, that components
children
are still rendered in the light DOM, so are accessible for external use, styling, and manipulation as there were before.Screenshots:
Detailed test instructions:
ContentButtonLayout
component, like https://gla1.test/wp-admin/admin.php?page=wc-admin&path=%2Fgoogle%2Fsetup-adsPros:
Shadow,
:host ...
selector styles are the class ones - applies to all instances..gla-some-class
indicates a special subset of instances to be selected.CSSStyleSheet
objectCons:
Fields for improvements / further exploration
ref
manually.shadowRoot
styles back to HTML.:host
and::slotted( foo )