-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Tip: Use CSS-in-JS #26366
Tip: Use CSS-in-JS #26366
Conversation
9b9ddca
to
951a081
Compare
packages/components/src/tip/index.js
Outdated
/** | ||
* External dependencies | ||
*/ | ||
import { jsx } from '@emotion/core'; |
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.
@saramarcondes Haiii!!!
Interesting idea! From my experience (in figuring out things in G2...), I personally would avoid the jsx pragma
based approach. Theme UI offers a similar API.
In practice, I found it to be pretty clunky (and slightly jarring if you're unfamiliar with the workflow).
I believe I understand why this was attempted ❤️ .
My guess was to avoid using styled
:) - which has a great dev workflow, with a slight trade-off in performance.
I would recommend continuing to use styled
as your continuing your CSS-in-JS journey within Gutenberg.
It would be consistent with the current (but limited) implementation.
I think introducing the JSX pragma may cause some fragmentation in tidying and refactoring things.
It looks like the E2E tests are unhappy with TypeScript things 😱 |
Fixing the tests are blocked by #26078 |
This avoids issues with not being able to successfully run a typecheck on the styles dependencies
b67e346
to
1c9d31b
Compare
@saramarcondes Thank you following up and adjusting the implementation ( With that, the it's a 👍 from me. However, it looks like the tests are unhappy. Hopefully #26078 can help resolve it! I left my comments in that PR. My TypeScript knowledge isn't good enough to thoughtfully and accurately go over those changes 😅 . |
Closing in favor of focusing on G2-components integration. |
Description
This PR updates the
Tip
component to use CSS-in-JS similar to how newer components do like theAnglePickerControl
andText
.How has this been tested?
I've confirmed there are no visual or behavioral differences between this branch and upstream in storybook.
Screenshots
Before:
After:
Types of changes
Non-breaking style change. The classname is preserved so it is still possible to target the
components-tip
class.Checklist: