search

JaspervanRiet / duck_router

A Flutter router with intents
MIT License
4 stars 3 forks source link

feat!: remove need to specify onPopInvoked #31

Closed JaspervanRiet closed 3 weeks ago

JaspervanRiet commented 3 weeks ago

Description

This is an iteration upon #28. There, we started supporting Flutter's onDidRemovePage callback. However, we had to make a compromise where users had to implement onPopInvoked when overriding Page.

This PR is an iteration on that PR. It removes the need to implement onPopInvoked, it is now handled automatically. We accomplish this by forcing usage of DuckPage, and by making DuckPage suitable for any type of Page via its new .custom constructor.

This is a breaking change, but I believe the last one around this topic. I have provided documentation in the README that should help users implement concepts such as dialogs.

Users can migrate by:

Related Issues

26

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

DelcoigneYves commented 3 weeks ago

Nice work!

I was a bit hesitant when seeing these changes at first sight, as I am prone to extending from MaterialPage and CupertinoPage instead of the general Page class. However, I found the same result can be achieved by overriding the createRoute method and return a MaterialPageRoute or CupertinoPageRoute. Since I think I'm probably not the only one extending pages this way, maybe it could be added to the documentation as well?

JaspervanRiet commented 3 weeks ago

Thanks @DelcoigneYves. Yes that was exactly the idea, that DuckPage is really nothing more than a wrapper for these advanced cases, that does some magic in the background. I have added some documentation highlighting the versatility of DuckPage, hopefully that helps?