✨ : Animation controller now has ability to repeat animation 'n' no. of times. #150764

aliasgar4558 · 2024-06-25T08:45:35Z

Right now animation controller does not have any way by which user can repeat animation with specific no. of times. Adding the changes in existing repeat method in which users can specific no. of times they want to repeat the animation. If not specified, it would simulate infinitely. [Existing repeat behaviour]

This PR fixes : #53262

Pre-launch Checklist

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I read the [Tree Hygiene] wiki page, which explains my responsibilities.
I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
I signed the [CLA].
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is [test-exempt].
I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on [Discord].

nate-thegrate

Thanks for contributing; overall I think this is a fantastic addition to the .repeat() API!

I decided to give a lot of feedback, so please take a look when you have a chance :)

packages/flutter/lib/src/animation/animation_controller.dart

nate-thegrate · 2024-06-25T23:26:03Z

packages/flutter/test/animation/animation_controller_test.dart

+      },
+    );
+
+    test(


Thanks for adding these tests, they look great!

packages/flutter/lib/src/animation/animation_controller.dart

aliasgar4558 · 2024-06-28T12:00:19Z

@nate-thegrate - Thanks for the review. I am glad you found this changes a good add to the framework. I will update your review comments in some time today.

nate-thegrate

Thanks for making those updates!

I don't think rounding is a good idea here, since it doesn't make things any more simple under the hood:

base ten	binary
5	101
256	10000000
0.125	0.001
0.1	0.0001100110011001100110011001100…

(See this thread for a few more details.)

Once we streamline the logic here, I think we'll be good to go!

packages/flutter/lib/src/animation/animation_controller.dart

nate-thegrate

One small suggestion; otherwise LGTM 👍

Thanks for putting in the work here!

packages/flutter/lib/src/animation/animation_controller.dart

nate-thegrate · 2024-07-03T00:09:14Z

Hi @aliasgar4558, thought I'd give a quick heads-up:

This PR will be merged once we get a second review. No guarantees as to when that will happen, but it'll probably be within a couple of days!

I also see several merge commits here. According to the Tree Hygeine docs, it's better to do an "update with rebase" instead.
(And unless tests are flaking, it might not be worth it to kick off all the checks again.)

aliasgar4558 · 2024-07-03T02:38:10Z

@nate-thegrate - Okay sure, No worries.

Moreover, I didn't get you when you say update with rebase. Are you suggesting that we should not update this branch with parent very oftenly now? And when we initiate second review, we can do that?

nate-thegrate · 2024-07-03T17:13:54Z

Yeah, generally, repeated merge commits might not be extremely helpful, and according to the wiki, things are more likely to go well when you do an "update with rebase".

nate-thegrate

Hey Aliasgar, sorry to pull a switcheroo after sending an approval, but I've come across some important information:

There's a popular independent package on pub.dev called flutter_animate that already implemented an AnimationController extension method with a "repeat count" option.

I like the way they chose to implement it, and I think we should tweak this PR to match for ease of migration.

packages/flutter/lib/src/animation/animation_controller.dart

packages/flutter/test/animation/animation_controller_test.dart

aliasgar4558 · 2024-07-17T04:36:30Z

@nate-thegrate Updated code, documentation & tests as per review comments.

nate-thegrate

Looks fantastic, thank you so much!

aliasgar4558 · 2024-07-17T04:52:41Z

Thanks @nate-thegrate for the valuable insights & suggestions.

nate-thegrate · 2024-07-17T18:37:34Z

There's a decent chance that rebasing with the latest updates to master could fix the failing check.

(Pushing an update is great as a workaround for flaky tests, but FYI I get an email each time it happens 🙃)

aliasgar4558 · 2024-07-17T18:41:19Z

So you recommend not to rebase very often? 😅
And shall I rebase with master branch now to get the latest changes??

Btw, it's a long thread of emails sorry 😅😅

nate-thegrate · 2024-07-17T18:46:29Z

So you recommend not to rebase very often?
And shall I rebase with master branch now to get the latest changes??

Yes sir!

Btw, it's a long thread of emails sorry

No worries! 🤠👍

…'n' no. of times. (flutter/flutter#150764)

aliasgar4558 · 2024-08-01T02:38:02Z

@nate-thegrate @chunhtai - thanks 👍🏼

…'n' no. of times. (flutter/flutter#150764)

Manual roll Flutter from 85960d2 to 4ff9462 (45 revisions) Manual roll requested by [email protected] flutter/flutter@85960d2...4ff9462 2024-08-01 [email protected] Fix local testing, gradle XML errors, and enable on CI. (flutter/flutter#152383) 2024-08-01 [email protected] Fix formatting issues in `search_anchor.0_test.dart` (flutter/flutter#152669) 2024-08-01 [email protected] Add tests for search anchor examples (flutter/flutter#152659) 2024-08-01 [email protected] Roll Flutter Engine from 4dc94d6f88ba to 7c4a44611abe (1 revision) (flutter/flutter#152665) 2024-08-01 [email protected] Roll Flutter Engine from f546fef7d7cd to 4dc94d6f88ba (1 revision) (flutter/flutter#152663) 2024-08-01 [email protected] Roll Flutter Engine from 0fbff219c498 to f546fef7d7cd (2 revisions) (flutter/flutter#152661) 2024-08-01 [email protected] Roll Flutter Engine from 32f788823f43 to 0fbff219c498 (5 revisions) (flutter/flutter#152658) 2024-08-01 [email protected] Manual roll Flutter Engine from 32f788823f43 to ed95b491f260 (3 revisions) (flutter/flutter#152654) 2024-08-01 [email protected] Manual roll Flutter Engine from 16332725788c to 32f788823f43 (11 revisions) (flutter/flutter#152648) 2024-07-31 [email protected] [CupertinoActionSheet] Make `_ActionSheetButtonBackground` stateless (flutter/flutter#152283) 2024-07-31 [email protected] Implementing null-aware logic in `/packages/flutter/` (flutter/flutter#152294) 2024-07-31 [email protected] Reintroduce verbose logging for hot reload flake (flutter/flutter#152639) 2024-07-31 [email protected] [material/menu_anchor.dart] Remove unused early key event listener (flutter/flutter#150915) 2024-07-31 [email protected] Improve `CupertinoCheckbox` fidelity (flutter/flutter#151441) 2024-07-31 [email protected] Update docs to support new Android version (flutter/flutter#152503) 2024-07-31 [email protected] [Android] Update integration test AVD dependency to use Android 35 emulators (flutter/flutter#152498) 2024-07-31 [email protected] Shift + click gesture support for SelectionArea on desktop platforms (flutter/flutter#148574) 2024-07-31 [email protected] Add ability to clip `Stepper` step content (flutter/flutter#152370) 2024-07-31 [email protected] Calendar font factor (flutter/flutter#152341) 2024-07-31 [email protected] Remove redundant usages of zones in skia_client.dart (flutter/flutter#149366) 2024-07-31 [email protected] Set up tests that verify we can build a fresh counter app across our Gradle/AGP/Kotlin support range (flutter/flutter#151568) 2024-07-31 [email protected] remove bringup from Windows tool_integration_tests_* (flutter/flutter#152599) 2024-07-31 [email protected] â�¨ : Animation controller now has ability to repeat animation 'n' no. of times. (flutter/flutter#150764) 2024-07-31 [email protected] Roll Flutter Engine from 3b31b21599d1 to 16332725788c (1 revision) (flutter/flutter#152631) 2024-07-31 [email protected] Roll Flutter Engine from b73367a30e9b to 3b31b21599d1 (8 revisions) (flutter/flutter#152625) 2024-07-31 [email protected] Roll Packages from 99e8606 to 46a712f (8 revisions) (flutter/flutter#152622) 2024-07-31 [email protected] Add tests for scaffold messenger examples (flutter/flutter#152536) 2024-07-31 [email protected] Add test for search_anchor.0.dart (flutter/flutter#152371) 2024-07-31 [email protected] Use decoration hint text as the default value for dropdown button hints (flutter/flutter#152474) 2024-07-31 [email protected] Deprecate invalid InputDecoration.collapsed parameters (flutter/flutter#152486) 2024-07-31 [email protected] increase sharding on Windows tool_integration_tests (flutter/flutter#152582) 2024-07-31 [email protected] Roll Flutter Engine from e2ece7e58480 to b73367a30e9b (4 revisions) (flutter/flutter#152592) 2024-07-31 [email protected] Roll Flutter Engine from 0b42657a184e to e2ece7e58480 (2 revisions) (flutter/flutter#152589) 2024-07-31 [email protected] Roll Flutter Engine from 08f9be3ab284 to 0b42657a184e (2 revisions) (flutter/flutter#152586) 2024-07-30 [email protected] Clarify and cleanup the test-exemption wording in tree-hygiene. (flutter/flutter#152402) 2024-07-30 [email protected] [web] Set COEP:credentialless on flutter run/drive. (flutter/flutter#152413) 2024-07-30 [email protected] Roll Flutter Engine from a4b88a37d511 to 08f9be3ab284 (5 revisions) (flutter/flutter#152583) 2024-07-30 [email protected] Marks Mac platform_channel_sample_test_macos to be flaky (flutter/flutter#151884) 2024-07-30 [email protected] Roll Flutter Engine from a6c5ff26c266 to a4b88a37d511 (2 revisions) (flutter/flutter#152575) 2024-07-30 [email protected] Reland #151599 (Add button semantics in list tile ) with a flag to control behavior. (flutter/flutter#152526) 2024-07-30 [email protected] Shift macOS/Android tests from Pixel 7 to mokey in staging (flutter/flutter#152571) 2024-07-30 [email protected] Roll Flutter Engine from 31bb9f98472a to a6c5ff26c266 (5 revisions) (flutter/flutter#152573) 2024-07-30 [email protected] Roll Packages from 247fb5f to 99e8606 (5 revisions) (flutter/flutter#152567) 2024-07-30 [email protected] Fix default avatar icon theme size for Material 2 (flutter/flutter#152307) ...

…of times. (flutter#150764) Right now animation controller does not have any way by which user can repeat animation with specific no. of times. Adding the changes in existing `repeat` method in which users can specific no. of times they want to repeat the animation. If not specified, it would simulate infinitely. [Existing repeat behaviour] This PR fixes : flutter#53262

…'n' no. of times. (flutter/flutter#150764)

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. a: animation Animation APIs labels Jun 25, 2024

goderbauer requested a review from nate-thegrate June 25, 2024 22:05

nate-thegrate reviewed Jun 25, 2024

View reviewed changes

nate-thegrate reviewed Jun 28, 2024

View reviewed changes

packages/flutter/lib/src/animation/animation_controller.dart Outdated Show resolved Hide resolved

packages/flutter/lib/src/animation/animation_controller.dart Outdated Show resolved Hide resolved

packages/flutter/lib/src/animation/animation_controller.dart Outdated Show resolved Hide resolved

nate-thegrate approved these changes Jun 29, 2024

View reviewed changes

packages/flutter/lib/src/animation/animation_controller.dart Outdated Show resolved Hide resolved

packages/flutter/lib/src/animation/animation_controller.dart Show resolved Hide resolved

aliasgar4558 force-pushed the Code/Improvement/repeatTimes_animation branch 6 times, most recently from 0b38c5f to 28c0ccb Compare July 11, 2024 01:51

nate-thegrate suggested changes Jul 11, 2024

View reviewed changes

packages/flutter/lib/src/animation/animation_controller.dart Outdated Show resolved Hide resolved

packages/flutter/lib/src/animation/animation_controller.dart Outdated Show resolved Hide resolved

packages/flutter/test/animation/animation_controller_test.dart Outdated Show resolved Hide resolved

aliasgar4558 force-pushed the Code/Improvement/repeatTimes_animation branch 3 times, most recently from a7d1efd to e42d9e8 Compare July 17, 2024 04:20

nate-thegrate approved these changes Jul 17, 2024

View reviewed changes

aliasgar4558 force-pushed the Code/Improvement/repeatTimes_animation branch 3 times, most recently from 01e1656 to ec2e6b4 Compare July 17, 2024 16:03

aliasgar4558 force-pushed the Code/Improvement/repeatTimes_animation branch from ec2e6b4 to 5f3c8da Compare July 17, 2024 18:47

engine-flutter-autoroll mentioned this pull request Jul 31, 2024

Roll Flutter from 031dc3d156b9 to 836e794b466a (72 revisions) flutter/packages#7265

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 31, 2024

836e794 ✨ : Animation controller now has ability to repeat animation …

0daf0ee

…'n' no. of times. (flutter/flutter#150764)

aliasgar4558 deleted the Code/Improvement/repeatTimes_animation branch August 1, 2024 02:37

engine-flutter-autoroll mentioned this pull request Aug 1, 2024

Roll Flutter from f817e51a3a34 to 7f4f1a0a6237 (74 revisions) flutter/packages#7275

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 1, 2024

836e794 ✨ : Animation controller now has ability to repeat animation …

b9e50c5

…'n' no. of times. (flutter/flutter#150764)

engine-flutter-autoroll mentioned this pull request Aug 1, 2024

Roll Flutter from f817e51a3a34 to 4ff9462be814 (75 revisions) flutter/packages#7277

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 1, 2024

836e794 ✨ : Animation controller now has ability to repeat animation …

63e3191

…'n' no. of times. (flutter/flutter#150764)

engine-flutter-autoroll mentioned this pull request Aug 1, 2024

Roll Flutter from f817e51a3a34 to 735958ef9a56 (76 revisions) flutter/packages#7279

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 1, 2024

836e794 ✨ : Animation controller now has ability to repeat animation …

84da653

…'n' no. of times. (flutter/flutter#150764)

engine-flutter-autoroll mentioned this pull request Aug 1, 2024

Roll Flutter from f817e51a3a34 to 5edec61f0e6d (77 revisions) flutter/packages#7281

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 1, 2024

836e794 ✨ : Animation controller now has ability to repeat animation …

4638864

…'n' no. of times. (flutter/flutter#150764)

engine-flutter-autoroll mentioned this pull request Aug 1, 2024

Roll Flutter from f817e51a3a34 to 619961d96cb1 (78 revisions) flutter/packages#7283

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 1, 2024

836e794 ✨ : Animation controller now has ability to repeat animation …

c587332

…'n' no. of times. (flutter/flutter#150764)

engine-flutter-autoroll mentioned this pull request Aug 2, 2024

Roll Flutter from e2e43987c4fb to 8818e60a653b (89 revisions) flutter/packages#7290

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 2, 2024

836e794 ✨ : Animation controller now has ability to repeat animation …

24358a0

…'n' no. of times. (flutter/flutter#150764)

engine-flutter-autoroll mentioned this pull request Aug 2, 2024

Roll Flutter from e2e43987c4fb to 9cc5d7f44638 (91 revisions) flutter/packages#7291

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 2, 2024

836e794 ✨ : Animation controller now has ability to repeat animation …

1ba5146

…'n' no. of times. (flutter/flutter#150764)

engine-flutter-autoroll mentioned this pull request Aug 2, 2024

Roll Flutter from e2e43987c4fb to f10a497e4abd (92 revisions) flutter/packages#7292

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 2, 2024

836e794 ✨ : Animation controller now has ability to repeat animation …

a816985

…'n' no. of times. (flutter/flutter#150764)

engine-flutter-autoroll mentioned this pull request Aug 2, 2024

Roll Flutter from e2e43987c4fb to 6206d8d73e35 (93 revisions) flutter/packages#7293

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 2, 2024

836e794 ✨ : Animation controller now has ability to repeat animation …

f058f7e

…'n' no. of times. (flutter/flutter#150764)

engine-flutter-autoroll mentioned this pull request Aug 3, 2024

Manual roll Flutter from 85960d24b9d2 to 4ff9462be814 (45 revisions) flutter/packages#7296

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 3, 2024

836e794 ✨ : Animation controller now has ability to repeat animation …

3c66083

…'n' no. of times. (flutter/flutter#150764)

Piinks mentioned this pull request Aug 9, 2024

Flutter Framework Team: Notable Commits #121415

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2024

836e794 ✨ : Animation controller now has ability to repeat animation …

f7dc75d

…'n' no. of times. (flutter/flutter#150764)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 12, 2024

836e794 ✨ : Animation controller now has ability to repeat animation …

97772d7

…'n' no. of times. (flutter/flutter#150764)

+                    },
+                  );
+                  test(

✨ : Animation controller now has ability to repeat animation 'n' no. of times. #150764

✨ : Animation controller now has ability to repeat animation 'n' no. of times. #150764

Uh oh!

Conversation

aliasgar4558 commented Jun 25, 2024

Pre-launch Checklist

Uh oh!

nate-thegrate left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nate-thegrate Jun 25, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

aliasgar4558 commented Jun 28, 2024

Uh oh!

nate-thegrate left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nate-thegrate left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nate-thegrate commented Jul 3, 2024

Uh oh!

aliasgar4558 commented Jul 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nate-thegrate commented Jul 3, 2024

Uh oh!

nate-thegrate left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aliasgar4558 commented Jul 17, 2024

Uh oh!

nate-thegrate left a comment

Choose a reason for hiding this comment

Uh oh!

aliasgar4558 commented Jul 17, 2024

Uh oh!

nate-thegrate commented Jul 17, 2024

Uh oh!

aliasgar4558 commented Jul 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nate-thegrate commented Jul 17, 2024

Uh oh!

aliasgar4558 commented Aug 1, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

aliasgar4558 commented Jul 3, 2024 •

edited

Loading

aliasgar4558 commented Jul 17, 2024 •

edited

Loading