Skip to content
This repository has been archived by the owner. It is now read-only.

fix(modal): Promise resolution after close animation #3057 #3056

Closed
wants to merge 2 commits into from

Conversation

@nauticalcoder
Copy link

nauticalcoder commented Dec 6, 2014

This change attempts to fix Issue #3057
This change moves the result promise resolution to after the close animation is finished.

This is to solve the issue where the result promise is currently getting resolved before the animation completes. If you display a message box in the promise handler, it is displaying before the modal closes.

This change moves the result promise resolution to after the close animation is finished.
@nauticalcoder nauticalcoder changed the title Promise resolution after close animation fix(modal): Promise resolution after close animation Dec 7, 2014
@nauticalcoder nauticalcoder changed the title fix(modal): Promise resolution after close animation fix(modal): Promise resolution after close animation #3057 Dec 7, 2014
@fiznool
Copy link

fiznool commented Feb 10, 2015

+1, is there any chance of this being merged in?

I'm currently just setting an arbitrary timeout to stop content inside my modal from changing while the animation runs, and it would be much better to have this fix.

@karianna
Copy link
Contributor

karianna commented Feb 10, 2015

@nauticalcoder Are you able to add some tests for this?

@nauticalcoder
Copy link
Author

nauticalcoder commented Feb 10, 2015

I will work on adding some tests for this. It might be a day or two.

@SnackyPete
Copy link

SnackyPete commented Mar 4, 2015

It would be better if this fix was designed as a Promise, rather than a callback. :)

@wesleycho wesleycho force-pushed the angular-ui:master branch from e373941 to 2c2dba6 Mar 23, 2015
@wesleycho
Copy link
Member

wesleycho commented Mar 31, 2015

Thanks for the effort, but I believe this is now addressed by @chrisirhc's work on transitioning to using ngAnimate for the modal.

@wesleycho wesleycho closed this Mar 31, 2015
@karianna karianna added this to the 0.13.0 milestone Mar 31, 2015
@wesleycho
Copy link
Member

wesleycho commented Aug 17, 2015

Hmm I closed this prematurely I believe, but this implementation is flawed - the problem is that it should be opt-in, and hidden behind a configuration option so as not to be a breaking change.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.