Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Stateful widget with didUpdateWidgetis not called #643

Open
karvulf opened this issue Oct 24, 2023 · 10 comments
Open

Bug: Stateful widget with didUpdateWidgetis not called #643

karvulf opened this issue Oct 24, 2023 · 10 comments

Comments

@karvulf
Copy link

karvulf commented Oct 24, 2023

Describe the bug
I have the following issue.
I will use the example bottom_navigation_mulitple_beamers for that.
My routerDelegate contains one BeamPage that has AppScreen as child.
AppScreen expects one parameter (the current path as lcoation).
The AppScreen contains a BottomNavigationBar. Now I navigate to the second tab and back (at this point didUpdateWidget is also not called).
When I use the browsers back button, the widget MyAppScreen isn't called even though the path has changed and should update the widget because the parameter changed. That results into the issue that the tabs are not changed.

Beamer version: (e.g. 2.0.0-dev.0, master, ...)
master (2.0.0-dev.0) with commit-id c3795ab39197cc3429c54c5ec3fa6a1202ddba88.

To Reproduce

Update the code of an example in main.dart with the following. This example code is almost the same as in the example app in bottom_navigation_multiple_beamers:

import 'package:beamer/beamer.dart';
import 'package:flutter/material.dart';

// DATA
const List<Map<String, String>> books = [
  {
    'id': '1',
    'title': 'Stranger in a Strange Land',
    'author': 'Robert A. Heinlein',
  },
  {
    'id': '2',
    'title': 'Foundation',
    'author': 'Isaac Asimov',
  },
  {
    'id': '3',
    'title': 'Fahrenheit 451',
    'author': 'Ray Bradbury',
  },
];

const List<Map<String, String>> articles = [
  {
    'id': '1',
    'title': 'Explaining Flutter Nav 2.0 and Beamer',
    'author': 'Toby Lewis',
  },
  {
    'id': '2',
    'title': 'Flutter Navigator 2.0 for mobile dev: 101',
    'author': 'Lulupointu',
  },
  {
    'id': '3',
    'title': 'Flutter: An Easy and Pragmatic Approach to Navigator 2.0',
    'author': 'Marco Muccinelli',
  },
];

// SCREENS
class BooksScreen extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: Text('Books'),
      ),
      body: ListView(
        children: books
            .map(
              (book) => ListTile(
                title: Text(book['title']!),
                subtitle: Text(book['author']!),
                onTap: () => context.beamToNamed('/books/${book['id']}'),
              ),
            )
            .toList(),
      ),
    );
  }
}

class BookDetailsScreen extends StatelessWidget {
  const BookDetailsScreen({required this.book});

  final Map<String, String> book;

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: Text(book['title']!),
      ),
      body: Padding(
        padding: const EdgeInsets.all(8.0),
        child: Text('Author: ${book['author']}'),
      ),
    );
  }
}

class ArticlesScreen extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(title: Text('Articles')),
      body: ListView(
        children: articles
            .map(
              (article) => ListTile(
                title: Text(article['title']!),
                subtitle: Text(article['author']!),
                onTap: () => context.beamToNamed('/articles/${article['id']}'),
              ),
            )
            .toList(),
      ),
    );
  }
}

class ArticleDetailsScreen extends StatelessWidget {
  const ArticleDetailsScreen({required this.article});

  final Map<String, String> article;

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: Text(article['title']!),
      ),
      body: Padding(
        padding: const EdgeInsets.all(8.0),
        child: Text('Author: ${article['author']}'),
      ),
    );
  }
}

// LOCATIONS
class BooksLocation extends BeamLocation<BeamState> {
  BooksLocation(RouteInformation routeInformation) : super(routeInformation);

  @override
  List<String> get pathPatterns => ['/books/:bookId'];

  @override
  List<BeamPage> buildPages(BuildContext context, BeamState state) => [
        BeamPage(
          key: ValueKey('books'),
          title: 'Books',
          type: BeamPageType.noTransition,
          child: BooksScreen(),
        ),
        if (state.pathParameters.containsKey('bookId'))
          BeamPage(
            key: ValueKey('book-${state.pathParameters['bookId']}'),
            title: books.firstWhere((book) =>
                book['id'] == state.pathParameters['bookId'])['title'],
            child: BookDetailsScreen(
              book: books.firstWhere(
                  (book) => book['id'] == state.pathParameters['bookId']),
            ),
          ),
      ];
}

class ArticlesLocation extends BeamLocation<BeamState> {
  ArticlesLocation(RouteInformation routeInformation) : super(routeInformation);

  @override
  List<String> get pathPatterns => ['/articles/:articleId'];

  @override
  List<BeamPage> buildPages(BuildContext context, BeamState state) => [
        BeamPage(
          key: ValueKey('articles'),
          title: 'Articles',
          type: BeamPageType.noTransition,
          child: ArticlesScreen(),
        ),
        if (state.pathParameters.containsKey('articleId'))
          BeamPage(
            key: ValueKey('articles-${state.pathParameters['articleId']}'),
            title: articles.firstWhere((article) =>
                article['id'] == state.pathParameters['articleId'])['title'],
            child: ArticleDetailsScreen(
              article: articles.firstWhere((article) =>
                  article['id'] == state.pathParameters['articleId']),
            ),
          ),
      ];
}

// APP
class AppScreen extends StatefulWidget {
  final String location;

  const AppScreen({Key? key, required this.location}) : super(key: key);

  @override
  _AppScreenState createState() => _AppScreenState();
}

class _AppScreenState extends State<AppScreen> {
  late int currentIndex;

  final routerDelegates = [
    BeamerDelegate(
      initialPath: '/books',
      locationBuilder: (routeInformation, _) {
        if (routeInformation.location!.contains('books')) {
          return BooksLocation(routeInformation);
        }
        return NotFound(path: routeInformation.location!);
      },
    ),
    BeamerDelegate(
      initialPath: '/articles',
      locationBuilder: (routeInformation, _) {
        if (routeInformation.location!.contains('articles')) {
          return ArticlesLocation(routeInformation);
        }
        return NotFound(path: routeInformation.location!);
      },
    ),
  ];

  @override
  void didUpdateWidget(covariant AppScreen oldWidget) {
    print('###Widget was updated! ${widget.location}');
    super.didUpdateWidget(oldWidget);
  }

  @override
  void didChangeDependencies() {
    super.didChangeDependencies();
    final uriString = Beamer.of(context).configuration.location!;
    currentIndex = uriString.contains('books') ? 0 : 1;
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: IndexedStack(
        index: currentIndex,
        children: [
          Beamer(
            routerDelegate: routerDelegates[0],
          ),
          Container(
            color: Colors.blueAccent,
            padding: const EdgeInsets.all(32.0),
            child: Beamer(
              routerDelegate: routerDelegates[1],
            ),
          ),
        ],
      ),
      bottomNavigationBar: BottomNavigationBar(
        currentIndex: currentIndex,
        items: [
          BottomNavigationBarItem(label: 'Books', icon: Icon(Icons.book)),
          BottomNavigationBarItem(label: 'Articles', icon: Icon(Icons.article)),
        ],
        onTap: (index) {
          if (index != currentIndex) {
            setState(() => currentIndex = index);
            routerDelegates[currentIndex].update(rebuild: false);
          }
        },
      ),
    );
  }
}

class MyApp extends StatelessWidget {
  final routerDelegate = BeamerDelegate(
    initialPath: '/books',
    locationBuilder: RoutesLocationBuilder(
      routes: {
        '*': (context, state, data) => BeamPage(
              key: ValueKey('app'),
              child: AppScreen(
                location: state.uri.path,
              ),
            ),
      },
    ),
  );

  @override
  Widget build(BuildContext context) {
    return MaterialApp.router(
      debugShowCheckedModeBanner: false,
      routerDelegate: routerDelegate,
      routeInformationParser: BeamerParser(),
      backButtonDispatcher: BeamerBackButtonDispatcher(
        delegate: routerDelegate,
      ),
    );
  }
}

void main() => runApp(MyApp());

Expected behavior
I would expect that the navigation works correctly with the back button and the widget gets updated when the path changed.

Desktop (please complete the following information):

  • OS: [e.g. iOS]
  • Browser [e.g. chrome, safari]
  • Version [e.g. 22]

Smartphone (please complete the following information):

  • Device: MacBook
  • OS: Ventura 13.4.1
  • Browser: Chrome
  • Version: 118.0.5993.70
@darkstarx
Copy link

darkstarx commented Oct 25, 2023

Right! It doesn't work and should not, because the Beamer.of calls Router.of which subscribes on the inherited widget _RouterScope that notifies only if

           routeInformationProvider != oldWidget.routeInformationProvider ||
           backButtonDispatcher != oldWidget.backButtonDispatcher ||
           routeInformationParser != oldWidget.routeInformationParser ||
           routerDelegate != oldWidget.routerDelegate ||
           routerState != oldWidget.routerState;

None of them change when you change the url in the browser url bar (of course, if you use the fragment url strategy, with a hash-tag sign in urls).
So, this is an obvious bug. It seems, the Beamer.of worked different when this example was written than current version.

@karvulf
Copy link
Author

karvulf commented Oct 25, 2023

So the only way to ensure that my widget can "refresh" is that the key of my BeamPage is changed after the uri changes when changing the tab (especially when using the browser back button)? @darkstarx I don't want that the screen is recreated after every tab change.

@darkstarx
Copy link

@karvulf Well, I'm not the author of this package, so.. I just can recommend you not to use this example of how to cook Beamer. You can use other examples that suppose to be working:
https://github.com/slovnicki/beamer/tree/master/examples/bottom_navigation
https://github.com/slovnicki/beamer/tree/master/examples/bottom_navigation_2

@karvulf
Copy link
Author

karvulf commented Oct 25, 2023

This example fitted very well for my usecase because it the widget holds the beamer location in separated delegates (see routerDelegates in _AppScreenState. Like this I was able to ensure that only the page is loaded that is displayed in the tab and won't be reloaded if I navigate to the tab again.

bottom_navigation:
This will definitely load the locations and will reload every tab page which is not desired.

bottom_navigation_2:
This holds all routes in the root routerDelegate, which is not supporting my usecase, because I have a nested location structure.

Only the third example is going to that what is solving my issues but unfortunately it seems harder to get this work if the locations are nested. @darkstarx

@darkstarx
Copy link

@karvulf ok, that's not a problem, but you should not rely on didChangeDependencies. You should listen the root delegate instead. Try this

class _AppScreenState extends State<AppScreen>
{
  final routerDelegates = [
    BeamerDelegate(
      initialPath: '/books',
      locationBuilder: (routeInformation, _) {
        if ('${routeInformation.uri}'.contains('books')) {
          return BooksLocation(routeInformation);
        }
        return NotFound(path: '${routeInformation.uri}');
      },
    ),
    BeamerDelegate(
      initialPath: '/articles',
      locationBuilder: (routeInformation, _) {
        if ('${routeInformation.uri}'.contains('articles')) {
          return ArticlesLocation(routeInformation);
        }
        return NotFound(path: '${routeInformation.uri}');
      },
    ),
  ];

  @override
  void dispose()
  {
    delegate?.removeListener(_onBeamerChanged);
    super.dispose();
  }

  @override
  void didChangeDependencies()
  {
    super.didChangeDependencies();
    final newDelegate = Beamer.of(context);
    if (delegate != newDelegate) {
      delegate?.removeListener(_onBeamerChanged);
      delegate = newDelegate;
      delegate?.addListener(_onBeamerChanged);
      _updateCurrentIndex();
    }
  }

  @override
  Widget build(final BuildContext context)
  {
    return Scaffold(
      body: IndexedStack(
        index: _currentIndex,
        children: [
          Beamer(
            routerDelegate: routerDelegates[0],
          ),
          Beamer(
            routerDelegate: routerDelegates[1],
          ),
        ],
      ),
      bottomNavigationBar: BottomNavigationBar(
        currentIndex: _currentIndex,
        items: const [
          BottomNavigationBarItem(label: 'Books', icon: Icon(Icons.book)),
          BottomNavigationBarItem(label: 'Articles', icon: Icon(Icons.article)),
        ],
        onTap: (index) {
          if (index != _currentIndex) {
            setState(() => _currentIndex = index);
            routerDelegates[_currentIndex].update(rebuild: false);
          }
        },
      ),
    );
  }

  void _updateCurrentIndex()
  {
    _currentIndex = '${delegate!.configuration.uri}'.contains('books') ? 0 : 1;
  }

  void _onBeamerChanged() => setState(_updateCurrentIndex);

  BeamerDelegate? delegate;
  late int _currentIndex;
}

But decisions like _currentIndex = '${delegate!.configuration.uri}'.contains('books') ? 0 : 1; are not for real project, try something more reliable than checking uri.

@karvulf
Copy link
Author

karvulf commented Oct 25, 2023

Ah I see, the listener could solve my issue, thank you, I will close the issue and try it with your proposed solution @darkstarx

@karvulf karvulf closed this as completed Oct 25, 2023
@darkstarx
Copy link

@karvulf Oh, I wouldn't close the issue, since the bug is still in the package documentation.

@karvulf karvulf reopened this Oct 25, 2023
@karvulf
Copy link
Author

karvulf commented Oct 25, 2023

As you wish :D @darkstarx

@stan-at-work
Copy link
Contributor

@slovnicki Could you change the docs, to update the outdated code?

@stan-at-work
Copy link
Contributor

@slovnicki could this isse be closed ?👆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants