diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fd68b0a8..431f97329 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ Changelog -------------- - **New**: Add WASM targets. +- **Behaviour Change**: `NavigatorImpl.goTo` no longer navigates if the `Screen` is equal to `Navigator.peek()` +- **Change**: `Navigator.goTo` now returns a Bool indicating navigation success - Mark `Presenter.Factory` as `@Stable`. - Mark `Ui.Factory` as `@Stable`. - Mark `CircuitContext` as `@Stable`. diff --git a/backstack/src/commonMain/kotlin/com/slack/circuit/backstack/BackStack.kt b/backstack/src/commonMain/kotlin/com/slack/circuit/backstack/BackStack.kt index a5438d640..1a28c6b08 100644 --- a/backstack/src/commonMain/kotlin/com/slack/circuit/backstack/BackStack.kt +++ b/backstack/src/commonMain/kotlin/com/slack/circuit/backstack/BackStack.kt @@ -41,8 +41,9 @@ public interface BackStack : Iterable { * @param record The record to push onto the stack. * @param resultKey An optional key that would be used to tag a result produced by this record. * The previous record on the stack will receive this key. + * @return If the [record] was successfully pushed onto the back stack */ - public fun push(record: R, resultKey: String? = null) + public fun push(record: R, resultKey: String? = null): Boolean /** * Push a new [Screen] onto the back stack. This will be enveloped in a [Record] and the new @@ -51,8 +52,9 @@ public interface BackStack : Iterable { * @param screen The screen to push onto the stack. * @param resultKey An optional key that would be used to tag a result produced by this record. * The previous record on the stack will receive this key. + * @return If the [screen] was successfully pushed onto the back stack */ - public fun push(screen: Screen, resultKey: String? = null) + public fun push(screen: Screen, resultKey: String? = null): Boolean /** * Attempt to pop the top item off of the back stack, returning the popped [Record] if popping was diff --git a/backstack/src/commonMain/kotlin/com/slack/circuit/backstack/SaveableBackStack.kt b/backstack/src/commonMain/kotlin/com/slack/circuit/backstack/SaveableBackStack.kt index a05967b49..5273184ec 100644 --- a/backstack/src/commonMain/kotlin/com/slack/circuit/backstack/SaveableBackStack.kt +++ b/backstack/src/commonMain/kotlin/com/slack/circuit/backstack/SaveableBackStack.kt @@ -21,6 +21,7 @@ import androidx.compose.runtime.saveable.Saver import androidx.compose.runtime.saveable.listSaver import androidx.compose.runtime.saveable.mapSaver import androidx.compose.runtime.saveable.rememberSaveable +import androidx.compose.runtime.snapshots.Snapshot import com.benasher44.uuid.uuid4 import com.slack.circuit.runtime.screen.PopResult import com.slack.circuit.runtime.screen.Screen @@ -90,18 +91,24 @@ internal constructor( public override val topRecord: Record? get() = entryList.firstOrNull() - public override fun push(screen: Screen, resultKey: String?) { + public override fun push(screen: Screen, resultKey: String?): Boolean { return push(screen, emptyMap(), resultKey) } - public fun push(screen: Screen, args: Map, resultKey: String?) { - push(Record(screen, args), resultKey) + public fun push(screen: Screen, args: Map, resultKey: String?): Boolean { + return push(Record(screen, args), resultKey) } - public override fun push(record: Record, resultKey: String?) { - entryList.add(0, record) - // Clear the cached pending result from the previous top record - entryList.getOrNull(1)?.apply { resultKey?.let(::prepareForResult) } + public override fun push(record: Record, resultKey: String?): Boolean { + val topRecord = Snapshot.withoutReadObservation { entryList.firstOrNull() } + // Guard pushing the exact same record value to the top, records.key is always unique so verify + // the parameters individually. + return if (topRecord?.screen != record.screen || topRecord.args != record.args) { + entryList.add(0, record) + // Clear the cached pending result from the previous top record + entryList.getOrNull(1)?.apply { resultKey?.let(::prepareForResult) } + true + } else false } override fun pop(result: PopResult?): Record? { diff --git a/backstack/src/commonTest/kotlin/com/slack/circuit/backstack/SaveableBackStackTest.kt b/backstack/src/commonTest/kotlin/com/slack/circuit/backstack/SaveableBackStackTest.kt index c6f169ecc..4298d3e78 100644 --- a/backstack/src/commonTest/kotlin/com/slack/circuit/backstack/SaveableBackStackTest.kt +++ b/backstack/src/commonTest/kotlin/com/slack/circuit/backstack/SaveableBackStackTest.kt @@ -11,6 +11,8 @@ import assertk.assertions.isEqualTo import assertk.assertions.isNotNull import com.slack.circuit.internal.test.TestScreen import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue class SaveableBackStackTest { @@ -100,6 +102,14 @@ class SaveableBackStackTest { assertThat(backStack.entryList.toList()).isEqualTo(restored.entryList.toList()) assertThat(backStack.stateStore.toMap()).isEqualTo(restored.stateStore.toMap()) } + + @Test + fun test_guard_pushing_same_top_record() { + val backStack = SaveableBackStack(TestScreen.RootAlpha) + assertTrue(backStack.push(TestScreen.ScreenA)) + assertTrue(backStack.push(TestScreen.ScreenB)) + assertFalse(backStack.push(TestScreen.ScreenB)) + } } private fun save(backStack: SaveableBackStack) = diff --git a/circuit-foundation/src/commonJvmTest/kotlin/com/slack/circuit/foundation/NavigatorTest.kt b/circuit-foundation/src/commonJvmTest/kotlin/com/slack/circuit/foundation/NavigatorTest.kt index 91ae164fb..71267f61c 100644 --- a/circuit-foundation/src/commonJvmTest/kotlin/com/slack/circuit/foundation/NavigatorTest.kt +++ b/circuit-foundation/src/commonJvmTest/kotlin/com/slack/circuit/foundation/NavigatorTest.kt @@ -29,7 +29,7 @@ class NavigatorTest { @Test fun popAtRoot() { val backStack = SaveableBackStack(TestScreen) - backStack.push(TestScreen) + backStack.push(TestScreen2) var onRootPop = 0 val navigator = NavigatorImpl(backStack) { onRootPop++ } diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/AnsweringNavigator.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/AnsweringNavigator.kt index 019f88af7..95edff732 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/AnsweringNavigator.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/AnsweringNavigator.kt @@ -124,9 +124,10 @@ public fun rememberAnsweringNavigator( } val answeringNavigator = remember { object : GoToNavigator { - override fun goTo(screen: Screen) { + override fun goTo(screen: Screen): Boolean { currentBackStack.push(screen, key) launched = true + return true } } } 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 b3feef71b..0b7b6c251 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 @@ -45,8 +45,9 @@ public fun CircuitContent( val navigator = remember(onNavEvent) { object : Navigator { - override fun goTo(screen: Screen) { + override fun goTo(screen: Screen): Boolean { onNavEvent(NavEvent.GoTo(screen)) + return true } override fun resetRoot( diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavigatorImpl.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavigatorImpl.kt index 9494527e9..2ca81a92f 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavigatorImpl.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavigatorImpl.kt @@ -54,8 +54,8 @@ internal class NavigatorImpl( check(!backStack.isEmpty) { "Backstack size must not be empty." } } - override fun goTo(screen: Screen) { - backStack.push(screen) + override fun goTo(screen: Screen): Boolean { + return backStack.push(screen) } override fun pop(result: PopResult?): Screen? { diff --git a/circuit-runtime/src/commonMain/kotlin/com/slack/circuit/runtime/Navigator.kt b/circuit-runtime/src/commonMain/kotlin/com/slack/circuit/runtime/Navigator.kt index 0b8525985..e9339a9fd 100644 --- a/circuit-runtime/src/commonMain/kotlin/com/slack/circuit/runtime/Navigator.kt +++ b/circuit-runtime/src/commonMain/kotlin/com/slack/circuit/runtime/Navigator.kt @@ -12,13 +12,18 @@ import kotlinx.collections.immutable.persistentListOf /** A Navigator that only supports [goTo]. */ @Stable public interface GoToNavigator { - public fun goTo(screen: Screen) + /** + * Navigate to the [screen]. + * + * @return If the navigator successfully went to the [screen] + */ + public fun goTo(screen: Screen): Boolean } /** A basic navigation interface for navigating between [screens][Screen]. */ @Stable public interface Navigator : GoToNavigator { - public override fun goTo(screen: Screen) + public override fun goTo(screen: Screen): Boolean public fun pop(result: PopResult? = null): Screen? @@ -82,7 +87,7 @@ public interface Navigator : GoToNavigator { ): ImmutableList public object NoOp : Navigator { - override fun goTo(screen: Screen) {} + override fun goTo(screen: Screen): Boolean = true override fun pop(result: PopResult?): Screen? = null diff --git a/circuit-test/src/androidUnitTest/kotlin/com/slack/circuit/test/FakeNavigatorTest.kt b/circuit-test/src/androidUnitTest/kotlin/com/slack/circuit/test/FakeNavigatorTest.kt index 8f5375e26..36388a8fe 100644 --- a/circuit-test/src/androidUnitTest/kotlin/com/slack/circuit/test/FakeNavigatorTest.kt +++ b/circuit-test/src/androidUnitTest/kotlin/com/slack/circuit/test/FakeNavigatorTest.kt @@ -4,6 +4,7 @@ package com.slack.circuit.test import com.google.common.truth.Truth.assertThat import com.slack.circuit.runtime.screen.Screen +import com.slack.circuit.test.FakeNavigator.GoToEvent import com.slack.circuit.test.FakeNavigator.ResetRootEvent import kotlinx.coroutines.test.runTest import kotlinx.parcelize.Parcelize @@ -34,6 +35,20 @@ class FakeNavigatorTest { assertThat(oldScreens).containsExactly(TestScreen1) assertThat(navigator.awaitResetRoot()).isEqualTo(ResetRootEvent(TestScreen1, oldScreens)) } + + @Test + fun goTo() = runTest { + val navigator = FakeNavigator(TestScreen1) + // Go to a second screen + assertThat(navigator.goTo(TestScreen2)).isTrue() + assertThat(navigator.awaitNextScreen()).isEqualTo(TestScreen2) + // Going to the same screen won't navigate + assertThat(navigator.goTo(TestScreen2)).isFalse() + assertThat(navigator.awaitNextGoTo()).isEqualTo(GoToEvent(TestScreen2, success = false)) + // Go to a third screen + assertThat(navigator.goTo(TestScreen3)).isTrue() + assertThat(navigator.awaitNextScreen()).isEqualTo(TestScreen3) + } } @Parcelize private data object TestScreen1 : Screen diff --git a/circuit-test/src/commonMain/kotlin/com/slack/circuit/test/FakeNavigator.kt b/circuit-test/src/commonMain/kotlin/com/slack/circuit/test/FakeNavigator.kt index 0fcf5e8b2..f654a1024 100644 --- a/circuit-test/src/commonMain/kotlin/com/slack/circuit/test/FakeNavigator.kt +++ b/circuit-test/src/commonMain/kotlin/com/slack/circuit/test/FakeNavigator.kt @@ -48,13 +48,14 @@ public class FakeNavigator internal constructor(private val delegate: Navigator) SaveableBackStack(root) ) - private val goToEvents = Turbine() + private val goToEvents = Turbine() private val resetRootEvents = Turbine() private val popEvents = Turbine() - override fun goTo(screen: Screen) { - delegate.goTo(screen) - goToEvents.add(screen) + override fun goTo(screen: Screen): Boolean { + val success = delegate.goTo(screen) + goToEvents.add(GoToEvent(screen, success)) + return success } override fun pop(result: PopResult?): Screen? { @@ -78,10 +79,13 @@ public class FakeNavigator internal constructor(private val delegate: Navigator) * * For non-coroutines users only. */ - public fun takeNextScreen(): Screen = goToEvents.takeItem() + public fun takeNextScreen(): Screen = goToEvents.takeItem().assertSuccessfulScreen() /** Awaits the next [Screen] that was navigated to or throws if no screens were navigated to. */ - public suspend fun awaitNextScreen(): Screen = goToEvents.awaitItem() + public suspend fun awaitNextScreen(): Screen = goToEvents.awaitItem().assertSuccessfulScreen() + + /** Awaits the next navigation [goTo] or throws if no goTo are performed. */ + public suspend fun awaitNextGoTo(): GoToEvent = goToEvents.awaitItem() /** Awaits the next navigation [resetRoot] or throws if no resets were performed. */ public suspend fun awaitResetRoot(): ResetRootEvent = resetRootEvents.awaitItem() @@ -99,6 +103,9 @@ public class FakeNavigator internal constructor(private val delegate: Navigator) goToEvents.expectNoEvents() } + /** Represents a recorded [Navigator.goTo] event. */ + public data class GoToEvent(val screen: Screen, val success: Boolean) + /** Represents a recorded [Navigator.pop] event. */ public data class PopEvent(val poppedScreen: Screen?, val result: PopResult? = null) @@ -109,4 +116,11 @@ public class FakeNavigator internal constructor(private val delegate: Navigator) val saveState: Boolean = false, val restoreState: Boolean = false, ) + + private fun GoToEvent.assertSuccessfulScreen(): Screen { + if (!success) { + throw AssertionError("Screen navigation was not successful") + } + return screen + } } diff --git a/circuitx/android/src/main/kotlin/com/slack/circuitx/android/AndroidScreenAwareNavigator.kt b/circuitx/android/src/main/kotlin/com/slack/circuitx/android/AndroidScreenAwareNavigator.kt index 413cb74e4..b7236443d 100644 --- a/circuitx/android/src/main/kotlin/com/slack/circuitx/android/AndroidScreenAwareNavigator.kt +++ b/circuitx/android/src/main/kotlin/com/slack/circuitx/android/AndroidScreenAwareNavigator.kt @@ -35,8 +35,9 @@ public interface AndroidScreen : Screen */ @Parcelize public data class IntentScreen(val intent: Intent, val options: Bundle? = null) : AndroidScreen { - public fun startWith(context: Context) { + public fun startWith(context: Context): Boolean { context.startActivity(intent, options) + return true } } @@ -45,8 +46,8 @@ private class AndroidScreenAwareNavigator( private val delegate: Navigator, private val starter: AndroidScreenStarter, ) : Navigator by delegate { - override fun goTo(screen: Screen) { - when (screen) { + override fun goTo(screen: Screen): Boolean { + return when (screen) { is AndroidScreen -> starter.start(screen) else -> delegate.goTo(screen) } @@ -61,7 +62,7 @@ private class AndroidScreenAwareNavigator( */ public fun interface AndroidScreenStarter { /** Starts the given [screen]. */ - public fun start(screen: AndroidScreen) + public fun start(screen: AndroidScreen): Boolean } /** @@ -78,6 +79,7 @@ public fun rememberAndroidScreenAwareNavigator(delegate: Navigator, context: Con AndroidScreenStarter { screen -> when (screen) { is IntentScreen -> screen.startWith(context) + else -> false } } } diff --git a/circuitx/effects/src/commonMain/kotlin/com/slack/circuitx/effects/ImpressionEffect.kt b/circuitx/effects/src/commonMain/kotlin/com/slack/circuitx/effects/ImpressionEffect.kt index dd82adf11..660cb7e51 100644 --- a/circuitx/effects/src/commonMain/kotlin/com/slack/circuitx/effects/ImpressionEffect.kt +++ b/circuitx/effects/src/commonMain/kotlin/com/slack/circuitx/effects/ImpressionEffect.kt @@ -72,9 +72,9 @@ public fun rememberImpressionNavigator( } private class OnNavEventNavigator(val delegate: Navigator, val onNavEvent: () -> Unit) : Navigator { - override fun goTo(screen: Screen) { + override fun goTo(screen: Screen): Boolean { onNavEvent() - delegate.goTo(screen) + return delegate.goTo(screen) } override fun pop(result: PopResult?): Screen? { diff --git a/circuitx/overlays/src/commonMain/kotlin/com/slack/circuitx/overlays/FullScreenOverlay.kt b/circuitx/overlays/src/commonMain/kotlin/com/slack/circuitx/overlays/FullScreenOverlay.kt index 59e56a9a3..08da881f1 100644 --- a/circuitx/overlays/src/commonMain/kotlin/com/slack/circuitx/overlays/FullScreenOverlay.kt +++ b/circuitx/overlays/src/commonMain/kotlin/com/slack/circuitx/overlays/FullScreenOverlay.kt @@ -67,7 +67,7 @@ internal class DispatchingOverlayNavigator( private val overlayNavigator: OverlayNavigator, private val onPop: () -> Unit, ) : Navigator { - override fun goTo(screen: Screen) { + override fun goTo(screen: Screen): Boolean { error("goTo() is not supported in full screen overlays!") } diff --git a/samples/star/src/androidMain/kotlin/com/slack/circuit/star/MainActivity.kt b/samples/star/src/androidMain/kotlin/com/slack/circuit/star/MainActivity.kt index 9fdfcd9c0..3bb83d3e6 100644 --- a/samples/star/src/androidMain/kotlin/com/slack/circuit/star/MainActivity.kt +++ b/samples/star/src/androidMain/kotlin/com/slack/circuit/star/MainActivity.kt @@ -95,7 +95,7 @@ class MainActivity @Inject constructor(private val circuit: Circuit) : AppCompat else -> error("Unknown AndroidScreen: $screen") } - private fun goTo(screen: OpenUrlScreen) { + private fun goTo(screen: OpenUrlScreen): Boolean { val scheme = CustomTabColorSchemeParams.Builder().setToolbarColor(0x000000).build() CustomTabsIntent.Builder() .setColorSchemeParams(COLOR_SCHEME_LIGHT, scheme) @@ -103,5 +103,6 @@ class MainActivity @Inject constructor(private val circuit: Circuit) : AppCompat .setShowTitle(true) .build() .launchUrl(this, Uri.parse(screen.url)) + return true } } diff --git a/samples/star/src/jvmMain/kotlin/com/slack/circuit/star/main.kt b/samples/star/src/jvmMain/kotlin/com/slack/circuit/star/main.kt index 02eb01787..ab385e271 100644 --- a/samples/star/src/jvmMain/kotlin/com/slack/circuit/star/main.kt +++ b/samples/star/src/jvmMain/kotlin/com/slack/circuit/star/main.kt @@ -45,8 +45,8 @@ fun main() { val navigator = remember(circuitNavigator) { object : Navigator by circuitNavigator { - override fun goTo(screen: Screen) { - when (screen) { + override fun goTo(screen: Screen): Boolean { + return when (screen) { is OpenUrlScreen -> openUrl(screen.url) else -> circuitNavigator.goTo(screen) } @@ -103,7 +103,8 @@ fun main() { } } -private fun openUrl(url: String) { +private fun openUrl(url: String): Boolean { val desktop = Desktop.getDesktop() desktop.browse(URI.create(url)) + return true }