refactor: remove material import from scrollable_semantics_test and selectable_region_context_menu_test by rkishan516 · Pull Request #186611 · flutter/flutter

rkishan516 · 2026-05-16T06:25:21Z

This PR remove material import from scrollable_semantics_test and selectable_region_context_menu_test

Pre-launch Checklist

gemini-code-assist

Code Review

This pull request removes Material library dependencies from the scrollable_semantics_test.dart and selectable_region_context_menu_test.dart files, replacing MaterialApp with TestWidgetsApp and SliverAppBar with a custom SliverPersistentHeader delegate. It also updates the cross-import checker to reflect these changes. Feedback identifies an inconsistency where a fixed-height delegate is used instead of a collapsing one, which may not fully replicate the original SliverAppBar behavior during scrolling.

gemini-code-assist · 2026-05-16T06:27:01Z

                      pinned: true,
-                      expandedHeight: kExpandedAppBarHeight,
-                      flexibleSpace: FlexibleSpaceBar(title: Text('App Bar')),
+                      delegate: _PinnedHeaderDelegate(height: kExpandedAppBarHeight),


In this test case, the SliverPersistentHeader is initialized with a fixed-height delegate (_PinnedHeaderDelegate(height: kExpandedAppBarHeight)). However, the original SliverAppBar would have collapsed to the default toolbar height (56.0) when scrolled.

To maintain consistency with the original behavior and with the other test case implemented below (line 348), consider using the collapsing constructor here as well. This ensures that if the test involves scrolling, the header behaves as expected.

Suggested change

delegate: _PinnedHeaderDelegate(height: kExpandedAppBarHeight),

delegate: _PinnedHeaderDelegate.collapsing(

minExtent: _kToolbarHeight,

maxExtent: kExpandedAppBarHeight,

),

navaronbracke

LGTM with nit

MarlonJD · 2026-05-22T12:10:30Z

selectable_region_context_menu_test.dart is already covered by #186672, which is approved and intentionally scoped to that file. Could this PR be narrowed to scrollable_semantics_test.dart plus its allowlist entry to avoid duplicate ownership/conflicts?

rkishan516 · 2026-05-22T15:59:23Z

Hey @MarlonJD Thanks for informing. But going forward if you pickup any new file, just ensure its not picked up by anyone else already. This PR was already handling scrollable_semantics_test and selectable_region_context_menu_test while we still ended up creating duplicate PRs #186623 and #186672.

MarlonJD · 2026-05-22T16:02:34Z

You are right, thanks for calling this out. I should have checked the existing ownership more carefully before picking up those files. #186623 is already closed, and I am closing #186672 in favor of this PR to avoid keeping duplicate work in the queue. I will do a stricter open-PR check before taking new files going forward.

rkishan516 · 2026-05-22T16:06:07Z

Thank you @MarlonJD, Normally we keep posting in issue itself when we are picking that issue but since this issue contains multiple files and many people were working on it, we needed this kind of communication. But great job, I will review your another pending PRs.

flutter-dashboard · 2026-05-26T07:43:13Z

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #186611 at sha dc824a2

Renzo-Olivares

Hi @rkishan516, thank you for the contribution. Had a small comment but mostly looks good to me.

Renzo-Olivares · 2026-05-29T20:45:38Z

                return Viewport(
                  offset: offset,
                  slivers: <Widget>[
-                    const SliverAppBar(


I recommend making a copy of this test in the material library widget tests so we don't lose out on SliverAppBar test coverage here.

I have added TODO for this, once material is unfrozen for changes, will add this.

I recommend filing an issue so we don't lose track of this and adding the link to the issue in the TODO.

Filed #189117 and added the link to the TODO in packages/flutter/test/widgets/scrollable_semantics_test.dart.

Renzo-Olivares

LGTM w/ one small comment.

…electable_region_context_menu_test

…n control variable

Renzo-Olivares

LGTM

navaronbracke

LGTM

navaronbracke · 2026-07-14T07:19:47Z

Looks like Linux web_canvaskit_tests_1 was cancelled. Rebase and kick the tree a little?

flutter/flutter@cf9e8af...846664b 2026-07-14 [email protected] Roll Skia from dfcff99566c3 to 88954ef8f36d (1 revision) (flutter/flutter#189440) 2026-07-14 [email protected] refactor: remove material import from scrollable_semantics_test and selectable_region_context_menu_test (flutter/flutter#186611) 2026-07-14 [email protected] Roll Skia from 3d1fc554f1a2 to dfcff99566c3 (17 revisions) (flutter/flutter#189428) 2026-07-14 [email protected] Roll Dart SDK from 2c587df8f05a to 05bf153370c4 (5 revisions) (flutter/flutter#189426) 2026-07-14 [email protected] [flutter_tools] Remove web hot reload flag (flutter/flutter#185994) 2026-07-14 [email protected] [iOS] Fix flaky keyboard animation test (flutter/flutter#189353) 2026-07-14 [email protected] [Impeller] Playground expanded role (flutter/flutter#188889) 2026-07-14 [email protected] Roll pub packages (flutter/flutter#189409) 2026-07-13 [email protected] Update lock-threads dependency to 6.0.2 (flutter/flutter#189053) 2026-07-13 49699333+dependabot[bot]@users.noreply.github.com Bump actions/labeler from 6.1.0 to 6.2.0 in the all-github-actions group (flutter/flutter#189396) 2026-07-13 [email protected] Remove outdated todo about `analysis bug on Windows` and update condition to also perform `analysis on windows` (flutter/flutter#189283) 2026-07-13 [email protected] Add more 0x0 size tests part 4 (flutter/flutter#185187) 2026-07-13 [email protected] Roll Packages from 20928d5 to ad2eab1 (18 revisions) (flutter/flutter#189387) 2026-07-13 [email protected] [flutter_tools] Fix ADB device listing output parsing regression (flutter/flutter#189369) 2026-07-13 [email protected] Stop running most Mac x64 builders that have Mac ARM equivalents on master (flutter/flutter#189301) 2026-07-13 [email protected] Move a few benchmarks from x64 Intel Macs to ARM (flutter/flutter#189377) 2026-07-13 [email protected] Add note that `hcpp` needs impeller (flutter/flutter#189382) 2026-07-13 [email protected] Roll Fuchsia Linux SDK from vhIlDkWIy21IrlB9E... to oOETA0ISPouDt2xBo... (flutter/flutter#189349) 2026-07-13 [email protected] [flutter_tools] Respect mustMatchAppBuild on Windows native assets (flutter/flutter#186788) 2026-07-13 [email protected] Roll Skia from 8bf65996caba to 3d1fc554f1a2 (2 revisions) (flutter/flutter#189350) 2026-07-13 [email protected] Roll Dart SDK from 0fc1668c4af4 to 2c587df8f05a (9 revisions) (flutter/flutter#189351) 2026-07-13 [email protected] [web] Fall back to full CJK fonts for characters not covered by split slices (flutter/flutter#188890) 2026-07-13 [email protected] Take Mac tool_integration_tests_* out of bringup (flutter/flutter#189368) 2026-07-13 [email protected] [hooks] Roll record_use to 1.0 and unpin (flutter/flutter#189366) 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

flutter-dashboard Bot added the CICD Run CI/CD label May 16, 2026

github-actions Bot added framework flutter/packages/flutter repository. See also f: labels. a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) f: scrolling Viewports, list views, slivers, etc. labels May 16, 2026

gemini-code-assist Bot reviewed May 16, 2026

View reviewed changes

navaronbracke previously approved these changes May 16, 2026

View reviewed changes

Comment thread packages/flutter/test/widgets/selectable_region_context_menu_test.dart Outdated

rkishan516 dismissed navaronbracke’s stale review via d2c9583 May 16, 2026 13:10

rkishan516 force-pushed the simple-cross-imports-1 branch from 78c40b9 to d2c9583 Compare May 16, 2026 13:10

github-actions Bot removed the CICD Run CI/CD label May 16, 2026

rkishan516 added the CICD Run CI/CD label May 16, 2026

chunhtai requested a review from Renzo-Olivares May 20, 2026 22:02

chunhtai mentioned this pull request May 20, 2026

Remove Material import from scrollable semantics test #186623

Closed

MarlonJD mentioned this pull request May 22, 2026

Remove Material import from selectable region context menu tests #186672

Closed

github-actions Bot removed the CICD Run CI/CD label May 26, 2026

rkishan516 added the CICD Run CI/CD label May 26, 2026

flutter-dashboard Bot added the will affect goldens Changes to golden files label May 26, 2026

github-actions Bot removed the CICD Run CI/CD label May 28, 2026

rkishan516 added the CICD Run CI/CD label May 28, 2026

Renzo-Olivares reviewed May 29, 2026

View reviewed changes

Renzo-Olivares added the waiting for response The Flutter team cannot make further progress on this issue until the original reporter responds label May 29, 2026

rkishan516 force-pushed the simple-cross-imports-1 branch from 1352eaa to b11b722 Compare June 2, 2026 15:42

github-actions Bot removed the CICD Run CI/CD label Jun 2, 2026

rkishan516 added the CICD Run CI/CD label Jun 2, 2026

github-actions Bot removed the waiting for response The Flutter team cannot make further progress on this issue until the original reporter responds label Jun 2, 2026

hannah-hyj requested a review from Renzo-Olivares June 10, 2026 21:51

Renzo-Olivares previously approved these changes Jul 8, 2026

View reviewed changes

rkishan516 mentioned this pull request Jul 8, 2026

Add SliverAppBar test coverage for showOnScreen semantics #189117

Open

rkishan516 dismissed Renzo-Olivares’s stale review via 3685059 July 8, 2026 06:46

rkishan516 requested a review from Renzo-Olivares July 8, 2026 07:00

rkishan516 force-pushed the simple-cross-imports-1 branch from 3685059 to 9e202e5 Compare July 8, 2026 07:01

dkwingsmt added this to Test cross-imports Review Queue Jul 8, 2026

github-project-automation Bot moved this to Todo in Test cross-imports Review Queue Jul 8, 2026

Renzo-Olivares reviewed Jul 8, 2026

View reviewed changes

Comment thread .github/actions/composite-flutter-setup/action.yml Outdated

rkishan516 force-pushed the simple-cross-imports-1 branch from 58ec59e to 8a8c133 Compare July 9, 2026 05:41

rkishan516 added 6 commits July 13, 2026 15:51

refactor: remove material import from scrollable_semantics_test and s…

bd6f20f

…electable_region_context_menu_test

test: update text selection control from existing empty text selectio…

3c13d55

…n control variable

chore: added todo for adding material ui test once unfrozen

51302ab

Add issue link to SliverAppBar TODO

ae69686

Include lockfiles in pub cache key

6a393fb

Revert pub cache key change

70cc774

Renzo-Olivares force-pushed the simple-cross-imports-1 branch from ec1c6bf to 70cc774 Compare July 13, 2026 22:51

Renzo-Olivares added CICD Run CI/CD and removed CICD Run CI/CD labels Jul 13, 2026

Renzo-Olivares approved these changes Jul 13, 2026

View reviewed changes

navaronbracke approved these changes Jul 14, 2026

View reviewed changes

Merge branch 'master' into simple-cross-imports-1

b3d437c

navaronbracke added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 14, 2026

auto-submit Bot added this pull request to the merge queue Jul 14, 2026

Merged via the queue into flutter:master with commit 2ac447c Jul 14, 2026
172 checks passed

github-project-automation Bot moved this from Todo to Done in Test cross-imports Review Queue Jul 14, 2026

flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 14, 2026

engine-flutter-autoroll mentioned this pull request Jul 14, 2026

Roll Flutter from cf9e8afe9a5e to 846664bf191a (24 revisions) flutter/packages#12200

Merged

-                      delegate: _PinnedHeaderDelegate(height: kExpandedAppBarHeight),
+                      delegate: _PinnedHeaderDelegate.collapsing(
+                        minExtent: _kToolbarHeight,
+                        maxExtent: kExpandedAppBarHeight,
+                      ),

Uh oh!

Conversation

rkishan516 commented May 16, 2026

Pre-launch Checklist

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 16, 2026

Choose a reason for hiding this comment

Uh oh!

navaronbracke left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MarlonJD commented May 22, 2026

Uh oh!

rkishan516 commented May 22, 2026

Uh oh!

MarlonJD commented May 22, 2026

Uh oh!

rkishan516 commented May 22, 2026

Uh oh!

flutter-dashboard Bot commented May 26, 2026

Uh oh!

Renzo-Olivares left a comment

Choose a reason for hiding this comment

Uh oh!

Renzo-Olivares May 29, 2026

Choose a reason for hiding this comment

Uh oh!

rkishan516 Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

Renzo-Olivares Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rkishan516 Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

Renzo-Olivares left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Renzo-Olivares left a comment

Choose a reason for hiding this comment

Uh oh!

navaronbracke left a comment

Choose a reason for hiding this comment

Uh oh!

navaronbracke commented Jul 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Renzo-Olivares Jul 8, 2026 •

edited

Loading