search

flutter / flutter

Flutter makes it easy and fast to build beautiful apps for mobile and beyond
https://flutter.dev
BSD 3-Clause "New" or "Revised" License
162.28k stars 26.66k forks source link

[go_router] iOS ONLY - scroll to top not working when using StatefulNavigationShell #131829

Open lkp-k opened 9 months ago

lkp-k commented 9 months ago

Is there an existing issue for this?

Steps to reproduce

  1. Set up GoRouter with StatefulShellRoute.indexedStack
  2. Use the navigationShell to build the navigation layout
  3. Tap status bar and scroll to top does not work

Expected results

Tapping the status bar should scroll to the top. For some reason using the navigationShell breaks this functionality.

Actual results

Scroll to top is not working.

Code sample

Router Setup ```dart final _rootNavigatorKey = GlobalKey(); final _shellNavigatorHomeKey = GlobalKey(debugLabel: 'shellHome'); final GoRouter router = GoRouter( initialLocation: "/home", navigatorKey: _rootNavigatorKey, routes: [ StatefulShellRoute.indexedStack( parentNavigatorKey: _rootNavigatorKey, builder: (context, state, navigationShell) { return Gatekeeper(navigationShell: navigationShell); }, branches: [ StatefulShellBranch( navigatorKey: _shellNavigatorHomeKey, routes: [ GoRoute( path: '/home', pageBuilder: (context, state) => const NoTransitionPage( child: HomeScreen(), ), routes: [ // child route ], ), ], ), ], ), ), ); ```
Using body: navigationShell breaks the scroll to top functionality ```dart class Gatekeeper extends HookConsumerWidget { final StatefulNavigationShell navigationShell; const Gatekeeper({ Key? key, required this.navigationShell, }) : super(key: key ?? const ValueKey('ScaffoldWithNestedNavigation')); void goBranch(int index) { navigationShell.goBranch( index, initialLocation: index == navigationShell.currentIndex, ); } Widget build(context, ref) { return Scaffold( key: key, body: navigationShell, // <----- this is what causes the issue. Replace this with anything else, like a ListView with 50 elements, and tapping status bar to scroll top should work bottomNavigationBar: BottomNavigationBar( showSelectedLabels: false, showUnselectedLabels: false, selectedFontSize: 12, unselectedFontSize: 12, iconSize: 25, currentIndex: navigationShell.currentIndex, onTap: goBranch, items: [ const BottomNavigationBarItem( icon: Icon(MaterialCommunityIcons.calendar_check_outline), label: "Home", ), ], ), ); } } ```
Home example ```dart class HomeScreen extends StatelessWidget { @override Widget build(context) { final items = List.generate(50, (i) => 'Item $i'); return Scaffold( body: ListView.builder( itemCount: items.length, prototypeItem: ListTile( title: Text(items.first), ), itemBuilder: (context, index) { return ListTile( title: Text(items[index]), ); }, ), ); } } ```

Screenshots or Video

Screenshots / Video demonstration [Upload media here]

Logs

Logs ```console [Paste your logs here] ```

Flutter Doctor output

Doctor output ```console [✓] Flutter (Channel stable, 3.10.6, on macOS 13.4.1 22F770820d darwin-x64, locale en-US) • Flutter version 3.10.6 on channel stable at /usr/local/Caskroom/flutter/2.2.3/flutter • Upstream repository https://github.com/flutter/flutter.git • Framework revision f468f3366c (3 weeks ago), 2023-07-12 15:19:05 -0700 • Engine revision cdbeda788a • Dart version 3.0.6 • DevTools version 2.23.1 [✓] Android toolchain - develop for Android devices (Android SDK version 32.1.0-rc1) • Android SDK at /Users/lkp-k/Library/Android/sdk • Platform android-33, build-tools 32.1.0-rc1 • ANDROID_HOME = /Users/lkp-k/Library/Android/sdk • Java binary at: /Applications/Android Studio.app/Contents/jre/Contents/Home/bin/java • Java version OpenJDK Runtime Environment (build 11.0.12+0-b1504.28-7817840) • All Android licenses accepted. [✓] Xcode - develop for iOS and macOS (Xcode 14.3.1) • Xcode at /Applications/Xcode.app/Contents/Developer • Build 14E300c • CocoaPods version 1.12.1 [✓] Chrome - develop for the web • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome [✓] Android Studio (version 2021.2) • Android Studio at /Applications/Android Studio.app/Contents • Flutter plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/9212-flutter • Dart plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/6351-dart • Java version OpenJDK Runtime Environment (build 11.0.12+0-b1504.28-7817840) [✓] VS Code (version 1.80.1) • VS Code at /Applications/Visual Studio Code.app/Contents • Flutter extension version 3.70.0 [✓] Connected device (3 available) • DARK MODE (mobile) • 00000000-000000000000000 • ios • iOS 16.5.1 20F75 • macOS (desktop) • macos • darwin-x64 • macOS 13.4.1 22F770820d darwin-x64 • Chrome (web) • chrome • web-javascript • Google Chrome 115.0.5790.114 [✓] Network resources • All expected network resources are available. ```
huycozy commented 9 months ago

Thanks for the report. I can reproduce this issue with below minimal sample code.

As far as I can see, this is a known issue with nested navigation: https://github.com/flutter/flutter/issues/85603. You can take a look at https://github.com/flutter/flutter/issues/85603#issuecomment-876798161 where the solution is pointing out there.

It seems we need to pass PrimaryScrollController down to shell route in the stack, but I haven't found a way to do it now. Labeling the issue for more ideas.

Complete sample code ```dart import 'package:flutter/material.dart'; import 'package:go_router/go_router.dart'; final _rootNavigatorKey = GlobalKey(); final _shellNavigatorHomeKey = GlobalKey(debugLabel: 'shellHome'); final GoRouter router = GoRouter( initialLocation: "/home", navigatorKey: _rootNavigatorKey, routes: [ StatefulShellRoute.indexedStack( parentNavigatorKey: _rootNavigatorKey, builder: (context, state, navigationShell) { return Gatekeeper(navigationShell: navigationShell); }, branches: [ StatefulShellBranch( navigatorKey: _shellNavigatorHomeKey, routes: [ GoRoute( path: '/home', pageBuilder: (context, state) => const NoTransitionPage( child: HomeScreen(), ), ), ], ), StatefulShellBranch( routes: [ GoRoute( path: '/details', pageBuilder: (context, state) => const NoTransitionPage( child: MyWidget(), ), ), ], ), ], ), ], ); void main() { runApp( MaterialApp.router( theme: ThemeData(useMaterial3: true), routerConfig: router, ), ); } class Gatekeeper extends StatelessWidget { final StatefulNavigationShell navigationShell; const Gatekeeper({ Key? key, required this.navigationShell, }) : super(key: key ?? const ValueKey('ScaffoldWithNestedNavigation')); void goBranch(int index) { navigationShell.goBranch( index, initialLocation: index == navigationShell.currentIndex, ); } @override Widget build(BuildContext context) { return Scaffold( body: navigationShell, bottomNavigationBar: BottomNavigationBar( showSelectedLabels: false, showUnselectedLabels: false, selectedFontSize: 12, unselectedFontSize: 12, iconSize: 25, currentIndex: navigationShell.currentIndex, onTap: goBranch, items: const [ BottomNavigationBarItem( icon: Icon(Icons.home), label: "Home", ), BottomNavigationBarItem( icon: Icon(Icons.ac_unit), label: "ABC", ), ], ), ); } } class HomeScreen extends StatelessWidget { const HomeScreen({super.key}); @override Widget build(context) { final items = List.generate(50, (i) => 'Item $i'); return ListView.builder( itemCount: items.length, prototypeItem: ListTile( title: Text(items.first), ), itemBuilder: (context, index) { return ListTile( title: Text(items[index]), ); }, ); } } class MyWidget extends StatelessWidget { const MyWidget({super.key}); @override Widget build(BuildContext context) { return const Placeholder(); } } ```
flutter doctor -v (stable and master) ```bash [✓] Flutter (Channel stable, 3.10.6, on macOS 13.5 22G74 darwin-x64, locale en-VN) • Flutter version 3.10.6 on channel stable at /Users/huynq/Documents/GitHub/flutter • Upstream repository https://github.com/flutter/flutter.git • Framework revision f468f3366c (3 weeks ago), 2023-07-12 15:19:05 -0700 • Engine revision cdbeda788a • Dart version 3.0.6 • DevTools version 2.23.1 [✓] Android toolchain - develop for Android devices (Android SDK version 32.0.0) • Android SDK at /Users/huynq/Library/Android/sdk • Platform android-33, build-tools 32.0.0 • ANDROID_HOME = /Users/huynq/Library/Android/sdk • Java binary at: /Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b802.4-9586694) • All Android licenses accepted. [✓] Xcode - develop for iOS and macOS (Xcode 15.0) • Xcode at /Applications/Xcode-beta.app/Contents/Developer • Build 15A5195m • CocoaPods version 1.11.3 [✓] Chrome - develop for the web • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome [✓] Android Studio (version 2022.2) • Android Studio at /Applications/Android Studio.app/Contents • Flutter plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/9212-flutter • Dart plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/6351-dart • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b802.4-9586694) [✓] VS Code (version 1.80.2) • VS Code at /Applications/Visual Studio Code.app/Contents • Flutter extension version 3.68.0 [✓] Connected device (2 available) • macOS (desktop) • macos • darwin-x64 • macOS 13.5 22G74 darwin-x64 • Chrome (web) • chrome • web-javascript • Google Chrome 115.0.5790.114 [✓] Network resources • All expected network resources are available. • No issues found! ``` ```bash [!] Flutter (Channel master, 3.13.0-16.0.pre.41, on macOS 13.5 22G74 darwin-x64, locale en-VN) • Flutter version 3.13.0-16.0.pre.41 on channel master at /Users/huynq/Documents/GitHub/flutter_master ! Warning: `flutter` on your path resolves to /Users/huynq/Documents/GitHub/flutter/bin/flutter, which is not inside your current Flutter SDK checkout at /Users/huynq/Documents/GitHub/flutter_master. Consider adding /Users/huynq/Documents/GitHub/flutter_master/bin to the front of your path. ! Warning: `dart` on your path resolves to /Users/huynq/Documents/GitHub/flutter/bin/dart, which is not inside your current Flutter SDK checkout at /Users/huynq/Documents/GitHub/flutter_master. Consider adding /Users/huynq/Documents/GitHub/flutter_master/bin to the front of your path. • Upstream repository https://github.com/flutter/flutter.git • Framework revision b3a4decda6 (3 hours ago), 2023-08-02 19:18:13 -0500 • Engine revision 9304c0074d • Dart version 3.2.0 (build 3.2.0-27.0.dev) • DevTools version 2.26.1 • If those were intentional, you can disregard the above warnings; however it is recommended to use "git" directly to perform update checks and upgrades. [✓] Android toolchain - develop for Android devices (Android SDK version 32.0.0) • Android SDK at /Users/huynq/Library/Android/sdk • Platform android-33, build-tools 32.0.0 • ANDROID_HOME = /Users/huynq/Library/Android/sdk • Java binary at: /Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b802.4-9586694) • All Android licenses accepted. [✓] Xcode - develop for iOS and macOS (Xcode 14.3) • Xcode at /Applications/Xcode.app/Contents/Developer • Build 14E222b • CocoaPods version 1.11.3 [✓] Chrome - develop for the web • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome [✓] Android Studio (version 2022.2) • Android Studio at /Applications/Android Studio.app/Contents • Flutter plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/9212-flutter • Dart plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/6351-dart • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b802.4-9586694) [✓] VS Code (version 1.80.2) • VS Code at /Applications/Visual Studio Code.app/Contents • Flutter extension version 3.70.0 [✓] Connected device (3 available) • RMX2001 (mobile) • EUYTFEUSQSRGDA6D • android-arm64 • Android 11 (API 30) • macOS (desktop) • macos • darwin-x64 • macOS 13.5 22G74 darwin-x64 • Chrome (web) • chrome • web-javascript • Google Chrome 115.0.5790.114 [✓] Network resources • All expected network resources are available. ! Doctor found issues in 1 category. ```
lkp-k commented 9 months ago

Thank you so much. I checked out the suggestion and it does work, and please let me know if I can help in some way as well.

Sorry I'm unfortunately not a pro at the internals of flutter implementations (trying to build MVPs fast) so please ignore this if it is wildly incorrect but since you offer GoRouter.of(context), would it made sense to offer a list or map of PrimaryScrollController per stack there, so for ex. GoRouter.of(context).primaryScrollControllers.byKey()?

In my case, I am using riverpod so I just created a primary scroll state, and utilized the key map. It does work, but it does feel like an unnecessary thing. Maybe the underlying StatefulNavigationShell can make a list, and use the index (already being used to show the right route) to give the right scroll controller in the context.

zsilverzweig commented 1 week ago

We have this on our app as well. Nothing new to add, but happy to pay for a coffee for someone to fix it ☕