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($state.transitionTo): trigger $stateChangeCancel appropriately #3039

Conversation

zeusdeux
Copy link

@zeusdeux zeusdeux commented Sep 26, 2016

This PR supersedes #3029

Current behaviour:

We are in state A. We start a transition to state B. $stateChangeStart
is triggered. Before $stateChangeSuccess is triggered for state B, we
start a transition back to state A. No state events are triggered and we
are left hanging without any notification.

Updated behaviour:

When the state change to B is superseded by the state change back to A,
$stateChangeCancel is broadcasted on $rootScope for the transition from
B -> A. This behaviour makes sure that for every $stateChangeStart there
is a corresponding $stateChange<Success|Error|Cancel> thus completing the
lifecycle.

Fixes issue #3027

PS: I have modified the existing test suite to assert on new behaviour

@christopherthielen
Copy link
Contributor

Good work! Can you update your PR so it doesn't change all the whitespace?

@zeusdeux
Copy link
Author

Ah damn. Auto save hooks. On it.

@zeusdeux
Copy link
Author

@christopherthielen Done!

Current behaviour:

We are in state A. We start a transition to state B. `$stateChangeStart`
is triggered. Before `$stateChangeSuccess` is triggered for state B, we
start a transition back to state A. No state events are triggered and we
are left hanging without any notification.

Updated behaviour:

When the state change to B is superseded by the state change back to A,
`$stateChangeCancel` is broadcasted on `$rootScope` for the transition from
B -> A. This behaviour makes sure that for every `$stateChangeStart` there
is a corresponding `$stateChange<Success|Error|Cancel>` thus completing the
lifecycle.

Fixes issue angular-ui#3027
@zeusdeux zeusdeux force-pushed the fix/transition-to-state-change-cancel branch from 6dc93a2 to 71b5b61 Compare September 26, 2016 15:57
@zeusdeux
Copy link
Author

Hey @christopherthielen, do you think this PR is merge ready?

I would love to have v0.3.2 as it would help release some features on my end :)

Thanks!

@christopherthielen
Copy link
Contributor

I think it's probably good to merge, but I'd like to play with it for a little bit before releasing 0.3.2

@zeusdeux
Copy link
Author

zeusdeux commented Oct 4, 2016

@christopherthielen Sounds good. Do you have a time estimate in mind? It would be very useful for me to be able to communicate that to my team.

@christopherthielen
Copy link
Contributor

I think #2696 should be fixed first, then I'm ready to release 0.3.2

@zeusdeux
Copy link
Author

zeusdeux commented Oct 24, 2016

@christopherthielen Could we get 0.3.2 w/o #2696 since it's been a month and I don't see any momentum on a fix for that. Thanks!

@christopherthielen christopherthielen merged commit ca7c366 into angular-ui:legacy Nov 3, 2016
@christopherthielen
Copy link
Contributor

@zeusdeux released 0.3.2

@zeusdeux
Copy link
Author

zeusdeux commented Nov 7, 2016

@christopherthielen 🙇

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

Successfully merging this pull request may close these issues.

2 participants