-
Notifications
You must be signed in to change notification settings - Fork 672
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
Fix tests for the Global component #2386
Fix tests for the Global component #2386
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
return ( | ||
<header> | ||
<Button | ||
suppressHydrationWarning |
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.
When I launched the Next example, I noticed the hydration warning popping up straight in your face. This component will cause it, because it's using useColorMode
without waiting until we're on the client, and we depend on the color scheme value in local storage. (We could render a skeleton for the button on the server instead, but that's not much better UX IMHO.)
data-s="" | ||
> | ||
|
||
.css-ck0s41 body{font-family:Georgia,serif;margin:0;} |
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.
I've read Emotion Global's code to figure it out:
- This class name shouldn't be there.
data-emotion
should becss-global
, notcss
.- Wait up, original Global uses
styles
instead ofcss
prop. Eureka moment. - We have JSX pragma configured in our tests.
sx
becomescss
, which becomesclassName
I've changed the name of the prop to styles
, because it will happen for the users. They're gonna use <Global>
alongside other JSX, in files where JSX goes through jsx
function from Theme UI.
Side effect of it is, we now have the same signature that Emotion Global has, just with different type of styles
, so the user can just change the import if they need to access their design tokens, and even change it back if they no longer need the theme there.
…not be named without a reference"
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 78c9019:
|
I've added comments below ⬇️