mt-akar / bottom_nav_layout

A quick and powerful Flutter layout with a bottom navbar.
https://pub.dev/packages/bottom_nav_layout
MIT License
34 stars 11 forks source link

Nested pages onWillPop handling #10

Open mohamoha6200 opened 2 years ago

mohamoha6200 commented 2 years ago

Hello , I'm back here after being stuck with an issue for a while , the problem im facing is that using willPopScope on top of the BottomNavLayout in order to control what happens when the android back button is pressed , makes it impossible of taking advantage of willPopScope again in the nested screens locally , I'm having few scenarios where I need to dismiss a sheet or an overlay on back btn press but instead the global willPopScope gets triggered and the local screen's one doesn't ! I noticed that if I hot reload (global willPopScope get's rebuilt) the local willPopScope starts triggering but the drawback is that the BottomNavLayout breaks completely with a "Bad state: No element" error , I tried using a state management solution to get the sheet controller into the global willPopScope and control it from there but it meant that it needs to be rebuilt but leads to BottomNavLayout breaking again ! I think this is more like a flutter issue , i've been looking into this thread : https://github.com/flutter/flutter/issues/47088 but don't you find it weird that the BottomNavLayout has to into "Bad state: No element" just for it getting rebuilt by a simple hot reload ? Thank you again for this great package man !

mt-akar commented 2 years ago

Thank you for the compliments and thank you for reaching out to me.

If I understand correctly, you want to use a WillPopScope widget in your screen located inside the BottomNavLayout widget. Correct me if my assumption is incorrect.

This is possible. How WillPopScope works is that when a back button even is generated, the lowest WillPopScope widget on the widget tree catches this even and its onWillPop method is fired. If this widget handles the back button press, it returns false to "consume" the back event. If it doesn't handle the back event, it should return true to bubble the back event higher on the widget tree for the next WillPopScope to handle it. This way, more than one WillPopScope can be used in a single widget tree. If none of the WillPopScope instances handle the back button event, it is eventually bubbled up the framework, which usually just exits the app.

I am assuming you are returning true, even though you handle the back event. Returning false from the onWillPop method should solve this issue. If somehow your back button press is ending up where you don't intend it to, I cannot help you further without seeing your code.

Now for the hot reload part, I believe I didn't get the problem exactly. It is unlikely that it is about this package. It is either about the framework or about your code.

I hope this solves your problem. Have a great day.

mohamoha6200 commented 2 years ago

Hi and thank you for the quick and thorough response as usual ! To answer , you're assumption is correct indeed , Im trying to handle back press events and the most practical case for me would be the one you mentioned where the lowest WillPopScope in the widget tree should be the one to receive the event and interpret it first and delegate it to a higher WillPopScope handler when true is returned , however , what's happening in my project is the exact opposite , the highest WillPopScope is the one being called by default first thing and no matter how many local WillPopScope s im adding locally they won't be fired at all unless a the highest widget containing the global WillPopScope is rebuilt (what a hot reload serves) that's when the behavior changes completely and the local WillPopScope starts to fire first the way you described , the downside of a rebuild as I mentioned is that is messes up the main BottomNavLayout behavior somehow making it fall into a "Bad state: No element" error. So as a workaround I tried to declare my sheets controllers (and the things I wanted to hide before popping screens) globally outside of the classes so I can access them in different layers and have the highest WillPopScope access them handle the back btn press according to their statuses so you can imagine how cumbersome the main WillPopScope has become.

I heard you when you said that the hot reload (rebuild) part shouldn't have to do with the package but rather either my code or the framework , so I tried to reproduce the issue in a sample project only containing BottomNavLayout and a simple WillPopScope and unfortunately the problem persisted here is the code :

https://codeshare.io/yonEze

Steps to reproduce :

Am I doing something terribly wrong or is this a framework/package issue ? Thank you for your time bro , have a wonderful new year.

mt-akar commented 2 years ago

I will look into this when I get the chance.

mohamoha6200 commented 2 years ago

Hey bro , I was wondering if you've managed to find some time for the issue in fact we're getting closer to the production phase of our app and sadly the functionality that is currently problematic to implement seems to be of high importance to the app flow ie : being able to switch from page to page with PageStack.push() (which needs setstate to take effect) without breaking the global on WillPopScope feature as shown in the sample code.

Many thanks !

mt-akar commented 2 years ago

I am sorry but I am away from home for a little while longer. I wish I could help sooner.

What I am wondering is that, how does an issue that only come up when hot reloaded affect your production environment? Or am I am missing something?

mohamoha6200 commented 2 years ago

No worries man things can wait , thank you for the concern. So the issue was first discovered after debugging and hot reloading but the hot reload itself wasn't the problem but rather the rebuilding of the Widget containing the WillPopScope and the BottomNavLayout even using a simple setState() , the rebuilding here is needed to push a different page into the pageStack which needs a setState() that eventually crashes the rest of the flow. So if the setstate wasn't needed the problem wouldn't rise but unfortunately it is in the .push(x) case.

mohamoha6200 commented 2 years ago

Hey bro , I take it that you could reproduce the issue in discussion , how complicated do you deem it to be ? and based on that would it be possible to estimate the time for resolution ? I'm open for any kind of workaround meanwhile if there is any. Please do keep me posted ;) appreciate it !

mt-akar commented 2 years ago

Okey. I will finally look at this properly. I will keep you updated.

Meanwhile, could you contact me on Discord via ViraL#2868? I do not want to flood this issue.