turbolinks / turbolinks-ios

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

Fix behavior when tapping on a same-page anchor link #126

Open domchristie opened 6 years ago

domchristie commented 6 years ago

This is an attempt to address the issue with tapping on same-page anchor links as noted by @javan here: https://github.com/turbolinks/turbolinks/pull/285#issuecomment-356318342.

The Problem

When tapping on a link to a same-page anchor, it is expected that the page should scroll to the anchor. In the browser, the hash location and scroll position is added to the history so the that visitor can jump back to where they came from. This behaviour is implemented in https://github.com/turbolinks/turbolinks/pull/285 but is still broken on turbolinks-ios/android.

In native Turbolinks apps, new visits are typically pushed onto a navigation stack. However in the case of same-page anchors, this doesn’t really make sense. Also, the concept of jumping back to a previous scroll position does not make sense, because “Back” usually pops the entire screen from the stack.

The Solution

This PR detects whether the proposed visit is to an anchor on the same page, and rather than start a new visit, it just scrolls to the anchor.

Notes on the implementation

This approach should be relatively easy to port to the turbolinks-android bridge, but there are a few things which could be improved, and I thought I’d get some feedback before proceeding.

~Reducing repetition of locationIsSamePageAnchor~

~The locationIsSamePageAnchor function could probably be moved to Turbolinks.Controller. This could then be used in turbolinks, turbolinks-ios, and turbolinks-android.~

UPDATE: locationIsSamePageAnchor has been moved to the controller as of https://github.com/turbolinks/turbolinks/pull/285/commits/652ddc99caaf605e27db14d580cfe4fd01e2dda3 and updated in this repo in https://github.com/turbolinks/turbolinks-ios/pull/126/commits/a50049e0af349584b470780df445ff895b4ec2ff.

visit.location goes out of sync

Bypassing the visit for same-page anchors means that the current visit’s location is not accurate. The actual location string will be something like http://example.com#anchor, whereas the visit’s location string will still be http://example.com. This could be fixed by adding a setter method to the visit e.g.

class Turbolinks.Visit
  constructor: (@controller, location, @action) ->
    …
    @setLocation(location)
    …

  setLocation: (location) ->
    @location = Turbolinks.Location.wrap(location)

and then be called from visitProposedToLocationWithAction:

visitProposedToLocationWithAction: function(location, action) {
    if (this.locationIsSamePageAnchor(location)) {
        this.controller.scrollToAnchor(location.anchor)
        this.currentVisit.setLocation(location)
    } else {
        this.postMessage("visitProposed", { location: location.absoluteURL, action: action })
    }
}

Bypassing the visit process also means that in-flight visits won’t be cancelled. I’m not sure if this is a big deal, or if there might be a way round it.


Also, thanks to @samoli for helping on this!

domchristie commented 6 years ago

I’ve added an example to the TurbolinksDemo project. The “Back to top” link won’t behave as expected unless version of Turbolinks is updated with a build from https://github.com/turbolinks/turbolinks/pull/285

domchristie commented 6 years ago

Friendly ping :) any thoughts on this?