Add `SingleChildScrollView` for `NavigationRail` #137415

mbfakourii · 2023-10-27T13:54:54Z

Description

Add SingleChildScrollView to NavigationRail for scrolling.
Closes: #89167

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.
All existing and new tests are passing.

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

flutter-dashboard · 2023-10-27T13:54:57Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie or stuartmorgan on the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

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

google-cla · 2023-10-27T13:54:59Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

goderbauer · 2023-11-03T21:44:20Z

Hey @mbfakourii, can you please add a test to this to make sure we never regress this? See the comment posted by the flutter-dashboard. Also, it looks like this change is failing some of the checks. Can you please take a look at that and address it? Thanks!

mbfakourii · 2023-11-04T10:00:55Z

Hey @mbfakourii, can you please add a test to this to make sure we never regress this? See the comment posted by the flutter-dashboard. Also, it looks like this change is failing some of the checks. Can you please take a look at that and address it? Thanks!

Yes, Sure. I added the test.
and regarding the flutter-dashboard fails, I executed the following command without changs this PR, and I got the same files again, as if these files are not related to this PR and NavigationRail !

../../bin/flutter test --test-randomize-ordering-seed=20231104 --fatal-warnings --no-track-widget-creation test/dart/ test/scheduler/ test/animation/ test/foundation/ test/semantics/ test/harness/ test/painting/ test/gestures/ test/examples/ test/rendering/ test/physics/ test/cupertino/ test/material/ test/services/

These fails are related to the file

flutter/packages/flutter/test/material/value_indicating_slider_test.dart

Please may check again

wjkoh · 2023-11-19T07:01:29Z

This is fantastic! I hope the PR is approved quickly.

flutter-dashboard · 2023-12-09T07:05:41Z

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

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.

goderbauer · 2023-12-11T23:04:14Z

Looks like some existing tests are failing, can you please take a look and fix those up?

mbfakourii · 2023-12-12T03:34:38Z

Looks like some existing tests are failing, can you please take a look and fix those up?

I checked the fail tests, please check here.

goderbauer · 2023-12-12T17:36:46Z

There's at least one failing test that's pretty clearly linked to the change here:

07:58 +8848 ~22: /b/s/w/ir/x/w/flutter/packages/flutter/test/material/navigation_rail_test.dart: Semantics - labelType=[none]                                                                          
══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following TestFailure was thrown running a test:
Expected: semantics node matching:
          TestSemantics(
            id: 0,
            rect: Rect.fromLTRB(0.0, 0.0, 2400.0, 1800.0),
            elevation: 0.0,
            thickness: 0.0,
            children: <TestSemantics>[
              TestSemantics(
                textDirection: TextDirection.ltr,
                children: <TestSemantics>[
                  TestSemantics(
                    children: <TestSemantics>[
                      TestSemantics(
                        flags: <SemanticsFlag>[SemanticsFlag.scopesRoute],
                        children: <TestSemantics>[
                          TestSemantics(
                            flags: <SemanticsFlag>[SemanticsFlag.isSelected,
SemanticsFlag.isFocusable],
                            actions: <SemanticsAction>[SemanticsAction.tap],
                            label: 'Abc
          Tab 1 of 4',
                            textDirection: TextDirection.ltr,
                            children: <TestSemantics>[
                            ],
                          ),
                          TestSemantics(
                            flags: <SemanticsFlag>[SemanticsFlag.isFocusable],
                            actions: <SemanticsAction>[SemanticsAction.tap],
                            label: 'Def
          Tab 2 of 4',
                            textDirection: TextDirection.ltr,
                            children: <TestSemantics>[
                            ],
                          ),
                          TestSemantics(
                            flags: <SemanticsFlag>[SemanticsFlag.isFocusable],
                            actions: <SemanticsAction>[SemanticsAction.tap],
                            label: 'Ghi
          Tab 3 of 4',
                            textDirection: TextDirection.ltr,
                            children: <TestSemantics>[
                            ],
                          ),
                          TestSemantics(
                            flags: <SemanticsFlag>[SemanticsFlag.isFocusable],
                            actions: <SemanticsAction>[SemanticsAction.tap],
                            label: 'Jkl
          Tab 4 of 4',
                            textDirection: TextDirection.ltr,
                            children: <TestSemantics>[
                            ],
                          ),
                          TestSemantics(
                            label: 'body',
                            textDirection: TextDirection.ltr,
                            children: <TestSemantics>[
                            ],
                          ),
                        ],
                      ),
                    ],
                  ),
                ],
              ),
            ],
          )
  Actual: SemanticsTester:<SemanticsTester for SemanticsNode#0(Rect.fromLTRB(0.0, 0.0, 2400.0,
1800.0))>
   Which: expected node id null to have 5 children but found 2.
          Current SemanticsNode tree:
            SemanticsNode#0
             │ Rect.fromLTRB(0.0, 0.0, 2400.0, 1800.0)
             │
             └─SemanticsNode#1
               │ Rect.fromLTRB(0.0, 0.0, 800.0, 600.0) scaled by 3.0x
               │ textDirection: ltr
               │
               └─SemanticsNode#2
                 │ Rect.fromLTRB(0.0, 0.0, 800.0, 600.0)
                 │ sortKey: OrdinalSortKey#1e145(order: 0.0)
                 │
                 └─SemanticsNode#3
                   │ Rect.fromLTRB(0.0, 0.0, 800.0, 600.0)
                   │ flags: scopesRoute
                   │
                   ├─SemanticsNode#4
                   │ │ Rect.fromLTRB(0.0, 8.0, 80.0, 184.0)
                   │ │ flags: hasImplicitScrolling
                   │ │ scrollExtentMin: 0.0
                   │ │ scrollPosition: 0.0
                   │ │ scrollExtentMax: 0.0
                   │ │
                   │ ├─SemanticsNode#5
                   │ │   Rect.fromLTRB(0.0, 0.0, 80.0, 44.0)
                   │ │   actions: tap
                   │ │   flags: isSelected, isFocusable
                   │ │   label:
                   │ │     "Abc
                   │ │     Tab 1 of 4"
                   │ │   textDirection: ltr
                   │ │
                   │ ├─SemanticsNode#6
                   │ │   Rect.fromLTRB(0.0, 44.0, 80.0, 88.0)
                   │ │   actions: tap
                   │ │   flags: isFocusable
                   │ │   label:
                   │ │     "Def
                   │ │     Tab 2 of 4"
                   │ │   textDirection: ltr
                   │ │
                   │ ├─SemanticsNode#7
                   │ │   Rect.fromLTRB(0.0, 88.0, 80.0, 132.0)
                   │ │   actions: tap
                   │ │   flags: isFocusable
                   │ │   label:
                   │ │     "Ghi
                   │ │     Tab 3 of 4"
                   │ │   textDirection: ltr
                   │ │
                   │ └─SemanticsNode#8
                   │     Rect.fromLTRB(0.0, 132.0, 80.0, 176.0)
                   │     actions: tap
                   │     flags: isFocusable
                   │     label:
                   │       "Jkl
                   │       Tab 4 of 4"
                   │     textDirection: ltr
                   │
                   └─SemanticsNode#9
                       Rect.fromLTRB(80.0, 290.0, 800.0, 310.0)
                       label: "body"
                       textDirection: ltr
          The semantics tree would have matched the following configuration:
            TestSemantics.root(
              children: <TestSemantics>[
                TestSemantics(
                  id: 1,
                  textDirection: TextDirection.ltr,
                  children: <TestSemantics>[
                    TestSemantics(
                      id: 2,
                      children: <TestSemantics>[
                        TestSemantics(
                          id: 3,
                          flags: <SemanticsFlag>[SemanticsFlag.scopesRoute],
                          children: <TestSemantics>[
                            TestSemantics(
                              id: 4,
                              flags: <SemanticsFlag>[SemanticsFlag.hasImplicitScrolling],
                              children: <TestSemantics>[
                                TestSemantics(
                                  id: 5,
                                  flags: <SemanticsFlag>[SemanticsFlag.isSelected,
SemanticsFlag.isFocusable],
                                  actions: <SemanticsAction>[SemanticsAction.tap],
                                  label: 'Abc\nTab 1 of 4',
                                  textDirection: TextDirection.ltr,
                                ),
                                TestSemantics(
                                  id: 6,
                                  flags: <SemanticsFlag>[SemanticsFlag.isFocusable],
                                  actions: <SemanticsAction>[SemanticsAction.tap],
                                  label: 'Def\nTab 2 of 4',
                                  textDirection: TextDirection.ltr,
                                ),
                                TestSemantics(
                                  id: 7,
                                  flags: <SemanticsFlag>[SemanticsFlag.isFocusable],
                                  actions: <SemanticsAction>[SemanticsAction.tap],
                                  label: 'Ghi\nTab 3 of 4',
                                  textDirection: TextDirection.ltr,
                                ),
                                TestSemantics(
                                  id: 8,
                                  flags: <SemanticsFlag>[SemanticsFlag.isFocusable],
                                  actions: <SemanticsAction>[SemanticsAction.tap],
                                  label: 'Jkl\nTab 4 of 4',
                                  textDirection: TextDirection.ltr,
                                ),
                              ],
                            ),
                            TestSemantics(
                              id: 9,
                              label: 'body',
                              textDirection: TextDirection.ltr,
                            ),
                          ],
                        ),
                      ],
                    ),
                  ],
                ),
              ],
            )

When the exception was thrown, this was the stack:
#4      main.<anonymous closure> (file:///b/s/w/ir/x/w/flutter/packages/flutter/test/material/navigation_rail_test.dart:2276:5)
<asynchronous suspension>
#5      testWidgetsWithLeakTracking.wrappedCallBack (package:leak_tracker_flutter_testing/src/test_widgets.dart:108:5)
<asynchronous suspension>
#6      testWidgets.<anonymous closure>.<anonymous closure> (package:flutter_test/src/widget_tester.dart:168:15)
<asynchronous suspension>
#7      TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:1017:5)
<asynchronous suspension>
<asynchronous suspension>
(elided one frame from package:stack_trace)

This was caught by the test expectation on the following line:
  file:///b/s/w/ir/x/w/flutter/packages/flutter/test/material/navigation_rail_test.dart line 2276
The test description was:
  Semantics - labelType=[none]
════════════════════════════════════════════════════════════════════════════════════════════════════

07:58 +8848 ~22 -1: /b/s/w/ir/x/w/flutter/packages/flutter/test/material/navigation_rail_test.dart: Semantics - labelType=[none] [E]                                                                   
  Test failed. See exception logs above.
  The test description was: Semantics - labelType=[none]

(from https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8761948290894190433/+/u/run_test.dart_for_framework_tests_shard_and_subshard_libraries/test_stdout).

That one will likely need to get fixed. If you see other unrelated test failures, try rebasing this PR with the latest master and see if they go away. The tree is currently green, so there are no unrelated existing failures.

mbfakourii · 2023-12-15T18:50:21Z

There's at least one failing test that's pretty clearly linked to the change here:

07:58 +8848 ~22: /b/s/w/ir/x/w/flutter/packages/flutter/test/material/navigation_rail_test.dart: Semantics - labelType=[none]                                                                          
══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following TestFailure was thrown running a test:
Expected: semantics node matching:
          TestSemantics(
            id: 0,
            rect: Rect.fromLTRB(0.0, 0.0, 2400.0, 1800.0),
            elevation: 0.0,
            thickness: 0.0,
            children: <TestSemantics>[
              TestSemantics(
                textDirection: TextDirection.ltr,
                children: <TestSemantics>[
                  TestSemantics(
                    children: <TestSemantics>[
                      TestSemantics(
                        flags: <SemanticsFlag>[SemanticsFlag.scopesRoute],
                        children: <TestSemantics>[
                          TestSemantics(
                            flags: <SemanticsFlag>[SemanticsFlag.isSelected,
SemanticsFlag.isFocusable],
                            actions: <SemanticsAction>[SemanticsAction.tap],
                            label: 'Abc
          Tab 1 of 4',
                            textDirection: TextDirection.ltr,
                            children: <TestSemantics>[
                            ],
                          ),
                          TestSemantics(
                            flags: <SemanticsFlag>[SemanticsFlag.isFocusable],
                            actions: <SemanticsAction>[SemanticsAction.tap],
                            label: 'Def
          Tab 2 of 4',
                            textDirection: TextDirection.ltr,
                            children: <TestSemantics>[
                            ],
                          ),
                          TestSemantics(
                            flags: <SemanticsFlag>[SemanticsFlag.isFocusable],
                            actions: <SemanticsAction>[SemanticsAction.tap],
                            label: 'Ghi
          Tab 3 of 4',
                            textDirection: TextDirection.ltr,
                            children: <TestSemantics>[
                            ],
                          ),
                          TestSemantics(
                            flags: <SemanticsFlag>[SemanticsFlag.isFocusable],
                            actions: <SemanticsAction>[SemanticsAction.tap],
                            label: 'Jkl
          Tab 4 of 4',
                            textDirection: TextDirection.ltr,
                            children: <TestSemantics>[
                            ],
                          ),
                          TestSemantics(
                            label: 'body',
                            textDirection: TextDirection.ltr,
                            children: <TestSemantics>[
                            ],
                          ),
                        ],
                      ),
                    ],
                  ),
                ],
              ),
            ],
          )
  Actual: SemanticsTester:<SemanticsTester for SemanticsNode#0(Rect.fromLTRB(0.0, 0.0, 2400.0,
1800.0))>
   Which: expected node id null to have 5 children but found 2.
          Current SemanticsNode tree:
            SemanticsNode#0
             │ Rect.fromLTRB(0.0, 0.0, 2400.0, 1800.0)
             │
             └─SemanticsNode#1
               │ Rect.fromLTRB(0.0, 0.0, 800.0, 600.0) scaled by 3.0x
               │ textDirection: ltr
               │
               └─SemanticsNode#2
                 │ Rect.fromLTRB(0.0, 0.0, 800.0, 600.0)
                 │ sortKey: OrdinalSortKey#1e145(order: 0.0)
                 │
                 └─SemanticsNode#3
                   │ Rect.fromLTRB(0.0, 0.0, 800.0, 600.0)
                   │ flags: scopesRoute
                   │
                   ├─SemanticsNode#4
                   │ │ Rect.fromLTRB(0.0, 8.0, 80.0, 184.0)
                   │ │ flags: hasImplicitScrolling
                   │ │ scrollExtentMin: 0.0
                   │ │ scrollPosition: 0.0
                   │ │ scrollExtentMax: 0.0
                   │ │
                   │ ├─SemanticsNode#5
                   │ │   Rect.fromLTRB(0.0, 0.0, 80.0, 44.0)
                   │ │   actions: tap
                   │ │   flags: isSelected, isFocusable
                   │ │   label:
                   │ │     "Abc
                   │ │     Tab 1 of 4"
                   │ │   textDirection: ltr
                   │ │
                   │ ├─SemanticsNode#6
                   │ │   Rect.fromLTRB(0.0, 44.0, 80.0, 88.0)
                   │ │   actions: tap
                   │ │   flags: isFocusable
                   │ │   label:
                   │ │     "Def
                   │ │     Tab 2 of 4"
                   │ │   textDirection: ltr
                   │ │
                   │ ├─SemanticsNode#7
                   │ │   Rect.fromLTRB(0.0, 88.0, 80.0, 132.0)
                   │ │   actions: tap
                   │ │   flags: isFocusable
                   │ │   label:
                   │ │     "Ghi
                   │ │     Tab 3 of 4"
                   │ │   textDirection: ltr
                   │ │
                   │ └─SemanticsNode#8
                   │     Rect.fromLTRB(0.0, 132.0, 80.0, 176.0)
                   │     actions: tap
                   │     flags: isFocusable
                   │     label:
                   │       "Jkl
                   │       Tab 4 of 4"
                   │     textDirection: ltr
                   │
                   └─SemanticsNode#9
                       Rect.fromLTRB(80.0, 290.0, 800.0, 310.0)
                       label: "body"
                       textDirection: ltr
          The semantics tree would have matched the following configuration:
            TestSemantics.root(
              children: <TestSemantics>[
                TestSemantics(
                  id: 1,
                  textDirection: TextDirection.ltr,
                  children: <TestSemantics>[
                    TestSemantics(
                      id: 2,
                      children: <TestSemantics>[
                        TestSemantics(
                          id: 3,
                          flags: <SemanticsFlag>[SemanticsFlag.scopesRoute],
                          children: <TestSemantics>[
                            TestSemantics(
                              id: 4,
                              flags: <SemanticsFlag>[SemanticsFlag.hasImplicitScrolling],
                              children: <TestSemantics>[
                                TestSemantics(
                                  id: 5,
                                  flags: <SemanticsFlag>[SemanticsFlag.isSelected,
SemanticsFlag.isFocusable],
                                  actions: <SemanticsAction>[SemanticsAction.tap],
                                  label: 'Abc\nTab 1 of 4',
                                  textDirection: TextDirection.ltr,
                                ),
                                TestSemantics(
                                  id: 6,
                                  flags: <SemanticsFlag>[SemanticsFlag.isFocusable],
                                  actions: <SemanticsAction>[SemanticsAction.tap],
                                  label: 'Def\nTab 2 of 4',
                                  textDirection: TextDirection.ltr,
                                ),
                                TestSemantics(
                                  id: 7,
                                  flags: <SemanticsFlag>[SemanticsFlag.isFocusable],
                                  actions: <SemanticsAction>[SemanticsAction.tap],
                                  label: 'Ghi\nTab 3 of 4',
                                  textDirection: TextDirection.ltr,
                                ),
                                TestSemantics(
                                  id: 8,
                                  flags: <SemanticsFlag>[SemanticsFlag.isFocusable],
                                  actions: <SemanticsAction>[SemanticsAction.tap],
                                  label: 'Jkl\nTab 4 of 4',
                                  textDirection: TextDirection.ltr,
                                ),
                              ],
                            ),
                            TestSemantics(
                              id: 9,
                              label: 'body',
                              textDirection: TextDirection.ltr,
                            ),
                          ],
                        ),
                      ],
                    ),
                  ],
                ),
              ],
            )

When the exception was thrown, this was the stack:
#4      main.<anonymous closure> (file:///b/s/w/ir/x/w/flutter/packages/flutter/test/material/navigation_rail_test.dart:2276:5)
<asynchronous suspension>
#5      testWidgetsWithLeakTracking.wrappedCallBack (package:leak_tracker_flutter_testing/src/test_widgets.dart:108:5)
<asynchronous suspension>
#6      testWidgets.<anonymous closure>.<anonymous closure> (package:flutter_test/src/widget_tester.dart:168:15)
<asynchronous suspension>
#7      TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:1017:5)
<asynchronous suspension>
<asynchronous suspension>
(elided one frame from package:stack_trace)

This was caught by the test expectation on the following line:
  file:///b/s/w/ir/x/w/flutter/packages/flutter/test/material/navigation_rail_test.dart line 2276
The test description was:
  Semantics - labelType=[none]
════════════════════════════════════════════════════════════════════════════════════════════════════

07:58 +8848 ~22 -1: /b/s/w/ir/x/w/flutter/packages/flutter/test/material/navigation_rail_test.dart: Semantics - labelType=[none] [E]                                                                   
  Test failed. See exception logs above.
  The test description was: Semantics - labelType=[none]

(from https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8761948290894190433/+/u/run_test.dart_for_framework_tests_shard_and_subshard_libraries/test_stdout).

That one will likely need to get fixed. If you see other unrelated test failures, try rebasing this PR with the latest master and see if they go away. The tree is currently green, so there are no unrelated existing failures.

I checked and tested this issue several times, I even returned the codes before I changed them, but I get the same error again! I don't think it is related to this PR!

goderbauer · 2024-01-19T23:10:24Z

In that output of the test there is a new scrollable semantics node that wasn't there before if you compare expected and actual:

                   ├─SemanticsNode#4
                   │ │ Rect.fromLTRB(0.0, 8.0, 80.0, 184.0)
                   │ │ flags: hasImplicitScrolling
                   │ │ scrollExtentMin: 0.0
                   │ │ scrollPosition: 0.0
                   │ │ scrollExtentMax: 0.0

This is node is almost guaranteed produced by the SingleChildScrollView added in this PR. So, in order to get the tests passing, this test will have to be adjusted to account for this extra node.

mbfakourii · 2024-01-20T08:16:29Z

In that output of the test there is a new scrollable semantics node that wasn't there before if you compare expected and actual:
                   ├─SemanticsNode#4
                   │ │ Rect.fromLTRB(0.0, 8.0, 80.0, 184.0)
                   │ │ flags: hasImplicitScrolling
                   │ │ scrollExtentMin: 0.0
                   │ │ scrollPosition: 0.0
                   │ │ scrollExtentMax: 0.0
This is node is almost guaranteed produced by the SingleChildScrollView added in this PR. So, in order to get the tests passing, this test will have to be adjusted to account for this extra node.

Fixed, can you please check again?

goderbauer

LGTM

QuncCccccc

LGTM! Nice improvement:) Thanks for the contribution!

…tter#137415)

Manual roll Flutter from 2f6fdf2 to ace9181 (57 revisions) Manual roll requested by [email protected] flutter/flutter@2f6fdf2...ace9181 2024-01-29 [email protected] Roll Flutter Engine from 9c3ebf67b5da to bedafa8794b6 (4 revisions) (flutter/flutter#142478) 2024-01-29 [email protected] onNavigationNotification for *App.router (flutter/flutter#142190) 2024-01-29 [email protected] Marks Mac_x64 framework_tests_misc to be unflaky (flutter/flutter#142118) 2024-01-29 [email protected] Roll Flutter Engine from 3e2b8975bd5b to 9c3ebf67b5da (1 revision) (flutter/flutter#142472) 2024-01-29 [email protected] Feat: TextField can scroll when disabled (flutter/flutter#140922) 2024-01-29 [email protected] Roll Flutter Engine from 436f91f3b06b to 3e2b8975bd5b (2 revisions) (flutter/flutter#142466) 2024-01-29 [email protected] Marks Mac_arm64 framework_tests_misc to be unflaky (flutter/flutter#142119) 2024-01-29 [email protected] Fix InputDecorationTheme copyWith fallback for iconColor (flutter/flutter#142462) 2024-01-29 [email protected] Add `SingleChildScrollView` for `NavigationRail` (flutter/flutter#137415) 2024-01-29 [email protected] [Windows Arm64] Run plugin test post-submit (flutter/flutter#141987) 2024-01-29 [email protected] Catch file system exceptions when trying to parse user-provided asset file paths (flutter/flutter#142214) 2024-01-29 [email protected] Implementing `switch` expressions in `foundation/` and `material/` (flutter/flutter#142279) 2024-01-29 [email protected] Roll Flutter Engine from bff1e46c0d65 to 436f91f3b06b (1 revision) (flutter/flutter#142455) 2024-01-29 [email protected] Opt out test from leak tracking. (flutter/flutter#142417) 2024-01-29 [email protected] Update Android minSdkVersion to 21 (flutter/flutter#142267) 2024-01-29 [email protected] Roll Flutter Engine from 3d87470655b1 to bff1e46c0d65 (1 revision) (flutter/flutter#142446) 2024-01-29 [email protected] Roll Packages from cbe8100 to 516648a (3 revisions) (flutter/flutter#142445) 2024-01-29 [email protected] Roll Flutter Engine from 1405cb7b6e74 to 3d87470655b1 (1 revision) (flutter/flutter#142444) 2024-01-29 [email protected] Roll Flutter Engine from 3e65f1720a6f to 1405cb7b6e74 (1 revision) (flutter/flutter#142432) 2024-01-29 [email protected] Roll Flutter Engine from 210ed1dfb8cf to 3e65f1720a6f (1 revision) (flutter/flutter#142429) 2024-01-29 [email protected] Roll Flutter Engine from f15cb86d31c3 to 210ed1dfb8cf (1 revision) (flutter/flutter#142426) 2024-01-29 [email protected] Roll Flutter Engine from f3d48be76630 to f15cb86d31c3 (11 revisions) (flutter/flutter#142425) 2024-01-27 [email protected] Roll Flutter Engine from 95e9a15fd909 to f3d48be76630 (1 revision) (flutter/flutter#142370) 2024-01-27 [email protected] Add no-shuffle to language_version_test.dart (flutter/flutter#142378) 2024-01-27 [email protected] Roll Flutter Engine from 2687ddb2655c to 95e9a15fd909 (8 revisions) (flutter/flutter#142369) 2024-01-27 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Flutter Engine from 2687ddb2655c to 2adad88a39f4 (4 revisions)" (flutter/flutter#142366) 2024-01-27 [email protected] Roll Flutter Engine from 2687ddb2655c to 2adad88a39f4 (4 revisions) (flutter/flutter#142362) 2024-01-27 [email protected] Roll Flutter Engine from 45c06c22d5c7 to 2687ddb2655c (1 revision) (flutter/flutter#142359) 2024-01-27 [email protected] Remove suspicious constant from input decorator layout (flutter/flutter#142342) 2024-01-27 [email protected] Roll Flutter Engine from 2e32acf4c31a to 45c06c22d5c7 (2 revisions) (flutter/flutter#142353) 2024-01-27 [email protected] Roll Flutter Engine from a65a1b55e06a to 2e32acf4c31a (1 revision) (flutter/flutter#142351) 2024-01-26 [email protected] Roll Flutter Engine from 525bd7dcf7f3 to a65a1b55e06a (11 revisions) (flutter/flutter#142347) 2024-01-26 [email protected] Relands "Add runWidget to bootstrap a widget tree without a default View" (flutter/flutter#142344) 2024-01-26 [email protected] Fix assertion failure when reordering two dimensional children (flutter/flutter#141504) 2024-01-26 [email protected] Limit `fuchsia_precache` in presubmit to engine rolls (flutter/flutter#142333) 2024-01-26 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.23.1 to 3.23.2 (flutter/flutter#142345) 2024-01-26 [email protected] refactor asset bundle code to not depend on the global Cache.flutterRoot (flutter/flutter#142277) 2024-01-26 [email protected] [flutter_tools] remove await runZonedGuarded() in tests (flutter/flutter#142336) 2024-01-26 [email protected] Only use iOS 17 physical devices in staging tests (flutter/flutter#142323) 2024-01-26 [email protected] Roll deps from dart-lang/native in templates (flutter/flutter#142322) 2024-01-26 [email protected] Remove duplicate global declaration of `UserMessages` (flutter/flutter#142281) 2024-01-26 [email protected] Redistribute iOS TESTOWNERS (flutter/flutter#142315) 2024-01-26 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Add runWidget to bootstrap a widget tree without a default View" (flutter/flutter#142339) 2024-01-26 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Flutter Engine from 525bd7dcf7f3 to 65d1291c3add (1 revision)" (flutter/flutter#142332) ...

TahaTesser · 2024-02-07T09:52:21Z

This seems to cause a regression, see #143061

goderbauer · 2024-02-07T18:46:11Z

Reason for revert: #143061, this is breaking some existing use cases, see also flutter/samples#2157. If we try this again, we need to add this in less breaking way.

auto-submit · 2024-02-07T18:46:26Z

Time to revert pull request flutter/flutter/137415 has elapsed.
You need to open the revert manually and process as a regular pull request.

This reverts commit fd7f45a.

Reverts #137415 Reverting due to #143061. this is breaking some existing use cases, see also flutter/samples#2157. If we try this again, we need to add this in less breaking way. Fixes #143061

This is modified and extended version of the merged and reverted PR #137415. It was reverted, because it broke a the sample app "Material 3 demo". This PR circumvents the breaking of existing code, by making the changes optional via constructor parameters. Default values for the parameters are functionally equivalent to the state before this change. The new key parameter for this is `scrollable`. Additionally this change includes the parameters `leadingAtTop: true` and `trailingAtBottom: false`. Fixes #89167 ## 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. If you need help, consider asking for advice on the #hackers-new channel on [Discord].  [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

…169421)  This is modified and extended version of the merged and reverted PR flutter#137415. It was reverted, because it broke a the sample app "Material 3 demo". This PR circumvents the breaking of existing code, by making the changes optional via constructor parameters. Default values for the parameters are functionally equivalent to the state before this change. The new key parameter for this is `scrollable`. Additionally this change includes the parameters `leadingAtTop: true` and `trailingAtBottom: false`. Fixes flutter#89167 ## 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. If you need help, consider asking for advice on the #hackers-new channel on [Discord].  [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

feat: Add SingleChildScrollView for NavigationRail

2f3e4b3

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Oct 27, 2023

mbfakourii added 2 commits October 27, 2023 18:25

fix code format

92a287f

Merge branch 'master' into navigationrail_scrollable

68bc289

HansMuller requested a review from goderbauer November 3, 2023 21:41

mbfakourii added 3 commits November 4, 2023 10:36

Merge branch 'master' into navigationrail_scrollable

5a00ff7

test: add NavigationRail can scroll in low height test

207c77c

Merge branch 'master' into navigationrail_scrollable

2660883

mbfakourii added 2 commits November 6, 2023 04:55

Merge branch 'master' into navigationrail_scrollable

6a4bfaf

Merge branch 'master' into navigationrail_scrollable

2449a4b

Merge branch 'master' into navigationrail_scrollable

3b34a3b

Merge branch 'master' into navigationrail_scrollable

189f4d8

mbfakourii added 2 commits December 15, 2023 19:06

Merge branch 'master' into navigationrail_scrollable

5fcc5a6

Merge branch 'flutter:master' into navigationrail_scrollable

5bd1f5a

mbfakourii added 2 commits January 20, 2024 07:58

Merge branch 'master' into navigationrail_scrollable

f65944d

fix: Fix Semantics in navigation_rail_test.dart

dc3e8c2

Merge branch 'master' into navigationrail_scrollable

3d547fe

goderbauer approved these changes Jan 29, 2024

View reviewed changes

goderbauer requested a review from QuncCccccc January 29, 2024 18:21

QuncCccccc approved these changes Jan 29, 2024

View reviewed changes

QuncCccccc added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 29, 2024

auto-submit bot merged commit fd7f45a into flutter:master Jan 29, 2024

engine-flutter-autoroll mentioned this pull request Jan 30, 2024

Manual roll Flutter from 2f6fdf2650d4 to ace9181172b6 (57 revisions) flutter/packages#6013

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 30, 2024

fd7f45a Add SingleChildScrollView for NavigationRail (flutter/flu…

2f4716e

…tter#137415)

engine-flutter-autoroll mentioned this pull request Jan 30, 2024

Roll Flutter from 2f6fdf2650d4 to ccb2b4439a43 (68 revisions) flutter/packages#6014

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 30, 2024

fd7f45a Add SingleChildScrollView for NavigationRail (flutter/flu…

30dd763

…tter#137415)

engine-flutter-autoroll mentioned this pull request Jan 30, 2024

Roll Flutter from 2f6fdf2650d4 to 72dc162ff052 (69 revisions) flutter/packages#6015

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 30, 2024

fd7f45a Add SingleChildScrollView for NavigationRail (flutter/flu…

f328fbd

…tter#137415)

HansMuller mentioned this pull request Feb 2, 2024

Flutter Framework Team: Notable Commits #121415

Closed

TahaTesser mentioned this pull request Feb 7, 2024

Material 3 demo doesn't load the left color sidebar with latest dev channel #143061

Closed

johnpryan mentioned this pull request Feb 7, 2024

Remove Expanded from NavigationRail flutter/samples#2157

Closed

goderbauer added the revert Autorevert PR (with "Reason for revert:" comment) label Feb 7, 2024

auto-submit bot removed the revert Autorevert PR (with "Reason for revert:" comment) label Feb 7, 2024

goderbauer added a commit that referenced this pull request Feb 7, 2024

Revert "Add SingleChildScrollView for NavigationRail (#137415)"

0c2f39a

This reverts commit fd7f45a.

goderbauer mentioned this pull request Feb 7, 2024

Revert "Add SingleChildScrollView for NavigationRail" #143097

Merged

mbfakourii deleted the navigationrail_scrollable branch February 11, 2024 06:46

pogojotz mentioned this pull request May 24, 2025

NavigationRail: optionally scrollable and more configurable #169421

Merged

9 tasks

pogojotz mentioned this pull request Jun 2, 2025

Making NavigationRail scrollable #89167

Closed

Add SingleChildScrollView for NavigationRail #137415

Add SingleChildScrollView for NavigationRail #137415

Uh oh!

Conversation

mbfakourii commented Oct 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pre-launch Checklist

Uh oh!

flutter-dashboard bot commented Oct 27, 2023

Uh oh!

google-cla bot commented Oct 27, 2023

Uh oh!

goderbauer commented Nov 3, 2023

Uh oh!

mbfakourii commented Nov 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wjkoh commented Nov 19, 2023

Uh oh!

flutter-dashboard bot commented Dec 9, 2023

Uh oh!

goderbauer commented Dec 11, 2023

Uh oh!

mbfakourii commented Dec 12, 2023

Uh oh!

goderbauer commented Dec 12, 2023

Uh oh!

mbfakourii commented Dec 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

goderbauer commented Jan 19, 2024

Uh oh!

mbfakourii commented Jan 20, 2024

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

QuncCccccc left a comment

Choose a reason for hiding this comment

Uh oh!

TahaTesser commented Feb 7, 2024

Uh oh!

goderbauer commented Feb 7, 2024

Uh oh!

auto-submit bot commented Feb 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Add `SingleChildScrollView` for `NavigationRail` #137415

Add `SingleChildScrollView` for `NavigationRail` #137415

mbfakourii commented Oct 27, 2023 •

edited

Loading

mbfakourii commented Nov 4, 2023 •

edited

Loading

mbfakourii commented Dec 15, 2023 •

edited

Loading