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

Implement route history further. Add support for html5 history. #14

Draft
wants to merge 2 commits into
base: routing-refactor
Choose a base branch
from

Conversation

yoodame
Copy link

@yoodame yoodame commented Mar 21, 2025

No description provided.

Comment on lines -544 to -545
(defn install-history! [history-impl]
(reset! history history-impl))
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved this the to route-history NS. Allows others to create custom implementations of the route history if they choose to.

@@ -144,9 +135,10 @@
(when (gobj/getValueByKeys evt "state")
(let [event-uid (gobj/getValueByKeys evt "state" "uid")]
(log/debug "Got pop state event." evt)
(scf/send! app :com.fulcrologic.statecharts.integration.fulcro.ui-routes/session
(swap! (:current-uid history) (constantly event-uid))
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensures current-uid is synced with the browser popstate event-uid.

Comment on lines 93 to 104
(-back! [this]
#?(:cljs
(do
(tap> "Going back")
(cond
(> (count @uid->history) 1) (do
(tap> "Going back YAW!")
(log/debug "Back to prior route" (some-> @uid->history last second))
(.back js/history))
:else (log/error "No prior route. Ignoring BACK request.")))))
(-current-route [this] (url->route fulcro-app))
(-recent-history [this] @uid->history))
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented back to sync the browser back button.

Copy link
Author

@yoodame yoodame left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@awkay I have several changes here for your review. So far these changes syncs up nicely with the browser popstate (back/forward), and drops the user into the appropriate state route from the current URL.

Still broken:

  • Reloading form edit breaks the form. We need to sync the sc route param some how.
  • Report controls do not update the sc route param

@awkay
Copy link
Member

awkay commented Mar 21, 2025

So, you've tested that a ui state that had data (from the initial event) and is properly re-started with the same event data?

Is that seeming sufficient? I assume that as long as the route state itself is established from outside properly, then any substate that needs to be established will be properly handled by the entry handlers from there. See any edge cases?

@yoodame
Copy link
Author

yoodame commented Mar 21, 2025

Im not confident it works well. I was able to reload the a form ui state by providing the route params ((ri/form-state {:route/target DocumentForm :route/path ["document"] :route/params [:id]}. However, it chokes after a second browser refresh. Guessing it happens when trying convert the url to a ui state route. Perhaps in the ru/current-url-state-params function?

Failed to execute 'atob' on 'Window': The string to be decoded is not correctly encoded.
Screenshot 2025-03-21 at 7 23 44 AM

Will share a screen recording with you directly.

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