Open lazaronixon opened 11 months ago
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!
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(_:)
.
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.
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
.
Interesting. Any chance you can reproduce this bug in the demo app for turbo-ios? If so we can rule out Turbo Navigator.
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.
@lazaronixon, if you apply this patch to turbo-ios, does the same issue occur?
https://github.com/hotwired/turbo-ios/pull/160#event-10968726377
No, not yet, it's not blinking anymore, but it still breaks the loader.
https://github.com/joemasilotti/TurboNavigator/assets/2651240/15d659bb-8e6b-45b4-98b4-39fc9fafa48b
Commenting to remind myself that this issue needs to be fixed before Turbo Navigator is officially upstreamed.
When I submit a form and use
redirect_to
instead ofrefresh_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.https://github.com/joemasilotti/TurboNavigator/assets/2651240/e747ddb7-7b22-454b-a294-89010b9e87c3