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

Location.manageHistory: Consider dispatching the event for changes to route #190

Open
3noch opened this issue Jan 17, 2018 · 2 comments
Open

Comments

@3noch
Copy link
Member

3noch commented Jan 17, 2018

manageHistory currently maintains changes to the route internally but any other JS code listening for route events won't be notified when these updates happen. We can dispatch this event and rely on the browser to update the Dynamic for us. There are a few choices to consider, however, namely if we want the event to bubble, etc.

dispatchEvent' :: Dom.Window -> JSM ()
dispatchEvent' window = do
  obj@(JS.Object o) <- JS.create
  JS.objSetPropertyByName obj (t_ "cancelable") True
  JS.objSetPropertyByName obj (t_ "bubbles")    True
  JS.objSetPropertyByName obj (t_ "view")       window
  event <- newPopStateEvent (t_ "popstate") $ Just $ pFromJSVal o
  dispatchEvent_ window event

browserHistoryWith only provides half of the story that manageHistory offers. However, if the browser were handling the updates for us, we could write manageHistory in terms of browserHistoryWith.

@ryantrinkle
Copy link
Member

This is an interesting idea, but I'm not sure exactly when it should be done or what other ramifications it might have. Do JS libraries usually do this - i.e. is there an established best-practice?

Separately, shouldn't something like this be used to construct the event?

@3noch
Copy link
Member Author

3noch commented Jan 17, 2018

That code is actually from reflex-dom-contrib. You're right that it can likely be modernized.

Good question about best practices. It could even be that we should add this to HistoryCommand so that the user can decide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants