Preserve anchors - Fix #110 #125

Merged
merged 1 commit into from Dec 1, 2012

Projects

None yet

4 participants

@reed

I came up with a few ways to fix the problem (#110), but I think this one is the best because it takes care of two issues.

Unless I'm missing something, the purpose of the reflectRedirectedUrl function is to update the newly pushed (from reflectNewUrl) history state in the event the ajax request is redirected. However, right now it's doing it whether there was a redirect or not, which most of the time will be unnecessary. The lost anchors are a side effect of this, since it replaces the current state with the X-XHR-Current-Location value from the server (which omits anchors). As far as I can tell, my solution preserves the anchors while eliminating unnecessary replaceState calls.

@kossnocorp

👍

@kossnocorp

You can swap conditions and remove brackets:

unless document.location.pathname + document.location.search is location = xhr.getResponseHeader 'X-XHR-Current-Location'

But in my opinion original location = xhr.getResponseHeader('X-XHR-Current-Location') is more much readable 😄.

@reed

It's just personal preference. I like the way I did it, but I'd have no issue with changing it. I'll leave it up to @dhh to decide how he wants it before adding another commit.

@kossnocorp

I see. Also since Turbolinks is completly bracketless it's pointless to try add brackets to improve readability. I'm just point you to opportunity remove two more brackets 😸.

@yasuoza

👍

@dhh dhh merged commit 5f156b3 into turbolinks:master Dec 1, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment