search

sockeqwe / mosby

A Model-View-Presenter / Model-View-Intent library for modern Android apps
http://hannesdorfmann.com/mosby/
Apache License 2.0
5.49k stars 841 forks source link

ViewPager on back stack calls Presenter's method detachView(retainInstance=true) #271

Closed Thomas-Vos closed 6 years ago

Thomas-Vos commented 6 years ago

Hi Hannes,

I'm having some issues with a ViewPager in the back stack.

Mosby Version: mvp-common:3.0.4 and viewstate:3.0.4

Expected behavior I have a Fragment called A that contains a ViewPager that contains Fragments B and C. When Fragment A is put on the back stack, the method detachView(retainInstance=false) gets called. At the same time, detachView(retainInstance=false) is supposed to be called for Fragments B and C.

Actual behavior (include a stacktrace if crash) When Fragment A is put on the back stack, the following happens:

As you can see, retainInstance is true for both B and C, but it should be false.

Steps to reproduce the behavior or link to a sample repository

  1. Create Fragment A containing a ViewPager that contains Fragments B and C.
  2. All Fragments call setRetainInstance(true) in the onCreate() method.
  3. The MVP delegate is instantiated like this: new FragmentMvpViewStateDelegateImpl<>(this, this, true, false);.
  4. Now put Fragment A on the backstack and see what happens.

Thanks, Thomas

sockeqwe commented 6 years ago

Hey, I see ... This bug seems to occur if you have setRetainInstance(true) and `new FragmentMvpViewStateDelegateImpl<>(..., keepPresenterAndViewStateOnBackstack = false)´ so you don't want to keep the presenter.

Beside that this is a bug (thanks for reporting), what is actually the reason behind using setRetainInstance(true) in that case? Does it work as expected with setRetainInstance(false)?

Thomas-Vos commented 6 years ago

Hello @sockeqwe, I want to keep the presenter on a configuration change, that's why I call setRetainInstance(true). I just tried without calling setRetainInstance(true); the presenter is not retained anymore on a configuration change, and I'm still having the same issue.

If I return to Fragment A (by pressing back or popping the back stack), the presenter is recreated anyway, so that's why I set keepPresenterAndViewStateOnBackstack to false. (issue #265)

I also noticed if you remove Fragment A from the back stack (go "up"), then Fragment B/C call detachView(retainInstance=true). I currently have worked around this by calling detachView(false) manually in the Fragment's onDestroy(). Is this a bug?

Thanks for your help, Thomas

sockeqwe commented 6 years ago

See #274. Feel free to reopen this issue if 3.1.0 (snapshot) doesn't solve this porblem.