Redirect to the same page break pull-to-refresh

[lazaronixon]

When I submit a form and use redirect_to instead of refresh_or_redirect_to the screen is replaced but the pull-to-refresh stops working. I was expecting that the "navigation to the same screen" was able to handle that.

  def update
    if @contact.update(contact_params)
      redirect_to @contact, notice: "Contact was successfully updated."
    else
      render :edit, status: :unprocessable_entity
    end
  end

Simulator.Screen.Recording.-.iPhone.15.-.2023-10-20.at.12.41.35.mp4

[joemasilotti]

Thanks for the bug report! I can confirm this is the case and that refresh_or_redirect_to does indeed fix the issue.

But for those only using redirect_to we should probably have a solution. I've identified that this line of code in Turbo is where the divergence occurs.

// turbo-ios/Source/Session/Session.swift#248

public func visitableDidRequestRefresh(_ visitable: Visitable) {
    guard visitable === topmostVisitable else { return }

    refreshing = true
    visitable.visitableWillRefresh()
    reload()
}

The guard statement fails so the refresh reload never actually occurs. Which looks to be caused by visitableDelegate being nil when the underlying view re-appears. This chain eventually should set topmostVisitable but won't be called.

// turbo-ios/Source/Visitable/VisitableViewController.swift

open override func viewDidAppear(_ animated: Bool) {
    super.viewDidAppear(animated)
    visitableDelegate?.visitableViewDidAppear(self)
}

I'm not sure yet if that's a bug in turbo-ios or being caused by Turbo Navigator without digging in more. If you have any ideas please let me know!

[joemasilotti]

Hmm, I think I might see what's going on. It looks like viewDidAppear(_:) isn't called when a modal is dismissed. Which means topmostVisitable is never set causing visitableDelegate to be nil.

I'm not entirely sure what the fix is here, to be honest. I think turbo-ios might have to change how visitableDelegate is set outside of viewDidAppear(_:).

[joemasilotti]

Confirmed. Applying the following diff presents the modals in a full screen context. Which then calls the right callback.

diff --git a/Sources/TurboNavigator.swift b/Sources/TurboNavigator.swift
index 4e90bcc..9c41a32 100644
--- a/Sources/TurboNavigator.swift
+++ b/Sources/TurboNavigator.swift
@@ -131,6 +131,7 @@ public class TurboNavigator {
                 pushOrReplace(on: modalNavigationController, with: controller, via: proposal)
             } else {
                 modalNavigationController.setViewControllers([controller], animated: false)
+                modalNavigationController.modalPresentationStyle = .fullScreen
                 navigationController.present(modalNavigationController, animated: true)
             }
             visit(controller, on: modalSession, with: proposal.options)

But we can't default that for everyone. So still need some alternate solution.

[lazaronixon]

You know what is weird, in Hey they use redirect_to when you rename a workflow, and it works. For the other screens they use refresh_or_redirect_to, I could see it in the proxy I have on my phone proxyman.

[joemasilotti]

Interesting. Any chance you can reproduce this bug in the demo app for turbo-ios? If so we can rule out Turbo Navigator.

[lazaronixon]

No, because the demo app doesn't handle "navigating to the same view", it pushes a new view to the stack, and in this case, the refresh still works.

[joemasilotti]

@lazaronixon, if you apply this patch to turbo-ios, does the same issue occur?

hotwired/turbo-ios#160 (comment)

[lazaronixon]

No, not yet, it's not blinking anymore, but it still breaks the loader.

Simulator.Screen.Recording.-.iPhone.15.-.2023-11-21.at.00.07.30.mp4

[joemasilotti]

Commenting to remind myself that this issue needs to be fixed before Turbo Navigator is officially upstreamed.