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

[framer] Support Framer color tokens for ThemeProvider #19451

Merged
merged 5 commits into from
Feb 2, 2020

Conversation

iKettles
Copy link
Contributor

This PR adds support for using Framer color tokens with the ThemeProvider wrapper component.

@mui-pr-bot
Copy link

mui-pr-bot commented Jan 28, 2020

No bundle size changes comparing 5c84559...8eed1f4

Generated by 🚫 dangerJS against 8eed1f4

@mbrookes
Copy link
Member

mbrookes commented Jan 28, 2020

@iKettles This is great! Thanks for looking into it.

The theme provider is templated, but it doesn't really need to be as its API doesn't change, so perhaps you could delete the template, and remove the ThemeProvider section from framerConfig.js, along with the primary, secondary and error keys from additionalProps.js.

Also, while not currently widely used, you could perhaps add the new info, warning and success colors.

A brief comment describing the regex, "Extract the color value from a string such as [example string]" or whatever makes sense would be good, although hopefully it won't ever need to be changed.

Finally, if there were other theme values you think could be useful to add, feel free!

@iKettles
Copy link
Contributor Author

@iKettles This is great! Thanks for looking into it.

The theme provider is templated, but it doesn't really need to be as it its API doesn't change, so perhaps you could delete the template, and remove the ThemeProvider section from framerConfig.js, along with the primary, secondary and error keys from additionalProps.js.

Also, while not currently widely used, you could perhaps add the new info, warning and success colors.

A brief comment describing the regex, "Extract the color value from a string such as [example string]" or whatever makes sense would be good, although hopefully it won't ever need to be changed.

Finally, if there were other theme values you think could be useful to add, feel free!

No worries! I've documented the parseColor util and stripped ThemeProvider from the build process as well as removing the template. I had to upgrade @material-ui/core to access the new palette props (also bumped @material-ui/icons).

I'm working on a branch that introduces a property control which allows users to upload JSON for an existing theme, as well as being able to set the theme via an override. Should be useful for users re-using existing themes/pulling them from a generator. I'll do this in a separate PR as I've got some things to iron out first.

Regarding the failed argos check - is this a false negative? The height seems to be a couple of pixels off in the visual diff.

Thanks for reviewing so quickly!

@mbrookes
Copy link
Member

mbrookes commented Jan 29, 2020

I'm working on a branch that introduces a property control which allows users to upload JSON for an existing theme.

Ha that's funny! We've discussed doing that for the docs, and I literally just thought before seeing your comment that we could do the same for Framer. The the other big win from this would be component overrides.

Not sure whether it should be the same or an alternate component, but let's see how it turns out in your PR.

The other key that might be interesting to add to this PR is shape.borderRadius, but I'll leave that up to you. It can always be added later.

Regarding the failed argos check - is this a false negative? The height seems to be a couple of pixels off in the visual diff.

Yes. There's a separate PR to remove the failing test.

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.

4 participants