Skip to content
This repository has been archived by the owner on Apr 21, 2020. It is now read-only.

Fix use of browser history #39

Merged
merged 2 commits into from
Feb 5, 2017
Merged

Fix use of browser history #39

merged 2 commits into from
Feb 5, 2017

Conversation

rgrempel
Copy link
Contributor

This will fix #38.

One problem was the a [href "#"] [onClick ...] ... the href essentially contradicts what the onClick wants to do. One would either have to use the correct href (and have no onClick), or (better) have no href and just rely on the onClick.

I'm presuming the purpose of the href "#" is to ensure that the cursor changes to a pointer, so I put that in manually.

I suppose we do lose the ":visited" styling this way (which we could get by using the correct href) -- in cases where distinct styling for ":visited" was critical, one could use the correct href and leave off the onClick.

The other issue relates to how to handle "default" URLs (that is, what page do we want to show for an empty hash). I've made some changes here that seem to improve that. I should work on this a bit and see if I can make it nicer in elm-route-url -- there is some interaction here between what elm-lang/navigation does and and what elm-route-url does and what evancz/url-parser does, and I'm not absolutely sure what the best approach is.

@rgrempel
Copy link
Contributor Author

This is ready for review, @amitaibu

@amitaibu
Copy link
Member

Maybe we can try to use href="javascript:;"?

@rgrempel
Copy link
Contributor Author

rgrempel commented Feb 4, 2017

But in that case, I don't understand what the purpose of having an href is? You wouldn't get any useful :visited styling from it. If the purpose is to get a cursor: pointer, then it actually seems better to me to do that directly, rather than in an obscure manner.

@amitaibu amitaibu merged commit 22f5604 into master Feb 5, 2017
@amitaibu
Copy link
Member

amitaibu commented Feb 5, 2017

Great, thanks.

@amitaibu amitaibu deleted the 38-browser-history branch February 5, 2017 06:03
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.

Browser history doesn't affect the app
2 participants