venantius / accountant

ClojureScript navigation for single-page applications, made simple.
Eclipse Public License 1.0
250 stars 39 forks source link

Navigating to the same path #40

Closed konrad-ch closed 7 years ago

konrad-ch commented 7 years ago

When i click on link defined like this [:a {:href "/page"}] and the current page is /page then the whole page refreshes. But when I'm switching between different pages then everything is fine. Is this a bug/intended design or I am doing something wrong? Anyone knows how to deal with that issue?

venantius commented 7 years ago

This is a bug.

konrad-ch commented 7 years ago

Thanks for quick reply. I've tried to fix it but can't seem to know where to look exactly... Strangely when the route has a query parameters then everything is working fine.. maybe it helps?

venantius commented 7 years ago

My guess is there's a bug somewhere here: https://github.com/venantius/accountant/blob/master/src/accountant/core.cljs#L90

konrad-ch commented 7 years ago

When i comment out this line https://github.com/venantius/accountant/blob/master/src/accountant/core.cljs#L97 its working fine. But do you think it could break something else?

venantius commented 7 years ago

Ah, that makes sense. That logic was originally to prevent a page transition if the page was the same, but what it does instead is to prevent Accountant from operating, which triggers a normal reload. I don't think there's a compelling reason for that line to be there. I'd accept a PR to delete it.

konrad-ch commented 7 years ago

Ok will do that, but one more thing- same problem occurs when using navigate!. Any hints about how to fix it? Edit: Sorry, I said "same problem" but actually page does not refreshes but nothing happens. Managed to fix that by just assigning value of token to old-route. So in fact whole logic associated with old-route can be thrown away. But again, I'm not quite sure if it is the correct solution.

smee commented 7 years ago

I think I added this line in https://github.com/venantius/accountant/pull/26. I didn't see the problem because I always have on-click listeners on each link which take care of preventing the navigation. I suggest we don't delete this line but rather split the condition into two cases: add history and prevent default or just prevent the default when the path is the same.