From a7882a161de383ab5e6b2a82940e62423d517f79 Mon Sep 17 00:00:00 2001 From: Chris Banes Date: Wed, 13 Mar 2024 17:41:37 +0000 Subject: [PATCH 01/14] Try out pausing presenters --- ...vigableCircuitViewModelStateAndroidTest.kt | 131 ++++++++++-------- .../circuit/foundation/CircuitContent.kt | 6 +- .../com/slack/circuit/foundation/Lifecycle.kt | 25 ++++ .../foundation/NavigableCircuitContent.kt | 107 ++++++++------ .../circuit/foundation/PauseablePresenter.kt | 49 +++++++ ...roidPredictiveBackNavigationDecoration.kt} | 11 +- .../GestureNavigationSaveableStateTest.kt | 129 ----------------- ...eTest.kt => GestureNavigationStateTest.kt} | 99 +++++++++---- .../slack/circuitx/gesturenavigation/Utils.kt | 14 +- .../gesturenavigation/TransitionUtils.kt | 3 + 10 files changed, 313 insertions(+), 261 deletions(-) create mode 100644 circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/Lifecycle.kt create mode 100644 circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PauseablePresenter.kt rename circuitx/gesture-navigation/src/androidMain/kotlin/com/slack/circuitx/gesturenavigation/{GestureNavigationDecoration.kt => AndroidPredictiveBackNavigationDecoration.kt} (94%) delete mode 100644 circuitx/gesture-navigation/src/androidUnitTest/kotlin/com/slack/circuitx/gesturenavigation/GestureNavigationSaveableStateTest.kt rename circuitx/gesture-navigation/src/androidUnitTest/kotlin/com/slack/circuitx/gesturenavigation/{GestureNavigationRetainedStateTest.kt => GestureNavigationStateTest.kt} (64%) diff --git a/circuit-foundation/src/androidUnitTest/kotlin/com/slack/circuit/foundation/NavigableCircuitViewModelStateAndroidTest.kt b/circuit-foundation/src/androidUnitTest/kotlin/com/slack/circuit/foundation/NavigableCircuitViewModelStateAndroidTest.kt index a36c3f163..b2d188d48 100644 --- a/circuit-foundation/src/androidUnitTest/kotlin/com/slack/circuit/foundation/NavigableCircuitViewModelStateAndroidTest.kt +++ b/circuit-foundation/src/androidUnitTest/kotlin/com/slack/circuit/foundation/NavigableCircuitViewModelStateAndroidTest.kt @@ -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 @@ -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 @@ -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 @@ -66,11 +64,9 @@ 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 @@ -78,37 +74,43 @@ class NavigableCircuitViewModelStateAndroidTest { 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") @@ -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") @@ -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") @@ -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") } @@ -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 + } +} diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt index 24422a6dc..a43409c4b 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt @@ -6,8 +6,10 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.SideEffect +import androidx.compose.runtime.getValue import androidx.compose.runtime.key import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier import com.slack.circuit.runtime.CircuitContext import com.slack.circuit.runtime.CircuitUiState @@ -147,7 +149,9 @@ public fun CircuitContent( onDispose(eventListener::onDisposePresent) } - val state = presenter.present() + val pauseablePresenter = remember(presenter) { presenter.toPauseablePresenter() } + + val state = pauseablePresenter.present() // TODO not sure why stateFlow + LaunchedEffect + distinctUntilChanged doesn't work here SideEffect { eventListener.onState(state) } diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/Lifecycle.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/Lifecycle.kt new file mode 100644 index 000000000..bc31be25a --- /dev/null +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/Lifecycle.kt @@ -0,0 +1,25 @@ +// Copyright (C) 2024 Slack Technologies, LLC +// SPDX-License-Identifier: Apache-2.0 +package com.slack.circuit.foundation + +import androidx.compose.runtime.ProvidableCompositionLocal +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.setValue +import androidx.compose.runtime.staticCompositionLocalOf + +public interface Lifecycle { + public val isPaused: Boolean +} + +internal class LifecycleImpl : Lifecycle { + internal var _isPaused by mutableStateOf(false) + override val isPaused: Boolean + get() = _isPaused +} + +public val LocalLifecycle: ProvidableCompositionLocal = staticCompositionLocalOf { + object : Lifecycle { + override val isPaused: Boolean = false + } +} diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavigableCircuitContent.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavigableCircuitContent.kt index 11bdc0c68..8f2e5f7b8 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavigableCircuitContent.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavigableCircuitContent.kt @@ -19,7 +19,9 @@ import androidx.compose.animation.slideOutHorizontally import androidx.compose.animation.togetherWith import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.Immutable +import androidx.compose.runtime.SideEffect import androidx.compose.runtime.compositionLocalOf import androidx.compose.runtime.currentCompositeKeyHash import androidx.compose.runtime.getValue @@ -58,7 +60,8 @@ public fun NavigableCircuitContent( circuit.onUnavailableContent, ) { val activeContentProviders = - backStack.buildCircuitContentProviders( + buildCircuitContentProviders( + backStack = backStack, navigator = navigator, circuit = circuit, unavailableRoute = unavailableRoute, @@ -101,36 +104,30 @@ public fun NavigableCircuitContent( val outerKey = "_navigable_registry_${currentCompositeKeyHash.toString(MaxSupportedRadix)}" val outerRegistry = rememberRetained(key = outerKey) { RetainedStateRegistry() } + println("Composing NavigableCircuitContent for ${backStack.toList()}") + 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 + println("--> NavigableCircuitContent content called for ${provider.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) + } + + DisposableEffect(Unit) { + println("NavigableCircuitContent for ${provider.record} added") + + onDispose { println("NavigableCircuitContent for ${provider.record} removed") } } + + SideEffect { println("--> NavigableCircuitContent content finished for ${provider.record}") } } } } @@ -163,37 +160,65 @@ public class RecordContentProvider( } @Composable -private fun BackStack.buildCircuitContentProviders( +private fun buildCircuitContentProviders( + backStack: BackStack, navigator: Navigator, circuit: Circuit, unavailableRoute: @Composable (screen: Screen, modifier: Modifier) -> Unit, ): ImmutableList> { val previousContentProviders = remember { mutableMapOf>() } + 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 { record -> + val recordInBackStackRetainChecker = + remember(lastBackStack, record) { + CanRetainChecker { lastBackStack.containsRecord(record, includeSaved = true) } + .also { println("Creating CanRetainChecker for $record") } + } + + val lifecycle = + remember { LifecycleImpl().also { println("Created LifecycleImpl $it for $record") } } + .apply { + _isPaused = lastBackStack.topRecord != record + println("Set LifecycleImpl.isPaused to $_isPaused for $record. $this") + } + + 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, + LocalLifecycle 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() diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PauseablePresenter.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PauseablePresenter.kt new file mode 100644 index 000000000..d590c8066 --- /dev/null +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PauseablePresenter.kt @@ -0,0 +1,49 @@ +// Copyright (C) 2024 Slack Technologies, LLC +// SPDX-License-Identifier: Apache-2.0 +package com.slack.circuit.foundation + +import androidx.compose.runtime.Composable +import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.saveable.rememberSaveableStateHolder +import androidx.compose.runtime.setValue +import com.slack.circuit.retained.LocalRetainedStateRegistry +import com.slack.circuit.retained.RetainedStateRegistry +import com.slack.circuit.retained.rememberRetained +import com.slack.circuit.runtime.CircuitUiState +import com.slack.circuit.runtime.presenter.Presenter + +public abstract class PauseablePresenter() : Presenter { + + private var uiState by mutableStateOf(null) + + @Composable + override fun present(): UiState { + val saveableStateHolder = rememberSaveableStateHolder() + + val isPaused = LocalLifecycle.current.isPaused + + if (!isPaused || uiState == null) { + val retainedStateRegistry = rememberRetained(key = "pauseable") { RetainedStateRegistry() } + CompositionLocalProvider(LocalRetainedStateRegistry provides retainedStateRegistry) { + saveableStateHolder.SaveableStateProvider("pauseable_presenter") { uiState = _present() } + } + } + + println("PauseablePresenter. isPaused: $isPaused. state: $uiState") + + return uiState!! + } + + @Composable protected abstract fun _present(): UiState +} + +public fun Presenter.toPauseablePresenter(): + PauseablePresenter { + if (this is PauseablePresenter) return this + // Else we wrap the presenter + return object : PauseablePresenter() { + @Composable override fun _present(): UiState = this@toPauseablePresenter.present() + } +} diff --git a/circuitx/gesture-navigation/src/androidMain/kotlin/com/slack/circuitx/gesturenavigation/GestureNavigationDecoration.kt b/circuitx/gesture-navigation/src/androidMain/kotlin/com/slack/circuitx/gesturenavigation/AndroidPredictiveBackNavigationDecoration.kt similarity index 94% rename from circuitx/gesture-navigation/src/androidMain/kotlin/com/slack/circuitx/gesturenavigation/GestureNavigationDecoration.kt rename to circuitx/gesture-navigation/src/androidMain/kotlin/com/slack/circuitx/gesturenavigation/AndroidPredictiveBackNavigationDecoration.kt index 01f543b8e..6ec9b5a49 100644 --- a/circuitx/gesture-navigation/src/androidMain/kotlin/com/slack/circuitx/gesturenavigation/GestureNavigationDecoration.kt +++ b/circuitx/gesture-navigation/src/androidMain/kotlin/com/slack/circuitx/gesturenavigation/AndroidPredictiveBackNavigationDecoration.kt @@ -20,6 +20,7 @@ import androidx.compose.material3.MaterialTheme import androidx.compose.runtime.Composable import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.SideEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableFloatStateOf import androidx.compose.runtime.mutableStateOf @@ -43,12 +44,12 @@ public actual fun GestureNavigationDecoration( onBackInvoked: () -> Unit, ): NavDecoration = when { - Build.VERSION.SDK_INT >= 34 -> AndroidPredictiveNavigationDecoration(onBackInvoked) + Build.VERSION.SDK_INT >= 34 -> AndroidPredictiveBackNavigationDecoration(onBackInvoked) else -> fallback } @RequiresApi(34) -private class AndroidPredictiveNavigationDecoration(private val onBackInvoked: () -> Unit) : +public class AndroidPredictiveBackNavigationDecoration(private val onBackInvoked: () -> Unit) : NavDecoration { @Composable override fun DecoratedContent( @@ -70,7 +71,11 @@ private class AndroidPredictiveNavigationDecoration(private val onBackInvoked: ( label = "GestureNavDecoration", ) - if (previous != null && !transition.isStateBeingAnimated { it.record == previous }) { + if ( + previous != null && + !transition.isPending && + !transition.isStateBeingAnimated { it.record == previous } + ) { // We display the 'previous' item in the back stack for when the user performs a gesture // to go back. // We only display it here if the transition is not running. When the transition is diff --git a/circuitx/gesture-navigation/src/androidUnitTest/kotlin/com/slack/circuitx/gesturenavigation/GestureNavigationSaveableStateTest.kt b/circuitx/gesture-navigation/src/androidUnitTest/kotlin/com/slack/circuitx/gesturenavigation/GestureNavigationSaveableStateTest.kt deleted file mode 100644 index a3594c905..000000000 --- a/circuitx/gesture-navigation/src/androidUnitTest/kotlin/com/slack/circuitx/gesturenavigation/GestureNavigationSaveableStateTest.kt +++ /dev/null @@ -1,129 +0,0 @@ -// Copyright (C) 2023 Slack Technologies, LLC -// SPDX-License-Identifier: Apache-2.0 -package com.slack.circuitx.gesturenavigation - -import androidx.activity.ComponentActivity -import androidx.compose.ui.test.assertTextEquals -import androidx.compose.ui.test.junit4.createAndroidComposeRule -import androidx.compose.ui.test.performClick -import androidx.test.ext.junit.runners.AndroidJUnit4 -import com.slack.circuit.backstack.rememberSaveableBackStack -import com.slack.circuit.foundation.CircuitCompositionLocals -import com.slack.circuit.foundation.NavigableCircuitContent -import com.slack.circuit.foundation.rememberCircuitNavigator -import com.slack.circuit.internal.test.TestContentTags.TAG_COUNT -import com.slack.circuit.internal.test.TestContentTags.TAG_GO_NEXT -import com.slack.circuit.internal.test.TestContentTags.TAG_INCREASE_COUNT -import com.slack.circuit.internal.test.TestContentTags.TAG_LABEL -import com.slack.circuit.internal.test.TestContentTags.TAG_POP -import com.slack.circuit.internal.test.TestCountPresenter.RememberType -import com.slack.circuit.internal.test.TestScreen -import com.slack.circuit.internal.test.createTestCircuit -import org.junit.Rule -import org.junit.Test -import org.junit.runner.RunWith -import org.robolectric.annotation.Config - -@Config(minSdk = 34) -@RunWith(AndroidJUnit4::class) -class GestureNavigationSaveableStateTest { - - @get:Rule val composeTestRule = createAndroidComposeRule() - - @Test - fun saveableStateScopedToBackstackWithKeysAndBackSwipes() { - saveableStateScopedToBackstack(true) { - composeTestRule.activityRule.scenario.performBackSwipeGesture() - } - } - - @Test - fun saveableStateScopedToBackstackWithoutKeysAndBackSwipes() { - saveableStateScopedToBackstack(false) { - composeTestRule.activityRule.scenario.performBackSwipeGesture() - } - } - - @Test - fun saveableStateScopedToBackstackWithKeysAndBackPress() { - saveableStateScopedToBackstack(true) { - composeTestRule.onTopNavigationRecordNodeWithTag(TAG_POP).performClick() - } - } - - @Test - fun saveableStateScopedToBackstackWithoutKeysAndBackPress() { - saveableStateScopedToBackstack(false) { - composeTestRule.onTopNavigationRecordNodeWithTag(TAG_POP).performClick() - } - } - - private fun saveableStateScopedToBackstack(useKeys: Boolean, pop: () -> Unit) { - composeTestRule.run { - val circuit = createTestCircuit(useKeys = useKeys, rememberType = RememberType.Saveable) - - setContent { - CircuitCompositionLocals(circuit) { - val backStack = rememberSaveableBackStack(TestScreen.ScreenA) - val navigator = - rememberCircuitNavigator( - backStack = backStack, - onRootPop = {}, // no-op for tests - ) - NavigableCircuitContent( - navigator = navigator, - backStack = backStack, - decoration = GestureNavigationDecoration(onBackInvoked = navigator::pop), - ) - } - } - - // Current: Screen A. Increase count to 1 - onTopNavigationRecordNodeWithTag(TAG_LABEL).assertTextEquals("A") - onTopNavigationRecordNodeWithTag(TAG_COUNT).assertTextEquals("0") - onTopNavigationRecordNodeWithTag(TAG_INCREASE_COUNT).performClick() - onTopNavigationRecordNodeWithTag(TAG_COUNT).assertTextEquals("1") - - // Navigate to Screen B. Increase count to 1 - onTopNavigationRecordNodeWithTag(TAG_GO_NEXT).performClick() - onTopNavigationRecordNodeWithTag(TAG_LABEL).assertTextEquals("B") - onTopNavigationRecordNodeWithTag(TAG_COUNT).assertTextEquals("0") - onTopNavigationRecordNodeWithTag(TAG_INCREASE_COUNT).performClick() - onTopNavigationRecordNodeWithTag(TAG_COUNT).assertTextEquals("1") - - // Navigate to Screen C. Increase count to 1 - onTopNavigationRecordNodeWithTag(TAG_GO_NEXT).performClick() - onTopNavigationRecordNodeWithTag(TAG_LABEL).assertTextEquals("C") - onTopNavigationRecordNodeWithTag(TAG_COUNT).assertTextEquals("0") - onTopNavigationRecordNodeWithTag(TAG_INCREASE_COUNT).performClick() - onTopNavigationRecordNodeWithTag(TAG_COUNT).assertTextEquals("1") - - // Pop to Screen B. Increase count from 1 to 2. - pop() - onTopNavigationRecordNodeWithTag(TAG_LABEL).assertTextEquals("B") - onTopNavigationRecordNodeWithTag(TAG_COUNT).assertTextEquals("1") - onTopNavigationRecordNodeWithTag(TAG_INCREASE_COUNT).performClick() - onTopNavigationRecordNodeWithTag(TAG_COUNT).assertTextEquals("2") - - // Navigate to Screen C. Assert that it's state was not saved - onTopNavigationRecordNodeWithTag(TAG_GO_NEXT).performClick() - onTopNavigationRecordNodeWithTag(TAG_LABEL).assertTextEquals("C") - onTopNavigationRecordNodeWithTag(TAG_COUNT).assertTextEquals("0") - - // Pop to Screen B. Assert that it's state was saved - pop() - onTopNavigationRecordNodeWithTag(TAG_LABEL).assertTextEquals("B") - onTopNavigationRecordNodeWithTag(TAG_COUNT).assertTextEquals("2") - - // Pop to Screen A. Assert that it's state was saved - pop() - onTopNavigationRecordNodeWithTag(TAG_LABEL).assertTextEquals("A") - onTopNavigationRecordNodeWithTag(TAG_COUNT).assertTextEquals("1") - - // Navigate to Screen B. Assert that it's state was not saved - onTopNavigationRecordNodeWithTag(TAG_GO_NEXT).performClick() - onTopNavigationRecordNodeWithTag(TAG_LABEL).assertTextEquals("B") - onTopNavigationRecordNodeWithTag(TAG_COUNT).assertTextEquals("0") - } - } -} diff --git a/circuitx/gesture-navigation/src/androidUnitTest/kotlin/com/slack/circuitx/gesturenavigation/GestureNavigationRetainedStateTest.kt b/circuitx/gesture-navigation/src/androidUnitTest/kotlin/com/slack/circuitx/gesturenavigation/GestureNavigationStateTest.kt similarity index 64% rename from circuitx/gesture-navigation/src/androidUnitTest/kotlin/com/slack/circuitx/gesturenavigation/GestureNavigationRetainedStateTest.kt rename to circuitx/gesture-navigation/src/androidUnitTest/kotlin/com/slack/circuitx/gesturenavigation/GestureNavigationStateTest.kt index 765d59d87..ce7c81cea 100644 --- a/circuitx/gesture-navigation/src/androidUnitTest/kotlin/com/slack/circuitx/gesturenavigation/GestureNavigationRetainedStateTest.kt +++ b/circuitx/gesture-navigation/src/androidUnitTest/kotlin/com/slack/circuitx/gesturenavigation/GestureNavigationStateTest.kt @@ -3,10 +3,14 @@ package com.slack.circuitx.gesturenavigation import androidx.activity.ComponentActivity +import androidx.compose.material.ExperimentalMaterialApi +import androidx.compose.runtime.remember import androidx.compose.ui.test.assertTextEquals import androidx.compose.ui.test.junit4.createAndroidComposeRule +import androidx.compose.ui.test.onRoot import androidx.compose.ui.test.performClick -import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.compose.ui.test.performTouchInput +import androidx.compose.ui.test.swipeRight import com.slack.circuit.backstack.rememberSaveableBackStack import com.slack.circuit.foundation.CircuitCompositionLocals import com.slack.circuit.foundation.NavigableCircuitContent @@ -22,45 +26,62 @@ import com.slack.circuit.internal.test.createTestCircuit import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith +import org.robolectric.ParameterizedRobolectricTestRunner import org.robolectric.annotation.Config -@Config(minSdk = 34) -@RunWith(AndroidJUnit4::class) -class GestureNavigationRetainedStateTest { - - @get:Rule val composeTestRule = createAndroidComposeRule() +enum class GestureNavDecorationOption { + AndroidPredictiveBack, + Cupertino, +} - @Test - fun retainedStateScopedToBackstackWithKeysAndBackSwipes() { - retainedStateScopedToBackstack(true) { - composeTestRule.activityRule.scenario.performBackSwipeGesture() - } +@OptIn(ExperimentalMaterialApi::class) +@Config(minSdk = 34) +@RunWith(ParameterizedRobolectricTestRunner::class) +class GestureNavigationStateTest( + private val decorationOption: GestureNavDecorationOption, + private val useKeys: Boolean, + private val useSwipe: Boolean, + private val rememberType: RememberType, +) { + companion object { + @JvmStatic + @ParameterizedRobolectricTestRunner.Parameters( + name = "{0}_useKeys={1}_useSwipe={2}_rememberType={3}" + ) + fun params() = + parameterizedParams() + .combineWithParameters( + GestureNavDecorationOption.Cupertino, + GestureNavDecorationOption.AndroidPredictiveBack, + ) + .combineWithParameters(true, false) // useKeys + .combineWithParameters(true, false) // useSwipe + .combineWithParameters(RememberType.Retained, RememberType.Saveable) } - @Test - fun retainedStateScopedToBackstackWithoutKeysAndBackSwipes() { - retainedStateScopedToBackstack(false) { - composeTestRule.activityRule.scenario.performBackSwipeGesture() - } - } + @get:Rule val composeTestRule = createAndroidComposeRule() - @Test - fun retainedStateScopedToBackstackWithKeysAndBackPress() { - retainedStateScopedToBackstack(true) { + private fun pop() { + if (useSwipe) { + when (decorationOption) { + GestureNavDecorationOption.AndroidPredictiveBack -> { + composeTestRule.activityRule.scenario.performGestureNavigationBackSwipe() + } + GestureNavDecorationOption.Cupertino -> { + composeTestRule.onRoot().performTouchInput { + swipeRight(startX = width * 0.2f, endX = width * 0.8f) + } + } + } + } else { composeTestRule.onTopNavigationRecordNodeWithTag(TAG_POP).performClick() } } @Test - fun retainedStateScopedToBackstackWithoutKeysAndBackPress() { - retainedStateScopedToBackstack(false) { - composeTestRule.onTopNavigationRecordNodeWithTag(TAG_POP).performClick() - } - } - - private fun retainedStateScopedToBackstack(useKeys: Boolean, pop: () -> Unit) { + fun retainedStateScopedToBackstack() { composeTestRule.run { - val circuit = createTestCircuit(useKeys = useKeys, rememberType = RememberType.Retained) + val circuit = createTestCircuit(useKeys = useKeys, rememberType = rememberType) setContent { CircuitCompositionLocals(circuit) { @@ -73,7 +94,17 @@ class GestureNavigationRetainedStateTest { NavigableCircuitContent( navigator = navigator, backStack = backStack, - decoration = GestureNavigationDecoration(onBackInvoked = navigator::pop), + decoration = + remember { + when (decorationOption) { + GestureNavDecorationOption.AndroidPredictiveBack -> { + AndroidPredictiveBackNavigationDecoration(onBackInvoked = navigator::pop) + } + GestureNavDecorationOption.Cupertino -> { + CupertinoGestureNavigationDecoration(onBackInvoked = navigator::pop) + } + } + }, ) } } @@ -81,41 +112,51 @@ class GestureNavigationRetainedStateTest { // Current: Screen A. Increase count to 1 onTopNavigationRecordNodeWithTag(TAG_LABEL).assertTextEquals("A") onTopNavigationRecordNodeWithTag(TAG_COUNT).assertTextEquals("0") + println("Increasing A") onTopNavigationRecordNodeWithTag(TAG_INCREASE_COUNT).performClick() onTopNavigationRecordNodeWithTag(TAG_COUNT).assertTextEquals("1") // Navigate to Screen B. Increase count to 1 + println("Going to B") onTopNavigationRecordNodeWithTag(TAG_GO_NEXT).performClick() onTopNavigationRecordNodeWithTag(TAG_LABEL).assertTextEquals("B") onTopNavigationRecordNodeWithTag(TAG_COUNT).assertTextEquals("0") + println("Increasing B") onTopNavigationRecordNodeWithTag(TAG_INCREASE_COUNT).performClick() onTopNavigationRecordNodeWithTag(TAG_COUNT).assertTextEquals("1") // Navigate to Screen C. Increase count to 1 + println("Going to C") onTopNavigationRecordNodeWithTag(TAG_GO_NEXT).performClick() onTopNavigationRecordNodeWithTag(TAG_LABEL).assertTextEquals("C") onTopNavigationRecordNodeWithTag(TAG_COUNT).assertTextEquals("0") + println("Increasing C") onTopNavigationRecordNodeWithTag(TAG_INCREASE_COUNT).performClick() onTopNavigationRecordNodeWithTag(TAG_COUNT).assertTextEquals("1") // Pop to Screen B. Increase count from 1 to 2. + println("Pop to B") pop() onTopNavigationRecordNodeWithTag(TAG_LABEL).assertTextEquals("B") onTopNavigationRecordNodeWithTag(TAG_COUNT).assertTextEquals("1") + println("Increasing B") onTopNavigationRecordNodeWithTag(TAG_INCREASE_COUNT).performClick() onTopNavigationRecordNodeWithTag(TAG_COUNT).assertTextEquals("2") // Navigate to Screen C. Assert that it's state was not retained + println("Go to C") onTopNavigationRecordNodeWithTag(TAG_GO_NEXT).performClick() onTopNavigationRecordNodeWithTag(TAG_LABEL).assertTextEquals("C") onTopNavigationRecordNodeWithTag(TAG_COUNT).assertTextEquals("0") // Pop to Screen B. Assert that it's state was retained + println("Popping to B") pop() onTopNavigationRecordNodeWithTag(TAG_LABEL).assertTextEquals("B") onTopNavigationRecordNodeWithTag(TAG_COUNT).assertTextEquals("2") // Pop to Screen A. Assert that it's state was retained + println("Popping to A") pop() onTopNavigationRecordNodeWithTag(TAG_LABEL).assertTextEquals("A") onTopNavigationRecordNodeWithTag(TAG_COUNT).assertTextEquals("1") diff --git a/circuitx/gesture-navigation/src/androidUnitTest/kotlin/com/slack/circuitx/gesturenavigation/Utils.kt b/circuitx/gesture-navigation/src/androidUnitTest/kotlin/com/slack/circuitx/gesturenavigation/Utils.kt index 11013508d..0e292ba64 100644 --- a/circuitx/gesture-navigation/src/androidUnitTest/kotlin/com/slack/circuitx/gesturenavigation/Utils.kt +++ b/circuitx/gesture-navigation/src/androidUnitTest/kotlin/com/slack/circuitx/gesturenavigation/Utils.kt @@ -27,7 +27,7 @@ internal fun BackEventCompat.copy( ): BackEventCompat = BackEventCompat(touchX = touchX, touchY = touchY, progress = progress, swipeEdge = swipeEdge) -internal fun ActivityScenario.performBackSwipeGesture() { +internal fun ActivityScenario.performGestureNavigationBackSwipe() { onActivity { activity -> val event = BackEventCompat( @@ -50,3 +50,15 @@ internal fun ActivityScenario.performBackSwipeGesture() { } } } + +fun parameterizedParams(): List> = emptyList() + +inline fun List>.combineWithParameters(vararg values: T): List> { + if (isEmpty()) return values.map { arrayOf(it) } + + return fold(emptyList()) { acc, args -> + val result = acc.toMutableList() + values.forEach { result += (args + it) } + result.toList() + } +} diff --git a/circuitx/gesture-navigation/src/commonMain/kotlin/com/slack/circuitx/gesturenavigation/TransitionUtils.kt b/circuitx/gesture-navigation/src/commonMain/kotlin/com/slack/circuitx/gesturenavigation/TransitionUtils.kt index 7c69886be..39d203b2a 100644 --- a/circuitx/gesture-navigation/src/commonMain/kotlin/com/slack/circuitx/gesturenavigation/TransitionUtils.kt +++ b/circuitx/gesture-navigation/src/commonMain/kotlin/com/slack/circuitx/gesturenavigation/TransitionUtils.kt @@ -9,6 +9,9 @@ internal fun Transition.isStateBeingAnimated(equals: (T) -> Boolean): Boo return isRunning && (equals(currentState) || equals(targetState)) } +internal val Transition<*>.isPending: Boolean + get() = this.currentState != this.targetState + /** * A holder class used by the `AnimatedContent` composables. This enables us to pass through all of * the necessary information as an argument, which is optimal for `AnimatedContent`. From ff394c35c77729b9b067eea8272049e97145db73 Mon Sep 17 00:00:00 2001 From: Chris Banes Date: Thu, 23 May 2024 17:43:28 +0100 Subject: [PATCH 02/14] Remove logging --- .../foundation/NavigableCircuitContent.kt | 21 +------------------ .../circuit/foundation/PauseablePresenter.kt | 4 +--- ...droidPredictiveBackNavigationDecoration.kt | 1 - 3 files changed, 2 insertions(+), 24 deletions(-) diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavigableCircuitContent.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavigableCircuitContent.kt index 8f2e5f7b8..d3fed056d 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavigableCircuitContent.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavigableCircuitContent.kt @@ -19,9 +19,7 @@ import androidx.compose.animation.slideOutHorizontally import androidx.compose.animation.togetherWith import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider -import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.Immutable -import androidx.compose.runtime.SideEffect import androidx.compose.runtime.compositionLocalOf import androidx.compose.runtime.currentCompositeKeyHash import androidx.compose.runtime.getValue @@ -104,12 +102,8 @@ public fun NavigableCircuitContent( val outerKey = "_navigable_registry_${currentCompositeKeyHash.toString(MaxSupportedRadix)}" val outerRegistry = rememberRetained(key = outerKey) { RetainedStateRegistry() } - println("Composing NavigableCircuitContent for ${backStack.toList()}") - CompositionLocalProvider(LocalRetainedStateRegistry provides outerRegistry) { decoration.DecoratedContent(activeContentProviders, backStack.size, modifier) { provider -> - println("--> NavigableCircuitContent content called for ${provider.record}") - val record = provider.record // Remember the `providedValues` lookup because this composition can live longer than @@ -120,14 +114,6 @@ public fun NavigableCircuitContent( CompositionLocalProvider(LocalBackStack provides backStack, *providedLocals) { provider.content(record) } - - DisposableEffect(Unit) { - println("NavigableCircuitContent for ${provider.record} added") - - onDispose { println("NavigableCircuitContent for ${provider.record} removed") } - } - - SideEffect { println("--> NavigableCircuitContent content finished for ${provider.record}") } } } } @@ -178,15 +164,10 @@ private fun buildCircuitContentProviders( val recordInBackStackRetainChecker = remember(lastBackStack, record) { CanRetainChecker { lastBackStack.containsRecord(record, includeSaved = true) } - .also { println("Creating CanRetainChecker for $record") } } val lifecycle = - remember { LifecycleImpl().also { println("Created LifecycleImpl $it for $record") } } - .apply { - _isPaused = lastBackStack.topRecord != record - println("Set LifecycleImpl.isPaused to $_isPaused for $record. $this") - } + remember { LifecycleImpl() }.apply { _isPaused = lastBackStack.topRecord != record } CompositionLocalProvider(LocalCanRetainChecker provides recordInBackStackRetainChecker) { // Now provide a new registry to the content for it to store any retained state in, diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PauseablePresenter.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PauseablePresenter.kt index d590c8066..52513dbfd 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PauseablePresenter.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PauseablePresenter.kt @@ -25,14 +25,12 @@ public abstract class PauseablePresenter() : Presenter val isPaused = LocalLifecycle.current.isPaused if (!isPaused || uiState == null) { - val retainedStateRegistry = rememberRetained(key = "pauseable") { RetainedStateRegistry() } + val retainedStateRegistry = rememberRetained { RetainedStateRegistry() } CompositionLocalProvider(LocalRetainedStateRegistry provides retainedStateRegistry) { saveableStateHolder.SaveableStateProvider("pauseable_presenter") { uiState = _present() } } } - println("PauseablePresenter. isPaused: $isPaused. state: $uiState") - return uiState!! } diff --git a/circuitx/gesture-navigation/src/androidMain/kotlin/com/slack/circuitx/gesturenavigation/AndroidPredictiveBackNavigationDecoration.kt b/circuitx/gesture-navigation/src/androidMain/kotlin/com/slack/circuitx/gesturenavigation/AndroidPredictiveBackNavigationDecoration.kt index 6ec9b5a49..b6953d915 100644 --- a/circuitx/gesture-navigation/src/androidMain/kotlin/com/slack/circuitx/gesturenavigation/AndroidPredictiveBackNavigationDecoration.kt +++ b/circuitx/gesture-navigation/src/androidMain/kotlin/com/slack/circuitx/gesturenavigation/AndroidPredictiveBackNavigationDecoration.kt @@ -20,7 +20,6 @@ import androidx.compose.material3.MaterialTheme import androidx.compose.runtime.Composable import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.LaunchedEffect -import androidx.compose.runtime.SideEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableFloatStateOf import androidx.compose.runtime.mutableStateOf From 9dede6cc1bf919953bc23be43088c814a1338e93 Mon Sep 17 00:00:00 2001 From: Chris Banes Date: Fri, 24 May 2024 08:21:06 +0100 Subject: [PATCH 03/14] LifecycleImp to MutableLifecycle --- .../com/slack/circuit/foundation/Lifecycle.kt | 17 ++++++++++------- .../foundation/NavigableCircuitContent.kt | 2 +- .../circuit/foundation/PauseablePresenter.kt | 5 ++--- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/Lifecycle.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/Lifecycle.kt index bc31be25a..c780b006e 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/Lifecycle.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/Lifecycle.kt @@ -3,23 +3,26 @@ package com.slack.circuit.foundation import androidx.compose.runtime.ProvidableCompositionLocal +import androidx.compose.runtime.Stable import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.setValue import androidx.compose.runtime.staticCompositionLocalOf +@Stable public interface Lifecycle { - public val isPaused: Boolean + public val isResumed: Boolean } -internal class LifecycleImpl : Lifecycle { - internal var _isPaused by mutableStateOf(false) - override val isPaused: Boolean - get() = _isPaused +internal class MutableLifecycle(isResumed: Boolean = false) : Lifecycle { + override var isResumed: Boolean by mutableStateOf(isResumed) } public val LocalLifecycle: ProvidableCompositionLocal = staticCompositionLocalOf { + staticLifecycle(true) +} + +private fun staticLifecycle(isResumed: Boolean): Lifecycle = object : Lifecycle { - override val isPaused: Boolean = false + override val isResumed: Boolean = isResumed } -} diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavigableCircuitContent.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavigableCircuitContent.kt index d3fed056d..0190617d1 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavigableCircuitContent.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavigableCircuitContent.kt @@ -167,7 +167,7 @@ private fun buildCircuitContentProviders( } val lifecycle = - remember { LifecycleImpl() }.apply { _isPaused = lastBackStack.topRecord != record } + remember { MutableLifecycle() }.apply { isResumed = lastBackStack.topRecord == record } CompositionLocalProvider(LocalCanRetainChecker provides recordInBackStackRetainChecker) { // Now provide a new registry to the content for it to store any retained state in, diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PauseablePresenter.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PauseablePresenter.kt index 52513dbfd..15826e2af 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PauseablePresenter.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PauseablePresenter.kt @@ -20,11 +20,10 @@ public abstract class PauseablePresenter() : Presenter @Composable override fun present(): UiState { + val lifecycle = LocalLifecycle.current val saveableStateHolder = rememberSaveableStateHolder() - val isPaused = LocalLifecycle.current.isPaused - - if (!isPaused || uiState == null) { + if (lifecycle.isResumed || uiState == null) { val retainedStateRegistry = rememberRetained { RetainedStateRegistry() } CompositionLocalProvider(LocalRetainedStateRegistry provides retainedStateRegistry) { saveableStateHolder.SaveableStateProvider("pauseable_presenter") { uiState = _present() } From a7f04ee2e5a1793e1adafe965ba53d668b43717e Mon Sep 17 00:00:00 2001 From: Chris Banes Date: Fri, 24 May 2024 08:36:13 +0100 Subject: [PATCH 04/14] Refactor PauseablePresenter to functions --- .../circuit/foundation/CircuitContent.kt | 6 +-- .../slack/circuit/foundation/PausableState.kt | 40 ++++++++++++++++ .../circuit/foundation/PauseablePresenter.kt | 46 ------------------- 3 files changed, 41 insertions(+), 51 deletions(-) create mode 100644 circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt delete mode 100644 circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PauseablePresenter.kt diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt index a43409c4b..0b6395c9d 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt @@ -6,10 +6,8 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.SideEffect -import androidx.compose.runtime.getValue import androidx.compose.runtime.key import androidx.compose.runtime.remember -import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier import com.slack.circuit.runtime.CircuitContext import com.slack.circuit.runtime.CircuitUiState @@ -149,9 +147,7 @@ public fun CircuitContent( onDispose(eventListener::onDisposePresent) } - val pauseablePresenter = remember(presenter) { presenter.toPauseablePresenter() } - - val state = pauseablePresenter.present() + val state = presenter.presentWithLifecycle() // TODO not sure why stateFlow + LaunchedEffect + distinctUntilChanged doesn't work here SideEffect { eventListener.onState(state) } diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt new file mode 100644 index 000000000..e9419db69 --- /dev/null +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt @@ -0,0 +1,40 @@ +// Copyright (C) 2024 Slack Technologies, LLC +// SPDX-License-Identifier: Apache-2.0 +package com.slack.circuit.foundation + +import androidx.compose.runtime.Composable +import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.saveable.rememberSaveableStateHolder +import androidx.compose.runtime.setValue +import com.slack.circuit.retained.LocalRetainedStateRegistry +import com.slack.circuit.retained.RetainedStateRegistry +import com.slack.circuit.retained.rememberRetained +import com.slack.circuit.runtime.CircuitUiState +import com.slack.circuit.runtime.presenter.Presenter + +@Composable +public fun Presenter.presentWithLifecycle( + isResumed: Boolean = LocalLifecycle.current.isResumed +): UiState = pausableState(isResumed) { present() } + +@Composable +public fun pausableState( + isResumed: Boolean = LocalLifecycle.current.isResumed, + produceState: @Composable () -> UiState, +): UiState { + var uiState: UiState? by remember { mutableStateOf(null) } + + val saveableStateHolder = rememberSaveableStateHolder() + + if (isResumed || uiState == null) { + val retainedStateRegistry = rememberRetained { RetainedStateRegistry() } + CompositionLocalProvider(LocalRetainedStateRegistry provides retainedStateRegistry) { + saveableStateHolder.SaveableStateProvider("pauseable_presenter") { uiState = produceState() } + } + } + + return uiState!! +} diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PauseablePresenter.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PauseablePresenter.kt deleted file mode 100644 index 15826e2af..000000000 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PauseablePresenter.kt +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright (C) 2024 Slack Technologies, LLC -// SPDX-License-Identifier: Apache-2.0 -package com.slack.circuit.foundation - -import androidx.compose.runtime.Composable -import androidx.compose.runtime.CompositionLocalProvider -import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.saveable.rememberSaveableStateHolder -import androidx.compose.runtime.setValue -import com.slack.circuit.retained.LocalRetainedStateRegistry -import com.slack.circuit.retained.RetainedStateRegistry -import com.slack.circuit.retained.rememberRetained -import com.slack.circuit.runtime.CircuitUiState -import com.slack.circuit.runtime.presenter.Presenter - -public abstract class PauseablePresenter() : Presenter { - - private var uiState by mutableStateOf(null) - - @Composable - override fun present(): UiState { - val lifecycle = LocalLifecycle.current - val saveableStateHolder = rememberSaveableStateHolder() - - if (lifecycle.isResumed || uiState == null) { - val retainedStateRegistry = rememberRetained { RetainedStateRegistry() } - CompositionLocalProvider(LocalRetainedStateRegistry provides retainedStateRegistry) { - saveableStateHolder.SaveableStateProvider("pauseable_presenter") { uiState = _present() } - } - } - - return uiState!! - } - - @Composable protected abstract fun _present(): UiState -} - -public fun Presenter.toPauseablePresenter(): - PauseablePresenter { - if (this is PauseablePresenter) return this - // Else we wrap the presenter - return object : PauseablePresenter() { - @Composable override fun _present(): UiState = this@toPauseablePresenter.present() - } -} From bcfb04f74f7435d7b59b03f874486ad223149865 Mon Sep 17 00:00:00 2001 From: Chris Banes Date: Fri, 24 May 2024 08:56:34 +0100 Subject: [PATCH 05/14] Add opt-out NonPausablePresenter --- .../kotlin/com/slack/circuit/foundation/CircuitContent.kt | 6 +++++- .../kotlin/com/slack/circuit/foundation/PausableState.kt | 8 ++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt index 0b6395c9d..921383151 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt @@ -147,7 +147,11 @@ public fun CircuitContent( onDispose(eventListener::onDisposePresent) } - val state = presenter.presentWithLifecycle() + val state = + when (presenter) { + is NonPausablePresenter -> presenter.present() + else -> presenter.presentWithLifecycle() + } // TODO not sure why stateFlow + LaunchedEffect + distinctUntilChanged doesn't work here SideEffect { eventListener.onState(state) } diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt index e9419db69..898143c1e 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt @@ -4,6 +4,7 @@ package com.slack.circuit.foundation import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.runtime.Stable import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember @@ -15,6 +16,13 @@ import com.slack.circuit.retained.rememberRetained import com.slack.circuit.runtime.CircuitUiState import com.slack.circuit.runtime.presenter.Presenter +/** + * By default [CircuitContent] will wrap presenters so that their [CircuitUiState]s are conflated + * when the presenter is paused. If this behavior is not wanted, you can make your presenter + * implement this interface to disable the behavior. + */ +@Stable public interface NonPausablePresenter : Presenter + @Composable public fun Presenter.presentWithLifecycle( isResumed: Boolean = LocalLifecycle.current.isResumed From 34106a0626f1a31760d8eb46753b67a0a1d84dc9 Mon Sep 17 00:00:00 2001 From: Chris Banes Date: Fri, 24 May 2024 09:01:39 +0100 Subject: [PATCH 06/14] Add more comments --- .../kotlin/com/slack/circuit/foundation/PausableState.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt index 898143c1e..829b54097 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt @@ -17,8 +17,8 @@ import com.slack.circuit.runtime.CircuitUiState import com.slack.circuit.runtime.presenter.Presenter /** - * By default [CircuitContent] will wrap presenters so that their [CircuitUiState]s are conflated - * when the presenter is paused. If this behavior is not wanted, you can make your presenter + * By default [CircuitContent] will wrap presenters so that the last emitted [CircuitUiState] is + * replayed when the presenter is paused. If this behavior is not wanted, [Presenter] should * implement this interface to disable the behavior. */ @Stable public interface NonPausablePresenter : Presenter @@ -40,7 +40,7 @@ public fun pausableState( if (isResumed || uiState == null) { val retainedStateRegistry = rememberRetained { RetainedStateRegistry() } CompositionLocalProvider(LocalRetainedStateRegistry provides retainedStateRegistry) { - saveableStateHolder.SaveableStateProvider("pauseable_presenter") { uiState = produceState() } + saveableStateHolder.SaveableStateProvider("pausable_state") { uiState = produceState() } } } From 0d76e4d00245d8dcfd1f072cd790a7fc82b236c7 Mon Sep 17 00:00:00 2001 From: Chris Banes Date: Fri, 24 May 2024 09:14:10 +0100 Subject: [PATCH 07/14] Update Retained to use Any keys --- .../circuit/foundation/CircuitContent.kt | 2 +- .../slack/circuit/foundation/PausableState.kt | 10 +++--- .../circuit/retained/AndroidContinuity.kt | 10 +++--- .../circuit/retained/RememberRetained.kt | 13 +++----- .../circuit/retained/RetainedStateRegistry.kt | 31 +++++++++---------- 5 files changed, 30 insertions(+), 36 deletions(-) diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt index 921383151..22adc760f 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt @@ -150,7 +150,7 @@ public fun CircuitContent( val state = when (presenter) { is NonPausablePresenter -> presenter.present() - else -> presenter.presentWithLifecycle() + else -> presenter.presentWithLifecycle(key = key ?: screen) } // TODO not sure why stateFlow + LaunchedEffect + distinctUntilChanged doesn't work here diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt index 829b54097..37bc60e19 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt @@ -25,11 +25,13 @@ import com.slack.circuit.runtime.presenter.Presenter @Composable public fun Presenter.presentWithLifecycle( - isResumed: Boolean = LocalLifecycle.current.isResumed -): UiState = pausableState(isResumed) { present() } + key: Any, + isResumed: Boolean = LocalLifecycle.current.isResumed, +): UiState = pausableState(key, isResumed) { present() } @Composable public fun pausableState( + key: Any, isResumed: Boolean = LocalLifecycle.current.isResumed, produceState: @Composable () -> UiState, ): UiState { @@ -38,9 +40,9 @@ public fun pausableState( val saveableStateHolder = rememberSaveableStateHolder() if (isResumed || uiState == null) { - val retainedStateRegistry = rememberRetained { RetainedStateRegistry() } + val retainedStateRegistry = rememberRetained(key = key) { RetainedStateRegistry() } CompositionLocalProvider(LocalRetainedStateRegistry provides retainedStateRegistry) { - saveableStateHolder.SaveableStateProvider("pausable_state") { uiState = produceState() } + saveableStateHolder.SaveableStateProvider(key = key) { uiState = produceState() } } } diff --git a/circuit-retained/src/androidMain/kotlin/com/slack/circuit/retained/AndroidContinuity.kt b/circuit-retained/src/androidMain/kotlin/com/slack/circuit/retained/AndroidContinuity.kt index d942617fb..f6c687bbe 100644 --- a/circuit-retained/src/androidMain/kotlin/com/slack/circuit/retained/AndroidContinuity.kt +++ b/circuit-retained/src/androidMain/kotlin/com/slack/circuit/retained/AndroidContinuity.kt @@ -16,12 +16,12 @@ import androidx.lifecycle.viewmodel.compose.viewModel internal class ContinuityViewModel : ViewModel(), RetainedStateRegistry { private val delegate = RetainedStateRegistryImpl(null) - override fun consumeValue(key: String): Any? { + override fun consumeValue(key: Any): Any? { return delegate.consumeValue(key) } override fun registerValue( - key: String, + key: Any, valueProvider: RetainedValueProvider, ): RetainedStateRegistry.Entry { return delegate.registerValue(key, valueProvider) @@ -31,7 +31,7 @@ internal class ContinuityViewModel : ViewModel(), RetainedStateRegistry { delegate.saveAll() } - override fun saveValue(key: String) { + override fun saveValue(key: Any) { delegate.saveValue(key) } @@ -44,10 +44,10 @@ internal class ContinuityViewModel : ViewModel(), RetainedStateRegistry { delegate.valueProviders.clear() } - @VisibleForTesting fun peekRetained(): Map> = delegate.retained.toMap() + @VisibleForTesting fun peekRetained(): Map> = delegate.retained.toMap() @VisibleForTesting - fun peekProviders(): Map> = + fun peekProviders(): Map> = delegate.valueProviders.toMap() object Factory : ViewModelProvider.Factory { diff --git a/circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RememberRetained.kt b/circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RememberRetained.kt index 5e8fcc241..dfd8527a6 100644 --- a/circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RememberRetained.kt +++ b/circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RememberRetained.kt @@ -66,7 +66,7 @@ import androidx.compose.runtime.remember * @param init A factory function to create the initial value of this state */ @Composable -public fun rememberRetained(vararg inputs: Any?, key: String? = null, init: () -> T): T { +public fun rememberRetained(vararg inputs: Any?, key: Any? = null, init: () -> T): T { val registry = LocalRetainedStateRegistry.current // Short-circuit no-ops if (registry === NoOpRetainedStateRegistry) { @@ -81,12 +81,7 @@ public fun rememberRetained(vararg inputs: Any?, key: String? = null, val compositeKey = currentCompositeKeyHash // key is the one provided by the user or the one generated by the compose runtime - val finalKey = - if (!key.isNullOrEmpty()) { - key - } else { - compositeKey.toString(MaxSupportedRadix) - } + val finalKey = key ?: compositeKey.toString(MaxSupportedRadix) val canRetainChecker = LocalCanRetainChecker.current ?: rememberCanRetainChecker() val holder = @@ -115,14 +110,14 @@ private const val MaxSupportedRadix = 36 private class RetainableHolder( private var registry: RetainedStateRegistry?, private var canRetainChecker: CanRetainChecker, - private var key: String, + private var key: Any, private var value: T, private var inputs: Array, private var hasBeenRestored: Boolean = false, ) : RetainedValueProvider, RememberObserver { private var entry: RetainedStateRegistry.Entry? = null - fun update(registry: RetainedStateRegistry?, key: String, value: T, inputs: Array) { + fun update(registry: RetainedStateRegistry?, key: Any, value: T, inputs: Array) { var entryIsOutdated = false if (this.registry !== registry) { this.registry = registry diff --git a/circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RetainedStateRegistry.kt b/circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RetainedStateRegistry.kt index 2bbb9e462..962f7c3f1 100644 --- a/circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RetainedStateRegistry.kt +++ b/circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RetainedStateRegistry.kt @@ -18,7 +18,7 @@ public interface RetainedStateRegistry { * * @param key Key used to save the value */ - public fun consumeValue(key: String): Any? + public fun consumeValue(key: Any): Any? /** * Registers the value provider. @@ -35,7 +35,7 @@ public interface RetainedStateRegistry { * @param valueProvider The value to provide * @return the registry entry which you can use to unregister the provider */ - public fun registerValue(key: String, valueProvider: RetainedValueProvider): Entry + public fun registerValue(key: Any, valueProvider: RetainedValueProvider): Entry /** * Executes all the registered value providers and combines these values into a map. We have a @@ -44,7 +44,7 @@ public interface RetainedStateRegistry { public fun saveAll() /** Executes the value providers registered with the given [key], and saves them for retrieval. */ - public fun saveValue(key: String) + public fun saveValue(key: Any) /** Releases all currently unconsumed values. Useful as a GC mechanism for the registry. */ public fun forgetUnclaimedValues() @@ -57,7 +57,7 @@ public interface RetainedStateRegistry { } internal interface MutableRetainedStateRegistry : RetainedStateRegistry { - val retained: MutableMap> + val retained: MutableMap> } /** @@ -65,9 +65,7 @@ internal interface MutableRetainedStateRegistry : RetainedStateRegistry { * * @param values The map of the restored values */ -public fun RetainedStateRegistry( - values: Map> = emptyMap() -): RetainedStateRegistry = +public fun RetainedStateRegistry(values: Map> = emptyMap()): RetainedStateRegistry = RetainedStateRegistryImpl(values.mapValues { it.value.toMutableList() }.toMutableMap()) /** CompositionLocal with a current [RetainedStateRegistry] instance. */ @@ -76,13 +74,13 @@ public val LocalRetainedStateRegistry: ProvidableCompositionLocal>?) : +internal class RetainedStateRegistryImpl(retained: MutableMap>?) : MutableRetainedStateRegistry { - override val retained: MutableMap> = retained?.toMutableMap() ?: mutableMapOf() - internal val valueProviders = mutableMapOf>() + override val retained: MutableMap> = retained?.toMutableMap() ?: mutableMapOf() + internal val valueProviders = mutableMapOf>() - override fun consumeValue(key: String): Any? { + override fun consumeValue(key: Any): Any? { val list = retained.remove(key) return if (!list.isNullOrEmpty()) { if (list.size > 1) { @@ -94,8 +92,7 @@ internal class RetainedStateRegistryImpl(retained: MutableMap } } - override fun registerValue(key: String, valueProvider: RetainedValueProvider): Entry { - require(key.isNotBlank()) { "Registered key is empty or blank" } + override fun registerValue(key: Any, valueProvider: RetainedValueProvider): Entry { valueProviders.getOrPut(key) { mutableListOf() }.add(valueProvider) return object : Entry { override fun unregister() { @@ -128,7 +125,7 @@ internal class RetainedStateRegistryImpl(retained: MutableMap valueProviders.clear() } - override fun saveValue(key: String) { + override fun saveValue(key: Any) { val providers = valueProviders[key] if (providers != null) { retained[key] = providers.map { it.invoke() } @@ -154,13 +151,13 @@ internal class RetainedStateRegistryImpl(retained: MutableMap } internal object NoOpRetainedStateRegistry : RetainedStateRegistry { - override fun consumeValue(key: String): Any? = null + override fun consumeValue(key: Any): Any? = null - override fun registerValue(key: String, valueProvider: RetainedValueProvider): Entry = NoOpEntry + override fun registerValue(key: Any, valueProvider: RetainedValueProvider): Entry = NoOpEntry override fun saveAll() {} - override fun saveValue(key: String) {} + override fun saveValue(key: Any) {} override fun forgetUnclaimedValues() {} From 109fe52481a9579c92743f9f7f576e9be2e71092 Mon Sep 17 00:00:00 2001 From: Chris Banes Date: Fri, 24 May 2024 09:21:13 +0100 Subject: [PATCH 08/14] More kdoc --- .../com/slack/circuit/foundation/PausableState.kt | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt index 37bc60e19..b664cec10 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt @@ -23,12 +23,27 @@ import com.slack.circuit.runtime.presenter.Presenter */ @Stable public interface NonPausablePresenter : Presenter +/** + * Presents the UI state when the lifecycle is resumed, otherwise will replay the last emitted UI + * state. + * + * @param key A unique key for the pausable state + * @param isResumed Whether the lifecycle is resumed + */ @Composable public fun Presenter.presentWithLifecycle( key: Any, isResumed: Boolean = LocalLifecycle.current.isResumed, ): UiState = pausableState(key, isResumed) { present() } +/** + * Wraps a composable state producer, which will replay the last emitted state instance when + * [isResumed] is `false`. + * + * @param key A unique key for the pausable state. + * @param isResumed Whether the lifecycle is resumed + * @param produceState A composable lambda function which produces the state + */ @Composable public fun pausableState( key: Any, From 76b5e9626caf5233ec41ad203eb2dbe1b44e5be8 Mon Sep 17 00:00:00 2001 From: Chris Banes Date: Sun, 26 May 2024 09:27:09 +0100 Subject: [PATCH 09/14] API feedback --- .../com/slack/circuit/foundation/Lifecycle.kt | 28 ------------ .../foundation/NavigableCircuitContent.kt | 4 +- .../slack/circuit/foundation/PausableState.kt | 14 +++--- .../circuit/foundation/RecordLifecycle.kt | 44 +++++++++++++++++++ 4 files changed, 53 insertions(+), 37 deletions(-) delete mode 100644 circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/Lifecycle.kt create mode 100644 circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/RecordLifecycle.kt diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/Lifecycle.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/Lifecycle.kt deleted file mode 100644 index c780b006e..000000000 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/Lifecycle.kt +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright (C) 2024 Slack Technologies, LLC -// SPDX-License-Identifier: Apache-2.0 -package com.slack.circuit.foundation - -import androidx.compose.runtime.ProvidableCompositionLocal -import androidx.compose.runtime.Stable -import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.setValue -import androidx.compose.runtime.staticCompositionLocalOf - -@Stable -public interface Lifecycle { - public val isResumed: Boolean -} - -internal class MutableLifecycle(isResumed: Boolean = false) : Lifecycle { - override var isResumed: Boolean by mutableStateOf(isResumed) -} - -public val LocalLifecycle: ProvidableCompositionLocal = staticCompositionLocalOf { - staticLifecycle(true) -} - -private fun staticLifecycle(isResumed: Boolean): Lifecycle = - object : Lifecycle { - override val isResumed: Boolean = isResumed - } diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavigableCircuitContent.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavigableCircuitContent.kt index 0190617d1..12c7b9fef 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavigableCircuitContent.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavigableCircuitContent.kt @@ -167,7 +167,7 @@ private fun buildCircuitContentProviders( } val lifecycle = - remember { MutableLifecycle() }.apply { isResumed = lastBackStack.topRecord == record } + 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, @@ -179,7 +179,7 @@ private fun buildCircuitContentProviders( CompositionLocalProvider( LocalRetainedStateRegistry provides recordRetainedStateRegistry, LocalCanRetainChecker provides CanRetainChecker.Always, - LocalLifecycle provides lifecycle, + LocalRecordLifecycle provides lifecycle, ) { CircuitContent( screen = record.screen, diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt index b664cec10..e95e629aa 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt @@ -28,33 +28,33 @@ import com.slack.circuit.runtime.presenter.Presenter * state. * * @param key A unique key for the pausable state - * @param isResumed Whether the lifecycle is resumed + * @param isActive Whether the presenter is active or not. */ @Composable public fun Presenter.presentWithLifecycle( key: Any, - isResumed: Boolean = LocalLifecycle.current.isResumed, -): UiState = pausableState(key, isResumed) { present() } + isActive: Boolean = LocalRecordLifecycle.current.isActive, +): UiState = pausableState(key, isActive) { present() } /** * Wraps a composable state producer, which will replay the last emitted state instance when - * [isResumed] is `false`. + * [isActive] is `false`. * * @param key A unique key for the pausable state. - * @param isResumed Whether the lifecycle is resumed + * @param isActive Whether the state producer should be active or not. * @param produceState A composable lambda function which produces the state */ @Composable public fun pausableState( key: Any, - isResumed: Boolean = LocalLifecycle.current.isResumed, + isActive: Boolean = LocalRecordLifecycle.current.isActive, produceState: @Composable () -> UiState, ): UiState { var uiState: UiState? by remember { mutableStateOf(null) } val saveableStateHolder = rememberSaveableStateHolder() - if (isResumed || uiState == null) { + if (isActive || uiState == null) { val retainedStateRegistry = rememberRetained(key = key) { RetainedStateRegistry() } CompositionLocalProvider(LocalRetainedStateRegistry provides retainedStateRegistry) { saveableStateHolder.SaveableStateProvider(key = key) { uiState = produceState() } diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/RecordLifecycle.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/RecordLifecycle.kt new file mode 100644 index 000000000..064fa3662 --- /dev/null +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/RecordLifecycle.kt @@ -0,0 +1,44 @@ +// Copyright (C) 2024 Slack Technologies, LLC +// SPDX-License-Identifier: Apache-2.0 +package com.slack.circuit.foundation + +import androidx.compose.runtime.ProvidableCompositionLocal +import androidx.compose.runtime.Stable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.setValue +import androidx.compose.runtime.staticCompositionLocalOf + +/** + * Represents the lifecycle of a navigation record in a [NavigableCircuitContent]. Will typically be + * stored in the [LocalRecordLifecycle] composition local, allowing presenters and UIs to observe + * whether the navigation record is active. + */ +@Stable +public interface RecordLifecycle { + /** + * Whether the record is currently active. Typically this will return true when the record is the + * top record in the back stack. + */ + public val isActive: Boolean +} + +internal class MutableRecordLifecycle(initial: Boolean = false) : RecordLifecycle { + override var isActive: Boolean by mutableStateOf(initial) +} + +/** + * Holds the current lifecycle for a record in a [NavigableCircuitContent]. + * + * For static [CircuitContent]s used outside of [NavigableCircuitContent], the default value will be + * a [RecordLifecycle] which always returned active. + */ +public val LocalRecordLifecycle: ProvidableCompositionLocal = + staticCompositionLocalOf { + staticRecordLifecycle(true) + } + +private fun staticRecordLifecycle(isActive: Boolean): RecordLifecycle = + object : RecordLifecycle { + override val isActive: Boolean = isActive + } From 8bbbb5088a06c7f5136755440b85d365c5922447 Mon Sep 17 00:00:00 2001 From: Chris Banes Date: Sun, 26 May 2024 09:32:18 +0100 Subject: [PATCH 10/14] Tweak pauseable state kdoc --- .../kotlin/com/slack/circuit/foundation/PausableState.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt index e95e629aa..ded031f7f 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt @@ -18,7 +18,7 @@ import com.slack.circuit.runtime.presenter.Presenter /** * By default [CircuitContent] will wrap presenters so that the last emitted [CircuitUiState] is - * replayed when the presenter is paused. If this behavior is not wanted, [Presenter] should + * replayed when the presenter is paused. If this behavior is not wanted, the [Presenter] should * implement this interface to disable the behavior. */ @Stable public interface NonPausablePresenter : Presenter @@ -50,7 +50,7 @@ public fun pausableState( isActive: Boolean = LocalRecordLifecycle.current.isActive, produceState: @Composable () -> UiState, ): UiState { - var uiState: UiState? by remember { mutableStateOf(null) } + var uiState: UiState? by remember(key) { mutableStateOf(null) } val saveableStateHolder = rememberSaveableStateHolder() From 94ccc2cb481dbc8540cfbe286bd622b0848fabb0 Mon Sep 17 00:00:00 2001 From: Chris Banes Date: Sun, 26 May 2024 12:16:37 +0100 Subject: [PATCH 11/14] Fix composition loop on KeyedCircuitContentTests --- .../slack/circuit/foundation/PausableState.kt | 24 ++++++++++++------- .../foundation/KeyedCircuitContentTests.kt | 4 ++-- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt index ded031f7f..218b878f1 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt @@ -1,5 +1,7 @@ // Copyright (C) 2024 Slack Technologies, LLC // SPDX-License-Identifier: Apache-2.0 +@file:Suppress("NOTHING_TO_INLINE") + package com.slack.circuit.foundation import androidx.compose.runtime.Composable @@ -27,11 +29,14 @@ import com.slack.circuit.runtime.presenter.Presenter * Presents the UI state when the lifecycle is resumed, otherwise will replay the last emitted UI * state. * + * The [CircuitUiState] class returned by the [Presenter] is required to implement [equals] and + * [hashCode] methods correctly, otherwise this function can create composition loops. + * * @param key A unique key for the pausable state * @param isActive Whether the presenter is active or not. */ @Composable -public fun Presenter.presentWithLifecycle( +public inline fun Presenter.presentWithLifecycle( key: Any, isActive: Boolean = LocalRecordLifecycle.current.isActive, ): UiState = pausableState(key, isActive) { present() } @@ -40,26 +45,29 @@ public fun Presenter.presentWithLifecycle( * Wraps a composable state producer, which will replay the last emitted state instance when * [isActive] is `false`. * + * The class [T] returned from [produceState] is required to implement [equals] and [hashCode] + * methods correctly, otherwise this function can create composition loops. + * * @param key A unique key for the pausable state. * @param isActive Whether the state producer should be active or not. * @param produceState A composable lambda function which produces the state */ @Composable -public fun pausableState( +public fun pausableState( key: Any, isActive: Boolean = LocalRecordLifecycle.current.isActive, - produceState: @Composable () -> UiState, -): UiState { - var uiState: UiState? by remember(key) { mutableStateOf(null) } + produceState: @Composable () -> T, +): T { + var state: T? by remember(key) { mutableStateOf(null) } val saveableStateHolder = rememberSaveableStateHolder() - if (isActive || uiState == null) { + if (isActive || state == null) { val retainedStateRegistry = rememberRetained(key = key) { RetainedStateRegistry() } CompositionLocalProvider(LocalRetainedStateRegistry provides retainedStateRegistry) { - saveableStateHolder.SaveableStateProvider(key = key) { uiState = produceState() } + saveableStateHolder.SaveableStateProvider(key = key) { state = produceState() } } } - return uiState!! + return state!! } diff --git a/circuit-foundation/src/jvmTest/kotlin/com/slack/circuit/foundation/KeyedCircuitContentTests.kt b/circuit-foundation/src/jvmTest/kotlin/com/slack/circuit/foundation/KeyedCircuitContentTests.kt index ed3aaf711..eb89cdc5c 100644 --- a/circuit-foundation/src/jvmTest/kotlin/com/slack/circuit/foundation/KeyedCircuitContentTests.kt +++ b/circuit-foundation/src/jvmTest/kotlin/com/slack/circuit/foundation/KeyedCircuitContentTests.kt @@ -198,7 +198,7 @@ class KeyedCircuitContentTests { } private class ScreenA(val num: Int) : Screen { - class State(val numSquare: Int, val retainedNumSquare: Int) : CircuitUiState + data class State(val numSquare: Int, val retainedNumSquare: Int) : CircuitUiState } private class ScreenAPresenter(val screen: ScreenA) : Presenter { @@ -221,7 +221,7 @@ private fun ScreenAUi(state: ScreenA.State, modifier: Modifier = Modifier) { private class ScreenB(val text: String) : Screen { - class State(val textReverse: String, val retainedTextReverse: String) : CircuitUiState + data class State(val textReverse: String, val retainedTextReverse: String) : CircuitUiState } private class ScreenBPresenter(val screen: ScreenB) : Presenter { From 59d41e0ef65d9e098859c81ebd808256bb7e0d8c Mon Sep 17 00:00:00 2001 From: Chris Banes Date: Sun, 26 May 2024 14:21:03 +0100 Subject: [PATCH 12/14] Update KeyedCircuitContentTests to use remember --- .../foundation/KeyedCircuitContentTests.kt | 101 +++++++++--------- 1 file changed, 52 insertions(+), 49 deletions(-) diff --git a/circuit-foundation/src/jvmTest/kotlin/com/slack/circuit/foundation/KeyedCircuitContentTests.kt b/circuit-foundation/src/jvmTest/kotlin/com/slack/circuit/foundation/KeyedCircuitContentTests.kt index eb89cdc5c..1386606fb 100644 --- a/circuit-foundation/src/jvmTest/kotlin/com/slack/circuit/foundation/KeyedCircuitContentTests.kt +++ b/circuit-foundation/src/jvmTest/kotlin/com/slack/circuit/foundation/KeyedCircuitContentTests.kt @@ -8,6 +8,7 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.MutableState import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier import androidx.compose.ui.platform.testTag @@ -16,7 +17,6 @@ import androidx.compose.ui.test.junit4.ComposeContentTestRule import androidx.compose.ui.test.junit4.createComposeRule import androidx.compose.ui.test.onNodeWithTag import com.slack.circuit.backstack.rememberSaveableBackStack -import com.slack.circuit.retained.rememberRetained import com.slack.circuit.runtime.CircuitUiState import com.slack.circuit.runtime.Navigator import com.slack.circuit.runtime.presenter.Presenter @@ -25,8 +25,8 @@ import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith -private const val TAG_UI_RETAINED = "TAG_UI_RETAINED" -private const val TAG_PRESENTER_RETAINED = "TAG_PRESENTER_RETAINED" +private const val TAG_UI_REMEMBERED = "TAG_UI_REMEMBERED" +private const val TAG_PRESENTER_REMEMBERED = "TAG_PRESENTER_REMEMBERED" private const val TAG_STATE = "TAG_STATE" /** @@ -62,33 +62,33 @@ class KeyedCircuitContentTests { val navigator = setUpTestOneContent() // Initial onNodeWithTag(TAG_STATE).assertTextEquals("1") - onNodeWithTag(TAG_UI_RETAINED).assertTextEquals("1") - onNodeWithTag(TAG_PRESENTER_RETAINED).assertTextEquals("1") + onNodeWithTag(TAG_UI_REMEMBERED).assertTextEquals("1") + onNodeWithTag(TAG_PRESENTER_REMEMBERED).assertTextEquals("1") // Push a new ScreenA navigator.goTo(ScreenA(2)) onNodeWithTag(TAG_STATE).assertTextEquals("4") - onNodeWithTag(TAG_UI_RETAINED).assertTextEquals("4") - onNodeWithTag(TAG_PRESENTER_RETAINED).assertTextEquals("4") + onNodeWithTag(TAG_UI_REMEMBERED).assertTextEquals("4") + onNodeWithTag(TAG_PRESENTER_REMEMBERED).assertTextEquals("4") // Push a new ScreenA navigator.goTo(ScreenA(3)) onNodeWithTag(TAG_STATE).assertTextEquals("9") - onNodeWithTag(TAG_UI_RETAINED).assertTextEquals("9") - onNodeWithTag(TAG_PRESENTER_RETAINED).assertTextEquals("9") + onNodeWithTag(TAG_UI_REMEMBERED).assertTextEquals("9") + onNodeWithTag(TAG_PRESENTER_REMEMBERED).assertTextEquals("9") // Push a new ScreenB navigator.goTo(ScreenB("abc")) onNodeWithTag(TAG_STATE).assertTextEquals("cba") - onNodeWithTag(TAG_UI_RETAINED).assertTextEquals("cba") - onNodeWithTag(TAG_PRESENTER_RETAINED).assertTextEquals("cba") + onNodeWithTag(TAG_UI_REMEMBERED).assertTextEquals("cba") + onNodeWithTag(TAG_PRESENTER_REMEMBERED).assertTextEquals("cba") // Back one navigator.pop() onNodeWithTag(TAG_STATE).assertTextEquals("9") - onNodeWithTag(TAG_UI_RETAINED).assertTextEquals("9") - onNodeWithTag(TAG_PRESENTER_RETAINED).assertTextEquals("9") + onNodeWithTag(TAG_UI_REMEMBERED).assertTextEquals("9") + onNodeWithTag(TAG_PRESENTER_REMEMBERED).assertTextEquals("9") // Back two navigator.pop() onNodeWithTag(TAG_STATE).assertTextEquals("4") - onNodeWithTag(TAG_UI_RETAINED).assertTextEquals("4") - onNodeWithTag(TAG_PRESENTER_RETAINED).assertTextEquals("4") + onNodeWithTag(TAG_UI_REMEMBERED).assertTextEquals("4") + onNodeWithTag(TAG_PRESENTER_REMEMBERED).assertTextEquals("4") } } @@ -98,33 +98,33 @@ class KeyedCircuitContentTests { var screenState by setUpTestTwoAContent() // Initial onNodeWithTag(TAG_STATE).assertTextEquals("1") - onNodeWithTag(TAG_UI_RETAINED).assertTextEquals("1") - onNodeWithTag(TAG_PRESENTER_RETAINED).assertTextEquals("1") + onNodeWithTag(TAG_UI_REMEMBERED).assertTextEquals("1") + onNodeWithTag(TAG_PRESENTER_REMEMBERED).assertTextEquals("1") // Set a new ScreenA screenState = ScreenA(2) onNodeWithTag(TAG_STATE).assertTextEquals("4") - onNodeWithTag(TAG_UI_RETAINED).assertTextEquals("4") - onNodeWithTag(TAG_PRESENTER_RETAINED).assertTextEquals("4") + onNodeWithTag(TAG_UI_REMEMBERED).assertTextEquals("4") + onNodeWithTag(TAG_PRESENTER_REMEMBERED).assertTextEquals("4") // Set a new ScreenA screenState = ScreenA(3) onNodeWithTag(TAG_STATE).assertTextEquals("9") - onNodeWithTag(TAG_UI_RETAINED).assertTextEquals("9") - onNodeWithTag(TAG_PRESENTER_RETAINED).assertTextEquals("9") + onNodeWithTag(TAG_UI_REMEMBERED).assertTextEquals("9") + onNodeWithTag(TAG_PRESENTER_REMEMBERED).assertTextEquals("9") // Set a new ScreenB screenState = ScreenB("abc") onNodeWithTag(TAG_STATE).assertTextEquals("cba") - onNodeWithTag(TAG_UI_RETAINED).assertTextEquals("cba") - onNodeWithTag(TAG_PRESENTER_RETAINED).assertTextEquals("cba") + onNodeWithTag(TAG_UI_REMEMBERED).assertTextEquals("cba") + onNodeWithTag(TAG_PRESENTER_REMEMBERED).assertTextEquals("cba") // Back to a ScreenA screenState = ScreenA(3) onNodeWithTag(TAG_STATE).assertTextEquals("9") - onNodeWithTag(TAG_UI_RETAINED).assertTextEquals("9") - onNodeWithTag(TAG_PRESENTER_RETAINED).assertTextEquals("9") + onNodeWithTag(TAG_UI_REMEMBERED).assertTextEquals("9") + onNodeWithTag(TAG_PRESENTER_REMEMBERED).assertTextEquals("9") // Back to another ScreenA screenState = ScreenA(2) onNodeWithTag(TAG_STATE).assertTextEquals("4") - onNodeWithTag(TAG_UI_RETAINED).assertTextEquals("4") - onNodeWithTag(TAG_PRESENTER_RETAINED).assertTextEquals("4") + onNodeWithTag(TAG_UI_REMEMBERED).assertTextEquals("4") + onNodeWithTag(TAG_PRESENTER_REMEMBERED).assertTextEquals("4") } } @@ -134,33 +134,33 @@ class KeyedCircuitContentTests { var screenState by setUpTestTwoBContent() // Initial onNodeWithTag(TAG_STATE).assertTextEquals("1") - onNodeWithTag(TAG_UI_RETAINED).assertTextEquals("1") - onNodeWithTag(TAG_PRESENTER_RETAINED).assertTextEquals("1") + onNodeWithTag(TAG_UI_REMEMBERED).assertTextEquals("1") + onNodeWithTag(TAG_PRESENTER_REMEMBERED).assertTextEquals("1") // Set a new ScreenA screenState = ScreenA(2) onNodeWithTag(TAG_STATE).assertTextEquals("4") - onNodeWithTag(TAG_UI_RETAINED).assertTextEquals("1") - onNodeWithTag(TAG_PRESENTER_RETAINED).assertTextEquals("1") + onNodeWithTag(TAG_UI_REMEMBERED).assertTextEquals("1") + onNodeWithTag(TAG_PRESENTER_REMEMBERED).assertTextEquals("1") // Set a new ScreenA screenState = ScreenA(3) onNodeWithTag(TAG_STATE).assertTextEquals("9") - onNodeWithTag(TAG_UI_RETAINED).assertTextEquals("1") - onNodeWithTag(TAG_PRESENTER_RETAINED).assertTextEquals("1") + onNodeWithTag(TAG_UI_REMEMBERED).assertTextEquals("1") + onNodeWithTag(TAG_PRESENTER_REMEMBERED).assertTextEquals("1") // Set a new ScreenB screenState = ScreenB("abc") onNodeWithTag(TAG_STATE).assertTextEquals("cba") - onNodeWithTag(TAG_UI_RETAINED).assertTextEquals("cba") - onNodeWithTag(TAG_PRESENTER_RETAINED).assertTextEquals("cba") + onNodeWithTag(TAG_UI_REMEMBERED).assertTextEquals("cba") + onNodeWithTag(TAG_PRESENTER_REMEMBERED).assertTextEquals("cba") // Back to a ScreenA screenState = ScreenA(3) onNodeWithTag(TAG_STATE).assertTextEquals("9") - onNodeWithTag(TAG_UI_RETAINED).assertTextEquals("9") - onNodeWithTag(TAG_PRESENTER_RETAINED).assertTextEquals("9") + onNodeWithTag(TAG_UI_REMEMBERED).assertTextEquals("9") + onNodeWithTag(TAG_PRESENTER_REMEMBERED).assertTextEquals("9") // Back to another ScreenA screenState = ScreenA(2) onNodeWithTag(TAG_STATE).assertTextEquals("4") - onNodeWithTag(TAG_UI_RETAINED).assertTextEquals("9") - onNodeWithTag(TAG_PRESENTER_RETAINED).assertTextEquals("9") + onNodeWithTag(TAG_UI_REMEMBERED).assertTextEquals("9") + onNodeWithTag(TAG_PRESENTER_REMEMBERED).assertTextEquals("9") } } @@ -198,13 +198,13 @@ class KeyedCircuitContentTests { } private class ScreenA(val num: Int) : Screen { - data class State(val numSquare: Int, val retainedNumSquare: Int) : CircuitUiState + data class State(val numSquare: Int, val rememberedNumSquare: Int) : CircuitUiState } private class ScreenAPresenter(val screen: ScreenA) : Presenter { @Composable override fun present(): ScreenA.State { - val square = rememberRetained { screen.num * screen.num } + val square = remember { screen.num * screen.num } return ScreenA.State(screen.num * screen.num, square) } } @@ -212,22 +212,25 @@ private class ScreenAPresenter(val screen: ScreenA) : Presenter { @Composable private fun ScreenAUi(state: ScreenA.State, modifier: Modifier = Modifier) { Column(modifier) { - val retained = rememberRetained { state.numSquare } - Text(text = "$retained", modifier = Modifier.testTag(TAG_UI_RETAINED)) + val remembered = remember { state.numSquare } + Text(text = "$remembered", modifier = Modifier.testTag(TAG_UI_REMEMBERED)) Text(text = "${state.numSquare}", modifier = Modifier.testTag(TAG_STATE)) - Text(text = "${state.retainedNumSquare}", modifier = Modifier.testTag(TAG_PRESENTER_RETAINED)) + Text( + text = "${state.rememberedNumSquare}", + modifier = Modifier.testTag(TAG_PRESENTER_REMEMBERED), + ) } } private class ScreenB(val text: String) : Screen { - data class State(val textReverse: String, val retainedTextReverse: String) : CircuitUiState + data class State(val textReverse: String, val rememberedTextReverse: String) : CircuitUiState } private class ScreenBPresenter(val screen: ScreenB) : Presenter { @Composable override fun present(): ScreenB.State { - val textReverse = rememberRetained { screen.text.reversed() } + val textReverse = remember { screen.text.reversed() } return ScreenB.State(screen.text.reversed(), textReverse) } } @@ -235,9 +238,9 @@ private class ScreenBPresenter(val screen: ScreenB) : Presenter { @Composable private fun ScreenBUi(state: ScreenB.State, modifier: Modifier = Modifier) { Column(modifier) { - val retained = rememberRetained { state.textReverse } - Text(text = retained, modifier = Modifier.testTag(TAG_UI_RETAINED)) + val remembered = remember { state.textReverse } + Text(text = remembered, modifier = Modifier.testTag(TAG_UI_REMEMBERED)) Text(text = state.textReverse, modifier = Modifier.testTag(TAG_STATE)) - Text(text = state.retainedTextReverse, modifier = Modifier.testTag(TAG_PRESENTER_RETAINED)) + Text(text = state.rememberedTextReverse, modifier = Modifier.testTag(TAG_PRESENTER_REMEMBERED)) } } From 17a382e099c147977d6412757c68ed2588d60137 Mon Sep 17 00:00:00 2001 From: Chris Banes Date: Sun, 26 May 2024 19:40:48 +0100 Subject: [PATCH 13/14] Revert retained keys change --- .../circuit/foundation/CircuitContent.kt | 2 +- .../slack/circuit/foundation/PausableState.kt | 8 +++-- .../circuit/retained/AndroidContinuity.kt | 10 +++--- .../circuit/retained/RememberRetained.kt | 13 +++++--- .../circuit/retained/RetainedStateRegistry.kt | 31 ++++++++++--------- 5 files changed, 37 insertions(+), 27 deletions(-) diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt index 22adc760f..921383151 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt @@ -150,7 +150,7 @@ public fun CircuitContent( val state = when (presenter) { is NonPausablePresenter -> presenter.present() - else -> presenter.presentWithLifecycle(key = key ?: screen) + else -> presenter.presentWithLifecycle() } // TODO not sure why stateFlow + LaunchedEffect + distinctUntilChanged doesn't work here diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt index 218b878f1..d9e018e4f 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/PausableState.kt @@ -37,7 +37,7 @@ import com.slack.circuit.runtime.presenter.Presenter */ @Composable public inline fun Presenter.presentWithLifecycle( - key: Any, + key: String? = null, isActive: Boolean = LocalRecordLifecycle.current.isActive, ): UiState = pausableState(key, isActive) { present() } @@ -54,7 +54,7 @@ public inline fun Presenter.presentWithLifec */ @Composable public fun pausableState( - key: Any, + key: String? = null, isActive: Boolean = LocalRecordLifecycle.current.isActive, produceState: @Composable () -> T, ): T { @@ -65,7 +65,9 @@ public fun pausableState( if (isActive || state == null) { val retainedStateRegistry = rememberRetained(key = key) { RetainedStateRegistry() } CompositionLocalProvider(LocalRetainedStateRegistry provides retainedStateRegistry) { - saveableStateHolder.SaveableStateProvider(key = key) { state = produceState() } + saveableStateHolder.SaveableStateProvider(key = key ?: "pausable_state") { + state = produceState() + } } } diff --git a/circuit-retained/src/androidMain/kotlin/com/slack/circuit/retained/AndroidContinuity.kt b/circuit-retained/src/androidMain/kotlin/com/slack/circuit/retained/AndroidContinuity.kt index f6c687bbe..d942617fb 100644 --- a/circuit-retained/src/androidMain/kotlin/com/slack/circuit/retained/AndroidContinuity.kt +++ b/circuit-retained/src/androidMain/kotlin/com/slack/circuit/retained/AndroidContinuity.kt @@ -16,12 +16,12 @@ import androidx.lifecycle.viewmodel.compose.viewModel internal class ContinuityViewModel : ViewModel(), RetainedStateRegistry { private val delegate = RetainedStateRegistryImpl(null) - override fun consumeValue(key: Any): Any? { + override fun consumeValue(key: String): Any? { return delegate.consumeValue(key) } override fun registerValue( - key: Any, + key: String, valueProvider: RetainedValueProvider, ): RetainedStateRegistry.Entry { return delegate.registerValue(key, valueProvider) @@ -31,7 +31,7 @@ internal class ContinuityViewModel : ViewModel(), RetainedStateRegistry { delegate.saveAll() } - override fun saveValue(key: Any) { + override fun saveValue(key: String) { delegate.saveValue(key) } @@ -44,10 +44,10 @@ internal class ContinuityViewModel : ViewModel(), RetainedStateRegistry { delegate.valueProviders.clear() } - @VisibleForTesting fun peekRetained(): Map> = delegate.retained.toMap() + @VisibleForTesting fun peekRetained(): Map> = delegate.retained.toMap() @VisibleForTesting - fun peekProviders(): Map> = + fun peekProviders(): Map> = delegate.valueProviders.toMap() object Factory : ViewModelProvider.Factory { diff --git a/circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RememberRetained.kt b/circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RememberRetained.kt index dfd8527a6..5e8fcc241 100644 --- a/circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RememberRetained.kt +++ b/circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RememberRetained.kt @@ -66,7 +66,7 @@ import androidx.compose.runtime.remember * @param init A factory function to create the initial value of this state */ @Composable -public fun rememberRetained(vararg inputs: Any?, key: Any? = null, init: () -> T): T { +public fun rememberRetained(vararg inputs: Any?, key: String? = null, init: () -> T): T { val registry = LocalRetainedStateRegistry.current // Short-circuit no-ops if (registry === NoOpRetainedStateRegistry) { @@ -81,7 +81,12 @@ public fun rememberRetained(vararg inputs: Any?, key: Any? = null, ini val compositeKey = currentCompositeKeyHash // key is the one provided by the user or the one generated by the compose runtime - val finalKey = key ?: compositeKey.toString(MaxSupportedRadix) + val finalKey = + if (!key.isNullOrEmpty()) { + key + } else { + compositeKey.toString(MaxSupportedRadix) + } val canRetainChecker = LocalCanRetainChecker.current ?: rememberCanRetainChecker() val holder = @@ -110,14 +115,14 @@ private const val MaxSupportedRadix = 36 private class RetainableHolder( private var registry: RetainedStateRegistry?, private var canRetainChecker: CanRetainChecker, - private var key: Any, + private var key: String, private var value: T, private var inputs: Array, private var hasBeenRestored: Boolean = false, ) : RetainedValueProvider, RememberObserver { private var entry: RetainedStateRegistry.Entry? = null - fun update(registry: RetainedStateRegistry?, key: Any, value: T, inputs: Array) { + fun update(registry: RetainedStateRegistry?, key: String, value: T, inputs: Array) { var entryIsOutdated = false if (this.registry !== registry) { this.registry = registry diff --git a/circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RetainedStateRegistry.kt b/circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RetainedStateRegistry.kt index 962f7c3f1..2bbb9e462 100644 --- a/circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RetainedStateRegistry.kt +++ b/circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RetainedStateRegistry.kt @@ -18,7 +18,7 @@ public interface RetainedStateRegistry { * * @param key Key used to save the value */ - public fun consumeValue(key: Any): Any? + public fun consumeValue(key: String): Any? /** * Registers the value provider. @@ -35,7 +35,7 @@ public interface RetainedStateRegistry { * @param valueProvider The value to provide * @return the registry entry which you can use to unregister the provider */ - public fun registerValue(key: Any, valueProvider: RetainedValueProvider): Entry + public fun registerValue(key: String, valueProvider: RetainedValueProvider): Entry /** * Executes all the registered value providers and combines these values into a map. We have a @@ -44,7 +44,7 @@ public interface RetainedStateRegistry { public fun saveAll() /** Executes the value providers registered with the given [key], and saves them for retrieval. */ - public fun saveValue(key: Any) + public fun saveValue(key: String) /** Releases all currently unconsumed values. Useful as a GC mechanism for the registry. */ public fun forgetUnclaimedValues() @@ -57,7 +57,7 @@ public interface RetainedStateRegistry { } internal interface MutableRetainedStateRegistry : RetainedStateRegistry { - val retained: MutableMap> + val retained: MutableMap> } /** @@ -65,7 +65,9 @@ internal interface MutableRetainedStateRegistry : RetainedStateRegistry { * * @param values The map of the restored values */ -public fun RetainedStateRegistry(values: Map> = emptyMap()): RetainedStateRegistry = +public fun RetainedStateRegistry( + values: Map> = emptyMap() +): RetainedStateRegistry = RetainedStateRegistryImpl(values.mapValues { it.value.toMutableList() }.toMutableMap()) /** CompositionLocal with a current [RetainedStateRegistry] instance. */ @@ -74,13 +76,13 @@ public val LocalRetainedStateRegistry: ProvidableCompositionLocal>?) : +internal class RetainedStateRegistryImpl(retained: MutableMap>?) : MutableRetainedStateRegistry { - override val retained: MutableMap> = retained?.toMutableMap() ?: mutableMapOf() - internal val valueProviders = mutableMapOf>() + override val retained: MutableMap> = retained?.toMutableMap() ?: mutableMapOf() + internal val valueProviders = mutableMapOf>() - override fun consumeValue(key: Any): Any? { + override fun consumeValue(key: String): Any? { val list = retained.remove(key) return if (!list.isNullOrEmpty()) { if (list.size > 1) { @@ -92,7 +94,8 @@ internal class RetainedStateRegistryImpl(retained: MutableMap>?) } } - override fun registerValue(key: Any, valueProvider: RetainedValueProvider): Entry { + override fun registerValue(key: String, valueProvider: RetainedValueProvider): Entry { + require(key.isNotBlank()) { "Registered key is empty or blank" } valueProviders.getOrPut(key) { mutableListOf() }.add(valueProvider) return object : Entry { override fun unregister() { @@ -125,7 +128,7 @@ internal class RetainedStateRegistryImpl(retained: MutableMap>?) valueProviders.clear() } - override fun saveValue(key: Any) { + override fun saveValue(key: String) { val providers = valueProviders[key] if (providers != null) { retained[key] = providers.map { it.invoke() } @@ -151,13 +154,13 @@ internal class RetainedStateRegistryImpl(retained: MutableMap>?) } internal object NoOpRetainedStateRegistry : RetainedStateRegistry { - override fun consumeValue(key: Any): Any? = null + override fun consumeValue(key: String): Any? = null - override fun registerValue(key: Any, valueProvider: RetainedValueProvider): Entry = NoOpEntry + override fun registerValue(key: String, valueProvider: RetainedValueProvider): Entry = NoOpEntry override fun saveAll() {} - override fun saveValue(key: Any) {} + override fun saveValue(key: String) {} override fun forgetUnclaimedValues() {} From edc10fd4caea33ac6e01f892f9be07644bffffcf Mon Sep 17 00:00:00 2001 From: Chris Banes Date: Sun, 26 May 2024 19:54:13 +0100 Subject: [PATCH 14/14] Update CHANGELOG --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 318de6849..a5a09ce88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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.