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

detachView(..) in MosbyPresenter not called when app goes to background #273

Closed ZahidRasheed closed 6 years ago

ZahidRasheed commented 6 years ago

I am trying to dispose my subscriptions like below in my presenter extending MvpBasePresenter

@Override public void detachView(boolean retainInstance) { compositeSubscription.clear(); super.detachView(retainInstance); } This works when I press the back key but when I press the home key, it never clear subscriptions.

MainActivity V ⇢ onPause() MainActivity V ⇠ onPause [8ms] MainActivity V ⇢ onStop() XYZController V ⇢ onDetach(view=android.widget.RelativeLayout{...}) V ⇠ onDetach [0ms] MainActivity V ⇠ onStop [6ms]

sockeqwe commented 6 years ago

Thanks for reporting! Yes, this is a known issue (there are some related issues like better backstack support etc) and I will try to fix this as soon as possible, hopefully within the next two weeks as it is involves slightly bigger internal changes.

On Do., 31. Aug. 2017, 19:54 Zahid Rasheed notifications@github.com wrote:

I am trying to dispose my subscriptions like below in my presenter extending MvpBasePresenter

@override https://github.com/override public void detachView(boolean retainInstance) { compositeSubscription.clear(); super.detachView(retainInstance); } This works when I press the back key but when I press the home key, it never clear subscriptions.

MainActivity V ⇢ onPause() MainActivity V ⇠ onPause [8ms] MainActivity V ⇢ onStop() XYZController V ⇢ onDetach(view=android.widget.RelativeLayout{...}) V ⇠ onDetach [0ms] MainActivity V ⇠ onStop [6ms]

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/sockeqwe/mosby/issues/273, or mute the thread https://github.com/notifications/unsubscribe-auth/AAjnrms7Ehp33-CdsUrCJqSKBx8TInbrks5sdvM4gaJpZM4PJMDc .

chris6647 commented 6 years ago

Our temporary fix involves having each presenter implement an interface Clearable, with an onCleared method. Each presenter calls that method on detachView, as well as our BaseController calling it onDetach (we are using Conductor, so if you're not, it would just be the equivalent method in i.e. Fragments) to ensure we are properly cleaning up after ourselves every time.

sockeqwe commented 6 years ago

Im working on that, seems that I can provide a snapshot with a fix for that tomorrow ( at least for MVI, MVP soon afterwards)

sockeqwe commented 6 years ago

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

ZahidRasheed commented 6 years ago

I just gave it a shot. So I removed the deprecated detachView(boolean retainInstance) and tried using detachView() and destroy() but apparently both of them never get called, not even when the view is detached.

sockeqwe commented 6 years ago

hm that is strange ... do you use MVI, MVP, MVP + ViewState? 3.1.0-snapshot?

ZahidRasheed commented 6 years ago

We are using MVP and mvp-conductor (3.0.0)

mvp version: 3.0.4

App goes into background Don't keep activities: On

    StartZoneController  V  ⇢ onViewBound(view=android.widget.RelativeLayout{384c681 V.E...... ......ID 0,0-0,0})
                         V  ⇠ onViewBound [9ms]
     StartZonePresenter  V  ⇢ attachView(view=com.example.startzone.presentation.StartZoneController@cca0326)
                         V  ⇠ attachView [10ms]
    StartZoneController  V  ⇢ onDetach(view=android.widget.RelativeLayout{384c681 V.E...... ......ID 0,0-1440,175})
                         V  ⇠ onDetach [0ms]
     StartZonePresenter  V  ⇢ detachView(retainInstance=false)
                         V  ⇠ detachView [1ms]

App goes into background Don't keep activities: Off

    StartZoneController  V  ⇢ onViewBound(view=android.widget.RelativeLayout{384c681 V.E...... ......ID 0,0-0,0})
                         V  ⇠ onViewBound [9ms]
     StartZonePresenter  V  ⇢ attachView(view=com.example.startzone.presentation.StartZoneController@cca0326)
                         V  ⇠ attachView [10ms]
    StartZoneController  V  ⇢ onDetach(view=android.widget.RelativeLayout{384c681 V.E...... ......ID 0,0-1440,175})
                         V  ⇠ onDetach [0ms]

^^ Presenter was never detached.

mvp version: 3.1.0-SNAPTSHOT

I can see that detachView is now deprecated. So now I am looking for: detachView() and destroy().

App goes into background Don't keep activities: On

    StartZoneController  V  ⇢ onViewBound(view=android.widget.RelativeLayout{384c681 V.E...... ......ID 0,0-0,0})
                         V  ⇠ onViewBound [9ms]
     StartZonePresenter  V  ⇢ attachView(view=com.example.startzone.presentation.StartZoneController@cca0326)
                         V  ⇠ attachView [10ms]
    StartZoneController  V  ⇢ onDetach(view=android.widget.RelativeLayout{384c681 V.E...... ......ID 0,0-1440,175})
                         V  ⇠ onDetach [0ms]
     StartZonePresenter  V  ⇢ detachView(retainInstance=false)
                         V  ⇠ detachView [1ms]

^^No detachView() or destroy()??

App goes into background Don't keep activities: Off

    StartZoneController  V  ⇢ onViewBound(view=android.widget.RelativeLayout{384c681 V.E...... ......ID 0,0-0,0})
                         V  ⇠ onViewBound [9ms]
     StartZonePresenter  V  ⇢ attachView(view=com.example.startzone.presentation.StartZoneController@cca0326)
                         V  ⇠ attachView [10ms]
    StartZoneController  V  ⇢ onDetach(view=android.widget.RelativeLayout{384c681 V.E...... ......ID 0,0-1440,175})
                         V  ⇠ onDetach [0ms]

^^No detachView(retainInstance=false), detachView() or destroy()??

sockeqwe commented 6 years ago

That seems like the desired behavior? If you app is in background, but hasn't stopped yet, there is still a view (although not visible on screen) that can be updated. Hence view not detached from presenter and therefore never called.

Since you are​ using Conductor: the new snapshot 3.1.0 is not compatible yet with current conductor-mosby plugin 3.0.x ...

On Di., 3. Okt. 2017, 11:51 Zahid Rasheed notifications@github.com wrote:

We are using MVP and mvp-conductor (3.0.0)

mvp version: 3.0.4

App goes into background Don't keep activities: On

StartZoneController  V  ⇢ onViewBound(view=android.widget.RelativeLayout{384c681 V.E...... ......ID 0,0-0,0})
                     V  ⇠ onViewBound [9ms]
 StartZonePresenter  V  ⇢ attachView(view=com.example.startzone.presentation.StartZoneController@cca0326)
                     V  ⇠ attachView [10ms]
StartZoneController  V  ⇢ onDetach(view=android.widget.RelativeLayout{384c681 V.E...... ......ID 0,0-1440,175})
                     V  ⇠ onDetach [0ms]
 StartZonePresenter  V  ⇢ detachView(retainInstance=false)
                     V  ⇠ detachView [1ms]

App goes into background Don't keep activities: Off

StartZoneController  V  ⇢ onViewBound(view=android.widget.RelativeLayout{384c681 V.E...... ......ID 0,0-0,0})
                     V  ⇠ onViewBound [9ms]
 StartZonePresenter  V  ⇢ attachView(view=com.example.startzone.presentation.StartZoneController@cca0326)
                     V  ⇠ attachView [10ms]
StartZoneController  V  ⇢ onDetach(view=android.widget.RelativeLayout{384c681 V.E...... ......ID 0,0-1440,175})
                     V  ⇠ onDetach [0ms]

^^ Presenter was never detached.

mvp version: 3.1.0-SNAPTSHOT

I can see that detachView is now deprecated. So now I am looking for: detachView() and destroy().

App goes into background Don't keep activities: On

StartZoneController  V  ⇢ onViewBound(view=android.widget.RelativeLayout{384c681 V.E...... ......ID 0,0-0,0})
                     V  ⇠ onViewBound [9ms]
 StartZonePresenter  V  ⇢ attachView(view=com.example.startzone.presentation.StartZoneController@cca0326)
                     V  ⇠ attachView [10ms]
StartZoneController  V  ⇢ onDetach(view=android.widget.RelativeLayout{384c681 V.E...... ......ID 0,0-1440,175})
                     V  ⇠ onDetach [0ms]
 StartZonePresenter  V  ⇢ detachView(retainInstance=false)
                     V  ⇠ detachView [1ms]

^^No detachView() or destroy()??

App goes into background Don't keep activities: Off

StartZoneController  V  ⇢ onViewBound(view=android.widget.RelativeLayout{384c681 V.E...... ......ID 0,0-0,0})
                     V  ⇠ onViewBound [9ms]
 StartZonePresenter  V  ⇢ attachView(view=com.example.startzone.presentation.StartZoneController@cca0326)
                     V  ⇠ attachView [10ms]
StartZoneController  V  ⇢ onDetach(view=android.widget.RelativeLayout{384c681 V.E...... ......ID 0,0-1440,175})
                     V  ⇠ onDetach [0ms]

^^No detachView(retainInstance=false), detachView() or destroy()??

— You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub https://github.com/sockeqwe/mosby/issues/273#issuecomment-333794203, or mute the thread https://github.com/notifications/unsubscribe-auth/AAjnrl9bLu_22bO6QFbg7N9vHPV92H59ks5sogObgaJpZM4PJMDc .

ZahidRasheed commented 6 years ago

Please feel free to close this issue. We had a confusion as we thought Presenter will get a detachView call back when the Controller gets onDetach callback.