From 9a6ae31e1eedacdfbd742686410f3a3fa405b1b3 Mon Sep 17 00:00:00 2001 From: Kishan Rathore <34465683+rkishan516@users.noreply.github.com> Date: Thu, 3 Apr 2025 02:57:07 +0530 Subject: [PATCH] Fix: Hero animation for page transition (#164469) Fix: Hero animation for page transition fixes: #163989 ## 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. --- packages/flutter/lib/src/material/page.dart | 9 +- .../src/material/page_transitions_theme.dart | 6 +- .../material/page_transitions_theme_test.dart | 4 +- .../flutter/test/widgets/heroes_test.dart | 148 ++++++++++-------- 4 files changed, 93 insertions(+), 74 deletions(-) diff --git a/packages/flutter/lib/src/material/page.dart b/packages/flutter/lib/src/material/page.dart index 05a417db95..98ac6da6cf 100644 --- a/packages/flutter/lib/src/material/page.dart +++ b/packages/flutter/lib/src/material/page.dart @@ -100,7 +100,14 @@ mixin MaterialRouteTransitionMixin on PageRoute { PageTransitionsBuilder? _getPageTransitionBuilder(BuildContext context) { final TargetPlatform platform = Theme.of(context).platform; final PageTransitionsTheme pageTransitionsTheme = Theme.of(context).pageTransitionsTheme; - return pageTransitionsTheme.builders[platform]; + return pageTransitionsTheme.builders[platform] ?? + switch (platform) { + TargetPlatform.iOS || TargetPlatform.macOS => const CupertinoPageTransitionsBuilder(), + TargetPlatform.android || + TargetPlatform.fuchsia || + TargetPlatform.windows || + TargetPlatform.linux => const ZoomPageTransitionsBuilder(), + }; } // The transitionDuration is used to create the AnimationController which is only diff --git a/packages/flutter/lib/src/material/page_transitions_theme.dart b/packages/flutter/lib/src/material/page_transitions_theme.dart index f33926d6d7..b1473a36bb 100644 --- a/packages/flutter/lib/src/material/page_transitions_theme.dart +++ b/packages/flutter/lib/src/material/page_transitions_theme.dart @@ -1077,8 +1077,8 @@ class PageTransitionsTheme with Diagnosticable { /// Constructs an object that selects a transition based on the platform. /// /// By default the list of builders is: [ZoomPageTransitionsBuilder] - /// for [TargetPlatform.android], and [CupertinoPageTransitionsBuilder] for - /// [TargetPlatform.iOS] and [TargetPlatform.macOS]. + /// for [TargetPlatform.android], [TargetPlatform.windows] and [TargetPlatform.linux] + /// and [CupertinoPageTransitionsBuilder] for [TargetPlatform.iOS] and [TargetPlatform.macOS]. const PageTransitionsTheme({ Map builders = _defaultBuilders, }) : _builders = builders; @@ -1088,6 +1088,8 @@ class PageTransitionsTheme with Diagnosticable { TargetPlatform.android: ZoomPageTransitionsBuilder(), TargetPlatform.iOS: CupertinoPageTransitionsBuilder(), TargetPlatform.macOS: CupertinoPageTransitionsBuilder(), + TargetPlatform.windows: ZoomPageTransitionsBuilder(), + TargetPlatform.linux: ZoomPageTransitionsBuilder(), }; /// The [PageTransitionsBuilder]s supported by this theme. diff --git a/packages/flutter/test/material/page_transitions_theme_test.dart b/packages/flutter/test/material/page_transitions_theme_test.dart index c733d81722..2124f9eb9f 100644 --- a/packages/flutter/test/material/page_transitions_theme_test.dart +++ b/packages/flutter/test/material/page_transitions_theme_test.dart @@ -22,14 +22,14 @@ void main() { case TargetPlatform.android: case TargetPlatform.iOS: case TargetPlatform.macOS: + case TargetPlatform.linux: + case TargetPlatform.windows: expect( theme.builders[platform], isNotNull, reason: 'theme builder for $platform is null', ); case TargetPlatform.fuchsia: - case TargetPlatform.linux: - case TargetPlatform.windows: expect( theme.builders[platform], isNull, diff --git a/packages/flutter/test/widgets/heroes_test.dart b/packages/flutter/test/widgets/heroes_test.dart index e0d9d37b79..f6abacba39 100644 --- a/packages/flutter/test/widgets/heroes_test.dart +++ b/packages/flutter/test/widgets/heroes_test.dart @@ -794,90 +794,100 @@ Future main() async { expect(find.byKey(secondKey), findsNothing); }); - testWidgets('Hero pop transition interrupted by a push', (WidgetTester tester) async { - await tester.pumpWidget( - MaterialApp( - routes: routes, - theme: ThemeData( - pageTransitionsTheme: const PageTransitionsTheme( - builders: { - TargetPlatform.android: FadeUpwardsPageTransitionsBuilder(), - }, + testWidgets( + 'Hero pop transition interrupted by a push', + (WidgetTester tester) async { + await tester.pumpWidget( + MaterialApp( + routes: routes, + theme: ThemeData( + pageTransitionsTheme: const PageTransitionsTheme( + builders: { + TargetPlatform.android: FadeUpwardsPageTransitionsBuilder(), + }, + ), ), ), - ), - ); + ); - // Pushes MaterialPageRoute '/two'. - await tester.tap(find.text('two')); - await tester.pump(); - await tester.pump(const Duration(seconds: 1)); + // Pushes MaterialPageRoute '/two'. + await tester.tap(find.text('two')); + await tester.pump(); + await tester.pump(const Duration(seconds: 1)); - // Now the secondKey Card on the '/2' route is visible - expect(find.byKey(secondKey), isOnstage); - expect(find.byKey(secondKey), isInCard); - expect(find.byKey(firstKey), findsNothing); + // Now the secondKey Card on the '/2' route is visible + expect(find.byKey(secondKey), isOnstage); + expect(find.byKey(secondKey), isInCard); + expect(find.byKey(firstKey), findsNothing); - // Pop MaterialPageRoute '/two'. - await tester.tap(find.text('pop')); + // Pop MaterialPageRoute '/two'. + await tester.tap(find.text('pop')); - // Start the flight of Hero 'a' from route '/two' to route '/'. Route '/two' - // is now offstage. - await tester.pump(); + // Start the flight of Hero 'a' from route '/two' to route '/'. Route '/two' + // is now offstage. + await tester.pump(); - final double initialHeight = tester.getSize(find.byKey(secondKey)).height; - final double finalHeight = tester.getSize(find.byKey(firstKey, skipOffstage: false)).height; - expect(finalHeight, lessThan(initialHeight)); // simplify the checks below + final double initialHeight = tester.getSize(find.byKey(secondKey)).height; + final double finalHeight = tester.getSize(find.byKey(firstKey, skipOffstage: false)).height; + expect(finalHeight, lessThan(initialHeight)); // simplify the checks below - // Build the first hero animation frame in the navigator's overlay. - await tester.pump(); + // Build the first hero animation frame in the navigator's overlay. + await tester.pump(); - // At this point the hero widgets have been replaced by placeholders - // and the destination hero has been moved to the overlay. - expect( - find.descendant(of: find.byKey(homeRouteKey), matching: find.byKey(firstKey)), - findsNothing, - ); - expect( - find.descendant(of: find.byKey(routeTwoKey), matching: find.byKey(secondKey)), - findsNothing, - ); - expect(find.byKey(firstKey), isOnstage); - expect(find.byKey(secondKey), findsNothing); + // At this point the hero widgets have been replaced by placeholders + // and the destination hero has been moved to the overlay. + expect( + find.descendant(of: find.byKey(homeRouteKey), matching: find.byKey(firstKey)), + findsNothing, + ); + expect( + find.descendant(of: find.byKey(routeTwoKey), matching: find.byKey(secondKey)), + findsNothing, + ); + expect(find.byKey(firstKey), isOnstage); + expect(find.byKey(secondKey), findsNothing); - // The duration of a MaterialPageRoute's transition is 300ms. - // At 150ms Hero 'a' is mid-flight. - await tester.pump(const Duration(milliseconds: 150)); - final double height150ms = tester.getSize(find.byKey(firstKey)).height; - expect(height150ms, lessThan(initialHeight)); - expect(height150ms, greaterThan(finalHeight)); + // The duration of a MaterialPageRoute's transition is 300ms. + // At 150ms Hero 'a' is mid-flight. + await tester.pump(const Duration(milliseconds: 150)); + final double height150ms = tester.getSize(find.byKey(firstKey)).height; + expect(height150ms, lessThan(initialHeight)); + expect(height150ms, greaterThan(finalHeight)); - // Push route '/two' before the pop transition from '/two' has finished. - await tester.tap(find.text('two')); + // Push route '/two' before the pop transition from '/two' has finished. + await tester.tap(find.text('two')); - // Restart the flight of Hero 'a'. Now it's flying from route '/' to - // route '/two'. - await tester.pump(); + // Restart the flight of Hero 'a'. Now it's flying from route '/' to + // route '/two'. + await tester.pump(); - // After flying in the opposite direction for 50ms Hero 'a' will - // be smaller than it was, but bigger than its initial size. - await tester.pump(const Duration(milliseconds: 50)); - final double height200ms = tester.getSize(find.byKey(firstKey)).height; - expect(height200ms, greaterThan(height150ms)); - expect(finalHeight, lessThan(height200ms)); + // After flying in the opposite direction for 50ms Hero 'a' will + // be smaller than it was, but bigger than its initial size. + await tester.pump(const Duration(milliseconds: 50)); + final double height200ms = tester.getSize(find.byKey(firstKey)).height; + expect(height200ms, greaterThan(height150ms)); + expect(finalHeight, lessThan(height200ms)); - // Hero a's return flight at 149ms. The outgoing (push) flight took - // 150ms so we should be just about back to where Hero 'a' started. - const double epsilon = 0.001; - await tester.pump(const Duration(milliseconds: 99)); - moreOrLessEquals(tester.getSize(find.byKey(firstKey)).height - initialHeight, epsilon: epsilon); + // Hero a's return flight at 149ms. The outgoing (push) flight took + // 150ms so we should be just about back to where Hero 'a' started. + const double epsilon = 0.001; + await tester.pump(const Duration(milliseconds: 99)); + moreOrLessEquals( + tester.getSize(find.byKey(firstKey)).height - initialHeight, + epsilon: epsilon, + ); - // The flight is finished. We're back to where we started. - await tester.pump(const Duration(milliseconds: 300)); - expect(find.byKey(secondKey), isOnstage); - expect(find.byKey(secondKey), isInCard); - expect(find.byKey(firstKey), findsNothing); - }); + // The flight is finished. We're back to where we started. + await tester.pump(const Duration(milliseconds: 300)); + expect(find.byKey(secondKey), isOnstage); + expect(find.byKey(secondKey), isInCard); + expect(find.byKey(firstKey), findsNothing); + }, + variant: const TargetPlatformVariant({ + TargetPlatform.android, + TargetPlatform.linux, + }), + ); testWidgets('Destination hero disappears mid-flight', (WidgetTester tester) async { const Key homeHeroKey = Key('home hero');