-
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
Use @emotion library for CSS #71
Conversation
…een eslint and stylelint.
This is so that we can use the breakpoints in CSS-in-JS. This is the same as https://github.com/WordPress/gutenberg/blob/ff46cbc3ffd24495144bd4f72c294adc65e64160/packages/components/src/utils/breakpoint-values.js.
export default { | ||
huge: '1440px', | ||
wide: '1280px', | ||
'x-large': '1080px', | ||
large: '960px', // admin sidebar auto folds | ||
medium: '782px', // adminbar goes big | ||
small: '600px', | ||
mobile: '480px', | ||
'zoomed-in': '280px', | ||
}; |
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.
Note that this file is the same as https://github.com/WordPress/gutenberg/blob/master/packages/components/src/utils/breakpoint-values.js. This is so that we can use the breakpoints in CSS-in-JS.
Nice and deep research! Sorry, for being grumpy ;) Could you help me build and verify the pros and cons list here? Pros:
Cons:
I'll take another deeper look tomorrow, as it is pretty late for me. Edit 2020.11.25 11:12: added performance to the cons, as per comment below |
Thanks for exploring this @ecgan , very cool, I haven't had a chance to dive into this space before. I've done a little digging today and I'm still not quite sure it is something that is being that heavily adopted within A8C just yet. For sure there is some experimentation and components slowly starting to use it more. But it seems like quite a few comments reflect this one From what I understand for the list of pros and cons above there are also some potential performance downsides (can't cache as well and JS rendering has a little more overhead?) and similar to what Tomek mentioned, the ambiguous class could potentially trip up HEs / making debugging more complicated. I know in the past we've had requests to add more obvious classes and ids to help them on certain projects. |
@tomalec ,
I'd rather you to be grumpy on me (because it means you've read it and you're talking to me about it), instead of not reading and not grumpy 😉 😄
A little side story: years ago before I work on react, I was like that too, using HTML, CSS and JS in conventional ways. I was more of an Angular (old and new versions) guy. The first time I looked at React / JSX, I didn't like it because it wasn't "cleanly separated" "pure HTML" and "pure JavaScript". Now, after years of working with React / JSX, I love it and I wouldn't want to go back to conventional (even Angular) ways. To me, the developer productivity gain is massive. When we use React / JSX, we are using JS to do HTML + JS. With CSS-in-JS, JavaScript is essentially the unifying language that to generate everything HTML + JS + CSS, which can be seen as collapsing the stack. I just managed to find back a post that I read long time ago, A Unified Styling Language, written in May 2017 with 12k claps now. It provides some very solid reasonings on using CSS in JS. It's a long article, but I strongly recommend reading this one. There are reasons why such CSS-in-JS libraries are so popular (e.g. styled-components repo with 30k GitHub stars). I do not expect us to adopt this overnight - that would be not realistic and not pragmatic (this is also the reason why I use the
Do you mean for CSS-in-JS, or pure conventional CSS / SCSS? In conventional CSS, it would be more difficult because class names in HTML are strings, and I have seen somewhere in the WC code base where there are string concatenations like below (an example): <div class={`length-${ props.isSuper ? '4' : '8'}`}> // class name can be length-4 or length-8 depending on props. With string manipulation like this, you can see it would be more difficult to have editor support compared with CSS-in-JS.
Well, this is possibly the next best thing when we can't have Shadow DOM 😆 |
@jconroy ,
Generally, if I were to make a trade-off between performance and developer productivity (i.e. readability, maintainability etc), I would usually choose the latter. When we optimize for performance, the benefits that we get will become smaller and smaller as technology advances. However, developer / human brain has largely remain the same, so if we choose to optimize for developer, we would give us more benefits in the long run.
Got you. With this CSS-in-JS approach, we can still do manual classes or ids to help them out, it can work in harmony 🙂 |
I've had a few more discussions and done some more digging to help with my thinking. Given the projects we work on seek to align with Gutenberg's approaches I believe we roughly have 2 paths ahead of us: 1 . Current approachwc-admin and woo blocks, for the most part, seek to align with current Gutenberg approaches
2. (Potential) Future approachThe G2 project is where Gutenberg is intended to be heading. Elements from this project will potentially begin making their way to Gutenberg early to mid-December this year - subject to community and Gutenberg being accepting of the changes. Some of the key things you'll notice are:
There is a new package @wp-g2/styles which is a wrapper for Emotion that can be used. I don't know fully what it adds for WP (on top of Emotion that is immediately relevant for us) so it might be overkill for us at the moment but it also might be a good opportunity to help test and provide feedback/contribute. I actually swapped it in (locally) within this PR and it worked without any further changes on our end. The main thing I noticed is an increase in bundle size of about 70kb. There is no guarantee everything from G2 will make it to core (or without changes) but it certainly seems like it is trajectory we are headed. Does any of this help or change your thinking @tomalec @ecgan to progress the discussion ? I don't have a super strong opinion either way but I'd like to move forward on this convo and get things progressing. We can always re-evaluate. Lets perhaps try and sync up in slack if we can get the 3 of us online at the same time later today. |
@ecgan Thanks for sharing your story and a great article! I'll share a scrap of my story: I started the conventional way, then got into fancy (back then libraries and frameworks), then got back to the Web Platform, and fell in love with Web Components and it's hard for me to imagine why and how one could go back. I hope I'll soon feel in love with React as well.
Great point, now I totally agree, that given we are already settled in React and with HTML-in-JS, CSS-in-JS is no longer a disadvantage. I'm removing it from my list above. However, we still need to be careful about being able to provide critical CSS upfront. I'm not sure how to we handle it currently. However, I think my "working canonical/native/vanilla/conventional" still stands here, as "-in-JS" is somewhat conventional with React ;)
Agree, and the number of reasons is nicely gathered in the article you linked 👍 In general, I believe the best code is no code.
I mean pure CSS and CSS in JS. Like CSS Custom properties (
|
In general, I agree. Unless we blow something in the face of a user or HE. Investigating styles for |
I'm happy to meet and chat :) Just want to note, that, I don't try to act blocky here. I'm just sharing my elaborate, newcomer thoughts and opinions. I'm happy to give Emotion a ride. |
@tomalec ,
I don't know either. From what I can see, we are not doing SSR.
FYI we are using some of that, an example in http://localhost:8888/wp-content/plugins/google-listings-and-ads/js/build/index.css?ver=1606408694 :root {
--wp-admin-theme-color: #007cba;
--wp-admin-theme-color-darker-10: #006ba1;
--wp-admin-theme-color-darker-20: #005a87; } which is generated from https://github.com/WordPress/gutenberg/blob/master/packages/base-styles/_default-custom-properties.scss. |
As per this demo PR #76, because there are many existing things in SCSS that we can't easily port over to JavaScript, we will be putting CSS-in-JS / Emotion library aside and stick to using SCSS way for now. |
Changes proposed in this Pull Request:
Closes #65 .
This PR looks into using emotion library to do CSS in JS. Emotion is used in gutenberg.
Some findings from this research:
@emotion/css
is framework-agnostic, you can use it in HTML with vanilla JS, no react required.@emotion/react
is react specific.@emotion/styled
is similar to styled-components library. It has a peer dependency on@emotion/react
, so you have to manually install@emotion/react
even though you are not using it explicitly.@emotion/react
and@emotion/css
both have acss
function but they are not the same! This gave me a gotcha headache. This is briefly mentioned here: https://emotion.sh/docs/package-summary@emotion/react
for react project, however I find it generate more noise and more tedious because you either have to modify babel config or put in extra JSX Pragma code at the top of the your JS files. More info here: https://emotion.sh/docs/css-prop . Because of this, I personally do not favor@emotion/react
.@emotion/styled
to create styled components. An example here: 239d1a3 . I feel this is ok, but my experience tells me we might end up having a lot ofStyledXyz
component and can be quite a bit of noise and mentally taxing to comprehend and navigate.@emotion/css
library here in this PR. Essentially it generates unique class names that we can use, without having to worry about class name conflict. You can see it's pretty much similar with previous SCSS approach. I think this is a more balanced approach and more welcoming to people who are not familiar.motivation-text
,title
,description
etc) to use thecss
function to auto-generate unique class names, so that they wouldn't be affected by CSS from other places (e.g. defined and clashed in global style sheet). I leave them out for now.Detailed test instructions:
npm install
andnpm run start
.Changelog Note:
Use emotion library for styling and CSS.