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

fix(ui-sref): handle state transition promise rejection #3029

Conversation

zeusdeux
Copy link

@zeusdeux zeusdeux commented Sep 22, 2016

In the current world, ui-sref does nothing when the promise returned by
$state.go is rejected. This PR gives ui-sref directive the ability to
emit $stateChangeCancel when the transition promise rejects.
The reason why I have chosen to implement this in the ui-sref directive
and not in the definition for $state.go or $state.transitionTo are
outlined in the issue below.
#3027

@zeusdeux zeusdeux force-pushed the fix/ui-sref-handle-failed-state-transition branch 2 times, most recently from 9658739 to 700a207 Compare September 22, 2016 09:17
In the current world, ui-sref does nothing when the promise returned by
$state.go is rejected. This PR gives ui-sref directive the ability to
emit $stateChangeCancel when the transition promise rejects.
The reason why I have chosen to implement this in the ui-sref directive
and not in the definition for $state.go or $state.transitionTo are
outlined in the issue below.

angular-ui#3027.
@zeusdeux zeusdeux force-pushed the fix/ui-sref-handle-failed-state-transition branch from 700a207 to 3b6c735 Compare September 22, 2016 09:47
@christopherthielen christopherthielen added this to the 0.3.2 milestone Sep 22, 2016
@christopherthielen
Copy link
Contributor

@zeusdeux as mentioned in the issue #3027 I think this belongs in the transitionTo code. If it only is implemented for ui-sref, the parity between stateChangeStart and (Success/Cancel/Error) is still broken.

I'd be happy to help ensure the equivalent code in transitionTo is correct.

@zeusdeux
Copy link
Author

@christopherthielen Agreed. Working on a patch for transitionTo

@zeusdeux
Copy link
Author

zeusdeux commented Sep 26, 2016

@christopherthielen Done here #3039. Please add that instead of this to 0.3.2 milestone.

Closing this in favour of that.

Thanks!

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