feat: add onEnd to AnimatedCrossFade#181455
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces an onEnd callback to the AnimatedCrossFade widget, which is triggered upon the completion of the cross-fade animation. The implementation correctly adds a status listener to the animation controller and invokes the callback when the animation is completed or dismissed. The changes are accompanied by a comprehensive set of tests that verify the new functionality in forward, reverse, and repeated animation scenarios. My feedback focuses on improving the readability and maintainability of the new tests by reducing code duplication, in line with the repository's style guide.
| testWidgets('AnimatedCrossFade onEnd callback is called when animation completes forward', ( | ||
| WidgetTester tester, | ||
| ) async { | ||
| int onEndCallCount = 0; | ||
|
|
||
| await tester.pumpWidget( | ||
| Directionality( | ||
| textDirection: TextDirection.ltr, | ||
| child: AnimatedCrossFade( | ||
| firstChild: const SizedBox(width: 100.0, height: 100.0), | ||
| secondChild: const SizedBox(width: 200.0, height: 200.0), | ||
| crossFadeState: CrossFadeState.showFirst, | ||
| duration: const Duration(milliseconds: 200), | ||
| onEnd: () { | ||
| onEndCallCount++; | ||
| }, | ||
| ), | ||
| ), | ||
| ); | ||
|
|
||
| expect(onEndCallCount, 0); | ||
|
|
||
| // Transition to showSecond. | ||
| await tester.pumpWidget( | ||
| Directionality( | ||
| textDirection: TextDirection.ltr, | ||
| child: AnimatedCrossFade( | ||
| firstChild: const SizedBox(width: 100.0, height: 100.0), | ||
| secondChild: const SizedBox(width: 200.0, height: 200.0), | ||
| crossFadeState: CrossFadeState.showSecond, | ||
| duration: const Duration(milliseconds: 200), | ||
| onEnd: () { | ||
| onEndCallCount++; | ||
| }, | ||
| ), | ||
| ), | ||
| ); | ||
|
|
||
| // Pump halfway through animation. | ||
| await tester.pump(const Duration(milliseconds: 100)); | ||
| expect(onEndCallCount, 0); | ||
|
|
||
| // Pump to complete animation and settle. | ||
| await tester.pumpAndSettle(); | ||
| expect(onEndCallCount, 1); | ||
| }); |
There was a problem hiding this comment.
This test contains duplicated code for building the AnimatedCrossFade widget. This can be simplified by extracting the widget creation into a helper function, similar to what's done in the 'AnimatedCrossFade onEnd callback is called for each animation cycle' test. This will make the test more readable and easier to maintain.
testWidgets('AnimatedCrossFade onEnd callback is called when animation completes forward', (
WidgetTester tester,
) async {
int onEndCallCount = 0;
Widget buildWidget(CrossFadeState state) {
return Directionality(
textDirection: TextDirection.ltr,
child: AnimatedCrossFade(
firstChild: const SizedBox(width: 100.0, height: 100.0),
secondChild: const SizedBox(width: 200.0, height: 200.0),
crossFadeState: state,
duration: const Duration(milliseconds: 200),
onEnd: () {
onEndCallCount++;
},
),
);
}
await tester.pumpWidget(buildWidget(CrossFadeState.showFirst));
expect(onEndCallCount, 0);
// Transition to showSecond.
await tester.pumpWidget(buildWidget(CrossFadeState.showSecond));
// Pump halfway through animation.
await tester.pump(const Duration(milliseconds: 100));
expect(onEndCallCount, 0);
// Pump to complete animation and settle.
await tester.pumpAndSettle();
expect(onEndCallCount, 1);
});References
- Code should be optimized for readability. In this case, the test code can be made more readable by reducing duplication. (link)
| testWidgets('AnimatedCrossFade onEnd callback is called when animation completes reverse', ( | ||
| WidgetTester tester, | ||
| ) async { | ||
| int onEndCallCount = 0; | ||
|
|
||
| await tester.pumpWidget( | ||
| Directionality( | ||
| textDirection: TextDirection.ltr, | ||
| child: AnimatedCrossFade( | ||
| firstChild: const SizedBox(width: 100.0, height: 100.0), | ||
| secondChild: const SizedBox(width: 200.0, height: 200.0), | ||
| crossFadeState: CrossFadeState.showSecond, | ||
| duration: const Duration(milliseconds: 200), | ||
| onEnd: () { | ||
| onEndCallCount++; | ||
| }, | ||
| ), | ||
| ), | ||
| ); | ||
|
|
||
| expect(onEndCallCount, 0); | ||
|
|
||
| // Reverse transition to showFirst. | ||
| await tester.pumpWidget( | ||
| Directionality( | ||
| textDirection: TextDirection.ltr, | ||
| child: AnimatedCrossFade( | ||
| firstChild: const SizedBox(width: 100.0, height: 100.0), | ||
| secondChild: const SizedBox(width: 200.0, height: 200.0), | ||
| crossFadeState: CrossFadeState.showFirst, | ||
| duration: const Duration(milliseconds: 200), | ||
| onEnd: () { | ||
| onEndCallCount++; | ||
| }, | ||
| ), | ||
| ), | ||
| ); | ||
|
|
||
| // Pump halfway through animation. | ||
| await tester.pump(const Duration(milliseconds: 100)); | ||
| expect(onEndCallCount, 0); | ||
|
|
||
| // Pump to complete animation and settle. | ||
| await tester.pumpAndSettle(); | ||
| expect(onEndCallCount, 1); | ||
| }); |
There was a problem hiding this comment.
Similar to the previous test, this one also has duplicated code for building the AnimatedCrossFade widget. Extracting this logic into a helper function would improve readability and maintainability.
testWidgets('AnimatedCrossFade onEnd callback is called when animation completes reverse', (
WidgetTester tester,
) async {
int onEndCallCount = 0;
Widget buildWidget(CrossFadeState state) {
return Directionality(
textDirection: TextDirection.ltr,
child: AnimatedCrossFade(
firstChild: const SizedBox(width: 100.0, height: 100.0),
secondChild: const SizedBox(width: 200.0, height: 200.0),
crossFadeState: state,
duration: const Duration(milliseconds: 200),
onEnd: () {
onEndCallCount++;
},
),
);
}
await tester.pumpWidget(buildWidget(CrossFadeState.showSecond));
expect(onEndCallCount, 0);
// Reverse transition to showFirst.
await tester.pumpWidget(buildWidget(CrossFadeState.showFirst));
// Pump halfway through animation.
await tester.pump(const Duration(milliseconds: 100));
expect(onEndCallCount, 0);
// Pump to complete animation and settle.
await tester.pumpAndSettle();
expect(onEndCallCount, 1);
});References
- Code should be optimized for readability. In this case, the test code can be made more readable by reducing duplication. (link)
7a3b394 to
f423bc7
Compare
loic-sharma
left a comment
There was a problem hiding this comment.
Thanks for the wonderful contribution!
Roll Flutter from dfd92b773219 to da72d5936d69 (39 revisions) flutter/flutter@dfd92b7...da72d59 2026-01-29 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[ Tool / Engine ] Cleanup x86 references (#181152)" (flutter/flutter#181643) 2026-01-29 [email protected] Roll Skia from 174db42b7300 to 89df65f8324c (2 revisions) (flutter/flutter#181636) 2026-01-29 [email protected] [material/menu_anchor.dart] Add animations to MenuAnchor. (flutter/flutter#176494) 2026-01-28 [email protected] Roll Skia from 8e09f8a82251 to 174db42b7300 (6 revisions) (flutter/flutter#181627) 2026-01-28 [email protected] Send statusBarTouch events via dedicated messages (flutter/flutter#179643) 2026-01-28 [email protected] test: Improve DropdownMenuFormField tests (flutter/flutter#181369) 2026-01-28 [email protected] Account for vec3 padding in Metal (flutter/flutter#181563) 2026-01-28 [email protected] [ Tool / Engine ] Cleanup x86 references (flutter/flutter#181152) 2026-01-28 [email protected] Fix the issue on macOS where, after a hot restart with multiple windows, unresponsive windows are left behind. (flutter/flutter#180287) 2026-01-28 [email protected] Roll Skia from 675444e94bc9 to 8e09f8a82251 (7 revisions) (flutter/flutter#181606) 2026-01-28 [email protected] Roll Packages from e37af11 to 1cb2148 (5 revisions) (flutter/flutter#181608) 2026-01-28 [email protected] fix: swap app and engine version in vk::ApplicationInfo (flutter/flutter#181432) 2026-01-28 [email protected] Adds impeller backend to skia gold client (flutter/flutter#181503) 2026-01-28 [email protected] Remove chrome_and_driver dependency where it's not needed (flutter/flutter#178174) 2026-01-28 [email protected] chore: Windows_mokey basic_material_app_win__compile !bringup (flutter/flutter#180985) 2026-01-28 [email protected] Fix generating both `settings.gradle` and `settings.gradle.kts` for plugins (flutter/flutter#181592) 2026-01-28 [email protected] Roll Skia from 6614f89de521 to 675444e94bc9 (3 revisions) (flutter/flutter#181587) 2026-01-28 [email protected] Fix `todayBorder` todayBorder color is incorrectly overridden by `todayForegroundColor` in CalendarDatePicker (flutter/flutter#178792) 2026-01-28 [email protected] Roll Skia from 8f1695a4b328 to 6614f89de521 (2 revisions) (flutter/flutter#181583) 2026-01-28 [email protected] feat: add RoundedSuperellipseInputBorder (flutter/flutter#177220) 2026-01-28 [email protected] Roll Dart SDK from 38e351498549 to f10dcbfca98f (2 revisions) (flutter/flutter#181582) 2026-01-28 [email protected] Roll Skia from f424d58e37a3 to 8f1695a4b328 (6 revisions) (flutter/flutter#181577) 2026-01-28 [email protected] Remove unused code paths in `PlatformViewsController.java` (flutter/flutter#181393) 2026-01-28 [email protected] feat: add onEnd to AnimatedCrossFade (flutter/flutter#181455) 2026-01-28 [email protected] Don't pass bounds to saveLayer call when painting ImageFilter (flutter/flutter#181353) 2026-01-28 [email protected] test: Clarify failure messages on gestures/debug_test.dart (flutter/flutter#181109) 2026-01-27 [email protected] Roll Fuchsia Linux SDK from akraNGn2lw4T1msgZ... to adhoq9ouVRh0xzkm3... (flutter/flutter#181571) 2026-01-27 [email protected] Roll Dart SDK from 4c7cb0a1d07d to 38e351498549 (4 revisions) (flutter/flutter#181570) 2026-01-27 [email protected] Make sure that an AnimatedPadding doesn't crash in 0x0 environment (flutter/flutter#181235) 2026-01-27 [email protected] Make sure that an ImageFiltered doesn't crash at 0x0 environment (flutter/flutter#181067) 2026-01-27 [email protected] Marks firebase_release_smoke_test to be unflaky (flutter/flutter#181308) 2026-01-27 [email protected] Fix remove material import box decoration test (flutter/flutter#181253) 2026-01-27 [email protected] Roll Skia from c2754838b077 to f424d58e37a3 (8 revisions) (flutter/flutter#181564) 2026-01-27 [email protected] Make sure that an AnimatedAlign doesn't crash in 0x0 environment (flutter/flutter#181361) 2026-01-27 [email protected] Make sure that an ImageIcon doesn't crash in 0x0 environment (flutter/flutter#181099) 2026-01-27 [email protected] Make sure that an AnimatedContainer doesn't crash in 0x0 environment (flutter/flutter#181198) 2026-01-27 [email protected] Make sure that an AnimatedRotation doesn't crash in 0x0 environment (flutter/flutter#181486) 2026-01-27 [email protected] Make sure that an AnimatedPositionedDirectional doesn't crash in 0x0 … (flutter/flutter#181451) 2026-01-27 [email protected] Merge changelog for 3.38.8. (flutter/flutter#181558) 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 [email protected],[email protected] on the revert to ensure that a human is aware of the problem. ...
feat: add onEnd to AnimatedCrossFade fixes: flutter#180682 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
feat: add onEnd to AnimatedCrossFade
fixes: #180682
Pre-launch Checklist
///).