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

Problems while using accountant #65

Open
pavel-klavik opened this issue Dec 14, 2019 · 4 comments
Open

Problems while using accountant #65

pavel-klavik opened this issue Dec 14, 2019 · 4 comments

Comments

@pavel-klavik
Copy link

I am using it together with bidi. In the main function, I am initializing it like this:

(accountant/configure-navigation!
    {:nav-handler       #(rf/dispatch [:app-state/changed-route %])
     :path-exists?      #(contains? #{:app/index :app/help}
                                    (:handler (b/match-route routes/routes %)))
     :reload-same-path? false})

I have several problems with it:

  1. Clicking on back and forward buttons in the browser changes the URL, but does not call :app-state/changed-route event. So it is ignored by the app.

  2. When I remove :reload-same-path?, each click on a link called :app-state/changed-route multiple times. But I want to allow reloading the same path to the user.

  3. Sometimes :app-state/changed-route is called multiple times anyway when I reload the page.

Any idea what I could be doing wrong?

@pavel-klavik
Copy link
Author

I did some further digging concerning 2. When I set :reload-same-path? true, then navigating to a different URL dispatches [:app-state/changed-route] twice, while navigating to the same URL dispatches it only once. I believe the reason might be the following part of the code:

(when (not= current-relative-href relative-href) ;; do not add duplicate html5 history state
           (. history (setToken relative-href title)))
         (.preventDefault e)
         (when reload-same-path?
           (events/dispatchEvent history (Event. path true))))))))

When navigating to different URL, both (. history (setToken relative-href title)) and (events/dispatchEvent history (Event. path true)) are called. Is this correct?

@pavel-klavik
Copy link
Author

3 is unrelated to accountant.

To solve 1, I have added:

(events/listen "popstate" #(rf/dispatch [:app-state/changed-route
                                         (->> js/document .-location .-href
                                              (re-find #"^.*//[^/]*(/.*)") last)]))

Is this the expected solution. Maybe adding something to the documentation that this has to be done independently?

To solve 2, I did a small hack in which I check in :app-state/changed-route whether routing to the same path did not occur in last 250 ms. But it would be good to avoid this.

@KingMob
Copy link

KingMob commented Feb 10, 2021

Seconding this. With reload-same-path? set to true, I also get double calls of the nav handler all the time. The issue is the (when reload-same-path? ... doesn't actually check the path. The code should probably be something more like:

(if (and (= current-relative-href relative-href) reload-same-path?)
  (events/dispatchEvent history (Event. path true))
  (. history (setToken relative-href title)))
(.preventDefault e)

@iku000888
Copy link
Collaborator

I don't actively use accountant anymore, but I can merge PRs.

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

No branches or pull requests

3 participants