Skip to content

fix(transitionTo): re-added the saved hash before broadcasting event #2464

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

Merged

Conversation

blokfyuh
Copy link
Contributor

Re-added the saved hash before broadcasting $stateChangeStart. This way, libraries using this event to do their magic will have the hash accessible through toParams. (e.g. https://github.com/Narzerus/angular-permission)

Thoughts/comments ?

@nateabele
Copy link
Contributor

Yeah, the patch makes sense. We just need a matching unit test to prevent regressions.

@blokfyuh
Copy link
Contributor Author

Hi guys,

I'm not an expert regarding contributions, so what kind of unit tests would you expect for this patch ? I mean wouldn't be redundant with the tests that @mismith wrote for #1867 ?

@nateabele
Copy link
Contributor

So, the idea is you want to have a unit test that fails against master but passes with your patch. In this case, that means asserting that the hash is available in a $stateChangeStart hook.

@nateabele
Copy link
Contributor

Test looks good. Almost there: http://prjs.radify.io/#/angular-ui/ui-router/pulls/2464

@blokfyuh blokfyuh force-pushed the blokfyuh-hash-fix-permissions-plugins branch from c1096e1 to a2ede2c Compare January 18, 2016 09:46
@blokfyuh
Copy link
Contributor Author

The subject looks good, the commits have been squashed. Do I pass ? :D

@nateabele
Copy link
Contributor

Almost! It's the subject of the commit, not the PR. Sorry about that. I'm gonna fix the language in the UI so it's clearer.

Re-added the saved hash before broadcasting $stateChangeStart. This way, libraries using this event to do their magic will have the hash accessible through toParams. (e.g. https://github.com/Narzerus/angular-permission)
@blokfyuh blokfyuh force-pushed the blokfyuh-hash-fix-permissions-plugins branch from a2ede2c to 8c1bf30 Compare January 19, 2016 09:31
@blokfyuh
Copy link
Contributor Author

No problem. Let me know if we are good this time.

@nateabele
Copy link
Contributor

Yesssss!

nateabele added a commit that referenced this pull request Jan 19, 2016
…plugins

fix(transitionTo): re-added the saved hash before broadcasting event
@nateabele nateabele merged commit 2d23875 into angular-ui:master Jan 19, 2016
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