bluelinelabs / Conductor

A small, yet full-featured framework that allows building View-based Android applications
Apache License 2.0
3.9k stars 345 forks source link

Controller is not detached when controller's root component is SwipeRefreshLayout #224

Closed palaima closed 7 years ago

palaima commented 7 years ago

I'm encountering some rare behaviour which happens time to time and I'm not able to find reproducible path.

There is RootController and few child controllers (ChildController1, ChildController2)

Basically what I'm doing I'm going back and forth from ChildController1 to ChildController2 and sometimes ChildController1 is not detached (lifecycle methods are not called) even so ChildController2 is attached.

BTW, LeakCanary immediately informs that ChildController1 was leaked

Does anyone encounters such a behaviour?

Happens with 2.x.x versions

palaima commented 7 years ago

Demonstration for issue.

conductor_screen_not_detached

NewTransferController -> RecipientController -> Back (works as expected) NewTransferController -> RecipientController --->>> NewTransferController is not detached

This is what I see in logs.

NewTransferController@9b192ee || CREATE
NewTransferController@9b192ee || CREATE_VIEW
NewTransferController@9b192ee || ATTACH

RecipientController@3633e34 || CREATE
RecipientController@3633e34 || CREATE_VIEW
RecipientController@3633e34 || ATTACH

NewTransferController is not detached

palaima commented 7 years ago

Even so RecipientController receives all touch events

palaima commented 7 years ago

Got 100% reproducible way.

Let's say Controller's layout root component is SwipeRefreshLayout Open that controller. Swipe down. While spinner is showing push other Controller -> this issue

palaima commented 7 years ago

If you wrap SwipeRefreshLayout into e.g. FrameLayout everything works as expected.

Checkout my fork https://github.com/palaima/Conductor There is demo app (myapplication) which represents this issue.

https://github.com/palaima/Conductor/blob/develop/myapplication/src/main/java/com/example/myapplication/controller/Child1Controller.java#L42

Change R.layout.controller_child_1 to R.layout.controller_child_1_issue

EricKuck commented 7 years ago

I haven't used SwipeRefreshLayout too much, but it does some weird stuff. I don't believe this is actually related to Conductor, since replacing your toChild2() method with this:

    @OnClick(R.id.to_child_2)
    public void toChild2() {
        ((ViewGroup)getView().getParent()).removeView(getView());
    }

doesn't seem to work either. I'll keep looking into it, just posting what I've found so far.

Calling refreshLayout.setEnabled(false); right before starting the transition does work if you need a quick workaround.

dimsuz commented 7 years ago

For the record we also had similar problem in our app when using SwipeRefreshLayout and Conductor. It was solved with some weird workaround too (I'd have to dig in to find which one exactly...)

EricKuck commented 7 years ago

After further investigation, this is an issue with the support lib. Here's a link to a bug report on it, where a Google engineer posts a workaround:

https://code.google.com/p/android/issues/detail?id=78062

There are several other open issues that are duplicates of this one. None of the issues I've found have been marked as fixed.

palaima commented 7 years ago

Great! this is not library issue. So I'm closing this issue. Thanks