ModalContainer doesn't tear down dialogs when destroyed

[zach-klippenstein]

It does this when its context's lifecycle is destroyed, but not its local lifecycle is being torn down. Another possible approach would be to tear down on detach. Not sure which is more correct yet.

[zach-klippenstein]

Discussion from slack:

Zach:

@ray Ryan Any reason by ModalContainer doesn’t just tear the dialogs down when the view is detached? It does so when its context’s lifecycle is destroyed, but container views’ primary lifecycle is their attach state, and this container just seems to assume it will be attached forever.
I think the container should tear them down when detached, and also make sure to re-show them when attached

Ray:

Totally agree, nice catch
Not sure the attach side is necessary?
Well, I guess we can’t count on update being called after attach

Zach:

if we tear down when we detach, then we need to re-setup when attached, otherwise if some parent just detaches and reattaches for some reason then the dialogs won’t show.
that’s unlikely, but it should work if done

Ray:

Yeah

Zach:

also the only reason i even noticed this was because i’m writing a test for some of the androidx integration that assumes that this is all wired correctly

Ray:

It would be great to get rid of the lifecycle thing. Just have to hope that Android’s nag comes after the activity content view is detached. Surely…

Zach:

actually the real bug right now is that the dialog teardown listens to the context’s lifecycle, not the local one that is now being provided from the parent container
that would be a less drastic change too, only a few lines and leave the attach behavior the same. At least then dialogs would go away correctly if the container will never be re-attached

Ray:

It’s such a wart. The code is entirely there to prevent noise in logcat, there’s no actually bad behavior.
I’d love to do your first proposal and light that block on fire
It’s already caused one or two memory leaks
Isn’t there also an old bug where we don’t tear down dialogs if updates go in an unexpected order? This might fix both

[rjrjr]

See also #204

[zach-klippenstein]

Yea, I think if we make ModalContainer do the right thing (either on detach or WorkflowLifecycleOwner destroy), #204 will get fixed as well.