search

neilff / redux-ui-router

ngRedux bindings for Angular UI Router
MIT License
143 stars 48 forks source link

angular-ui-router@1.0.0-beta.1 Compatibility #61

Closed matt328 closed 8 years ago

matt328 commented 8 years ago

Should redux-ui-router@0.5.2 be compatible with angular-ui-router@1.0.0-beta.1? I'm seeing a few issues, the router.currentState in the redux state never updates from what it is initially, and also it seems forward and back buttons and typing in a hash fragment doesn't do anything at all.

I would like to be able to use the new version of UI Router since it supports components instead templates and controllerAs.

hally9k commented 8 years ago

0.5.2 doesn't support for angular-ui-router@1.0.0-beta.1 by the looks of things. I might have a look at this over the weekend but no promises. Feel free to submit a PR :smile:

matt328 commented 8 years ago

It looks like ui-router completely replaced state events with a transitions service. They provide an optional ng1_state_events module that will probably serve as a workaround until you are ready to drop support for ui-router < 1.0.0.

hally9k commented 8 years ago

@matt328 You should be all set now thanks to this PR: https://github.com/neilff/redux-ui-router/pull/64 I'm going to leave this open for comment on the upgrade to 1.0 beta for now. Keen to hear your feedback.

Nighthawk14 commented 8 years ago

Hey @hally9k testing right now, it seems that the params issue is fixed. But there seems to be an issue when you refresh a page with a params e.g. /foo/1 (where 1 is /:id) the param is not propagated after refresh and the properties fromParams and toParams of the onStart and onSuccess actions don't contain it.

ps: I tried with and without html5location enabled ps2: I tried serving the files via webpack-dev-server or via nginx, same result.

hally9k commented 8 years ago

Hi, nice one for finding this! If you want to document your findings in an issue I'll have a jam as well and between us we'll see if we cant muster a PR to fix this!

hally9k commented 8 years ago

This upgrade broke the previous params, there are getting set to the current params... I will submit a PR shortly. It would be cool if someone other than myself had a little look over it before I merge it... two pairs of eyes and all that... @Nighthawk14 ? @rayronvictor ?

hally9k commented 8 years ago

Here you go... Totally open to comment, didn't feel entirely clean tbh... :https://github.com/neilff/redux-ui-router/pull/68