cedvdb/phone_form_field

Select country dialog fails when called from nested navigator

autolainen opened this issue · 4 comments

autolainen commented

Hello lib owner!
I encountered a bug - when I call select country dialog and tap on any country, it doesn't close but instead a page under it is closed. I dig a little bit into this and found that when dialog is opened, root navigator is used (this is by default) but when .pop() is invoked there is no indication that root navigator should be used. When you have just root navigator in your app this is fine but in my case I use nested navigator so .pop() is called on wrong navigator. So line 194 in country_selector_navigator.dart should look like:
onCountrySelected: (country) =>
Navigator.of(context, rootNavigator: true).pop(country),

cedvdb commented

That's because flutter is mixing two concepts that have no business in being mixed up together. Namely: Navigation and Overlays.

Anyway, it seems to me that rootNavigator should be used in all instance of the selector navigator class.

So yeah I'll accept a PR that does that. Ultimately I believe this is a design mistake from the flutter side

autolainen commented

Anyway, it seems to me that rootNavigator should be used in all instance of the selector navigator class.

I've checket all the rest of the variants namely ".bottomSheet(), .draggableBottomSheet(), .modalBottomSheet(), .searchDelegate()" and they are working as expected. This is because they all use current (nested in my case) navigator. So I updated just .dialog().

Please review my PR

autolainen commented

That's because flutter is mixing two concepts that have no business in being mixed up together. Namely: Navigation and Overlays.

Sorry but didn't quite get where overlays are used here. All the ways to show country selector use navigator. Or I am missing something?

Overlays is totally different concept. It is used for hero animations and similar cases.

cedvdb commented

I didn't mean overlay in the flutter overlay sense but in a broad sense. All things like dialog bottom sheets etc have nothing to do with URLs, yet they are coupled with the routing mechanism on a logical level which is in my opinion a modelling mistake. I'm convinced this will lead to a lot of confusion when people start using nested routers. I've been vocal in the past that I believed this was a mistake.