Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

Modal dialog does not disappear with 0.13 and Angular 1.3 when destroying a passed scope #3787

Closed
bradyisom opened this issue Jun 9, 2015 · 10 comments

Comments

@bradyisom
Copy link

Angular version: 1.3.16
ui-bootstrap version: 0.13.0

This is related to, but not the same as #3633 and #3620. If you create a new scope to pass to $modal.open, it needs to be destroyed when the dialog closes to prevent memory leaks. The only event we currently get to perform this action is the result promise. However, this resolves before the closing animation is executed. If you destroy the new scope at that point, the animation cannot execute, because of its dependency on ngClass to remove the 'in' CSS class.

Here is a Plunkr that shows the behavior: http://plnkr.co/edit/yaFvaOGu1NUxkzXwfFuJ?p=preview

This same Plunkr works when using version 0.12.1, so it is a breaking change.

@bradyisom
Copy link
Author

Workaround is to defer destroying the scope using $timeout or similar method.

@Oceanswave
Copy link

+1

@wesleycho wesleycho added this to the 0.13.1 (Performance) milestone Jun 11, 2015
@wesleycho wesleycho self-assigned this Jun 11, 2015
@wesleycho
Copy link
Contributor

So we would be able to adapt the internals so that the promise is not resolved until after the modal window closes, but this could potentially cause a problem - this would be a breaking change that may be undesired, since current behavior is to resolve immediately after the functions are called.

One thing that should be noted is that the $scope is never used directly - the service will always do $scope.$new(), so it always works with a descendent scope that gets garbage collected appropriately. Given this, I think your workaround is fine as the resolution for this issue and that there should not be a library modification to support this, except potentially an additional option that gets called on completion of animation of the modal fade out.

Thoughts?

@wesleycho wesleycho removed their assignment Jun 11, 2015
@bradyisom
Copy link
Author

Having an additional option or promise that gets resolved on completion of animation would be sufficient.

However, another option would be to just put the resolve in an $evalAsync block. That way the animation would kick off before parent scopes get destroyed. It would still resolve almost immediately after the close functions are called.

@wesleycho
Copy link
Contributor

It looks like $evalAsync does not work - however, $timeout does work, so I will open a PR shortly with this fix.

@malterb
Copy link

malterb commented Jun 16, 2015

+1

disabling the animation as a workaround works as well

@skidvd
Copy link

skidvd commented Jun 18, 2015

+1

1 similar comment
@fhawkes
Copy link

fhawkes commented Jun 19, 2015

+1

@SimonNodel-AI
Copy link

I've ran into same scenario with angular 1.4.1. Disabling animation solved my problem too.

@chrisirhc chrisirhc self-assigned this Jul 4, 2015
@chrisirhc
Copy link
Contributor

I'm working on a fix now and it should fix this issue.
Working Plunker with snapshot:
http://plnkr.co/edit/cTE4sNZIf08guSaDmzFH?p=preview

chrisirhc added a commit to chrisirhc/angular-ui-bootstrap that referenced this issue Jul 4, 2015
chrisirhc added a commit to chrisirhc/angular-ui-bootstrap that referenced this issue Jul 4, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants