turbolinks / turbolinks-ios

Native iOS adapter for building hybrid apps with Turbolinks 5
MIT License
881 stars 92 forks source link

WebView Content scrolls under navigation bar when going back #94

Closed michiels closed 6 years ago

michiels commented 7 years ago

I have a problem with my Turbolinks iOS app that my webview content scrolls under the navigation bar when my app goes back to that page. The app consists of two screens. The first where you choose a year, and then that links to a page where some content is displayed. When you go back, the year selector screen suddenly scrolls under the navigation bar.

Here are three screenshots to illustrate the problem:

Initial position:

img_2844

After tapping a year (new VisitableViewController on navstack):

img_2845

Then tapping back via the navigation bar:

img_2846

This seems similar to https://github.com/turbolinks/turbolinks-ios/issues/46. Setting the following does actually help, but is not the behavior I want.

    override func viewWillAppear(animated: Bool) {
        super.viewWillAppear(animated)
        self.edgesForExtendedLayout = .None
    }

Since this seem like a pretty obvious think that happens (also in Basecamp app for example), what am I doing wrong or is this a bug that has been recently introduced? It seems that Turbolinks scrolls the webview to the wrong position after going back.

samoli commented 7 years ago

I'm also having this problem. I think it's because of the change in 65657ae9fd03850a2fdcceee153cf63970fea132

When activateWebView is called after pressing back, it looks like the insets top & bottom are frequently 0. Switching back to the old behavior, or removing the line updateContentInsets() from activateWebView seems to fix it.

cc @dginsburg

zachwaugh commented 7 years ago

Are you still experiencing this? We're not having this issue in our app.

henrik commented 7 years ago

We're seeing a possibly related issue that we were able to reproduce in the demo app (see below). Simulated iPhone 8 Plus, iOS 11.

What we're seeing is this:

Here's a gif:

turbug

To reproduce, we made the pages longer so we could scroll them. Here's that diff:

diff --git a/TurbolinksDemo/server/lib/turbolinks_demo/views/index.erb b/TurbolinksDemo/server/lib/turbolinks_demo/views/index.erb
index 99f4e7d..bc7e14e 100644
--- a/TurbolinksDemo/server/lib/turbolinks_demo/views/index.erb
+++ b/TurbolinksDemo/server/lib/turbolinks_demo/views/index.erb
@@ -9,3 +9,7 @@
   <li><a href="https://github.com/turbolinks/turbolinks" target="_blank">Follow an external link</a> to open it in Safari.</li>
   <li><a href="#" data-behavior="post-message">Post a message from JavaScript</a> to see how it can be handled natively.</li>
 </ul>
+
+<p>Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
+<p>Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
+<p>Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
diff --git a/TurbolinksDemo/server/lib/turbolinks_demo/views/one.erb b/TurbolinksDemo/server/lib/turbolinks_demo/views/one.erb
index aa346ff..f3406ff 100644
--- a/TurbolinksDemo/server/lib/turbolinks_demo/views/one.erb
+++ b/TurbolinksDemo/server/lib/turbolinks_demo/views/one.erb
@@ -7,3 +7,7 @@
 <p><strong>or</strong></p>

 <p><a href="/two" data-turbolinks-action="replace">Here's a link to another page</a> that does a <em>replace</em> instead of an <em>advance</em>
+
+<p>Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
+<p>Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
+<p>Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
michiels commented 6 years ago

Hi @zachwaugh

Yes. I'm still experiencing this. Just upgraded to 2.0.0 Pod and re-tried with the same app as my original issue.

calleluks commented 6 years ago

I have investigated this further and found that:

I wonder if letting the WKWebView be the first subview of the view controller's content view rather than wrapping it in a VisitableView would let us rely on the automatic adjustment of scroll view insets?

@zachwaugh, how did you solve this for the iOS 11 version of the Basecamp app?

zachwaugh commented 6 years ago

Thanks for the investigation @calleerlandsson! A while back we switched to opaque nav bars in Basecamp due to some design changes, which is probably why it hasn't affected us.

There are new APIs in iOS 11 for handling insets, such has UIScrollView.contentInsetAdjustmentBehavior ( https://developer.apple.com/documentation/uikit/uiscrollview/2902261-contentinsetadjustmentbehavior), as well as the other safe area inset APIs. I'm planning on refactoring how we handle all that in Turbolinks and see if we can't do a better job there, but I don't have immediate time to do that.

zachwaugh commented 6 years ago

I did some investigation today and believe I fixed the main issues mentioned in this PR - https://github.com/turbolinks/turbolinks-ios/pull/116. Can anyone try that and see if they still any issues?

michiels commented 6 years ago

@zachwaugh Awesome. That was some rapid " but I don't have immediate time to do that." ;-) I'll see how I can get the PR branch in my Podfile (never did it before) and test it.

michiels commented 6 years ago

@zachwaugh / all

I can confirm this works with Pod built with #116 branch. In the following GIF you do not see the scroll position "jumping" when pressing back:

2017-11-03 21 13 05

henrik commented 6 years ago

Tried merging this and the swift-4 branch, and then used that in our Podfile, but it seems our issues remain :/ We'll dig further later in the week and post an update.

zachwaugh commented 6 years ago

@henrik hmm, you should be able to just use the fix-scrolling branch as it's based off the swift-4 branch. Let me know what you find out, but that branch fixed it in the cases I could find.

henrik commented 6 years ago

Hey @zachwaugh. We've dug further and it turns out the issue in the GIF above was fixed (thank you so much!), but another issue remains, that we haven't yet mentioned.

We have implemented a "tap the current tab bar item again to scroll to top" feature, but it doesn't account for the navigation bar. Here's another GIF of us first tapping the tab bar item again (scrolls wrong), then the clock (scrolls correctly).

scrollbug

This is the code we use to scroll:

session.webView.scrollView.setContentOffset(CGPoint(x: 0, y: 0), animated: true)

Any ideas? We'll keep digging.

zachwaugh commented 6 years ago

You'll need to take into account the content inset of the scroll view. That's true for scroll views in general (not just related to Turbolinks), we use an extension on UIScrollView for doing a similar thing in our app:

extension UIScrollView {
    func scrollToTop() {
        setContentOffset(CGPoint(x: 0, y: -contentInset.top), animated: true)
    }
}
zachwaugh commented 6 years ago

And thanks for confirming the fix! Merged this and swift-4 branch into master and tagged a new v3.0.0 release.