firka/flutter
firka/flutter
5
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Suppress previous route transition if current route is fullscreenDialog (#159312)

Browse Source 
Fixes #159289

Cupertino and Material routes were looking to see if the next route was
a full screen dialog route, and not transitioning accordingly. However
with the updates to allow [mixed transitions in an
app](https://github.com/flutter/flutter/pull/150031), any route will try
and transition if a delegatedTransition is available. This PR makes it
so that Cupertino and Material routes that are fullscreen dialogs will
use `canTransitionFrom` to tell the previous route not to transition.

Before:

[388677067-d301238d-6615-42a7-b60a-611c61136d88.webm](https://github.com/user-attachments/assets/eea99ee9-0bc8-4981-a950-08f99a7fdb3e)

After:


https://github.com/user-attachments/assets/7deb5143-cd67-4696-95ec-9d7df329dfa4



## 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.
- [ ] 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.

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

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
This commit is contained in:
Mitchell Goodwin 2024-11-26 15:59:14 -08:00 committed by GitHub
parent 830ec5a3fb
commit 4a4d7a7dac
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 108 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
packages/flutter/lib/src/cupertino/route.dart
View File

@ -175,6 +175,12 @@ mixin CupertinoRouteTransitionMixin<T> on PageRoute<T> {
((nextRoute is CupertinoRouteTransitionMixin) || nextRouteHasDelegatedTransition);
}
@override
bool canTransitionFrom(TransitionRoute<dynamic> previousRoute) {
// Supress previous route from transitioning if this is a fullscreenDialog route.
return previousRoute is PageRoute && !fullscreenDialog;
}
@override
Widget buildPage(BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation) {
final Widget child = buildContent(context);

6
packages/flutter/lib/src/material/page.dart
View File

@ -124,6 +124,12 @@ mixin MaterialRouteTransitionMixin<T> on PageRoute<T> {
((nextRoute is MaterialRouteTransitionMixin) || nextRouteHasDelegatedTransition);
}
@override
bool canTransitionFrom(TransitionRoute<dynamic> previousRoute) {
// Supress previous route from transitioning if this is a fullscreenDialog route.
return previousRoute is PageRoute && !fullscreenDialog;
}
@override
Widget buildPage(
BuildContext context,

48
packages/flutter/test/cupertino/route_test.dart
View File

@ -2691,6 +2691,54 @@ void main() {
await tester.pumpAndSettle();
});
testWidgets('fullscreen routes do not transition previous route', (WidgetTester tester) async {
await tester.pumpWidget(
CupertinoApp(
initialRoute: '/',
onGenerateRoute: (RouteSettings settings) {
if (settings.name == '/') {
return PageRouteBuilder<void>(
pageBuilder: (BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation) {
return CupertinoPageScaffold(
navigationBar: const CupertinoNavigationBar(
middle: Text('Page 1'),
),
child: Container()
);
},
);
}
return CupertinoPageRoute<void>(
builder: (BuildContext context) {
return CupertinoPageScaffold(
navigationBar: const CupertinoNavigationBar(
middle: Text('Page 2'),
),
child: Container(),
);
},
fullscreenDialog: true,
);
},
),
);
expect(find.text('Page 1'), findsOneWidget);
expect(find.text('Page 2'), findsNothing);
final double pageTitleDX = tester.getTopLeft(find.text('Page 1')).dx;
tester.state<NavigatorState>(find.byType(Navigator)).pushNamed('/next');
await tester.pump();
await tester.pump(const Duration(milliseconds: 100));
// Second page transition has started.
expect(find.text('Page 2'), findsOneWidget);
// First page has not moved.
expect(tester.getTopLeft(find.text('Page 1')).dx, equals(pageTitleDX));
});
testWidgets('Setting CupertinoDialogRoute.requestFocus to false does not request focus on the dialog', (WidgetTester tester) async {
late BuildContext savedContext;
final FocusNode focusNode = FocusNode();

48
packages/flutter/test/material/page_test.dart
View File

@ -656,6 +656,54 @@ void main() {
expect(tester.getTopLeft(find.text('Page 2')), Offset.zero);
}, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS, TargetPlatform.macOS }));
testWidgets('test fullscreen routes do not transition previous route', (WidgetTester tester) async {
await tester.pumpWidget(
MaterialApp(
initialRoute: '/',
onGenerateRoute: (RouteSettings settings) {
if (settings.name == '/') {
return PageRouteBuilder<void>(
pageBuilder: (BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation) {
return Scaffold(
appBar: AppBar(
title: const Text('Page 1'),
),
body: Container()
);
},
);
}
return MaterialPageRoute<void>(
builder: (BuildContext context) {
return Scaffold(
appBar: AppBar(
title: const Text('Page 2'),
),
body: Container(),
);
},
fullscreenDialog: true,
);
},
),
);
expect(find.text('Page 1'), findsOneWidget);
expect(find.text('Page 2'), findsNothing);
final double pageTitleDX = tester.getTopLeft(find.text('Page 1')).dx;
tester.state<NavigatorState>(find.byType(Navigator)).pushNamed('/next');
await tester.pump();
await tester.pump(const Duration(milliseconds: 100));
// Second page transition has started.
expect(find.text('Page 2'), findsOneWidget);
// First page has not moved.
expect(tester.getTopLeft(find.text('Page 1')).dx, equals(pageTitleDX));
}, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS, TargetPlatform.macOS }));
testWidgets('test adaptable transitions switch during execution', (WidgetTester tester) async {
await tester.pumpWidget(
MaterialApp(