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

Forward element key when calculating height auto #250

Merged
merged 1 commit into from Oct 3, 2018

Conversation

Projects
None yet
2 participants
@einarlove
Contributor

einarlove commented Oct 3, 2018

Fixes some part of #249

@drcmda drcmda merged commit ca1e401 into drcmda:master Oct 3, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@einarlove

This comment has been minimized.

Show comment
Hide comment
@einarlove

einarlove Oct 4, 2018

Contributor

@drcmda Would it be possible to release a patch version to NPM? I'm quite dependent on this fix and I can't install from this commit directly via github:drcmda/react-spring#ca1e401ea1e33237042b8d2787f50f703ee37911 because your build script is dependent on YARN to work.

Contributor

einarlove commented Oct 4, 2018

@drcmda Would it be possible to release a patch version to NPM? I'm quite dependent on this fix and I can't install from this commit directly via github:drcmda/react-spring#ca1e401ea1e33237042b8d2787f50f703ee37911 because your build script is dependent on YARN to work.

@drcmda

This comment has been minimized.

Show comment
Hide comment
@drcmda

drcmda Oct 4, 2018

Owner

no problem, i am trying to get one last fix in

Owner

drcmda commented Oct 4, 2018

no problem, i am trying to get one last fix in

@drcmda

This comment has been minimized.

Show comment
Hide comment
@drcmda

drcmda Oct 4, 2018

Owner

@einarlove published @5.9.1-beta.1

It contains bugfixes for your transition case as well. https://codesandbox.io/embed/mmvk3p3xw8

Owner

drcmda commented Oct 4, 2018

@einarlove published @5.9.1-beta.1

It contains bugfixes for your transition case as well. https://codesandbox.io/embed/mmvk3p3xw8

@einarlove

This comment has been minimized.

Show comment
Hide comment
@einarlove

einarlove Oct 4, 2018

Contributor

Awesome. Tried it out and the forwarding key works as expected. You mentioned you fixed keyframes. Is that in 5.9.1-beta.1? Because I still see bugs with it.

Contributor

einarlove commented Oct 4, 2018

Awesome. Tried it out and the forwarding key works as expected. You mentioned you fixed keyframes. Is that in 5.9.1-beta.1? Because I still see bugs with it.

@einarlove

This comment has been minimized.

Show comment
Hide comment
@einarlove

einarlove Oct 4, 2018

Contributor

I recorded the inconsistent behavior of my Keyframes within a Transition which also animates height:auto.

The gif shows 4 examples on how the spinner around the button stops at random points. When used outside the Transition it correctly animates from 0 to 100% of the circle with Easing.linear

Contributor

einarlove commented Oct 4, 2018

I recorded the inconsistent behavior of my Keyframes within a Transition which also animates height:auto.

The gif shows 4 examples on how the spinner around the button stops at random points. When used outside the Transition it correctly animates from 0 to 100% of the circle with Easing.linear

@einarlove einarlove deleted the einarlove:patch-1 branch Oct 4, 2018

@drcmda

This comment has been minimized.

Show comment
Hide comment
@drcmda

drcmda Oct 4, 2018

Owner

I only looked at reacts transitiongroup for now, see the sandbox i posted. This one can run arrays of animations. I am still working on the other things you have posted in the issuetracker. Generic transition can only define a single animation for enter/leave/update. Would be interesting to allow arrays in general, but this will be a major refactoring of the entire structure - it won't a quick fix. TransitionGroup on the other hand is way more complex to use, but gives you more control.

If you want, post your component into another sandbox and i can look at it.

Owner

drcmda commented Oct 4, 2018

I only looked at reacts transitiongroup for now, see the sandbox i posted. This one can run arrays of animations. I am still working on the other things you have posted in the issuetracker. Generic transition can only define a single animation for enter/leave/update. Would be interesting to allow arrays in general, but this will be a major refactoring of the entire structure - it won't a quick fix. TransitionGroup on the other hand is way more complex to use, but gives you more control.

If you want, post your component into another sandbox and i can look at it.

@einarlove

This comment has been minimized.

Show comment
Hide comment
@einarlove

einarlove Oct 5, 2018

Contributor

I'll try to post an example where it fails. Might time some time since it's hard to decouple my components from my project atm.

—————

Example where it fails: #249 (comment)

Contributor

einarlove commented Oct 5, 2018

I'll try to post an example where it fails. Might time some time since it's hard to decouple my components from my project atm.

—————

Example where it fails: #249 (comment)

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