csells / go_router

The purpose of the go_router for Flutter is to use declarative routes to reduce complexity, regardless of the platform you're targeting (mobile, web, desktop), handling deep linking from Android, iOS and the web while still allowing an easy-to-use developer experience.
https://gorouter.dev
441 stars 97 forks source link

Nested Navigation Example - Browser's back button is changing the address without changing the tab #182

Closed osaxma closed 2 years ago

osaxma commented 2 years ago

Issue can be observed in this screen recording:

https://user-images.githubusercontent.com/46427323/142935109-bfb5375a-ce85-4e1f-8cc4-69e1ec305f7b.mov

Flutter version: 2.5.3 (stable) go_router version: 2.2.8

csells commented 2 years ago

Hmm... That looks like a regression. @lulupointu any thoughts on this?

lulupointu commented 2 years ago

This is missing:

  @override
  void didUpdateWidget(covariant FamilyTabsPage oldWidget) {
    if (oldWidget.index != widget.index && widget.index != _controller.index) {
      _controller.animateTo(
        widget.index,
        duration: const Duration(milliseconds: 300),
      );
    }
    super.didUpdateWidget(oldWidget);
  }

Also this is not needed:

  @override
  void didChangeDependencies() {
    super.didChangeDependencies();
    _controller.index = widget.index;
  }

Explanation time !

First, let's understand why this example did work (almost):

So if the user used at least once _tab, then changed the url, didChangeDependencies was called and the index changes.

Now everything changed 8 days ago, look at this change

This basically cancelled any call to didChangeDependencies that was caused by GoRouter.

Now you know why Provider as listen: false, and also why it warns you when you try to use listen: true outside a build method :wink:

csells commented 2 years ago

Nice! Not only did you provide a bug fix but you also found why it used to work and suddenly stopped. Amazing!

@osaxma this fix is part of v2.2.9. can you confirm that it's fixed on your end?

osaxma commented 2 years ago

Yes, it's working now!