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

Enhanced enum features for AnimationStatus #147801

Merged
merged 13 commits into from
May 16, 2024

Conversation

nate-thegrate
Copy link
Member

@nate-thegrate nate-thegrate commented May 3, 2024

Based on issue #147799, this pull request adds two AnimationStatus getters.

bool get isRunning => switch (this) {
  forward   || reverse   => true,
  completed || dismissed => false,
};

bool get aimedForward => switch (this) {
  forward || completed => true,
  reverse || dismissed => false,
};

I also added a .toggle() method for animation controllers that makes use of aimedForward.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. a: animation Animation APIs labels May 3, 2024
@nate-thegrate nate-thegrate marked this pull request as ready for review May 3, 2024 20:12
Comment on lines +190 to +191
controller.dispose();

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that a controller was being reassigned here without a .dispose() call, so I added this as well.

@nate-thegrate nate-thegrate marked this pull request as draft May 15, 2024 02:11
@nate-thegrate nate-thegrate marked this pull request as ready for review May 15, 2024 05:38
packages/flutter/lib/src/animation/animation.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/animation/animation.dart Outdated Show resolved Hide resolved
///
/// Useful for toggling animations back and forth.
/// {@endtemplate}
bool get aimedForward => switch (this) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not super in love with this name. I wish I had a better suggestion...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

me too 🥲

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I asked gemini, it came up with the very explicit isForwardOrCompleted name. Still not great, I like it slightly better then aimedForward, though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with that!

/// The most recently returned [TickerFuture], if any, is marked as having been
/// canceled, meaning the future never completes and its [TickerFuture.orCancel]
/// derivative future completes with a [TickerCanceled] error.
TickerFuture toggle({ double? from }) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you document the parameter?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure can!

I copied the documentation over from the forward/reverse functions, so I'll document the parameter for those as well.

Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

///
/// Useful for toggling animations back and forth.
/// {@endtemplate}
bool get aimedForward => switch (this) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I asked gemini, it came up with the very explicit isForwardOrCompleted name. Still not great, I like it slightly better then aimedForward, though.

@nate-thegrate nate-thegrate added the autosubmit Merge PR when tree becomes green via auto submit App label May 16, 2024
@auto-submit auto-submit bot merged commit 6b38361 into flutter:master May 16, 2024
73 checks passed
@nate-thegrate nate-thegrate deleted the aimed-forward branch May 16, 2024 17:58
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 17, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 17, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request May 17, 2024
flutter/flutter@0d22d91...00425ef

2024-05-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from d575e511f9e5 to a19d3722922d (2 revisions) (flutter/flutter#148534)
2024-05-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from 8d1a1d8d7b48 to d575e511f9e5 (5 revisions) (flutter/flutter#148529)
2024-05-17 engine-flutter-autoroll@skia.org Manual roll Flutter Engine from 6fa734d68688 to 8d1a1d8d7b48 (8 revisions) (flutter/flutter#148528)
2024-05-17 32538273+ValentinVignal@users.noreply.github.com Add test for material_banner.0.dart and material_banner.1.dart (flutter/flutter#148452)
2024-05-17 nate.w5687@gmail.com `switch` statement cleanup (flutter/flutter#148382)
2024-05-16 goderbauer@google.com const vs. non-const widget build benchmark (flutter/flutter#148261)
2024-05-16 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 4.1.5 to 4.1.6 (flutter/flutter#148516)
2024-05-16 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Flutter Engine from 6fa734d68688 to 1850def2ccff (1 revision) (#148507)" (flutter/flutter#148517)
2024-05-16 stuartmorgan@google.com Add PrivacyInfo.xcprivacy to plugin template (flutter/flutter#148485)
2024-05-16 stuartmorgan@google.com Fix iOS reference in macOS Cocoapods error (flutter/flutter#148506)
2024-05-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 6fa734d68688 to 1850def2ccff (1 revision) (flutter/flutter#148507)
2024-05-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 460df6caef0e to 6fa734d68688 (4 revisions) (flutter/flutter#148500)
2024-05-16 nate.w5687@gmail.com Enhanced enum features for `AnimationStatus` (flutter/flutter#147801)
2024-05-16 matej.knopp@gmail.com [macOS] codesign native assets during embed (flutter/flutter#148310)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
auto-submit bot pushed a commit that referenced this pull request May 20, 2024
PR #147801 introduced some convenient `AnimationStatus` getters, but I just realized that `AnimationController` now has 2 getters for the same thing: `isAnimating` and `isRunning`.

The intent of this pull request is to correct that mistake, and implement the getters in the appropriate places.
TecHaxter pushed a commit to TecHaxter/flutter_packages that referenced this pull request May 22, 2024
…r#6753)

flutter/flutter@0d22d91...00425ef

2024-05-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from d575e511f9e5 to a19d3722922d (2 revisions) (flutter/flutter#148534)
2024-05-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from 8d1a1d8d7b48 to d575e511f9e5 (5 revisions) (flutter/flutter#148529)
2024-05-17 engine-flutter-autoroll@skia.org Manual roll Flutter Engine from 6fa734d68688 to 8d1a1d8d7b48 (8 revisions) (flutter/flutter#148528)
2024-05-17 32538273+ValentinVignal@users.noreply.github.com Add test for material_banner.0.dart and material_banner.1.dart (flutter/flutter#148452)
2024-05-17 nate.w5687@gmail.com `switch` statement cleanup (flutter/flutter#148382)
2024-05-16 goderbauer@google.com const vs. non-const widget build benchmark (flutter/flutter#148261)
2024-05-16 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 4.1.5 to 4.1.6 (flutter/flutter#148516)
2024-05-16 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Flutter Engine from 6fa734d68688 to 1850def2ccff (1 revision) (#148507)" (flutter/flutter#148517)
2024-05-16 stuartmorgan@google.com Add PrivacyInfo.xcprivacy to plugin template (flutter/flutter#148485)
2024-05-16 stuartmorgan@google.com Fix iOS reference in macOS Cocoapods error (flutter/flutter#148506)
2024-05-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 6fa734d68688 to 1850def2ccff (1 revision) (flutter/flutter#148507)
2024-05-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 460df6caef0e to 6fa734d68688 (4 revisions) (flutter/flutter#148500)
2024-05-16 nate.w5687@gmail.com Enhanced enum features for `AnimationStatus` (flutter/flutter#147801)
2024-05-16 matej.knopp@gmail.com [macOS] codesign native assets during embed (flutter/flutter#148310)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: animation Animation APIs autosubmit Merge PR when tree becomes green via auto submit App framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants