Skip to content

docs(Dialog): MdDialogConfig ViewContainerRef description #3747

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

Closed
wants to merge 6 commits into from
Closed

docs(Dialog): MdDialogConfig ViewContainerRef description #3747

wants to merge 6 commits into from

Conversation

M-a-c
Copy link
Contributor

@M-a-c M-a-c commented Mar 23, 2017

#3651
I looked to find an example of what this is actually used for.
I tried to explain it a little bit more. (line 2)

https://github.com/angular/material2/blob/master/src/lib/core/portal/portal.ts#L79

Does this JsDoc summary suffice?

I think this comment should be that large because its a rather specific definition. and is a more advanced component.

Based on https://github.com/angular/material2/blob/master/src/lib/core/portal/portal.ts#L79
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Mar 23, 2017
@M-a-c M-a-c changed the title JsDoc MdDialogConfig #3651 JsDoc MdDialogConfig Mar 23, 2017
M-a-c added 2 commits March 23, 2017 15:29
Fixed lint.
more specific, added MdDialog.open().
@M-a-c M-a-c changed the title JsDoc MdDialogConfig docs(Dialog): MdDialogConfig ViewContainerRef description Mar 23, 2017
/** Where the attached component should live in Angular's *logical* component tree.
* This is different from where the component *renders*, which is determined by
* the PortalHost (the component that was passed into MdDialog.open().
* Note the origin is necessary when the host is outside of the Angular application context. */
Copy link
Member

Choose a reason for hiding this comment

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

This isn't quite right. A little more accurate would be:

/** 
  * Where the attached component should live in Angular's *logical* component tree.
  * This affects what is available for injection and the change detection order for the 
  * component instantiated inside of the dialog. This does not affect where the dialog 
  * content will be rendered.
  */

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this makes more sense. I'll switch that out.

More accurate wording
This affects what is available for injection and the change detection order for the component instantiated inside of the dialog
/** Where the attached component should live in Angular's *logical* component tree.
* This affects what is available for injection and the change detection order for the
* component instantiated inside of the dialog. This does not affect where the dialog
* content will be rendered. */
Copy link
Member

Choose a reason for hiding this comment

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

The JsDoc syntax isn't quite right. It goes

/**
 * Line one...
 * Line two...
 * Line three ...
 */

@M-a-c
Copy link
Contributor Author

M-a-c commented Mar 27, 2017

@jelbourn LGTY?
Sorry about the styling misunderstanding, I have looked through the style guide now. I apologize, I should have read that before contributing.

@M-a-c
Copy link
Contributor Author

M-a-c commented Mar 28, 2017

Noticed that my branches are all messed up.
closing this creating new branch/w pr, using just these changes.
see #3825

@M-a-c M-a-c closed this Mar 28, 2017
@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 Sep 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes PR author has agreed to Google's Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants