Skip to content
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

Merged
merged 7 commits into from
Jan 22, 2023

Conversation

hasparus
Copy link
Member

@hasparus hasparus commented Jan 22, 2023

I've added comments below ⬇️

@vercel
Copy link

vercel bot commented Jan 22, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
theme-ui ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Jan 22, 2023 at 5:57PM (UTC)

return (
<header>
<Button
suppressHydrationWarning
Copy link
Member Author

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;}
Copy link
Member Author

@hasparus hasparus Jan 22, 2023

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:

  1. This class name shouldn't be there.
  2. data-emotion should be css-global, not css.
  3. Wait up, original Global uses styles instead of css prop. Eureka moment.
  4. We have JSX pragma configured in our tests.
  5. sx becomes css, which becomes className

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.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jan 22, 2023

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:

Sandbox Source
next-theme-ui-example Configuration
gatsby-plugin-theme-ui-example Configuration

@hasparus hasparus merged commit ac36c53 into 2207-global-component Jan 22, 2023
@hasparus hasparus deleted the 2207-global-component-hasparus branch January 22, 2023 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant