Button: Use CSS-in-JS #26340
Button: Use CSS-in-JS #26340
Conversation
| const getBaseStyles = ( props ) => { | ||
| if ( props.isPrimary ) { | ||
| return buttonStyles.primary.styles; | ||
| } | ||
|
|
||
| if ( props.isSecondary ) { | ||
| return buttonStyles.secondary.styles; | ||
| } | ||
|
|
||
| if ( props.isTertiary ) { | ||
| return buttonStyles.tertiary.styles; | ||
| } | ||
|
|
||
| if ( props.isLink ) { | ||
| if ( props.isDestructive ) { | ||
| return css` | ||
| ${ buttonStyles.link.styles } | ||
| ${ buttonStyles.link.destructive } | ||
| `; | ||
| } | ||
| return buttonStyles.link.styles; | ||
| } | ||
|
|
||
| // check pure destructive last to allow for destructive links | ||
| if ( props.isDestructive ) { | ||
| return buttonStyles.destructive.styles; | ||
| } | ||
|
|
||
| return buttonStyles.shared.buttonBase; | ||
| }; | ||
|
|
||
| const getBusyStyles = ( props ) => { | ||
| if ( ! props.isBusy ) { | ||
| return ''; | ||
| } | ||
|
|
||
| if ( props.isPrimary ) { | ||
| return buttonStyles.busy.primary; | ||
| } | ||
|
|
||
| return buttonStyles.busy.generic; | ||
| }; | ||
|
|
||
| const getPressedStyles = ( props ) => { | ||
| if ( props.isPressed ) { | ||
| return buttonStyles.shared.pressed; | ||
| } | ||
|
|
||
| return ''; | ||
| }; | ||
|
|
||
| const getSmallStyles = ( props, hasIcon, hasText ) => { | ||
| if ( props.isSmall ) { | ||
| if ( hasIcon && ! hasText ) { | ||
| return buttonStyles.shared.smallOnlyIcon; | ||
| } | ||
|
|
||
| return buttonStyles.shared.small; | ||
| } | ||
|
|
||
| return ''; | ||
| }; | ||
|
|
||
| const getIconStyles = ( hasIcon, hasText ) => { | ||
| if ( hasIcon ) { | ||
| if ( hasText ) { | ||
| return buttonStyles.shared.iconWithText; | ||
| } | ||
|
|
||
| return buttonStyles.shared.icon; | ||
| } | ||
|
|
||
| return ''; | ||
| }; |
There was a problem hiding this comment.
I don't love this whole slew of functions. Any ideas for how to improve this or different approaches that could be taken to accomplish the same thing would be very welcome.
| 200: '#ddd', // Used for most borders. | ||
| 300: '#ddd', // Used for most borders. | ||
| 200: '#e0e0e0', |
There was a problem hiding this comment.
I'm not sure about this change. These colors originally did not correspond with the colors in the Sass variables. This change aligns them, but maybe they weren't meant to be aligned in the first place?
| @@ -169,6 +170,7 @@ export const COLORS = { | |||
| darkOpacity: DARK_OPACITY, | |||
| darkOpacityLight: DARK_OPACITY_LIGHT, | |||
| mediumGray: G2.mediumGray, | |||
| gray: G2.gray, | |||
There was a problem hiding this comment.
Also not sure if exposing gray in this way is desirable or if these are meant to be aliased with names.
|
Size Change: +2.47 kB (0%) Total Size: 1.2 MB
ℹ️ View Unchanged
|
|
@saramarcondes Haiii!! Oh wow! Tackling the This will be an interesting one. An important one as well! Besides Buttons being used everywhere (often with custom/ad-hoc adjustments)... Buttons are one of the more complex elements within a Component library/framework. This is because they have sooo many variations (e.g. size, variant, states, content (like icon), etc...), and all of these variations can be mixed together. This complexity is often mirrored in the style code, regardless of language (e.g. Sass, Less, JS, etc...). With all that being said... I think figuring out how to do this would help set the tone for how we can approach CSS-in-JS in the components package moving forward. That was something I've experienced in past UI framework projects, with the most recent one being G2 Components. I noticed this PR went with the JSX pragma approach. I left my thoughts on CSS-in-JS JSX pragma in one of your other PRs I'll copy and paste a part of my thoughts here to make this PR easier to follow: I would recommend continuing to use styled as your continuing your CSS-in-JS journey within Gutenberg. I think introducing the JSX pragma may cause some fragmentation in tidying and refactoring things. |
sarayourfriend
requested review from
ajitbohra,
chrisvanpatten and
ellatrix
as code owners
|
@saramarcondes Haii! I've added a "resolver" to hopefully fix some of the style conflicts between Emotion and the other styles, as it relates to specificity and ordering 🙏 . Please let me know if that code makes sense. Feel free to make any modification as needed! |
|
@saramarcondes Haii! I've made so more updates to the resolver. Hopefully this helps 🙏 📝 VariablesAnother thing that's worth noting is that the CSS-in-JS solution does not automatically support CSS variable fallbacks. It's understandable that Button is using quite a few variables, as it relates to the admin theme color. Previously, these fallback values were manually added. We really need to support a more automated way to do this. I wrote a library specifically to try and tackle this issue: Of course, if we want to use this (or any Emotion/Stylis plugin), we'll need to create a top/app level Provider. One that has a customized Emotion instance with the plugin setup. Something like this: |
|
@saramarcondes Since we're talking about this potential Provider more and more... I wonder if it may be a good idea to move the "magical" resolver solution to it. That way, it's a little bit more explicit in that, the |
|
@ItsJonQ I think that's a good idea. In the current state of things I don't think this PR is mergable given that it would interact with style tags introduced outside of wp/components, which seems like something a general component library should not do. |
|
Since we're exploring the idea for a StyleProvider... perhaps we can pick this up again: I had created one several months ago. We can add the new resolver in that :) |
|
Closing in favor of focusing on G2-components integration. |
Description
Use CSS-in-JS for the
Buttoncomponent.Note: This is just one approach of many that can be taken. I'm not attached to it in any way and dislike parts of it (see inline comments). Hopefully @ItsJonQ will have some guidance 😺
How has this been tested?
Confirmed there are no visual differences between this branch and the production storybook buttons. Also verified in Gutenberg itself that buttons continue to appear and work the same.
Screenshots
Before:
After:
Types of changes
Non-breaking CSS-in-JS change. This change preserves the existing classnames ensuring anyone targeting them will still be able to do so (for example, storybook itself).
Checklist: