Skip to content

refactor: remove material from list_view_viewporting_test and page_forward_transitions_test#183564

Merged
auto-submit[bot] merged 5 commits into
flutter:masterfrom
rkishan516:simple-cross-imports-3
Apr 3, 2026
Merged

refactor: remove material from list_view_viewporting_test and page_forward_transitions_test#183564
auto-submit[bot] merged 5 commits into
flutter:masterfrom
rkishan516:simple-cross-imports-3

Conversation

@rkishan516

Copy link
Copy Markdown
Contributor

This PR removes Material imports from list_view_viewporting_test and page_forward_transitions_test.

This PR also adds onGenerateRoute on TestWidgetsApp.

part of: #177415

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
  • All existing and new tests are passing.

@github-actions github-actions Bot added framework flutter/packages/flutter repository. See also f: labels. f: scrolling Viewports, list views, slivers, etc. labels Mar 12, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors list_view_viewporting_test and page_forward_transitions_test to remove dependencies on the Material library. This is achieved by replacing Material widgets like MaterialApp, Theme, and SliverAppBar with non-Material alternatives such as TestWidgetsApp, IconTheme, and SliverToBoxAdapter. As part of this, TestWidgetsApp is enhanced to support onGenerateRoute, and corresponding tests are added. My review includes suggestions to properly dispose of HeroController instances in the updated tests to prevent resource leaks, and a correction to the documentation for the new onGenerateRoute property in TestWidgetsApp.

Comment thread packages/flutter/test/widgets/page_forward_transitions_test.dart
Comment thread packages/flutter/test/widgets/page_forward_transitions_test.dart
Comment on lines +129 to +130
/// When provided, this takes over route generation from [home] and [routes].
/// The [pageRouteBuilder] is not used when [onGenerateRoute] is specified.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This documentation is slightly incorrect. onGenerateRoute is a fallback for home and routes, it doesn't take them over. Also, pageRouteBuilder is still used for routes from home and routes; it's just not used for routes generated by onGenerateRoute.

  /// This callback is used if [routes] and [home] do not contain the requested
  /// route.
  ///
  /// The [pageRouteBuilder] is not used for routes created by this callback.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess Gemini is correct here but no big deal either way.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updating.

Comment thread packages/flutter/test/widgets/page_forward_transitions_test.dart
Comment thread packages/flutter/test/widgets/list_view_viewporting_test.dart Outdated
Comment thread packages/flutter/test/widgets/list_view_viewporting_test.dart Outdated
Comment thread packages/flutter/test/widgets/list_view_viewporting_test.dart Outdated
Comment thread packages/flutter/test/widgets/list_view_viewporting_test.dart Outdated
Comment thread packages/flutter/test/widgets/list_view_viewporting_test.dart Outdated
@rkishan516 rkishan516 force-pushed the simple-cross-imports-3 branch from 7949c8d to f8ab710 Compare March 12, 2026 14:13
justinmc
justinmc previously approved these changes Mar 12, 2026

@justinmc justinmc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 👍

/// });
/// ```
///
/// For tests that need full control over route generation:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for documenting this!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

All thanks to gemini, I always update docs with gemini.

Comment on lines +129 to +130
/// When provided, this takes over route generation from [home] and [routes].
/// The [pageRouteBuilder] is not used when [onGenerateRoute] is specified.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess Gemini is correct here but no big deal either way.

expect(find.text('Details Page'), findsNothing);
});

testWidgets('onGenerateRoute builds routes from callback', (WidgetTester tester) async {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for testing the changes to TestWidgetsApp too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yaa, we are testing other properites, so added for this also.

victorsanni
victorsanni previously approved these changes Mar 12, 2026

@victorsanni victorsanni left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, only had one comment.

@rkishan516 rkishan516 dismissed stale reviews from victorsanni and justinmc via 07bd6a9 March 13, 2026 01:18
@rkishan516 rkishan516 added the CICD Run CI/CD label Mar 24, 2026
@rkishan516 rkishan516 force-pushed the simple-cross-imports-3 branch from 07bd6a9 to 1d30539 Compare March 24, 2026 02:02
@github-actions github-actions Bot removed the CICD Run CI/CD label Mar 24, 2026
@rkishan516 rkishan516 added CICD Run CI/CD autosubmit Merge PR when tree becomes green via auto submit App labels Mar 24, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 24, 2026
@auto-submit

auto-submit Bot commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/flutter/183564, because This PR has not met approval requirements for merging. Changes were requested by {navaronbracke}, please make the needed changes and resubmit this PR.
The PR author is a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

  • Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

@github-actions github-actions Bot removed the CICD Run CI/CD label Mar 24, 2026
@rkishan516 rkishan516 added the CICD Run CI/CD label Mar 24, 2026
@github-actions github-actions Bot removed the CICD Run CI/CD label Mar 31, 2026
@rkishan516 rkishan516 added the CICD Run CI/CD label Mar 31, 2026
@justinmc justinmc added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 31, 2026
@auto-submit auto-submit Bot added this pull request to the merge queue Apr 3, 2026
Merged via the queue into flutter:master with commit be2b171 Apr 3, 2026
161 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Test cross-imports Review Queue Apr 3, 2026
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Apr 3, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 4, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 4, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 5, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 5, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 6, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 6, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 6, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 6, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 6, 2026
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Apr 6, 2026
flutter/flutter@7245c3f...9cd60b5

2026-04-06 [email protected] Fix go/ links in rbe.mde (flutter/flutter#184672)
2026-04-06 [email protected] Roll Packages from d31df66 to 5299279 (2 revisions) (flutter/flutter#184663)
2026-04-06 [email protected] Add suggestion for what types of skills are helpful (flutter/flutter#184661)
2026-04-06 [email protected] Roll Fuchsia Linux SDK from hrLZV-ij47FXnv_m5... to 1rcChbOv4nSTVkUxs... (flutter/flutter#184657)
2026-04-06 [email protected] Skill to find dart or flutter revision from a hash (flutter/flutter#184589)
2026-04-06 [email protected] Roll Skia from 207c6721c64b to 163dfdf500c7 (7 revisions) (flutter/flutter#184650)
2026-04-05 [email protected] Roll Skia from 52b79372764c to 207c6721c64b (1 revision) (flutter/flutter#184636)
2026-04-05 [email protected] Roll Fuchsia Linux SDK from D2z-jMnRVbcwcraY-... to hrLZV-ij47FXnv_m5... (flutter/flutter#184623)
2026-04-05 [email protected] Roll Skia from 500bc1651c86 to 52b79372764c (1 revision) (flutter/flutter#184621)
2026-04-04 [email protected] Roll Fuchsia Linux SDK from PpL3Bn2YMb2h9LbdK... to D2z-jMnRVbcwcraY-... (flutter/flutter#184611)
2026-04-04 [email protected] Roll Dart SDK from 102a3d1c7fe5 to 79f728f5d66e (1 revision) (flutter/flutter#184610)
2026-04-04 [email protected] Roll Skia from 5e5a7f252b5e to 500bc1651c86 (1 revision) (flutter/flutter#184609)
2026-04-04 [email protected] Roll Skia from ec209c206aa5 to 5e5a7f252b5e (2 revisions) (flutter/flutter#184606)
2026-04-04 [email protected] Roll Dart SDK from f7be88117622 to 102a3d1c7fe5 (2 revisions) (flutter/flutter#184604)
2026-04-04 [email protected] refactor: remove material from color and image filter test (flutter/flutter#183563)
2026-04-04 [email protected] Roll Skia from 50f3a9aaa185 to ec209c206aa5 (1 revision) (flutter/flutter#184601)
2026-04-03 [email protected] Roll Skia from 4ecb7b28459f to 50f3a9aaa185 (3 revisions) (flutter/flutter#184599)
2026-04-03 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll ICU from ee5f27adc28b to ff7995a708a1 (5 revisions) (#184566)" (flutter/flutter#184586)
2026-04-03 [email protected] Add integration scripts and tools for SwiftPM add to app (flutter/flutter#184204)
2026-04-03 [email protected] Roll Dart SDK from 46f49142acd9 to f7be88117622 (1 revision) (flutter/flutter#184596)
2026-04-03 [email protected] [AGP 9] Add Disable Built-in Kotlin and newDSL Migratorrs (flutter/flutter#184255)
2026-04-03 [email protected] Roll Skia from 3b0f0f04f97c to 4ecb7b28459f (7 revisions) (flutter/flutter#184594)
2026-04-03 [email protected] refactor: remove material from list_view_viewporting_test and page_forward_transitions_test (flutter/flutter#183564)
2026-04-03 [email protected] [Dot shorthands] Migrate examples/api/lib/cupertino (flutter/flutter#183964)
2026-04-03 [email protected] Downgrade CocoaPods doctor check to warning and fix build error messaging (flutter/flutter#184511)
2026-04-03 [email protected] Roll Skia from 4134f8091147 to 3b0f0f04f97c (2 revisions) (flutter/flutter#184582)
2026-04-03 [email protected] Remove live_text_utils cross-imports from material and cupertino tests (flutter/flutter#184517)
2026-04-03 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[data_assets] Cleanup tests (#184209)" (flutter/flutter#184575)
2026-04-03 [email protected] Roll Packages from 66bf7ec to d31df66 (3 revisions) (flutter/flutter#184578)
2026-04-03 [email protected] Roll Skia from 5d847ba5c4aa to 4134f8091147 (1 revision) (flutter/flutter#184573)

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.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@rkishan516 rkishan516 deleted the simple-cross-imports-3 branch April 8, 2026 15:49
mbcorona pushed a commit to mbcorona/flutter that referenced this pull request Apr 15, 2026
…rward_transitions_test (flutter#183564)

This PR removes Material imports from list_view_viewporting_test and
page_forward_transitions_test.

This PR also adds onGenerateRoute on TestWidgetsApp.

part of: flutter#177415

## 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels.

Projects

Development

Successfully merging this pull request may close these issues.

4 participants