Skip to content

Commit

Permalink
Uses DisposeOnViewTreeLifecycleDestroyed for ComposeView
Browse files Browse the repository at this point in the history
Fixes a problem where removing a `ComposeView` from the view hierarchy and then reintroducing it breaks things. For example this happens when `DialogCollator` calls `dimiss()` / `show()` on `Dialog` windows to re-order them. But it could also happen with things like `RecyclerView`, or anything that might do interesting classic view management. This kind of issue is why `ViewTreeLifecycleOwner` was invented, and why we've worked so hard to keep it working.
  • Loading branch information
rjrjr committed Jul 31, 2024
1 parent 040ece4 commit 698aa80
Show file tree
Hide file tree
Showing 2 changed files with 184 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.squareup.workflow1.ui.compose
import android.content.Context
import android.view.View
import android.view.ViewGroup
import android.widget.FrameLayout
import androidx.activity.ComponentDialog
import androidx.compose.foundation.clickable
import androidx.compose.foundation.text.BasicText
Expand Down Expand Up @@ -30,10 +31,14 @@ import com.squareup.workflow1.ui.Compatible
import com.squareup.workflow1.ui.NamedScreen
import com.squareup.workflow1.ui.Screen
import com.squareup.workflow1.ui.ScreenViewFactory
import com.squareup.workflow1.ui.ScreenViewFactory.Companion.forWrapper
import com.squareup.workflow1.ui.ScreenViewFactory.Companion.fromCode
import com.squareup.workflow1.ui.ScreenViewHolder
import com.squareup.workflow1.ui.ViewEnvironment
import com.squareup.workflow1.ui.ViewRegistry
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
import com.squareup.workflow1.ui.WorkflowViewStub
import com.squareup.workflow1.ui.Wrapper
import com.squareup.workflow1.ui.internal.test.IdleAfterTestRule
import com.squareup.workflow1.ui.internal.test.IdlingDispatcherRule
import com.squareup.workflow1.ui.internal.test.WorkflowUiTestActivity
Expand Down Expand Up @@ -651,6 +656,163 @@ internal class ComposeViewTreeIntegrationTest {
.assertIsDisplayed()
}

@Test fun composition_handles_overlay_reordering() {
val composeA: Screen = VanillaComposeRendering(
compatibilityKey = "0",
) {
var counter by rememberSaveable { mutableStateOf(0) }
BasicText(
"Counter: $counter",
Modifier
.clickable { counter++ }
.testTag(CounterTag)
)
}

val composeB: Screen = VanillaComposeRendering(
compatibilityKey = "1",
) {
var counter by rememberSaveable { mutableStateOf(0) }
BasicText(
"Counter2: $counter",
Modifier
.clickable { counter++ }
.testTag(CounterTag2)
)
}

scenario.onActivity {
it.setRendering(
BodyAndOverlaysScreen(
EmptyRendering,
listOf(
TestOverlay(composeA),
TestOverlay(composeB),
// When we move this to the front, both of the other previously-upstream-
// now-downstream dialogs will be dismissed and re-shown.
TestOverlay(EmptyRendering)
)
)
)
}

composeRule.onNodeWithTag(CounterTag)
.assertTextEquals("Counter: 0")
.performClick()
.assertTextEquals("Counter: 1")

composeRule.onNodeWithTag(CounterTag2)
.assertTextEquals("Counter2: 0")
.performClick()
.assertTextEquals("Counter2: 1")

// Reorder the overlays, dialogs will be dismissed and re-shown to preserve order.

scenario.onActivity {
it.setRendering(
BodyAndOverlaysScreen(
EmptyRendering,
listOf(
TestOverlay(EmptyRendering),
TestOverlay(composeB),
TestOverlay(composeA),
)
)
)
}

// Are they still responsive?

composeRule.onNodeWithTag(CounterTag)
.assertTextEquals("Counter: 1")
.performClick()
.assertTextEquals("Counter: 2")

composeRule.onNodeWithTag(CounterTag2)
.assertTextEquals("Counter2: 1")
.performClick()
.assertTextEquals("Counter2: 2")
}


@Test fun composition_under_view_stub_handles_overlay_reordering() {
val composeA: Screen = VanillaComposeRendering(
compatibilityKey = "0",
) {
var counter by rememberSaveable { mutableStateOf(0) }
BasicText(
"Counter: $counter",
Modifier
.clickable { counter++ }
.testTag(CounterTag)
)
}

val composeB: Screen = VanillaComposeRendering(
compatibilityKey = "1",
) {
var counter by rememberSaveable { mutableStateOf(0) }
BasicText(
"Counter2: $counter",
Modifier
.clickable { counter++ }
.testTag(CounterTag2)
)
}

scenario.onActivity {
it.setRendering(
BodyAndOverlaysScreen(
EmptyRendering,
listOf(
TestOverlay(ViewStubWrapper(composeA)),
TestOverlay(ViewStubWrapper(composeB)),
// When we move this to the front, both of the other previously-upstream-
// now-downstream dialogs will be dismissed and re-shown.
TestOverlay(EmptyRendering)
)
)
)
}

composeRule.onNodeWithTag(CounterTag)
.assertTextEquals("Counter: 0")
.performClick()
.assertTextEquals("Counter: 1")

composeRule.onNodeWithTag(CounterTag2)
.assertTextEquals("Counter2: 0")
.performClick()
.assertTextEquals("Counter2: 1")

// Reorder the overlays, dialogs will be dismissed and re-shown to preserve order.

scenario.onActivity {
it.setRendering(
BodyAndOverlaysScreen(
EmptyRendering,
listOf(
TestOverlay(EmptyRendering),
TestOverlay(ViewStubWrapper(composeB)),
TestOverlay(ViewStubWrapper(composeA)),
)
)
)
}

// Are they still responsive?

composeRule.onNodeWithTag(CounterTag)
.assertTextEquals("Counter: 1")
.performClick()
.assertTextEquals("Counter: 2")

composeRule.onNodeWithTag(CounterTag2)
.assertTextEquals("Counter2: 1")
.performClick()
.assertTextEquals("Counter2: 2")
}

private fun WorkflowUiTestActivity.setBackstack(vararg backstack: Screen) {
setRendering(
BackStackScreen.fromList(listOf<AndroidScreen<*>>(EmptyRendering) + backstack.asList())
Expand All @@ -668,6 +830,26 @@ internal class ComposeViewTreeIntegrationTest {
}
}

data class ViewStubWrapper<C : Screen>(
override val content: C
) : Screen, Wrapper<Screen, C>, AndroidScreen<ViewStubWrapper<C>> {
override fun <D : Screen> map(transform: (C) -> D) = ViewStubWrapper(transform(content))

override val viewFactory: ScreenViewFactory<ViewStubWrapper<C>> =
fromCode { _, initialEnvironment, context, _ ->
val stub = WorkflowViewStub(context)

FrameLayout(context)
.apply {
this.addView(stub)
}.let {
ScreenViewHolder(initialEnvironment, it) { r, e ->
stub.show(r.content, e)
}
}
}
}

/**
* This is our own custom lovingly handcrafted implementation that creates [ComposeView]
* itself, bypassing [ScreenComposableFactory] entirely. Allows us to mess with alternative
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import androidx.compose.runtime.CompositionLocalProvider
import androidx.compose.runtime.remember
import androidx.compose.ui.platform.ComposeView
import androidx.compose.ui.platform.LocalLifecycleOwner
import androidx.compose.ui.platform.ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed
import androidx.compose.ui.viewinterop.AndroidView
import androidx.lifecycle.setViewTreeLifecycleOwner
import com.squareup.workflow1.ui.Screen
Expand Down Expand Up @@ -125,6 +126,7 @@ public fun <ScreenT : Screen> ScreenComposableFactory<ScreenT>.asViewFactory():
container: ViewGroup?
): ScreenViewHolder<ScreenT> {
val view = ComposeView(context)
view.setViewCompositionStrategy(DisposeOnViewTreeLifecycleDestroyed)
return ScreenViewHolder(initialEnvironment, view) { newRendering, environment ->

// Update the state whenever a new rendering is emitted.
Expand Down

0 comments on commit 698aa80

Please sign in to comment.