Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
seanpdoyle opened this issue Feb 10, 2024 · 1 comment

Comments

@seanpdoyle
Copy link
Contributor

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.

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-02-09.at.19.15.22.mp4

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
Copy link
Contributor Author

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() {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant