Navigator - Guard against a goTo to the current screen

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`.

backstack/src/commonMain/kotlin/com/slack/circuit/backstack/BackStack.kt

@@ -41,8 +41,9 @@ public interface BackStack<R : Record> : Iterable<R> {
    * @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<R : Record> : Iterable<R> {
    * @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

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<String, Any?>, resultKey: String?) {
-    push(Record(screen, args), resultKey)
+  public fun push(screen: Screen, args: Map<String, Any?>, 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? {

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) =

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++ }

circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/AnsweringNavigator.kt

@@ -124,9 +124,10 @@ public fun <T : PopResult> 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
       }
     }
   }

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(

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? {

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<Screen>
 
   public object NoOp : Navigator {
-    override fun goTo(screen: Screen) {}
+    override fun goTo(screen: Screen): Boolean = true
 
     override fun pop(result: PopResult?): Screen? = null
 

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

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<Screen>()
+  private val goToEvents = Turbine<GoToEvent>()
   private val resetRootEvents = Turbine<ResetRootEvent>()
   private val popEvents = Turbine<PopEvent>()
 
-  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
+  }
 }

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
         }
       }
     }

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? {

circuitx/overlays/src/commonMain/kotlin/com/slack/circuitx/overlays/FullScreenOverlay.kt

@@ -67,7 +67,7 @@ internal class DispatchingOverlayNavigator(
   private val overlayNavigator: OverlayNavigator<Unit>,
   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!")
   }
 

samples/star/src/androidMain/kotlin/com/slack/circuit/star/MainActivity.kt

@@ -95,13 +95,14 @@ 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)
       .setColorSchemeParams(COLOR_SCHEME_DARK, scheme)
       .setShowTitle(true)
       .build()
       .launchUrl(this, Uri.parse(screen.url))
+    return true
   }
 }

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
 }