-
Notifications
You must be signed in to change notification settings - Fork 6.8k
feat(dialog): allow disableClose option to be updated #4964
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(dialog): allow disableClose option to be updated #4964
Conversation
jelbourn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Being able to update disableClose with this API is somewhat problematic because it means that some config properties can be updated live but other can't (e.g., position, size). This should be consistent one way or the other.
I would change the approach to add disableClose as a property to the dialog ref which is initially set from the config but can be subsequently updated.
42a11e9 to
6506b10
Compare
|
Addressed the feedback @jelbourn. |
jelbourn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just needs rebease
6506b10 to
7e1bb72
Compare
|
Needs rebase =) |
* Exposes the disableClose option via the `MdDialogRef` and allows for it to be updated. * Makes the `containerInstance` private to `MdDialogRef` since it doesn't make sense for it to be public anymore. * Completes the `backdropClick` observable once the associated `overlayRef` is destroyed. This avoids having to unsubscribe manually or having to use `Observable.first`. Fixes angular#3938.
7e1bb72 to
656c1fa
Compare
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
MdDialogRefand allows for it to be updated.containerInstanceprivate toMdDialogRefsince it doesn't make sense for it to be public anymore.backdropClickobservable once the associatedoverlayRefis destroyed. This avoids having to unsubscribe manually or having to useObservable.first.Fixes #3938.