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 createGlobalStyles + keyframes #2029

Merged
merged 2 commits into from Sep 24, 2018

Conversation

Projects
None yet
4 participants
@probablyup
Contributor

probablyup commented Sep 24, 2018

I'm not sure if this is the right solution. Ideally we wouldn't run the css helper until render time but since we're hashing the completed rules I'm not sure if that can be changed. It wasn't. But the PR has been updated to the correct solution.

Fixes #2027

@probablyup probablyup requested a review from kitten Sep 24, 2018

@probablyup probablyup force-pushed the fix-cgs-keyframes branch from 3abecc7 to aff8a76 Sep 24, 2018

@probablyup probablyup requested a review from imbhargav5 Sep 24, 2018

@imbhargav5

This comment has been minimized.

Member

imbhargav5 commented Sep 24, 2018

@probablyup Flow being flow. LOL.

@probablyup

This comment has been minimized.

Contributor

probablyup commented Sep 24, 2018

I’m going to delete it. Drives me friggin crazy. Separate PR though

@imbhargav5

This comment has been minimized.

Member

imbhargav5 commented Sep 24, 2018

Looks good to me. Perhaps we can add one test case where animation is added within interpolation just to be doubly sure?

${props => props.animate ?  `animation: ${keyframeAnimation1} ease 1s` :  `` }
@Fer0x

This comment has been minimized.

Contributor

Fer0x commented Sep 24, 2018

Offtopic: should we add some warning when someone tries to use keyframes within react-native?

@mxstbr

This comment has been minimized.

Member

mxstbr commented Sep 24, 2018

@Fer0x yeah that seems smart! People have been confused about that. Wanna PR it?

@probablyup

This comment has been minimized.

Contributor

probablyup commented Sep 24, 2018

@imbhargav5 that example actually wouldn't work because of the lack of css helper haha.

@imbhargav5

This comment has been minimized.

Member

imbhargav5 commented Sep 24, 2018

Alright then, I think we can merge this if all is good?

@imbhargav5

LGTM!

@mxstbr

mxstbr approved these changes Sep 24, 2018

LGTM what a simple fix

@probablyup probablyup merged commit a0549a9 into develop Sep 24, 2018

2 checks passed

bundlesize ./dist/styled-components.min.js: 14.74KB < maxSize 16KB (gzip)(1.52KB smaller than master, good job!)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@probablyup probablyup deleted the fix-cgs-keyframes branch Sep 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment