API doc comment updates - all high priority entries #124
Conversation
This covers all high priority API entries assigned to @chalin in angulardart#116 (and a few more).
| /// | ||
| /// `ngModel` binds an existing domain model to a form control. For a two-way | ||
| /// binding, use `[(ngModel)]` to ensure the model updates in both directions. | ||
| /// Learn more about `ngModel` in the Guide [Forms](docs/guide/forms.html#ngModel) |
There was a problem hiding this comment.
Do we use "Guide" this way elsewhere. OK to leave for now, but I'm curious.
| } | ||
|
|
||
| /// Creates a [Provider]. | ||
| /// Creates an injector [Provider] for the given [token] based on a given |
There was a problem hiding this comment.
We try to have full sentences between bulleted lists, and we don't use ", or" or ", and" in lists.
Also, this breaks the rule about descriptions starting with a one-sentence paragraph.
How about something like this:
/// Creates a [Provider] for the given [token].
///
/// In addition to the token, you must specify one of the following:
///
/// - A class
/// - A value
/// - A factory function
/// - An existing token
| /// - Another token | ||
| /// | ||
| /// The `token` is most commonly a class or [OpaqueToken-class.html]. | ||
| /// The [token] is most commonly a class or an opaque token. More details |
There was a problem hiding this comment.
Let's avoid the "can be found" indirectness, and be clearer about where the link goes. How about:
For more details about providers, see [Injector providers][di] in the Dependency Injection page.
| /// | ||
| /// Each application has its own private injector as well. When there are | ||
| /// multiple applications on a page, Angular treats each application injector's | ||
| /// services as private to that application. |
There was a problem hiding this comment.
Do we have an example for bootstrapping multiple apps?
There was a problem hiding this comment.
Not anymore (we did, in the doc examples, but there were issues so I worked around them), ... at least not that I am aware of.
| /// | ||
| /// {@example docs/template-syntax/lib/app_component.html region=Template-4} | ||
| /// | ||
| /// See the [Template Syntax section on `ngFor`][guide] for details. |
There was a problem hiding this comment.
Unsure about the capitalization for "Template Syntax" here. WDYT @kwalrath?
There was a problem hiding this comment.
Since it's a page name, the caps are good here.
It could be clearer that it's a page name, though. And the "for details" should be first, since seeing that page isn't mandatory. Something like:
/// For details, see the [`ngFor` discussion in the Template Syntax page]...
There was a problem hiding this comment.
fixed as Kathy suggested, here and in all other places that used a similar phrase.
| /// Causes the element and its contents to appear in the DOM conditionally. | ||
| /// Causes an element and its contents to be added/removed from the DOM | ||
| /// conditionally, based on the value of the supplied boolean template | ||
| /// expression. |
There was a problem hiding this comment.
Move 'conditionally' before added/removed?
|
None of these changes are crucial to make right now, and this PR significantly improves the docs. I'm inclined to accept the PR now, so we can test the process of accepting doc PRs... |
Contributes to angulardart#116. This is a followup to angulardart#124. This commit has updates for all remaining entries in the spreadsheet except for `CanActivate`, which I think should be handled once the router chapter is created.
Contributes to angulardart#116. This is a followup to angulardart#124. This commit has updates for all remaining entries in the spreadsheet except for `CanActivate`, which I think should be handled once the router chapter is created.
Post-review edits to address comments made in angulardart#124
5 participants
This covers all high priority API entries assigned to @chalin in #116 (and a few more); only 9 entries in the list remain.
Contributes to #116.
Supersedes #123 (ngIf has been included in this commit).
cc @matanlurey @vikerman @kwalrath @thso