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

Fix issues with overlap #2

Merged
merged 1 commit into from
Jan 9, 2018
Merged

Fix issues with overlap #2

merged 1 commit into from
Jan 9, 2018

Conversation

mario
Copy link

@mario mario commented Jan 2, 2018

Signed-off-by: Mario Danic mario@lovelyhq.com

Signed-off-by: Mario Danic <mario@lovelyhq.com>
@chris6647
Copy link
Owner

Could you give me some more information as to the issue this fixes? Along with steps to reproduce the issue it fixes. I just see that you move some of the setup into onAttach instead, and the commit message just mentions something with overlap.

@mario
Copy link
Author

mario commented Jan 4, 2018

Of course. Since onViewBound is invoked only once, there's the following problem:

  • 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 :)

@chris6647
Copy link
Owner

Turns out that we actually had the same issue (just still within a single activity) in our backlog, and this does indeed fix it :) Thanks!

@chris6647 chris6647 merged commit 2c31011 into chris6647:feature/bottom-navigation-view-controller Jan 9, 2018
@mario
Copy link
Author

mario commented Jan 9, 2018

Awesome! Glad I was of help! If you'd at any point like to know where your work is used, let me know and I'll point you to it! :)

@mario mario deleted the feature/bottom-navigation-view-controller branch January 9, 2018 08:51
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.

2 participants