gskinner / flutter_animate

Add beautiful animated effects & builders in Flutter, via an easy, highly customizable unified API.
BSD 3-Clause "New" or "Revised" License
938 stars 80 forks source link

Can reverseCurve support be added on CurvedAnimation? #95

Open fingerart opened 1 year ago

fingerart commented 1 year ago

such as:

.slideX(
    curve: Curves.bounceIn,
    reverseCurve: Curves.bounceOut,
)
gskinner commented 1 year ago

Thanks for the PR. I did a code review and it looks good. I'm going to do some tests to make sure it all works as expected, and then plan to accept it.

fingerart commented 1 year ago

Currently in the latest version of flutter, using controller.repeat(reverse: true) will not execute reverseCurve correctly, which is an internal bug of flutter. I think flutter_animate support reverseCurve is not affected. For details, see: https://github.com/flutter/flutter/issues/129776

fingerart commented 1 year ago

In the case of unfixed, use controller.forward and controller.reverse respectively to make reverseCurve take effect.

gskinner commented 1 year ago

I ran into that immediately on testing this, but I think it may be the expected behavior per the docs:

If the parent animation changes direction without first reaching the
[AnimationStatus.completed] or [AnimationStatus.dismissed] status, the
[CurvedAnimation] stays on the same curve (albeit in the opposite
direction) to avoid visual discontinuities.

If so, it severely limits the usefulness of this parameter because it only applies when you manually call reverse() on the AnimationController. At that point, you may as well just adjust the curve parameter directly.

Another quick note on the PR — ListenEffect manually applies curve. I think it is the only effect that does this, but it would need to be modified to use an animation directly in order to maintain parity with the behavior described above.

My gut on this is to defer accepting the PR until the Flutter issue is resolved one way or the other, but I'm open to feedback.

gskinner commented 1 year ago

As an aside, if you're able to share your specific use-case for this parameter (bonus points for visuals), that would also help me suss out the value proposition.

Also — thanks for the issue and PR!

shtse8 commented 11 months ago

@gskinner I also require this parameter. In game development, it's common to create animations that start quickly and end slowly, and this applies to both showing and hiding elements. However, using the same curve for the reverse process results in an element hiding slowly at the start and quickly at the end. This can negatively impact the user experience, as the transition appears excessively slow when an element starts to hide.

gskinner commented 11 months ago

@shtse8 - per this comment: https://github.com/gskinner/flutter_animate/issues/95#issuecomment-1652180362

I'm trying to weigh the added maintenance against the value given that the reverseCurve is only applied if you are manually calling reverse(), at which point you could conceivably just rebuild the animation with a different curve.

This param is easily implemented, but it has to be added (and maintained) across ~100 different places. I'm also a bit concerned that the inherent limitations in Flutter (described above) will cause confusion. And of course, it's always much easier to add to an API than remove later.

Feedback is very welcome. Thanks!

shtse8 commented 11 months ago

Thank you for your insights on the matter. You're correct that we can manage the reverse animations by rebuilding them with a different curve. However, this approach means we lose the convenience of the default settings and will have to manually adjust each animation individually throughout the entire project.

fingerart commented 11 months ago

Flutter's built-in CurvedAnimation already meets some behaviors of many existing components. I cannot directly modify this class, which will cause inconsistent behavior of other components. Therefore, it is recommended to copy a copy and put it in flutter_animate, and then modify it.

https://github.com/fingerart/flutter/blob/8f1a0f8f48a93c48a66ffe95a3eef74605284d5d/packages/flutter/lib/src/animation/animations.dart#L427-L430