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

Update the visitableURL on reload #77

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

leonvogt
Copy link
Contributor

Hi there,

Sorry for the spam lately 😅
While working on a feature I've noticed that Hotwire Native iOS handles the pull to refresh feature slightly different than Android.

We use HTML tabs on some views and a bit of JavaScript to update the URL whenever a user switches tabs.
This ensures that when the page is refreshed, the correct tab is selected.
The JavaScript code looks something like this:

tab.addEventListener('click', () => {
  const currentURL = new URL(window.location);
  currentURL.searchParams.set('view_part', tab.dataset.bsTarget)
  window.history.replaceState(history.state, "", currentURL)
})

Here's the current behavior:

Android

Android.mp4

iOS

iOS.mp4

On iOS, when a refresh is triggered, it reloads the (visitable)URL that was set at the start of the ViewController.
This PR updates iOS to behave like Android by ensuring it reloads the current URL from the webview instead.

Comment on lines +88 to +91
if let currentURL = webView.url {
visitable.visitableURL = currentURL
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this approach is maybe too naive? I'm happy to adjust if needed.

@ibrahima
Copy link

ibrahima commented Jan 30, 2025

Hi @leonvogt! Saw your comment on #48, I was trying to see if it would resolve my issue. But I am not sure how to use your fork of the library in my Xcode project, do you have any suggestions? I tried overriding the revision in Package.resolved but I don't know if it works that way. I set it to this and it didn't seem to resolve my issue but I don't know whether it's actually using that version or not.

		{
			"identity": "hotwire-native-ios",
			"kind": "remoteSourceControl",
			"location": "https://github.com/leonvogt/hotwire-native-ios",
			"state": {
				"revision": "272249c5c825338a61f6fa4dcd1429a7e1fde789",
				"version": "1.1.1"
			}
		},

(I am also building the app through fastlane as I don't have access to my Mac computer right now... so that's also a confounding factor.)

Edit: I found this solution on StackOverflow and after updating the pbxproj as well, it does use your version!. It seems that in my case, this branch fixes the issue that the URL is stuck on the old one, so pull to refresh after a turbo frame navigation does work correctly. So that should fix #48.

However, it doesn't add an entry to the navigation stack even though I have data-turbo-action="advance" set, so that's still an issue IMO. But I don't know whether that's going to require a change to a different part of the code... I guess it's a different issue from #48 then.

@leonvogt
Copy link
Contributor Author

Hey @ibrahima,
Happy to hear that you figured it out and it's working for you!

However, it doesn't add an entry to the navigation stack even though I have data-turbo-action="advance" set, so that's still an issue IMO

Yeah, I guess that's a separate issue. The native Turbo adapter would need to intercept the request and propagate it as a ordinary request. So you get a new ViewController on the stack. But I guess that would open a whole new set of problems, since your server may only response with the new Turbo Frame content and not with a whole page.
Also, you maybe don't always want that kind of behaviour. In the example of #48 with pagination, it would be kind of weird to push something on the stack, every time you click on a pagination link.

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

Successfully merging this pull request may close these issues.

2 participants