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

Routing on page load #28

Closed
wants to merge 2 commits into from
Closed

Routing on page load #28

wants to merge 2 commits into from

Conversation

nikgraf
Copy link
Contributor

@nikgraf nikgraf commented Apr 29, 2013

A proposal to fix the issue of Browsers behave differently on page load. If you start listening with a router in Chrome this will fire a handler on page load. In Firefox it doesn't.

Browsers tend to handle the popstate event differently on page load. Chrome and Safari always emit a popstate event on page load, but Firefox doesn't. see https://developer.mozilla.org/en-US/docs/DOM/window.onpopstate

To create a consistent behavior I copied the concept of Backbone's routing. Backbone routes by default when you starting the history. see https://github.com/documentcloud/backbone/blob/master/backbone.js#L1417
We also could implement a silent option to prevent routing on app start.

Further Changes:

  • Added a couple more tests.
  • Listen returns a bool which indicates if an UrlPattern matched on page load.

@pavelgj
Copy link
Contributor

pavelgj commented Apr 30, 2013

I totally suppor the idea, it's an important thing to fix. If I read correctly, you made the router stateful so that it remembers the last called handler and doesn't trigger the handler twice for the same URL, and in the listen() you invoke the handle() explicitly, so in some browsers it'd be called twice but that would be OK with the first change.

I'm probably fine with this approach, except I probably wouldn't want to merge it into the master branch because we're working on a new version and it would be much easier to fix this there as most of this is already done. Can you please open an issue instead?

@nikgraf
Copy link
Contributor Author

nikgraf commented May 2, 2013

For sure. I opened an issue. #32
If this new branch is getting more stable, i wouldn't mind implementing it by myself.

cheers
Nik

@nikgraf nikgraf closed this May 2, 2013
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.

2 participants