Skip to content

docs(material/dialog): state when notification observables close #21336

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

Merged
merged 2 commits into from
Dec 14, 2020
Merged

docs(material/dialog): state when notification observables close #21336

merged 2 commits into from
Dec 14, 2020

Conversation

mrm1st3r
Copy link
Contributor

It's currently not stated if and when the notification observables from MatDialogRef are closed. This causes questions and discussions both online and offline (in my team at work to be precise).

See different answers and comments in:

I think this matter should be mentioned in the documentation.

@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Dec 12, 2020
@@ -14,7 +14,7 @@ let dialogRef = dialog.open(UserProfileComponent, {
```

The `MatDialogRef` provides a handle on the opened dialog. It can be used to close the dialog and to
receive notification when the dialog has been closed.
receive notification when the dialog has been closed. Any notification Observables will close with the dialog.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not something you added, but we might as well fix it. The "receive notification" should be "receive notifications".

Also I think the correct terminology is that the observables will be "completed" when the dialog is closed, not that they'll be "closed".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, the terminology was indeed not correct. I was thinking of Subscriptions instead of Observables.
I fixed both, but had to rephrase slightly.

Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@crisbeto crisbeto added docs This issue is related to documentation action: merge The PR is ready for merge by the caretaker merge safe target: patch This PR is targeted for the next patch release labels Dec 13, 2020
@mmalerba mmalerba merged commit 8cd497b into angular:master Dec 14, 2020
mmalerba pushed a commit that referenced this pull request Dec 14, 2020
)

* docs(material/dialog): state when notification observables close

* docs(material/dialog): improve wording

(cherry picked from commit 8cd497b)
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jan 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement docs This issue is related to documentation target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants