Skip to content

Commit

Permalink
Add lifecycle aware Presenters (#1282)
Browse files Browse the repository at this point in the history
This PR adds a new `Lifecycle` interface, which presenters and UI can
observe to know when they are 'paused'. The API is rudimentary at the
moment and will change before this is ready to land.

A bundled `PauseablePresenter` class can be extended, enabling clients
to automatically add pausing ability to their presenters. This impl will
simply return the last emitted `UiState` when the presenter is paused.
Again, name TBD.

### Other things:

- `GestureNavigationRetainedStateTest` and
`GestureNavigationSaveableStateTest` have been combined into a
parameterized `GestureNavigationStateTest`. This new test also now tests
`CupertinoGestureNavigationDecoration` so we get extra coverage.
- Changed `rememberRetained`'s `key` param to `Any` to be consistent
with all of the other `remember` functions.

### TODO:

- [x] Saveable is currently broken by these changes. This needs to be
fixed before landing.
- [x] Remove all of the logging code.
- [x] Add comments
- [x] Fix the last remaining failing test
  • Loading branch information
chrisbanes authored May 26, 2024
1 parent 9e7d982 commit bb09273
Show file tree
Hide file tree
Showing 12 changed files with 392 additions and 310 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ Changelog
- **New**: Add `FakeNavigator` functions to check for the lack of pop/resetRoot events.
- **New**: Add `FakeNavigator` constructor param to add additional screens to the backstack.
- **New**: Add support for static UIs. In some cases, a UI may not need a presenter to compute or manage its state. Examples of this include UIs that are stateless or can derive their state from a single static input or an input [Screen]'s properties. In these cases, make your _screen_ implement the `StaticScreen` interface. When a `StaticScreen` is used, Circuit will internally allow the UI to run on its own and won't connect it to a presenter if no presenter is provided.
- **New**: Add `RecordLifecycle` and `LocalRecordLifecycle` composition local, allowing UIs and presenters to observe when they are 'active'. Currently, a record is 'active' when it is the top record on the back stack.
- **Behaviour Change**: Presenters are now 'paused' and replay their last emitted `CircuitUiState` when they are not active. Presenters can opt-out of this behavior by implementing `NonPausablePresenter`.
- **Behaviour Change**: `NavigatorImpl.goTo` no longer navigates if the `Screen` is equal to `Navigator.peek()`.
- **Behaviour Change**: `Presenter.present` is now annotated with `@ComposableTarget("presenter")`. This helps prevent use of Compose UI in the presentation logic as the compiler will emit a warning if you do. Note this does not appear in the IDE, so it's recommended to use `allWarningsAsErrors` to fail the build on this event.
- **Change**: `Navigator.goTo` now returns a Bool indicating navigation success.
Expand Down
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()
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
}
}
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
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
// 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

0 comments on commit bb09273

Please sign in to comment.