-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
feat(dialog): allow disableClose option to be updated #4964
Conversation
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. |
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. |
MdDialogRef
and allows for it to be updated.containerInstance
private toMdDialogRef
since it doesn't make sense for it to be public anymore.backdropClick
observable once the associatedoverlayRef
is destroyed. This avoids having to unsubscribe manually or having to useObservable.first
.Fixes #3938.