Skip to content

Updated for the new transitions hooks #296

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
merged 2 commits into from
May 14, 2016
Merged

Updated for the new transitions hooks #296

merged 2 commits into from
May 14, 2016

Conversation

flick36
Copy link
Contributor

@flick36 flick36 commented May 8, 2016

When updated ui-router in #283 didn't realize that the stateChange* events have been deprecated and disabled by default in ui-router 1.0* (my bad) and the new way to do this is using the new Transition hooks as explained here: https://github.com/angular-ui/ui-router/releases/tag/1.0.0alpha0 so this fixes #295

event.preventDefault();
return $state.go('app.login');
}
let redirectToLogin = ['$auth', '$transition$', ($auth) => {
Copy link
Owner

Choose a reason for hiding this comment

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

is $transitions needed here?
And minification safe di works through ngInject.. so can you please move $auth to the top so that we avoid the duplication (between '$auth' and $auth)

Copy link
Contributor Author

@flick36 flick36 May 11, 2016

Choose a reason for hiding this comment

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

It doesn't work if i move them to the top, they're special injectables for the onBefore method, note that the $transitions it's not the same as $transition$ used here, the $transition$ returns the current transition and like i said it's a special injectable for the onBefore method

See the docs

Copy link
Owner

Choose a reason for hiding this comment

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

Oh okay my bad
Thanks!
but now since we don't need $transition$ can't we remove it?

And could you also please try 1 more thing?
If it's possible to remove `'$auth', '$transition$' with this line:

 let redirectToLogin = ($auth) => {
     'ngInject';

I think ngInject should manage the dependencies properly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jadjoubran done, so far so good.

@jadjoubran
Copy link
Owner

Thanks!

@jadjoubran jadjoubran added the bug label May 11, 2016
@jadjoubran jadjoubran merged commit d167e79 into jadjoubran:master May 14, 2016
@jadjoubran
Copy link
Owner

Great thanks 🚀

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

Successfully merging this pull request may close these issues.

$rootScope.$on('$stateChangeStart') it's never fired therefore can't restrict auth
2 participants