-
Notifications
You must be signed in to change notification settings - Fork 877
docs(template-syntax): review and update Dart to new doc design #1465
docs(template-syntax): review and update Dart to new doc design #1465
Conversation
@Foxandxss, @wardbell, @kwalrath, @thso : this is ready for review. As usual, it might be easiest to review while ignoring differences in whitespace. |
<!-- #enddocregion NgSwitch --> | ||
|
||
<!-- with <template> --> | ||
<template [ngSwitchWhen]="'Eenie'"><span>Eenie</span></template> |
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.
Why this change?
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.
The objective was to show the result of desugaring the *ngSwitchWhen
without any optimizations. The chapter material is complex enough, so I preferred that the reader not be forced to recall that if the template expression is a string constant then:
<foo [x]="'s'">
is equivalent to
<foo x="s">
Besides, this is what matches the TS example. Let me know if you agree with this reasoning.
LGTM for Dart code. |
The TS part is good for me. |
@kwalrath : any updates on a review of the Dart-side prose? |
866d7c6
to
3c50a15
Compare
Taking a look now...
|
:marked | ||
Our Angular application manages what the user sees and does through the interaction of a Component class instance (the *component*) and its user-facing template. | ||
Our Angular application manages what the user sees and it achieves this through the interaction of a Component class instance (the *component*) and its user-facing template. |
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.
It's good to change the original sentence, since it was imprecise, but the new version is rather run-on. How about:
Our Angular application manages what the user sees and can do, achieving this through the...
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.
done
LGTM, once you fix the minor problems I noted. |
5200874
to
a8c456e
Compare
I believe that I have addressed all of your concerns. (See my replies to specific line comments; in particular about expression chaining.) |
51d1de5
to
cda1294
Compare
Added callout now that @vicb has clarified issues related to style property naming.
cda1294
to
7abed79
Compare
@kwalrath : can this be merged, if not what can I do to help make that happen? (I'd like to get the changes to |
Fixes #1460, contributes to #1508, #1522.