search

venantius / accountant

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

use custom url transformer to preserve hash and query params #29

Closed jalehman closed 7 years ago

jalehman commented 7 years ago

Ran into issue #19, and the PR (#24) that was submitted last year to fix it and #23.

It appears that there were some outstanding issues with that PR, and it appears to have been abandoned. This one incorporates that patch and resolves the issues that @venantius mentioned.

To summarize, this PR fixes #19 and #23, and incorporates the patches in PR #24.

venantius commented 7 years ago

I'm not doing too much work in ClojureScript these days. I assume you've confirmed that this works as expected?

jalehman commented 7 years ago

I actually just noticed that while it correctly avoided duplication of query params and preserved the hash in back events, it did not preserve the query params in back events.

I've fixed the issue locally -- would you prefer an additional commit in this PR, or a new PR with one commit?

venantius commented 7 years ago

Feel free to add it to this PR.

jalehman commented 7 years ago

Doh. Forgot about an earlier commit that I made for the sake of development -- will issue a new PR with a single commit. Sorry about that!

venantius commented 7 years ago

you know you can just force-push to your branch and it'll update this PR, right?

jalehman commented 7 years ago

That would have been way smarter. Think I screwed things over now by rushing to a poor solution (branch recreated on GH).

Or at least I think so, since It won't allow me to reopen any more.