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
165.01k stars 27.19k forks source link

Navigator 2: Screen not update when I delete the last x pages from Navigator.pages #82437

Open SchabanBo opened 3 years ago

SchabanBo commented 3 years ago

Steps to Reproduce

  1. Run flutter create bug.
  2. Update main as follows:
    Navigato 2 Code
  import 'package:flutter/material.dart';

  void main() {
    runApp(MyApp());
  }

  class AppState extends ChangeNotifier {
    final List<int> _indexs = List.generate(5, (index) => index);

    int _selectedIndex = 0;

    int get selectedIndex => _selectedIndex;

    set selectedIndex(int id) {
      _selectedIndex = id;
      notifyListeners();
    }

    List<int> get indexs => _indexs;
  }

  class MyApp extends StatefulWidget {
    @override
    State<StatefulWidget> createState() => _MyAppState();
  }

  class _MyAppState extends State<MyApp> {
    final IndexRouterDelegate _routerDelegate = IndexRouterDelegate();
    final IndexRouteInformationParser _routeInformationParser =
        IndexRouteInformationParser();

    @override
    void dispose() {
      _routerDelegate.dispose();
      super.dispose();
    }

    @override
    Widget build(BuildContext context) {
      return MaterialApp.router(
        title: 'Index App',
        routerDelegate: _routerDelegate,
        routeInformationParser: _routeInformationParser,
      );
    }
  }

  class IndexRouteInformationParser extends RouteInformationParser<int> {
    @override
    Future<int> parseRouteInformation(RouteInformation routeInformation) async {
      if (routeInformation.location == '/') {
        return 0;
      }
      final uri = Uri.parse(routeInformation.location!);
      final id = int.tryParse(uri.pathSegments[0]);
      return id!;
    }

    @override
    RouteInformation restoreRouteInformation(int path) =>
        RouteInformation(location: '/$path');
  }

  class IndexRouterDelegate extends RouterDelegate<int>
      with ChangeNotifier, PopNavigatorRouterDelegateMixin<int> {
    @override
    final GlobalKey<NavigatorState> navigatorKey;
    final AppState _appState = AppState();

    IndexRouterDelegate() : navigatorKey = GlobalKey<NavigatorState>() {
      _appState.addListener(() => notifyListeners());
    }

    @override
    void dispose() {
      _appState.dispose();
      super.dispose();
    }

    @override
    int get currentConfiguration => _appState.selectedIndex;

    List<Page> get getPages {
      final result = <Page>[];
      for (var i = 0; i <= _appState.selectedIndex; i++)
        result.add(IndexPage(i: _appState.indexs[i], state: _appState));
      print('Pages count : ${result.length}');
      return List.unmodifiable(result);
    }

    @override
    Widget build(BuildContext context) {
      return Navigator(
        key: navigatorKey,
        pages: getPages,
        onPopPage: (route, result) {
          if (!route.didPop(result)) {
            return false;
          }
          return true;
        },
      );
    }

    @override
    Future<void> setNewRoutePath(int path) async {
      _appState.selectedIndex = path;
    }
  }

  class IndexPage extends Page<dynamic> {
    final int i;
    final AppState state;

    IndexPage({
      required this.i,
      required this.state,
    }) : super(key: ValueKey(i));

    @override
    Route<dynamic> createRoute(BuildContext context) {
      return MaterialPageRoute(
        settings: this,
        builder: (BuildContext context) {
          return IndexScreen(
            index: i,
            state: state,
          );
        },
      );
    }
  }

  class IndexScreen extends StatelessWidget {
    final int index;
    final AppState state;
    IndexScreen({
      required this.state,
      required this.index,
    });

    @override
    Widget build(BuildContext context) {
      return Scaffold(
        appBar: AppBar(),
        drawer: Drawer(
          child: ListView(
            children: [
              for (var i = 0; i < state.indexs.length; i++)
                ListTile(
                  title: Text("$i"),
                  onTap: () => state.selectedIndex = i,
                )
            ],
          ),
        ),
        body: Padding(
          padding: const EdgeInsets.all(8.0),
          child: Center(
              child:
                  Text('$index', style: Theme.of(context).textTheme.headline6)),
        ),
      );
    }
  }
```

  1. Follow these steps

    • Run the app
    • Open Drawer
    • Go to page 1
    • Go to page 2
    • Open Drawer and go back to page 0 or 1

Expected results: Page 0 or 1 is shown

Actual results: Page 2 is shown (although it is not in Navigator.page) Check the logs when you are navigating, the pages.length will be logged

Logs ## flutter doctor -v [√] Flutter (Channel dev, 2.3.0-1.0.pre, on Microsoft Windows [Version 10.0.19042.928], locale en-US) • Flutter version 2.3.0-1.0.pre at C:\Tools\flutter • Upstream repository https://github.com/flutter/flutter.git • Framework revision d97f41caed (13 days ago), 2021-04-30 12:35:21 -0700 • Engine revision e7939e091e • Dart version 2.14.0 (build 2.14.0-48.0.dev) [√] Android toolchain - develop for Android devices (Android SDK version 30.0.3) • Android SDK at C:\Users\Schab\AppData\Local\Android\sdk • Platform android-30, build-tools 30.0.3 • Java binary at: C:\Program Files\Android\Android Studio\jre\bin\java • Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b01) • All Android licenses accepted. [√] Chrome - develop for the web • CHROME_EXECUTABLE = C:\Users\Schab\xmlhttperror.cmd [√] Visual Studio - develop for Windows (Visual Studio Community 2019 16.9.4) • Visual Studio at C:\Program Files (x86)\Microsoft Visual Studio\2019\Community • Visual Studio Community 2019 version 16.9.31205.134 • Windows 10 SDK version 10.0.19041.0 [√] Android Studio (version 4.1.0) • Android Studio at C:\Program Files\Android\Android Studio • 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 1.8.0_242-release-1644-b01) [√] VS Code (version 1.56.1) • VS Code at C:\Users\Schab\AppData\Local\Programs\Microsoft VS Code • Flutter extension version 3.22.0 [√] Connected device (3 available) • Windows (desktop) • windows • windows-x64 • Microsoft Windows [Version 10.0.19042.928] • Chrome (web) • chrome • web-javascript • Google Chrome 90.0.4430.212 • Edge (web) • edge • web-javascript • Microsoft Edge 90.0.818.49 • No issues found!

https://user-images.githubusercontent.com/49782771/118126030-0447ae80-b3f8-11eb-8f2b-f0219b2b6f78.mp4

darshankawar commented 3 years ago

@SchabanBo Although I see the same behavior as you mentioned, but since you are using Navigator 2.0 concept in your code, I'd like you to go through this issue that discussed challenges faced using Navigator 2.0, https://github.com/flutter/flutter/issues/69315.

If you go through that link, it was closed in favor of providing your feedback using navigator 2.0 usability api here, https://github.com/flutter/uxr/wiki/Navigator-2.0-API-Usability-Research and https://github.com/flutter/uxr/issues/6

You are welcome to share this behavior in linked issue to get more attention to it.

But I'll label this issue here in case if it's needed here.

darshankawar commented 3 years ago
Screenshot 2021-05-13 at 6 47 27 PM
flutter doctor -v ``` [✓] Flutter (Channel stable, 2.0.6, on Mac OS X 10.15.4 19E2269 darwin-x64, locale en-GB) • Flutter version 2.0.6 at /Users/dhs/documents/fluttersdk/flutter • Framework revision 1d9032c7e1 (6 days ago), 2021-04-29 17:37:58 -0700 • Engine revision 05e680e202 • Dart version 2.12.3 [!] Xcode - develop for iOS and macOS • Xcode at /Applications/Xcode.app/Contents/Developer • Xcode 12.3, Build version 12C33 ! CocoaPods 1.9.3 out of date (1.10.0 is recommended). CocoaPods is used to retrieve the iOS and macOS platform side's plugin code that responds to your plugin usage on the Dart side. Without CocoaPods, plugins will not work on iOS or macOS. For more info, see https://flutter.dev/platform-plugins To upgrade see https://guides.cocoapods.org/using/getting-started.html#installation for instructions. [✓] Chrome - develop for the web • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome [✓] Android Studio (version 4.1) • 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 1.8.0_242-release-1644-b3-6915495) [✓] VS Code (version 1.55.0) • VS Code at /Applications/Visual Studio Code.app/Contents • Flutter extension version 3.20.0 [✓] Connected device (3 available) • SM G975F (mobile) • RZ8M802WY0X • android-arm64 • Android 10 (API 29) • Darshan's iphone (mobile) • 21150b119064aecc249dfcfe05e259197461ce23 • ios • iOS 14.4.1 • macOS (desktop) • macos • darwin-x64 • Mac OS X 10.15.4 19E2269 darwin-x64 • Chrome (web) • chrome • web-javascript • Google Chrome 89.0.4389.114 ! Doctor found issues in 1 category. ```
SchabanBo commented 3 years ago

I really tried to understand what is happing here. and it seems this problem is related to the Drawer. if you change the Index screen to this

class IndexScreen extends StatelessWidget {
  final int index;
  final AppState state;
  IndexScreen({
    required this.state,
    required this.index,
  });

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(),
      drawer: Drawer(
        child: ListView(
          children: [
            for (var i = 0; i < state.indexs.length; i++)
              ListTile(
                title: Text("$i"),
                onTap: () => state.selectedIndex = i,
              )
          ],
        ),
      ),
      body: Padding(
        padding: const EdgeInsets.all(8.0),
        child: Center(
            child: Column(
          children: [
            Text('$index', style: Theme.of(context).textTheme.headline6),
            for (var i = 0; i < state.indexs.length; i++)
              ListTile(
                title: Text("$i"),
                onTap: () => state.selectedIndex = i,
              )
          ],
        )),
      ),
    );
  }
}

and navigate from the ListTile in the column, it works fine. but if you navigate from the drawer you will get this weird behavior.

yjbanov commented 3 years ago

/cc @goderbauer this looks like a framework-side issue

goderbauer commented 3 years ago

/cc @chunhtai

chunhtai commented 3 years ago

The page update attempt to pop the page but it popped the open drawer instead. You will need to manually close it. See the code

              onTap: () {
                  // Close the drawer.
                  Navigator.of(context).pop();
                  state.selectedIndex = i;
                },

We should add an error message says the page update failed to pop the page, and warn the developer that it can't be popped automatically.

Alternatively we can add a new API to force pop a route. I am not sure if this is a sane thing to do. @goderbauer Do you have any suggestion?

```dart import 'package:flutter/material.dart'; void main() { runApp(MyApp()); } class AppState extends ChangeNotifier { final List _indexs = List.generate(5, (index) => index); int _selectedIndex = 0; int get selectedIndex => _selectedIndex; set selectedIndex(int id) { _selectedIndex = id; notifyListeners(); } List get indexs => _indexs; } class MyApp extends StatefulWidget { @override State createState() => _MyAppState(); } class _MyAppState extends State { final IndexRouterDelegate _routerDelegate = IndexRouterDelegate(); final IndexRouteInformationParser _routeInformationParser = IndexRouteInformationParser(); @override void dispose() { _routerDelegate.dispose(); super.dispose(); } @override Widget build(BuildContext context) { return MaterialApp.router( title: 'Index App', routerDelegate: _routerDelegate, routeInformationParser: _routeInformationParser, ); } } class IndexRouteInformationParser extends RouteInformationParser { @override Future parseRouteInformation(RouteInformation routeInformation) async { if (routeInformation.location == '/') { return 0; } final uri = Uri.parse(routeInformation.location!); final id = int.tryParse(uri.pathSegments[0]); return id!; } @override RouteInformation restoreRouteInformation(int path) => RouteInformation(location: '/$path'); } class IndexRouterDelegate extends RouterDelegate with ChangeNotifier, PopNavigatorRouterDelegateMixin { @override final GlobalKey navigatorKey; final AppState _appState = AppState(); IndexRouterDelegate() : navigatorKey = GlobalKey() { _appState.addListener(() => notifyListeners()); } @override void dispose() { _appState.dispose(); super.dispose(); } @override int get currentConfiguration => _appState.selectedIndex; List get getPages { final result = []; for (var i = 0; i <= _appState.selectedIndex; i++) result.add(IndexPage(i: _appState.indexs[i], state: _appState)); print('Pages count : ${result}'); return List.unmodifiable(result); } @override Widget build(BuildContext context) { return Navigator( key: navigatorKey, pages: getPages, onPopPage: (route, result) { if (!route.didPop(result)) { return false; } return true; }, ); } @override Future setNewRoutePath(int path) async { _appState.selectedIndex = path; } } class IndexPage extends Page { final int i; final AppState state; IndexPage({ required this.i, required this.state, }) : super(key: ValueKey(i)); @override Route createRoute(BuildContext context) { return MaterialPageRoute( settings: this, builder: (BuildContext context) { return IndexScreen( index: i, state: state, ); }, ); } } class IndexScreen extends StatelessWidget { final int index; final AppState state; IndexScreen({ required this.state, required this.index, }); @override Widget build(BuildContext context) { return Scaffold( appBar: AppBar(), drawer: Drawer( child: ListView( children: [ for (var i = 0; i < state.indexs.length; i++) ListTile( title: Text("$i"), onTap: () { // Close the drawer. Navigator.of(context).pop(); state.selectedIndex = i; }, ) ], ), ), body: Padding( padding: const EdgeInsets.all(8.0), child: Center( child: Text('$index', style: Theme.of(context).textTheme.headline6)), ), ); } } ```
exaby73 commented 2 years ago

Tested on current stable and master version, issue still persists. Platforms tested are:

Video https://user-images.githubusercontent.com/50555895/166640101-df6a8353-21e3-48f8-af8a-d3039e7cc40b.mov
flutter doctor -v ``` [✓] Flutter (Channel master, 2.13.0-0.0.pre.886, on macOS 12.3.1 21E258 darwin-arm, locale en-US) • Flutter version 2.13.0-0.0.pre.886 at /Users/nabeelparkar/fvm/versions/master • Upstream repository https://github.com/flutter/flutter.git • Framework revision e9c0ee1e08 (3 hours ago), 2022-05-04 00:29:07 -0400 • Engine revision 0b46116471 • Dart version 2.18.0 (build 2.18.0-82.0.dev) • DevTools version 2.12.2 [✓] Android toolchain - develop for Android devices (Android SDK version 32.0.0-rc1) • Android SDK at /Users/nabeelparkar/Library/Android/sdk/ • Platform android-32, build-tools 32.0.0-rc1 • ANDROID_SDK_ROOT = /Users/nabeelparkar/Library/Android/sdk/ • Java binary at: /opt/homebrew/opt/openjdk@11/bin/java • Java version OpenJDK Runtime Environment Homebrew (build 11.0.15+0) • All Android licenses accepted. [✓] Xcode - develop for iOS and macOS (Xcode 13.3.1) • Xcode at /Applications/Xcode.app/Contents/Developer • CocoaPods version 1.11.3 [✓] Chrome - develop for the web • CHROME_EXECUTABLE = /Applications/Brave Browser.app/Contents/MacOS/Brave Browser [!] Android Studio (not installed) • Android Studio not found; download from https://developer.android.com/studio/index.html (or visit https://flutter.dev/docs/get-started/install/macos#android-setup for detailed instructions). [✓] IntelliJ IDEA Ultimate Edition (version 2022.1) • IntelliJ at /Applications/IntelliJ IDEA.app • Flutter plugin version 67.0.4 • Dart plugin version 221.5588 [✓] VS Code (version 1.66.2) • VS Code at /Applications/Visual Studio Code.app/Contents • Flutter extension version 3.40.0 [✓] Connected device (2 available) • macOS (desktop) • macos • darwin-arm64 • macOS 12.3.1 21E258 darwin-arm • Chrome (web) • chrome • web-javascript • Brave Browser 101.1.38.111 [✓] HTTP Host Availability • All required HTTP hosts are available ! Doctor found issues in 1 category. ```