badoo / RIBs

Badoo's take on RIBs
Apache License 2.0
162 stars 50 forks source link

Lifecycle events can be missing #44

Open Nublo opened 5 years ago

Nublo commented 5 years ago

Let's consider next situation. You added container RIB with Screen1Rib. Than during work with this RIB you decided to replace it to another Screen2Rib inside same container. Screen2Rib will not receive onStart and onResume events because of this code:

fun onAttach(savedInstanceState: Bundle?) {
    tag = savedInstanceState?.getString(KEY_TAG) ?: tag
    lifecycleRelay.accept(ACTIVE)
    ribLifecycleRegistry.handleLifecycleEvent(Lifecycle.Event.ON_CREATE)
    onAttach(ribLifecycleRegistry, savedInstanceState)
}

I think when you attaching/replacing RIB you should receive lifecycleEvents.

zsoltk commented 5 years ago

@Nublo start/stop can be done trivially (all Nodes that go to back stack, basically have their views detached, are stopped).

Pause/resume is more tricky in case of overlays, as content below overlays should be conceptually paused. I can see how it can be done probably after #53 is merged.

Also, most cases (currently in all our cases) overlays result in an AlertDialog being showed, which will trigger onPause on hosting Activity, which will pause all on-screen Nodes. That means that even the Node inside the AlertDialog will be paused 😬 . Probably can be solved by not forwarding pause to non-view-parented Nodes?

Anyway, pause/resume seems trickier. If start/stop is enough for now, it can be added relatively easily, so we can probably do it in two parts. What do you think?

Nublo commented 5 years ago

Sure, splitting into to separate issues is totally fine. Right now we have issues only with start/stop.

zsoltk commented 5 years ago

I believe showing a dialog as an overlay through portal (introduced in #83) solves the tricky case I mentioned in my last comment, which means this ticket is probably solved. Will check later.