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

Feat add container to modal options #6571

Open
wants to merge 4 commits into
base: development
Choose a base branch
from

Conversation

evan-qualls
Copy link

PR Checklist

Before creating new PR, please take a look at checklist below to make sure that you've done everything that needs to be done before we can merge it.

  • [X ] read and followed the CONTRIBUTING.md guide.
  • [ X] built and tested the changes locally.
  • [X ] added/updated tests.
  • [X ] added/updated API documentation.
  • [ X] added/updated demos.

evan-qualls and others added 3 commits April 8, 2023 14:04
feat(modal): Add container property to ModalOptions

These changes will allow developers to optionally place render modals within the application context, allowing components injected via BsModalService to use ViewEncapsulation and support change detection.
* Added container<string | ElementRef> property to ModalOptions.
* Updated modalConfigDefaults with "container: 'body' " to prevent breaking changes/behavior
* Updated BsModalService and Modal-Directive so that the container element is no longer hard-coded to 'body' but rather config.container
Updated documentation pages
Updated test case for ModalOptions.container property
These changes will allow developers to optionally place render modals within the application context, allowing components injected via BsModalService to use ViewEncapsulation and support change detection.
* Added container<string | ElementRef> property to ModalOptions.
* Updated modalConfigDefaults with "container: 'body' " to prevent breaking changes/behavior
* Updated BsModalService and Modal-Directive so that the container element is no longer hard-coded to 'body' but rather config.container
@evan-qualls
Copy link
Author

evan-qualls commented Apr 10, 2023

@valorkin @SvetlanaMuravlova Tagging you two because you are the people with the most recent activity. These changes would solve a lot of headaches for me. I work for a company that has a very large application with dozens of modals with customizations which use ViewEncapsulation.None because modals are hardcoded to be appended to the body. The lack of view encapsulation has caused a lot of problems depending on which dev on our team codes the modal...

I'd rather use this existing codebase instead of writing my own separate solution. I'm certainly open to hearing if my approach here needs adjusting.

image

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

Successfully merging this pull request may close these issues.

1 participant