Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds BottomNavigationController to the demo app #316

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

chris6647
Copy link

Googling around revealed quite a few issues for Conductor showing interrest in Conductor and Bottom Navigation together. This is an example implementation of that, but where each MenuItem has a separate backstack.

The first implementation I did, was something that @EricKuck mentioned in a related issue (#156 (comment)) where I created a ChangeHandlerFrameLayout pr MenuItem, but the main problem with that, was how each of the Controllers in each of the ChangeHandlerFrameLayout with their own Router, all had their view active and themselves being in attached state at the same time. We needed to be able to trust the lifecycle methods, and thus we gave this another attempt.

After the multiple ViewGroup/Router attempt, I tried managing the backstack state by keeping track of the RouterTransactions, but the Bundle constructor was package private, and thus I couldn't easily recreate it. Same goes internally used Backstack implementation.

So even if there was overhead in saving/restoring the entire Router pr MenuItem, it allowed me to consistently keep track of them individually and have the handleBack() and restoration functionality as we wanted. Which is what this implementation is.

I hope it will help someone - even if this doesn't make it into the demo app, and perhaps get some comments to improve the solution or do something completely different all together.

Some relevant issues:
#27 (comment)
#260

Showcasing a BottomNavigationView implementation with separate,
retained backstacks for each MenuItem.
@StefMa
Copy link
Contributor

StefMa commented Oct 10, 2017

I really love it and it works perfect.
Thanks @chris6647

I do really like to see it into the conductor samples.
Any reasons why not still merged @EricKuck ? 🤔

Provides concrete implementation example,
using enums to map between MenuItem IDs and Controllers,
together with the same reflection Controllers use internally,
when restoring themselves.

Fixes issue where Controllers in the childRouter,
would get incorrect lifecycle callbacks:
Previously, upon clearing the child Router,
if that Router had > 1 backstack entries,
the root Controller would get an onAttach/onCreateView callback,
and then immediately get torn down.
This caused issues when that Controller manipulated something
beyond its own scope (i.e. showing a dialog),
as that change would not be cleaned up.

Now the Routers are properly torn down and restored,
without any premature lifecycle callbacks.
This is done in a manner similar to how RouterPagerAdapter,
works.

Upon clicking the same menu item,
as the user is already on,
it resets the backstack to the initial controller.

Fixes issue with restoration when BottomNavigationController
itself is in the backstack.
@chris6647
Copy link
Author

@StefMa Thanks for your kind words! Please do check out my recent changes to it, as it improves it quite a bit, and fixes some bugs :)

mario and others added 3 commits January 2, 2018 13:52
Signed-off-by: Mario Danic <[email protected]>
…oller

Fixes issue where lastActiveChildRouter was null when it neednt
steps to reproduce issue prior to fix:
start another activity
go back
lastActiveChildRouter will be null
therefore new controller would be created causing UI overlap (one controller on top of another)
also clicking back button would crash hard since lastActiveChildRouter doesn't exist :)
…ly to need re-attach,

which would cause old views to be present still (or all together gone)
i.e. tapping quickly between menu items.
This removes the need for manual tracking of lastActiveRouter and the cachedInstanceStates.
…with Bundle args, as well as to a tab with a backstack

Refactors to use the same navigateTo method internally, as can be called externally.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants