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

Fix historyState bug with tg-remote #97

Closed
wants to merge 7 commits into from
Closed

Conversation

qq99
Copy link
Contributor

@qq99 qq99 commented Jul 16, 2015

Fixes #96

If you were to call Turbolinks.loadPage directly (bypassing Turbolinks.visit), rememberReferer would not be called. tg-remote does exactly this via Page.refresh and supplying an xhr in page.coffee.

This PR duplicates a call to rememberReferer, but should have no effect in the Turbolinks.visit case since it's just performing the same thing twice. It's feels a little unfortunate that it had to be duplicated

I think that this is a proper fix because:

  • if you navigate from page A > B > C without this patch, a tg-remote that didn't result in a redirect would have B as the referer, and re-push C onto the stack to result in A > B > C > C
  • a partial replace should probably care about its referer much in the same way that a regular navigation cares about its referer

I've tested it out on my own project with the back & forward buttons in these scenarios:

  • regular navigations
  • tg-remote with only an action (aka xhr but no onlyKeys)
  • tg-remote with an action and refresh-on-success (xhr + onlyKeys)

Also good for testing is the example app (added a new button + test for this kind of behaviour)

Don't merge: still need to fix:

  • tg-remote GET where you want to replace parts of the page and update the URL in history
  • tg-remote GET where you want to replace parts of the page and keep URL as it was (no history altering)

cc @Shopify/tnt

@qq99
Copy link
Contributor Author

qq99 commented Sep 23, 2016

Closing this, too old, forgot what I was doing with it exactly.
Lives on in #156

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

Successfully merging this pull request may close these issues.

rememberReferer does not always seem to be performing correctly
1 participant