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.

Sign up for GitHub

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

Jump to bottom

feat: add GlobalStyles via theme #1230

Merged
merged 10 commits into from
Sep 8, 2021
Merged

feat: add GlobalStyles via theme #1230

merged 10 commits into from
Sep 8, 2021

Conversation

viktoria-schwarz
Copy link
Contributor

@changeset-bot
Copy link

changeset-bot bot commented Aug 23, 2021 •
edited
Loading

🦋 Changeset detected

Latest commit: a22bfd4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
docs Patch
@marigold/components Patch
@marigold/system Patch
@marigold/theme-b2b Patch
@marigold/theme-unicorn Patch
@marigold/icons Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the type:feature New feature or component label Aug 23, 2021
@viktoria-schwarz viktoria-schwarz changed the title feat(styles): first version GlobalStyles wip: first version GlobalStyles Aug 23, 2021
@coveralls
Copy link

coveralls commented Aug 23, 2021 •
edited
Loading

Coverage Status

Coverage remained the same at 100.0% when pulling a22bfd4 on rootstyles into afd5085 on main.

@viktoria-schwarz
Copy link
Contributor Author

@sebald with a wrapping <Box> I could make it work. Or should I try to use emotion's Global? I tried to use it but couldn't access the theme (theme.styles.root) in a proper way... what do you think?

@viktoria-schwarz viktoria-schwarz changed the title wip: first version GlobalStyles feat: add GlobalStyles via theme Aug 24, 2021
@github-actions github-actions bot added the type:docs Improvements or additions to documentation label Aug 24, 2021
@sebald
Copy link
Member

sebald commented Aug 24, 2021

I would use <Global> since it does some extra stuff and optimization.

But I works not expose anything of that and keep it private to the <MarigoldProvider>.

And one more thing: since we don't do the styles thing, this is rather a theme-ui thing, use put "root" on the root theme object? 😄

@viktoria-schwarz
Copy link
Contributor Author

In the <body> the styles work but not in the <html> when injected via the root in the theme. Not sure if I'm missing something 🤔

@sebald
Copy link
Member

sebald commented Aug 24, 2021

@viktoria-schwarz I just took a quick look, but I think you copy/pasted something out shouldn't have 🤪

@viktoria-schwarz viktoria-schwarz marked this pull request as ready for review August 25, 2021 10:15
@ti10le ti10le merged commit ebd6e26 into main Sep 8, 2021
@ti10le ti10le deleted the rootstyles branch September 8, 2021 08:58
@viktoria-schwarz viktoria-schwarz mentioned this pull request Sep 8, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sebald sebald sebald left review comments

@ti10le ti10le ti10le approved these changes

Assignees
No one assigned
Labels
type:docs Improvements or additions to documentation type:feature New feature or component
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@viktoria-schwarz @coveralls @sebald @ti10le