feat(modal): Adds defaultContainer to $modalProvider

[Lalem001]

Includes basic tests, with adjustments to toHaveModalsOpen and toHaveBackdrop.

Fixes: #2688

[chrisirhc]

Excellent! Thanks! I don't have any cycles to review this at the moment, but if anyone would like to test it or review it, please go ahead.

[dubejf]

It would probably be better to add appendTo as a modalOptions instead of being a global setting on the $modalProvider.

Also, if you want to keep the $modalProvider.appendTo, it should be renamed to $modalProvider.defaultContainer.

[Lalem001]

Sorry for the delay.

$modalProvider.appendTo has been changed to $modalProvider.defaultContainer.

I am under the impression that this implementation was not built to handle modals on different containers, thus I believe adding appendTo to modalOptions would cause problems for some people. Support for modals on different containers could be done in the future if requested.

[pepopowitz]

One issue I noticed is that you also moved the OPENED_MODAL_CLASS and backdrop element to the container element, instead of the body. When those are applied at the container, the user can scroll the modal off the screen, and the backdrop will not cover the entire screen if they had already scrolled.

I think the OPENED_MODAL_CLASS and backdrop element should remain on the body, rather than the container.

[Lalem001]

@pleepleus88 I have not run into this issue in my environment, but I can see how it could happen in theory.

Perhaps it would be a good idea to rename defaultContainer to modalContainer, and add a backdropContainer attribute?

[pepopowitz]

That makes sense, and seems like it would work. 👍

[wesleycho]

Can you update this with current master?

[Lalem001]

I will close this one, and make a new PR with latest master. Will link to new PR here when its ready.

[Lalem001]

@wesleycho Regarding the change suggested above with separate containers for modal and backdrop, how would you prefer it be done?

Like:

$modalProvider.modalContainer = body;
$modalProvider.backdropContainer = body;

Or:

$modalProvider.defaultContainers = {
    modal: body,
    backdrop: body
}

Other?

[chrisirhc]

Fyi @Lalem001 , you can force push to the branch that this PR is based on to update it, so you don't need to create a new PR.