Popping can also cause back navigation between tabs when current stack is depleted

[mateherber]

Hi @ncapdevi,

This is the first rough implementation of our use case where developer can control with the builder flag what happens when trying to pop from a stack that already has only the root fragment (so far it was an unsupported exception).

Depending on the selected strategy the following will happen:

• Current Tab: This is the original way, exception will be raised
• Unique Tab History: Navigation will be triggered backwards in history (each tab is kept only once)
• Unlimited Tab History: Navigation will be triggered backwards in exactly in the order the user was switching between tabs

(Tab switching can only happen if we're on the root fragment of the current tab, if not the fragments will be popped from the current stack)

We kept backward compatibility with one change - popFragment will return boolean to indicate whether any pop or switch happened - (however in Current Tab mode we kept the exception to remain backward compatible)

We added test cases for the different strategies and updated the sample app to use the Unique Tab History.

Please review if you have time and we're open to any suggestion / changes.

Thanks!

[ncapdevi]

Hey @mateherber , sorry for the slight delay, I'm going through this PR now. First of all, great quality PR across the board, so thank you very much for that. Secondly, I've been slightly on the fence about this conceptually, and there's two reasons for hesitation. One, I'm generally a firm believer that keeping a library's scope as small and focused as possible is ideal. It's also important to define what that scope is. Two, I was/am unsure about the navigational feel of popping back between tabs (that being said, it shouldn't necessarily be up to the library to make that decision). Answering the second concern first, Material Design seems to be pretty open on the topic, and pressing back to return to a different tab seems to fall within the guidelines, so it seems pretty reasonable and I'm getting less concerned about the library easily allowing it. Regarding the first concern, the reason that it still slightly exists, is because pretty much everything being done could be done on a layer below the FragNavController. By that I mean, when you receive the back press, it wouldn't be unreasonable to check if it is the root fragment, and if it was, determine how you wanted to handle it there (e.g., switch to a previous tab). That being said you'd then have to manage your own stack of tab indices, but it's not horrible unreasonable since you have an interface for that already. Now, all that being said, the whole point of the library is to manage these back stacks, and to manage them elegantly, so I could definitely see this falling within the scope of that definition.

I think in general I lean towards the whole concept more than I lean away from it, but wanted to at least bring to light some of my hesitancies or concerns and give you a chance to speak your mind as well. I'm going to spend some more time with it and look at some of the nuances of how this might affect things both currently, as well as moving forward (will this open up more feature creep?) and look forward to your reply.

Thanks again for the great PR

[ncapdevi]

Also, turns out Spotify's app does this type of back stack management (pressing back moves through the tabs) and it felt perfectly natural to me, so I'm less concerned about it than before.

[mateherber]

Hi @ncapdevi,

Sorry for not replying earlier I saw your review just yesterday night. First of all thanks for reviewing our PR we hope it aligned with the project's standards. We understand your concerns as we had the same thoughts but we had to achieve our goal so let me share some history / share thoughts.

We are currently moving towards bottom navigation in our app and we wanted to achieve that in a way that we don't have to rewrite our entire application. First we checked out many apps in the store who already using this navigation model (Youtube, Instagram, Spotify, etc.). We wanted to have the same(ish) mechanics as those apps since Android users are most probably using those applications and familiar with them. We found that almost (if not) all of them are having the following navigation:

• Upper back works the same way as hardware back (on those pages that are in the bottom navigation content).
• Tapping back first popping fragments from the current fragment stack and when it is empty it starts to navigate backwards in "tab history" and start to pop fragments there

We found your library which we like pretty much and had most of the functionality we needed so we started to use it and as you suggested we added this backward navigation logic by wrapping the lib. Then we thought probably many users of the lib would like the same behavior to be supported out of the box so they don't have to tackle with wrapping. Hence we created the pr. We tried to be really careful to add this as an option not as default mechanism or anything forced. So if historical users of the lib continue to use it without the extra flags it will keep working just as before.

Regarding the material guidelines we agree that it is not really specific about how navigation should work but considering Youtube for instance is made by Google (who by the way doesn't comply with the guidelines a lot of times) we thought that can be a "sample" implementation for many developers.

Again thanks for review and merge and we hope we added useful functionality for some not just some unnecessary implementation for our specific weird case.

We also plan to add one more option to be able to control between show/hide and attach/detach. We already done the work but we still are making it worthy for a PR.

[mateherber]

Oh and also I'm planning to add README sections for both changes of course

[ncapdevi]

@mateherber Yes, The most time I spent with the PR, the more in favor of it I was and am happy with the addition of it. I'm going to be pushing an updated version of the library with it here shortly. The readme PR would be very greatly appreciated. Also, side note, I've been pretty heavily considering transitioning the library over to Kotlin, which would allow for a couple reassessments with design change, and would probably warrant a Major point release (3.0). Since your PR (and future PRs) are greatly appreciated, I wanted to give you a heads up about that possibility and a chance to speak out in favor or against that, as well as chime in with any recommendations/requests. Thanks!

[mateherber]

Hi @ncapdevi!

We're very well aligned with Kotlin. We were thinking about introducing kotlin with our PR just voted against it since we didn't want to introduce Kotlin as dependency we were not sure how would you like that. We're happy with transitioning to kotlin even we can help with rewrite! Last weeks were pretty busy for us preparing the bottom navigation version of the app, after release we'll ramp up the FragNav contribution :) - Stay Tuned