Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add lifecycle aware Presenters #1282

Merged
merged 15 commits into from
May 26, 2024
Merged

Add lifecycle aware Presenters #1282

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
a7882a1
Try out pausing presenters
chrisbanes Mar 13, 2024
ff394c3
Remove logging
chrisbanes May 23, 2024
9dede6c
LifecycleImp to MutableLifecycle
chrisbanes May 24, 2024
a7f04ee
Refactor PauseablePresenter to functions
chrisbanes May 24, 2024
bcfb04f
Add opt-out NonPausablePresenter
chrisbanes May 24, 2024
34106a0
Add more comments
chrisbanes May 24, 2024
0d76e4d
Update Retained to use Any keys
chrisbanes May 24, 2024
109fe52
More kdoc
chrisbanes May 24, 2024
76b5e96
API feedback
chrisbanes May 26, 2024
b15676c
Merge branch 'refs/heads/main' into cb/pause-presenter
chrisbanes May 26, 2024
8bbbb50
Tweak pauseable state kdoc
chrisbanes May 26, 2024
94ccc2c
Fix composition loop on KeyedCircuitContentTests
chrisbanes May 26, 2024
59d41e0
Update KeyedCircuitContentTests to use remember
chrisbanes May 26, 2024
17a382e
Revert retained keys change
chrisbanes May 26, 2024
edc10fd
Update CHANGELOG
chrisbanes May 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 74 additions & 57 deletions ...UnitTest/kotlin/com/slack/circuit/foundation/NavigableCircuitViewModelStateAndroidTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: Apache-2.0
package com.slack.circuit.foundation

import androidx.compose.ui.test.MainTestClock
import androidx.compose.ui.test.SemanticsMatcher
import androidx.compose.ui.test.assertAll
import androidx.compose.ui.test.assertAny
Expand Down Expand Up @@ -36,13 +37,12 @@ class NavigableCircuitViewModelStateAndroidTest {
@Test
fun retainedStateScopedToBackstackWithRecreations() {
composeTestRule.run {
mainClock.autoAdvance = false
mainClock.autoAdvance = true

// Current: Screen A. Increase count to 1
onNodeWithTag(TAG_LABEL).assertTextEquals("A")
onNodeWithTag(TAG_COUNT).assertTextEquals("0")
onNodeWithTag(TAG_INCREASE_COUNT).performClick()
mainClock.advanceTimeByFrame()
chrisbanes marked this conversation as resolved.
Show resolved Hide resolved
onNodeWithTag(TAG_COUNT).assertTextEquals("1")

// Now recreate the Activity and assert that the values were retained
Expand All @@ -52,11 +52,9 @@ class NavigableCircuitViewModelStateAndroidTest {

// Navigate to Screen B. Increase count to 1
onNodeWithTag(TAG_GO_NEXT).performClick()
mainClock.advanceTimeBy(1_000)
onNodeWithTag(TAG_LABEL).assertTextEquals("B")
onNodeWithTag(TAG_COUNT).assertTextEquals("0")
onNodeWithTag(TAG_INCREASE_COUNT).performClick()
mainClock.advanceTimeByFrame()
onNodeWithTag(TAG_COUNT).assertTextEquals("1")

// Now recreate the Activity and assert that the values were retained
Expand All @@ -66,49 +64,53 @@ class NavigableCircuitViewModelStateAndroidTest {

// Navigate to Screen C. Increase count to 1
onNodeWithTag(TAG_GO_NEXT).performClick()
mainClock.advanceTimeBy(1_000)
onNodeWithTag(TAG_LABEL).assertTextEquals("C")
onNodeWithTag(TAG_COUNT).assertTextEquals("0")
onNodeWithTag(TAG_INCREASE_COUNT).performClick()
mainClock.advanceTimeByFrame()
onNodeWithTag(TAG_COUNT).assertTextEquals("1")

// Now recreate the Activity and assert that the values were retained
scenario.recreate()
onNodeWithTag(TAG_LABEL).assertTextEquals("C")
onNodeWithTag(TAG_COUNT).assertTextEquals("1")

// Pop to Screen B. Increase count from 1 to 2.
onNodeWithTag(TAG_POP).performClick()

// Part-way through pop, both screens should be visible
onEachFrameWhileMultipleScreens(hasTestTag(TAG_LABEL)) {
onAllNodesWithTag(TAG_LABEL)
.assertCountEquals(2)
.assertAny(hasTextExactly("C"))
.assertAny(hasTextExactly("B"))
onAllNodesWithTag(TAG_COUNT).assertCountEquals(2).assertAll(hasTextExactly("1"))
mainClock.withAutoAdvance(false) {
// Pop to Screen B
onNodeWithTag(TAG_POP).performClick()

// Part-way through pop, both screens should be visible
onEachFrameWhileMultipleScreens(hasTestTag(TAG_LABEL)) {
onAllNodesWithTag(TAG_LABEL)
.assertCountEquals(2)
.assertAny(hasTextExactly("C"))
.assertAny(hasTextExactly("B"))
onAllNodesWithTag(TAG_COUNT).assertCountEquals(2).assertAll(hasTextExactly("1"))
}
}

// Increase count from 1 to 2.
onNodeWithTag(TAG_LABEL).assertTextEquals("B")
onNodeWithTag(TAG_COUNT).assertTextEquals("1")
onNodeWithTag(TAG_INCREASE_COUNT).performClick()
mainClock.advanceTimeByFrame()
onNodeWithTag(TAG_COUNT).assertTextEquals("2")

// Navigate to Screen C. Assert that it's state was not retained
onNodeWithTag(TAG_GO_NEXT).performClick()

// Part-way through push, both screens should be visible
onEachFrameWhileMultipleScreens(hasTestTag(TAG_LABEL)) {
onAllNodesWithTag(TAG_LABEL)
.assertCountEquals(2)
.assertAny(hasTextExactly("C"))
.assertAny(hasTextExactly("B"))
onAllNodesWithTag(TAG_COUNT)
.assertCountEquals(2)
.assertAny(hasTextExactly("0"))
.assertAny(hasTextExactly("2"))
mainClock.withAutoAdvance(false) {
// Navigate to Screen C
onNodeWithTag(TAG_GO_NEXT).performClick()

// Part-way through push, both screens should be visible
onEachFrameWhileMultipleScreens(hasTestTag(TAG_LABEL)) {
onAllNodesWithTag(TAG_LABEL)
.assertCountEquals(2)
.assertAny(hasTextExactly("C"))
.assertAny(hasTextExactly("B"))
onAllNodesWithTag(TAG_COUNT)
.assertCountEquals(2)
.assertAny(hasTextExactly("0"))
.assertAny(hasTextExactly("2"))
}
}
// Assert that Screen C's state was retained
onNodeWithTag(TAG_LABEL).assertTextEquals("C")
onNodeWithTag(TAG_COUNT).assertTextEquals("0")

Expand All @@ -117,20 +119,23 @@ class NavigableCircuitViewModelStateAndroidTest {
onNodeWithTag(TAG_LABEL).assertTextEquals("C")
onNodeWithTag(TAG_COUNT).assertTextEquals("0")

// Pop to Screen B. Assert that it's state was retained
onNodeWithTag(TAG_POP).performClick()

// Part-way through pop, both screens should be visible
onEachFrameWhileMultipleScreens(hasTestTag(TAG_LABEL)) {
onAllNodesWithTag(TAG_LABEL)
.assertCountEquals(2)
.assertAny(hasTextExactly("C"))
.assertAny(hasTextExactly("B"))
onAllNodesWithTag(TAG_COUNT)
.assertCountEquals(2)
.assertAny(hasTextExactly("0"))
.assertAny(hasTextExactly("2"))
mainClock.withAutoAdvance(false) {
// Pop to Screen B
onNodeWithTag(TAG_POP).performClick()

// Part-way through pop, both screens should be visible
onEachFrameWhileMultipleScreens(hasTestTag(TAG_LABEL)) {
onAllNodesWithTag(TAG_LABEL)
.assertCountEquals(2)
.assertAny(hasTextExactly("C"))
.assertAny(hasTextExactly("B"))
onAllNodesWithTag(TAG_COUNT)
.assertCountEquals(2)
.assertAny(hasTextExactly("0"))
.assertAny(hasTextExactly("2"))
}
}
// Assert that Screen B's state was retained
onNodeWithTag(TAG_LABEL).assertTextEquals("B")
onNodeWithTag(TAG_COUNT).assertTextEquals("2")

Expand All @@ -139,20 +144,23 @@ class NavigableCircuitViewModelStateAndroidTest {
onNodeWithTag(TAG_LABEL).assertTextEquals("B")
onNodeWithTag(TAG_COUNT).assertTextEquals("2")

// Pop to Screen A. Assert that it's state was retained
onNodeWithTag(TAG_POP).performClick()

// Part-way through pop, both screens should be visible
onEachFrameWhileMultipleScreens(hasTestTag(TAG_LABEL)) {
onAllNodesWithTag(TAG_LABEL)
.assertCountEquals(2)
.assertAny(hasTextExactly("B"))
.assertAny(hasTextExactly("A"))
onAllNodesWithTag(TAG_COUNT)
.assertCountEquals(2)
.assertAny(hasTextExactly("2"))
.assertAny(hasTextExactly("1"))
mainClock.withAutoAdvance(false) {
// Pop to Screen A
onNodeWithTag(TAG_POP).performClick()

// Part-way through pop, both screens should be visible
onEachFrameWhileMultipleScreens(hasTestTag(TAG_LABEL)) {
onAllNodesWithTag(TAG_LABEL)
.assertCountEquals(2)
.assertAny(hasTextExactly("B"))
.assertAny(hasTextExactly("A"))
onAllNodesWithTag(TAG_COUNT)
.assertCountEquals(2)
.assertAny(hasTextExactly("2"))
.assertAny(hasTextExactly("1"))
}
}
// Assert that Screen B's state was retained
onNodeWithTag(TAG_LABEL).assertTextEquals("A")
onNodeWithTag(TAG_COUNT).assertTextEquals("1")

Expand All @@ -163,7 +171,6 @@ class NavigableCircuitViewModelStateAndroidTest {

// Navigate to Screen B. Assert that it's state was not retained
onNodeWithTag(TAG_GO_NEXT).performClick()
mainClock.advanceTimeBy(1_000)
onNodeWithTag(TAG_LABEL).assertTextEquals("B")
onNodeWithTag(TAG_COUNT).assertTextEquals("0")
}
Expand All @@ -188,3 +195,13 @@ class NavigableCircuitViewModelStateAndroidTest {
}
}
}

private fun MainTestClock.withAutoAdvance(value: Boolean, block: () -> Unit) {
val currentAutoAdvance = this.autoAdvance
try {
this.autoAdvance = value
block()
} finally {
this.autoAdvance = currentAutoAdvance
}
}
6 changes: 5 additions & 1 deletion circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,11 @@ public fun <UiState : CircuitUiState> CircuitContent(
onDispose(eventListener::onDisposePresent)
}

val state = presenter.present()
val state =
when (presenter) {
is NonPausablePresenter<UiState> -> presenter.present()
else -> presenter.presentWithLifecycle()
}

// TODO not sure why stateFlow + LaunchedEffect + distinctUntilChanged doesn't work here
SideEffect { eventListener.onState(state) }
Expand Down
88 changes: 47 additions & 41 deletions ...-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavigableCircuitContent.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ public fun <R : Record> NavigableCircuitContent(
circuit.onUnavailableContent,
) {
val activeContentProviders =
backStack.buildCircuitContentProviders(
buildCircuitContentProviders(
backStack = backStack,
navigator = navigator,
circuit = circuit,
unavailableRoute = unavailableRoute,
Expand Down Expand Up @@ -103,33 +104,15 @@ public fun <R : Record> NavigableCircuitContent(

CompositionLocalProvider(LocalRetainedStateRegistry provides outerRegistry) {
decoration.DecoratedContent(activeContentProviders, backStack.size, modifier) { provider ->
// We retain the record's retained state registry for as long as the back stack
chrisbanes marked this conversation as resolved.
Show resolved Hide resolved
// contains the record
val record = provider.record
val recordInBackStackRetainChecker =
remember(backStack, record) {
CanRetainChecker { backStack.containsRecord(record, includeSaved = true) }
}

CompositionLocalProvider(LocalCanRetainChecker provides recordInBackStackRetainChecker) {
// Remember the `providedValues` lookup because this composition can live longer than
// the record is present in the backstack, if the decoration is animated for example.
val values = remember(record) { providedValues[record] }?.provideValues()
val providedLocals = remember(values) { values?.toTypedArray() ?: emptyArray() }
// Remember the `providedValues` lookup because this composition can live longer than
// the record is present in the backstack, if the decoration is animated for example.
val values = remember(record) { providedValues[record] }?.provideValues()
val providedLocals = remember(values) { values?.toTypedArray() ?: emptyArray() }

// Now provide a new registry to the content for it to store any retained state in,
// along with a retain checker which is always true (as upstream registries will
// maintain the lifetime), and the other provided values
val recordRetainedStateRegistry =
rememberRetained(key = record.registryKey) { RetainedStateRegistry() }
CompositionLocalProvider(
LocalRetainedStateRegistry provides recordRetainedStateRegistry,
LocalCanRetainChecker provides CanRetainChecker.Always,
LocalBackStack provides backStack,
*providedLocals,
) {
provider.content(record)
}
CompositionLocalProvider(LocalBackStack provides backStack, *providedLocals) {
provider.content(record)
}
}
}
Expand Down Expand Up @@ -163,37 +146,60 @@ public class RecordContentProvider<R : Record>(
}

@Composable
private fun <R : Record> BackStack<R>.buildCircuitContentProviders(
private fun <R : Record> buildCircuitContentProviders(
backStack: BackStack<R>,
navigator: Navigator,
circuit: Circuit,
unavailableRoute: @Composable (screen: Screen, modifier: Modifier) -> Unit,
): ImmutableList<RecordContentProvider<R>> {
val previousContentProviders = remember { mutableMapOf<String, RecordContentProvider<R>>() }

val lastBackStack by rememberUpdatedState(backStack)
val lastNavigator by rememberUpdatedState(navigator)
val lastCircuit by rememberUpdatedState(circuit)
val lastUnavailableRoute by rememberUpdatedState(unavailableRoute)

return iterator()
fun createRecordContent() =
movableContentOf<R> { record ->
val recordInBackStackRetainChecker =
remember(lastBackStack, record) {
CanRetainChecker { lastBackStack.containsRecord(record, includeSaved = true) }
}

val lifecycle =
remember { MutableRecordLifecycle() }.apply { isActive = lastBackStack.topRecord == record }

CompositionLocalProvider(LocalCanRetainChecker provides recordInBackStackRetainChecker) {
// Now provide a new registry to the content for it to store any retained state in,
// along with a retain checker which is always true (as upstream registries will
// maintain the lifetime), and the other provided values
val recordRetainedStateRegistry =
rememberRetained(key = record.registryKey) { RetainedStateRegistry() }

CompositionLocalProvider(
LocalRetainedStateRegistry provides recordRetainedStateRegistry,
LocalCanRetainChecker provides CanRetainChecker.Always,
LocalRecordLifecycle provides lifecycle,
) {
CircuitContent(
screen = record.screen,
navigator = lastNavigator,
circuit = lastCircuit,
unavailableContent = lastUnavailableRoute,
key = record.key,
)
}
}
}

return lastBackStack
.iterator()
.asSequence()
.map { record ->
// Query the previous content providers map, so that we use the same
// RecordContentProvider instances across calls.
previousContentProviders.getOrPut(record.key) {
RecordContentProvider(
record = record,
content =
movableContentOf { record ->
CircuitContent(
screen = record.screen,
modifier = Modifier,
navigator = lastNavigator,
circuit = lastCircuit,
unavailableContent = lastUnavailableRoute,
key = record.key,
)
},
)
RecordContentProvider(record = record, content = createRecordContent())
}
}
.toImmutableList()
Expand Down
Loading
Loading