olivernn / davis.js

RESTful degradable JavaScript routing using pushState
http://davisjs.com
532 stars 58 forks source link

Refactoring of Davis.hashRouting extension #7

Closed jbaudanza closed 13 years ago

jbaudanza commented 13 years ago

I recently backported all of my custom hash location handling into the Davis.hashRouting extension.

https://github.com/jbaudanza/davis.js/blob/86e75bd1cd5b1e2219c5e3989e1a0bcea4035f1d/src/extensions/davis.hashRouting.js

I added the following functionality:

I've also included some tests. Let me know what you think.

olivernn commented 13 years ago

All looks good to me, just a couple of points.

I think by default there should be no prefix, mostly because, and I could be wrong here, the bang part of the hash bang is something specific to getting a site indexed by google and is not necessary for hash based routing.

I'm not sure about automatically converting between hash and non hash location schemes. My initial thoughts were that this is something that could be quite app specific and should be handled by app developers themselves.

Having said that I don't have any experience in building an app that does fallback to hash based routing where pushState is not available, so would be interested to hear any issues that you encountered and if there were any app specific things that you needed to cover.

Last thing, the poll interval is set at 500ms, perhaps this should be configurable, I think 500ms might introduce a noticeable delay, generally anything that takes longer than 100ms is perceivable by the user.

If you put this in a pull request I'd be happy to merge.

olivernn commented 13 years ago

This has been merged and is available in the latest release