Skip to content
This repository has been archived by the owner on Sep 25, 2021. It is now read-only.

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

domchristie
Copy link

@domchristie domchristie commented Jan 16, 2018

This is an attempt to address the issue with tapping on same-page anchor links as noted by @javan here: turbolinks/turbolinks#285 (comment).

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 turbolinks/turbolinks#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
turbolinks/turbolinks@652ddc9 and updated in this repo in a50049e.

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!

Do not start a visit if proposed location is an anchor on the same-page.
@domchristie
Copy link
Author

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 turbolinks/turbolinks#285

@domchristie
Copy link
Author

Friendly ping :) any thoughts on this?

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

Successfully merging this pull request may close these issues.

1 participant