From 4503f2f459c34d8ce5a7debc19c14457bb439900 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: DelegateTransition for cupertino sheet route (#164675) Fix: DelegateTransition for cupertino sheet route fixes: #163954 ## 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/widgets/routes.dart | 2 +- .../flutter/test/cupertino/sheet_test.dart | 70 ++++++++++++++++++- .../flutter/test/widgets/routes_test.dart | 46 ++++++++++++ 3 files changed, 115 insertions(+), 3 deletions(-) diff --git a/packages/flutter/lib/src/widgets/routes.dart b/packages/flutter/lib/src/widgets/routes.dart index b48f25d4b3..f873ee588b 100644 --- a/packages/flutter/lib/src/widgets/routes.dart +++ b/packages/flutter/lib/src/widgets/routes.dart @@ -1583,7 +1583,7 @@ abstract class ModalRoute extends TransitionRoute with LocalHistoryRoute secondaryAnimation, Widget child, ) { - if (receivedTransition == null) { + if (receivedTransition == null || secondaryAnimation.isDismissed) { return buildTransitions(context, animation, secondaryAnimation, child); } diff --git a/packages/flutter/test/cupertino/sheet_test.dart b/packages/flutter/test/cupertino/sheet_test.dart index 42a671a76b..5e2005fdd2 100644 --- a/packages/flutter/test/cupertino/sheet_test.dart +++ b/packages/flutter/test/cupertino/sheet_test.dart @@ -4,6 +4,7 @@ import 'package:flutter/cupertino.dart'; import 'package:flutter/material.dart'; +import 'package:flutter_localizations/flutter_localizations.dart'; import 'package:flutter_test/flutter_test.dart'; import '../widgets/navigator_utils.dart'; @@ -810,8 +811,73 @@ void main() { await tester.pumpAndSettle(); final Finder clipRRectFinder = find.byType(ClipRRect); - final Rect clipRect = tester.getRect(clipRRectFinder); - expect(clipRect.center, equals(const Offset(400, 300))); + expect(clipRRectFinder, findsNothing); + }); + + testWidgets('Sheet transition does not interfere after popping', (WidgetTester tester) async { + final GlobalKey homeKey = GlobalKey(); + final GlobalKey sheetKey = GlobalKey(); + final GlobalKey popupMenuButtonKey = GlobalKey(); + + await tester.pumpWidget( + CupertinoApp( + localizationsDelegates: const >[ + GlobalMaterialLocalizations.delegate, + GlobalWidgetsLocalizations.delegate, + GlobalCupertinoLocalizations.delegate, + ], + home: CupertinoPageScaffold( + key: homeKey, + child: CupertinoListTile( + onTap: () { + showCupertinoSheet( + context: homeKey.currentContext!, + pageBuilder: (BuildContext context) { + return CupertinoPageScaffold( + key: sheetKey, + child: const Center(child: Text('Page 2')), + ); + }, + ); + }, + title: const Text('ListItem 0'), + trailing: Material( + type: MaterialType.transparency, + child: PopupMenuButton( + key: popupMenuButtonKey, + itemBuilder: (BuildContext context) { + return >[ + const PopupMenuItem(child: Text('Item 0')), + const PopupMenuItem(child: Text('Item 1')), + ]; + }, + ), + ), + ), + ), + ), + ); + + await tester.tap(find.text('ListItem 0')); + await tester.pumpAndSettle(); + + expect(find.text('Page 2'), findsOneWidget); + + final TestGesture gesture = await tester.startGesture(const Offset(100, 200)); + await gesture.moveBy(const Offset(0, 350)); + await tester.pump(); + + await gesture.up(); + await tester.pumpAndSettle(); + + expect(find.text('Page 2'), findsNothing); + expect(find.text('ListItem 0'), findsOneWidget); + + await tester.tap(find.byKey(popupMenuButtonKey)); + await tester.pumpAndSettle(); + + expect(find.text('Item 0'), findsOneWidget); + expect(tester.takeException(), isNull); }); group('drag dismiss gesture', () { diff --git a/packages/flutter/test/widgets/routes_test.dart b/packages/flutter/test/widgets/routes_test.dart index 92ffb7a2da..ef27822b90 100644 --- a/packages/flutter/test/widgets/routes_test.dart +++ b/packages/flutter/test/widgets/routes_test.dart @@ -766,6 +766,52 @@ void main() { expect(secondaryAnimationPageOne.parent, kAlwaysDismissedAnimation); }); + testWidgets( + 'delegated transitions are removed when secondary animation is dismissed and next route is removed', + (WidgetTester tester) async { + final GlobalKey navigator = GlobalKey(); + await tester.pumpWidget( + MaterialApp( + navigatorKey: navigator, + theme: ThemeData( + pageTransitionsTheme: const PageTransitionsTheme( + builders: { + TargetPlatform.android: CupertinoPageTransitionsBuilder(), + }, + ), + ), + home: const Text('home'), + ), + ); + + // Push first page with custom transition builder. + final Route firstRoute = CupertinoSheetRoute( + builder: (_) { + return const Text('Page One'); + }, + ); + + navigator.currentState!.push(firstRoute); + await tester.pumpAndSettle(); + + expect(find.text('Page One'), findsOneWidget); + final Finder cupertinoSheetDelegatedTransitionFinder = find.ancestor( + of: find.ancestor(of: find.byType(ClipRRect), matching: find.byType(AnimatedBuilder)), + matching: find.byType(ScaleTransition), + ); + expect(cupertinoSheetDelegatedTransitionFinder, findsOneWidget); + + navigator.currentState!.pop(); + await tester.pumpAndSettle(); + + // Verify home is still visible without transitions. + expect(find.text('home'), findsOneWidget); + + // Verify the delegated transition is removed. + expect(cupertinoSheetDelegatedTransitionFinder, findsNothing); + }, + ); + testWidgets('secondary animation is kDismissed after train hopping finishes and pop', ( WidgetTester tester, ) async {