feat(modal): dismiss all modals on $locationChangeSuccess

[pkozlowski-opensource]

Closes #1334

@angular-ui/bootstrap could you guys have a look and let me know what you think? I'm not sure about the API / set of supported options.

[chrisirhc]

Actually, regarding this, I don't think this should be a default behavior. I've had a use case where location changes have no impact at all on the current modal state of the application.

I think this is a case where we could have a wiki page to document a snippet to allow users to add to their applications if they desire such a behavior (alternatives as discussed in #1334 can also be added to that page).

[chrisirhc]

However, as I read through the code, it looks like we're not handling the case where the modal's scope (the name of that property is a little misleading, it's actually the modal's scope parent) is destroyed.

In such a case, perhaps the modal should be dismissed automatically as it no longer belongs to any non-destroyed scope (changes to its scope no longer matter) and probably is no longer relevant to the application.

In the default case where its parent scope is the $rootScope, nothing happens.

What do you think?

[Narretz]

I think this should be configurable at least. Global config is probably easier than per modal.

[bekos]

What about just exposing a dismissAll(reason) and let the developers decide when to call?

[chrisirhc]

+1 to exposing dismissAll and also adding the $locationSuccessChange handler snippet and this issue for users who desire such behavior.

[bekos]

Also, it would be nice to provide a beforeDismiss(reason) function that is able to cancel the actual dismiss of the modal. This way we can define behavior per instance and a hook for custom logic.

[pkozlowski-opensource]

OK, I agree that exposing dismissAll and leaving it up to the users makes more sense, going to update this PR accordingly.

@chrisirhc I see what you are saying about scopes and I think we should be taking care of host scope destroy somehow, but in practice I think it is rather rare use case as a modal covers "normal" UI so it is not that easy to destroy the host scope. Let's fix it when someone comes up with a legit use-case.

[bekos]

@pkozlowski-opensource You don't like the idea about beforeDismiss? Just curious :-)

[pkozlowski-opensource]

@bekos no, I like it :-) But I just wanted to push minimal changes, cut a release and focus on 1.2 + $ngAnimate compatibility changes.

[bekos]

@pkozlowski-opensource OK :-) Just a question on the commit. Shouldn't this be exposed to the $modal? Sorry if I am wrong here but I am not very familiar with this code :-)