joemasilotti / TurboNavigator

A drop-in class for Turbo Native apps to handle common navigation flows.
MIT License
208 stars 13 forks source link

Server-side redirect from "modal" route to "basic" route breaks navigation #79

Open seanpdoyle opened 8 months ago

seanpdoyle commented 8 months ago

According to the path configuration file, the client handles GET /navigations requests with the "basic" stack, and GET /modal/new requests with the "modal" stack.

This commit modifies the modals#new action to redirect to navigations#show when the ?redirect query parameter is present.

This GET to GET redirect causes issues on the client, since at request-time, the client thinks its handling a "modal" navigation, but at response-time, the client is served a route that it'd handle as a "basic"` navigation.

The resulting behavior is a bug through the following sequence:

  1. present the "modal" stack by animating up from the bottom of the screen
  2. render an empty screen
  3. dismiss the "modal" stack
  4. pop the entirety of the "basic" stack
  5. replace the root with the navigations#show response

Once the navigation completes, the old root screen is entirely unavailable. There is no navigation "Back" button, gestures don't pop off the stack, and pull to refresh re-requests the navigations#show page.

https://github.com/joemasilotti/TurboNavigator/assets/2575027/3aa14bb1-3c89-4a43-ad5c-9bcb4aa8e1da

The reproduction code is available at https://github.com/seanpdoyle/TurboNavigator/tree/redirect-get-modal-to-get-basic

diff --git a/Demo/Server/app/controllers/modals_controller.rb b/Demo/Server/app/controllers/modals_controller.rb
index 1bf606f..839cf19 100644
--- a/Demo/Server/app/controllers/modals_controller.rb
+++ b/Demo/Server/app/controllers/modals_controller.rb
@@ -1,5 +1,8 @@
 class ModalsController < ApplicationController
   def new
+    if params[:redirect]
+      redirect_to navigation_path
+    end
   end

   def show
diff --git a/Demo/Server/app/views/dashboards/show.html.erb b/Demo/Server/app/views/dashboards/show.html.erb
index 1192c87..ddb3271 100644
--- a/Demo/Server/app/views/dashboards/show.html.erb
+++ b/Demo/Server/app/views/dashboards/show.html.erb
@@ -19,6 +19,12 @@
     icon: "bi-arrow-up",
     name: "Modal navigation",
     description: "Present a modal screen." %>
+
+  <%= render "navigations/item",
+    path: new_modal_path(redirect: true),
+    icon: "bi-arrow-up",
+    name: "Redirect from Modal to Basic",
+    description: "Redirect from a modal screen to a screen on the stack." %>
 </div>

 <div class="d-flex gap-2 justify-content-sm-center mt-5">
seanpdoyle commented 8 months ago

In comparison, the @hotwired/turbo-android/demo gracefully handles the redirect:

Screen_recording_20240210_120001.webm

The following diff is the only necessary change to exhibit the desired behavior:

@@ -39,7 +39,8 @@ class MainSessionNavHostFragment : TurboSessionNavHostFragment() {

     override val pathConfigurationLocation: TurboPathConfiguration.Location
         get() = TurboPathConfiguration.Location(
-            assetFilePath = "json/configuration.json"
+            assetFilePath = "json/configuration.json",
+            remoteFileUrl = "http://10.0.2.2/configurations/ios_v1.json"
         )

     override fun onSessionCreated() {