Preserve url fragment on page reload

[beyang]

This fixes a bug where the URL hash disappears when a page is first loaded.

For instance, if you had a link <a href="#someanchor">link</a> on your page and clicked on it, the link would work correctly (it would scroll to #someanchor), but if you copied and pasted the URL into a new tab and navigated to the page afresh, the hash would disappear and no scrolling would occur.

See also: #510

[beyang]

closed pending comments #510

[timkindberg]

You don't have to close a pull request while waiting on comments elsewhere.

[beyang]

Thanks for the FYI @timkindberg. I'd happily make preserveHash an option on the options param; however there's some further discussion over at #510. Seems like some people want the hash as part of the state, and my PR doesn't do that. Want to comment on that thread? What's the protocol for decisions like this?

[timkindberg]

I'll probably comment over there, but I'm now curious, when would you NOT want to preserve a hash? I think never.

[beyang]

Well, transitionTo() is used for both state changes and "redirects" (e.g., when you navigate to a URL that matches a state, but includes extra text and the router rewrites the URL to something that matches the state exactly). In the latter case, you obviously want to preserve the hash, but in the former case, you probably don't. Otherwise, you'd be transitioning to a completely new state but still inheriting the hash from the previous one.

[nateabele]

@beyang It sounds like we may need to factor out a different method for that then.

[beyang]

@nateabele, I'm not sure I follow.

2 questions:
1 (semantics): Are these the semantics we want (i.e., the hash is not part of the stateParams)?
2 (implementation): Are you suggesting having a separate method from transitionTo with "redirect"-style semantics (i.e., preserving the hash). This would definitely be used in the registerState function, but should it also be used with the redirect portion of the transitionTo function itself (or anywhere else)? I'm new to this code, and it's not immediately clear to me what the intended semantics of transitionTo should be, so any pointers would be great :)

[nateabele]

1 (semantics): Are these the semantics we want (i.e., the hash is not part of the stateParams)?

Correct, the hash is logically separate from $stateParams. The discussion about allowing it to be a parameter is a somewhat different question, and I'm still undecided as to whether that's a good idea.

2 (implementation): Are you suggesting having a separate method from transitionTo [...]

After re-reading the discussion, I actually don't remember what I had in mind when I left my previous comment. What makes the most sense to me right now is to begin factoring out the relevant portions of transitionTo() into internal functions.

Since this issue only affects the initial page load, the initial page load just needs a slightly different code path, which can call these functions directly, rather than adding a parameter to an already very complex & highly parameterized public method.

Let me know if the above makes sense.

[beyang]

@nateabele, upon closer observation, this seems to be a bug-fix, not a feature :) The first couple of lines of transitionTo were a little too clever, and the function was not respecting the "updateLocation" semantics of the options param (https://github.com/angular-ui/ui-router/wiki/Quick-Reference#updatelocation-or-options).

On initial state registration, updateLocation is set to false for the exact purpose that it doesn't want to overwrite the URL (and the hash along with it). This PR fixes that issue and adds a test for it.

[nateabele]

@beyang Ohhhkay, I think I see now.

[U]pon closer observation, this seems to be a bug-fix, not a feature :)

Definitely agreed on that count.

[T]he function was not respecting the "updateLocation" semantics of the options param

Nope, actually, allowing options to accept a boolean was deprecated in 0.2 and subsequently removed. The docs need an update. /cc @timkindberg :-)

Rather, the correct fix would be to change the transitionTo() call in registerState(), like so:

$urlRouterProvider.when(state.url, ['$match', '$stateParams', function ($match, $stateParams) {
  if ($state.$current.navigable != state || !equalForKeys($match, $stateParams)) {
    $state.transitionTo(state, $match, { location: false });
  }
}]);

I'll let you update the patch. The test should be able to remain the same. Please don't forget to squash your commits, and follow the Angular commit message format. Thanks!

[timkindberg]

It is deprecated in 0.3 I thought, regardless, it is already on my list of assignments. Hopefully I'll get around to my list here soon. Been very busy lately.

[nateabele]

@timkindberg Don't feel bad, I'll just keep piling things onto your todo list.

[beyang]

@nateabele see the updated PR. thanks!

[nateabele]

@beyang Brilliant, looks great. Thanks for all your efforts on this.

[beyang]

thank you for making an awesome library :)

[cyberwombat]

Are there docs on how to enable this? I have the latest UI Router and named anchors do not work for me when loaded from address bar.

[nateabele]

@cyberwombat I don't think you're asking for the same thing as this PR addresses. What do you mean by named anchors?

[cyberwombat]

@nateabele - seems like the original entry to this issue describes it. Basically my nameed anchors, i.e. <a name="about"></a> work fine if I have a link on the page that looks like <a href="#about">About</a>. This causes the page to shift to location as well as update the URL. If I press reload at that point (which would be the same as a bookmark/paste as original posted mentions) then it does not find the page anymore and 404s. I have tried adding some basic regex to the router but my results were not successful. I would end up with things like http://example.com/about instead of the desired http://example.com#about or even http://example.com. Ideally I would like the hash anchor to be preserved but would be ok with it stripped away as long as things dont break. It seems OP question was about that but i am not sure the PR was - may be about the non html5 fragments.

[nateabele]

If I press reload at that point (which would be the same as a bookmark/paste as original posted mentions) then it does not find the page anymore and 404s.

This sounds like a server configuration issue, which would be unrelated to UI Router. Please see https://github.com/angular-ui/ui-router/wiki/Frequently-Asked-Questions#how-to-configure-your-server-to-work-with-html5mode