Persist visit options if a response is present #155
Conversation
| return options | ||
| } else if let unhandledVisitOptions { | ||
| /// Next visit is happening. Use the previous visit options. | ||
| self.unhandledVisitOptions = nil |
There was a problem hiding this comment.
Make sure URLs match between visit options
There was a problem hiding this comment.
Not exactly. Would we keep the visit options around indefinitely until the next visit or should the next visit use them? If the former, URL matching makes sense. Otherwise, this implementation works for our purposes.
What do you think? Sorry to say I'm still trying to understand when to expect a responseHTML and I also don't think Android is doing it 🤔.
|
@olivaresf is this work related to #136 (comment)? What kind of community support could help push this forward? |
|
Coincidentally, my plan is to test this PR thoroughly sometime next week. Let me test this and #160 as I believe we're both sort of circling around the same issue and then I'll come back to you. |
|
Hey @olivaresf, we'd love to get this wrapped up as it also fixes #186. Is all that's left adding tests and addressing the comment? I'm no Android expert, but @jayohms called out that the Android library handles this in a separate view model to keep track of the visit options in between requests. It's then used here. I wonder if there's an approach that might work better on iOS following that model? If you've already explored that then disregard, just wanted to explore a closer implementation between the two libraries. |
|
Thanks for your patience! Still trying to come back to this, but I've had other priorities.
Yep. If you can help me with that, I'd appreciate it. If not, I'll make some time tomorrow to test this out. |
…s-response * origin/main: Fix when snapshot cache is cleared (#185) Stale content (#184) Note feature parity in CONTRIBUTING doc Provide backwards compatibility support for Turbo 7.x and Turbolinks 5 Support visit.isPageRefresh and avoid displaying the activity indicator and screenshots when the page is refreshing Support Turbo 8 same-page refreshes and include debug logging rename loadrRequest to load (#141) Add CONTRIBUTING.md (#90) Update README window.Turbo reference (#68) # Conflicts: # Source/Session/Session.swift
|
I've added a unit test for this behavior. As far as I can tell, I'm mimicking Android's behavior:
I've tested this on Basecamp and it works as I expected. Let me know if there's anything else needed. |
Persist visit options for when a response HTML is provided. I've seen this fix an issue where the flash alert would not be displayed, if provided via visit options.