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

Can reverseCurve support be added on CurvedAnimation? #95

Open
fingerart opened this issue Jun 29, 2023 · 9 comments
Open

Can reverseCurve support be added on CurvedAnimation? #95

fingerart opened this issue Jun 29, 2023 · 9 comments
Labels
enhancement New feature or request flutter issue This issue is dependent on issues in Flutter or Dart

Comments

@fingerart
Copy link

such as:

.slideX(
    curve: Curves.bounceIn,
    reverseCurve: Curves.bounceOut,
)
@gskinner
Copy link
Owner

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
Copy link
Author

fingerart commented Jul 26, 2023

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: flutter/flutter#129776

@fingerart
Copy link
Author

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

@gskinner gskinner added enhancement New feature or request flutter issue This issue is dependent on issues in Flutter or Dart labels Jul 26, 2023
@gskinner
Copy link
Owner

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
Copy link
Owner

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
Copy link

shtse8 commented Nov 19, 2023

@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
Copy link
Owner

gskinner commented Nov 19, 2023

@shtse8 - per this comment:
#95 (comment)

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
Copy link

shtse8 commented Nov 20, 2023

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request flutter issue This issue is dependent on issues in Flutter or Dart
Projects
None yet
Development

No branches or pull requests

3 participants