Skip to content

Remove Material import from navigator_on_did_remove_page_test #184776

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
xfce0 wants to merge 1 commit into
base: master
Choose a base branch
from
+32 −15
Open

Remove Material import from navigator_on_did_remove_page_test #184776

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion dev/bots/check_tests_cross_imports.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ class TestsCrossImportChecker {
'packages/flutter/test/widgets/tap_region_test.dart',
'packages/flutter/test/widgets/navigator_test.dart',
'packages/flutter/test/widgets/navigator_restoration_test.dart',
'packages/flutter/test/widgets/navigator_on_did_remove_page_test.dart',
'packages/flutter/test/widgets/scrollable_semantics_test.dart',
'packages/flutter/test/widgets/form_test.dart',
};
Expand Down
46 changes: 32 additions & 14 deletions packages/flutter/test/widgets/navigator_on_did_remove_page_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,27 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:flutter/material.dart';
import 'package:flutter/widgets.dart';
import 'package:flutter_test/flutter_test.dart';

class _TestPage extends Page<void> {
const _TestPage({required super.key, required this.child});

final Widget child;
Comment on lines +8 to +11
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

According to the Flutter style guide, all members (including private ones) should have public-quality documentation using ///. Adding documentation helps explain the purpose of this test helper class and its fields.

/// A [Page] implementation used for testing the [Navigator].
class _TestPage extends Page<void> {
  /// Creates a [_TestPage].
  const _TestPage({required super.key, required this.child});

  /// The widget to display as the content of the page.
  final Widget child;
References
  1. Use /// for public-quality documentation, even on private members. (link)


@override
Route<void> createRoute(BuildContext context) {
return PageRouteBuilder<void>(
settings: this,
transitionDuration: Duration.zero,
reverseTransitionDuration: Duration.zero,
pageBuilder: (BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation) {
return child;
},
);
}
}

void main() {
Future<void> buildPages(
List<Page<void>> pages,
Expand All @@ -26,13 +44,13 @@ void main() {
testWidgets('Page API will not call onDidRemovePage', (WidgetTester tester) async {
final removedPages = <Page<void>>[];

const page = MaterialPage<void>(key: ValueKey<String>('page'), child: Text('page'));
const page1 = MaterialPage<void>(key: ValueKey<String>('page1'), child: Text('page1'));
const page2 = MaterialPage<void>(key: ValueKey<String>('page2'), child: Text('page2'));
const page3 = MaterialPage<void>(key: ValueKey<String>('page3'), child: Text('page3'));
const page4 = MaterialPage<void>(key: ValueKey<String>('page4'), child: Text('page4'));
const page5 = MaterialPage<void>(key: ValueKey<String>('page5'), child: Text('page5'));
const page6 = MaterialPage<void>(key: ValueKey<String>('page6'), child: Text('page6'));
const page = _TestPage(key: ValueKey<String>('page'), child: Text('page'));
const page1 = _TestPage(key: ValueKey<String>('page1'), child: Text('page1'));
const page2 = _TestPage(key: ValueKey<String>('page2'), child: Text('page2'));
const page3 = _TestPage(key: ValueKey<String>('page3'), child: Text('page3'));
const page4 = _TestPage(key: ValueKey<String>('page4'), child: Text('page4'));
const page5 = _TestPage(key: ValueKey<String>('page5'), child: Text('page5'));
const page6 = _TestPage(key: ValueKey<String>('page6'), child: Text('page6'));
await buildPages(<Page<void>>[page], tester, removedPage: removedPages);

expect(find.text('page'), findsOneWidget);
Expand All @@ -58,8 +76,8 @@ void main() {
final key = GlobalKey<NavigatorState>();
final removedPage = <Page<void>>[];

const page = MaterialPage<void>(key: ValueKey<String>('page'), child: Text('page'));
const page1 = MaterialPage<void>(key: ValueKey<String>('page1'), child: Text('page1'));
const page = _TestPage(key: ValueKey<String>('page'), child: Text('page'));
const page1 = _TestPage(key: ValueKey<String>('page1'), child: Text('page1'));
await buildPages(<Page<void>>[page, page1], tester, removedPage: removedPage, navKey: key);

expect(find.text('page1'), findsOneWidget);
Expand All @@ -81,15 +99,15 @@ void main() {
final key = GlobalKey<NavigatorState>();
final removedPage = <Page<void>>[];

const page = MaterialPage<void>(key: ValueKey<String>('page'), child: Text('page'));
const page1 = MaterialPage<void>(key: ValueKey<String>('page1'), child: Text('page1'));
const page = _TestPage(key: ValueKey<String>('page'), child: Text('page'));
const page1 = _TestPage(key: ValueKey<String>('page1'), child: Text('page1'));
await buildPages(<Page<void>>[page, page1], tester, removedPage: removedPage, navKey: key);

expect(find.text('page1'), findsOneWidget);

key.currentState!.pushReplacement(
MaterialPageRoute<void>(
builder: (_) {
PageRouteBuilder<void>(
pageBuilder: (BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation) {
return const Text('new page');
},
),
Comment on lines +109 to 113
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For consistency with _TestPage and to ensure the test remains fast and deterministic, consider setting transitionDuration and reverseTransitionDuration to Duration.zero here as well. This avoids unnecessary animation delays during the test execution.

Suggested change
PageRouteBuilder<void>(
pageBuilder: (BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation) {
return const Text('new page');
},
),
PageRouteBuilder<void>(
transitionDuration: Duration.zero,
reverseTransitionDuration: Duration.zero,
pageBuilder: (BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation) {
return const Text('new page');
},
),

Expand Down
Loading