Merge pull request #107 from hotwired/modal-navigation-action

Improved `default` context visit behavior when dismissing the `modal` context

Author: jayohms

navigation-fragments/src/main/java/dev/hotwire/navigation/destinations/HotwireDestination.kt

@@ -137,13 +137,23 @@ interface HotwireDestination : BridgeDestination {
         newPathProperties: PathConfigurationProperties,
         action: VisitAction
     ): NavOptions {
-        val modal = newPathProperties.context == PresentationContext.MODAL
-        val clearAll = newPathProperties.presentation == Presentation.CLEAR_ALL
-        val animate = action != VisitAction.REPLACE &&
+        val navigatingToModalContext = pathProperties.context == PresentationContext.DEFAULT &&
+                newPathProperties.context == PresentationContext.MODAL
+
+        val navigatingWithinModalContext = pathProperties.context == PresentationContext.MODAL &&
+                newPathProperties.context == PresentationContext.MODAL
+
+        val dismissingModalContext = pathProperties.context == PresentationContext.MODAL &&
+                newPathProperties.context == PresentationContext.DEFAULT
+
+        val animate = (navigatingToModalContext || dismissingModalContext) ||
+                (action != VisitAction.REPLACE &&
                 newPathProperties.presentation != Presentation.REPLACE &&
-                newPathProperties.presentation != Presentation.REPLACE_ROOT
+                newPathProperties.presentation != Presentation.REPLACE_ROOT)
+
+        val clearAll = newPathProperties.presentation == Presentation.CLEAR_ALL
 
-        return if (modal) {
+        return if (navigatingToModalContext || navigatingWithinModalContext || dismissingModalContext) {
             navOptions {
                 anim {
                     enter = if (animate) R.anim.enter_slide_in_bottom else 0

navigation-fragments/src/main/java/dev/hotwire/navigation/navigator/NavigatorRule.kt

@@ -82,10 +82,12 @@ internal class NavigatorRule(
         val locationIsCurrent = locationsAreSame(newLocation, currentLocation)
         val locationIsPrevious = locationsAreSame(newLocation, previousLocation)
         val replace = newVisitOptions.action == VisitAction.REPLACE
+        val dismissModalContext = currentPresentationContext == PresentationContext.MODAL &&
+                newPresentationContext == PresentationContext.DEFAULT
 
         return when {
             locationIsCurrent && isAtStartDestination -> Presentation.REPLACE_ROOT
-            locationIsPrevious -> Presentation.POP
+            locationIsPrevious || dismissModalContext -> Presentation.POP
             locationIsCurrent || replace -> Presentation.REPLACE
             else -> Presentation.PUSH
         }
@@ -134,9 +136,16 @@ internal class NavigatorRule(
             return null
         }
 
+        // When dismissing a modal to navigate in the "default" context, override the visit
+        // action. A visit proposal with an original "replace" action is respected by first
+        // dismissing the modal. But we don't want to also "replace" the underlying "default"
+        // context screen (unless the urls are the same).
+        val locationIsPrevious = locationsAreSame(newLocation, previousLocation)
+        val action = if (locationIsPrevious) VisitAction.REPLACE else VisitAction.ADVANCE
+
         return SessionModalResult(
             location = newLocation,
-            options = newVisitOptions,
+            options = newVisitOptions.copy(action = action),
             bundle = newBundle,
             shouldNavigate = newProperties.presentation != Presentation.NONE
         )

navigation-fragments/src/test/kotlin/dev/hotwire/navigation/navigator/NavigatorRuleTest.kt

@@ -17,6 +17,7 @@ import dev.hotwire.core.turbo.config.PathConfiguration.Location
 import dev.hotwire.core.turbo.nav.Presentation
 import dev.hotwire.core.turbo.nav.PresentationContext
 import dev.hotwire.core.turbo.nav.QueryStringPresentation
+import dev.hotwire.core.turbo.visit.VisitAction
 import dev.hotwire.core.turbo.visit.VisitOptions
 import org.assertj.core.api.Assertions.assertThat
 import org.assertj.core.api.Assertions.assertThatThrownBy
@@ -37,6 +38,7 @@ class NavigatorRuleTest {
     private val homeUrl = "https://hotwired.dev/home"
     private val newHomeUrl = "https://hotwired.dev/new-home"
     private val featureUrl = "https://hotwired.dev/feature"
+    private val featureTwoUrl = "https://hotwired.dev/feature-two"
     private val newUrl = "https://hotwired.dev/feature/new"
     private val editUrl = "https://hotwired.dev/feature/edit"
     private val recedeUrl = "https://hotwired.dev/custom/recede"
@@ -203,6 +205,56 @@ class NavigatorRuleTest {
         assertThat(rule.newNavOptions).isEqualTo(navOptions)
     }
 
+    @Test
+    fun `navigating back to feature from modal context with replace action maintains replace`() {
+        controller.navigate(webDestinationId, locationArgs(featureUrl))
+        controller.navigate(webModalDestinationId, locationArgs(newUrl))
+        val rule = getNavigatorRule(featureUrl, VisitOptions(action = VisitAction.REPLACE))
+
+        // Current destination
+        assertThat(rule.previousLocation).isEqualTo(featureUrl)
+        assertThat(rule.currentLocation).isEqualTo(newUrl)
+        assertThat(rule.currentPresentationContext).isEqualTo(PresentationContext.MODAL)
+        assertThat(rule.isAtStartDestination).isFalse()
+
+        // New destination
+        assertThat(rule.newLocation).isEqualTo(featureUrl)
+        assertThat(rule.newPresentationContext).isEqualTo(PresentationContext.DEFAULT)
+        assertThat(rule.newPresentation).isEqualTo(Presentation.POP)
+        assertThat(rule.newQueryStringPresentation).isEqualTo(QueryStringPresentation.REPLACE)
+        assertThat(rule.newNavigationMode).isEqualTo(NavigatorMode.DISMISS_MODAL)
+        assertThat(rule.newModalResult?.location).isEqualTo(featureUrl)
+        assertThat(rule.newModalResult?.options?.action).isEqualTo(VisitAction.REPLACE)
+        assertThat(rule.newDestinationUri).isEqualTo(webUri)
+        assertThat(rule.newDestination).isNotNull()
+        assertThat(rule.newNavOptions).isEqualTo(navOptions)
+    }
+
+    @Test
+    fun `navigating to new feature from modal context with replace action changes to advance`() {
+        controller.navigate(webDestinationId, locationArgs(featureUrl))
+        controller.navigate(webModalDestinationId, locationArgs(newUrl))
+        val rule = getNavigatorRule(featureTwoUrl, VisitOptions(action = VisitAction.REPLACE))
+
+        // Current destination
+        assertThat(rule.previousLocation).isEqualTo(featureUrl)
+        assertThat(rule.currentLocation).isEqualTo(newUrl)
+        assertThat(rule.currentPresentationContext).isEqualTo(PresentationContext.MODAL)
+        assertThat(rule.isAtStartDestination).isFalse()
+
+        // New destination
+        assertThat(rule.newLocation).isEqualTo(featureTwoUrl)
+        assertThat(rule.newPresentationContext).isEqualTo(PresentationContext.DEFAULT)
+        assertThat(rule.newPresentation).isEqualTo(Presentation.POP)
+        assertThat(rule.newQueryStringPresentation).isEqualTo(QueryStringPresentation.REPLACE)
+        assertThat(rule.newNavigationMode).isEqualTo(NavigatorMode.DISMISS_MODAL)
+        assertThat(rule.newModalResult?.location).isEqualTo(featureTwoUrl)
+        assertThat(rule.newModalResult?.options?.action).isEqualTo(VisitAction.ADVANCE)
+        assertThat(rule.newDestinationUri).isEqualTo(webUri)
+        assertThat(rule.newDestination).isNotNull()
+        assertThat(rule.newNavOptions).isEqualTo(navOptions)
+    }
+
     @Test
     fun `navigate from modal to same modal`() {
         controller.navigate(webModalDestinationId, locationArgs(newUrl))