Skip to content
This repository was archived by the owner on Dec 4, 2017. It is now read-only.

docs(template-syntax): review and update Dart to new doc design #1465

Merged
merged 4 commits into from
May 25, 2016

Conversation

chalin
Copy link
Contributor

@chalin chalin commented May 21, 2016

Fixes #1460, contributes to #1508, #1522.

  • Dart
    • update prose to new doc design.
    • enable e2e tests. Note: tests pass.
  • TS: copyedits and changes to support Dart doc.

@chalin
Copy link
Contributor Author

chalin commented May 21, 2016

@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>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this change?

Copy link
Contributor Author

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.

@thso
Copy link
Contributor

thso commented May 21, 2016

LGTM for Dart code.

@Foxandxss
Copy link
Member

The TS part is good for me.

@chalin
Copy link
Contributor Author

chalin commented May 24, 2016

@kwalrath : any updates on a review of the Dart-side prose?

@chalin chalin force-pushed the chalin-guide-template-syntax-0521 branch from 866d7c6 to 3c50a15 Compare May 24, 2016 13:40
@kwalrath
Copy link
Contributor

kwalrath commented May 24, 2016 via email

: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.
Copy link
Contributor

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...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@kwalrath
Copy link
Contributor

LGTM, once you fix the minor problems I noted.

@chalin chalin force-pushed the chalin-guide-template-syntax-0521 branch 2 times, most recently from 5200874 to a8c456e Compare May 24, 2016 15:39
@chalin
Copy link
Contributor Author

chalin commented May 24, 2016

I believe that I have addressed all of your concerns. (See my replies to specific line comments; in particular about expression chaining.)

@chalin
Copy link
Contributor Author

chalin commented May 24, 2016

@kwalrath : I pushed a new separate commit (adding a Dart difference call out), now that @vicb can confirmed the recommended style property naming scheme.

@chalin chalin force-pushed the chalin-guide-template-syntax-0521 branch from 51d1de5 to cda1294 Compare May 25, 2016 14:22
@chalin chalin force-pushed the chalin-guide-template-syntax-0521 branch from cda1294 to 7abed79 Compare May 25, 2016 21:24
@chalin
Copy link
Contributor Author

chalin commented May 25, 2016

@kwalrath : can this be merged, if not what can I do to help make that happen? (I'd like to get the changes to _util-fns.jade for work on other chapter).

@kwalrath kwalrath merged commit a664f46 into angular:master May 25, 2016
@chalin chalin deleted the chalin-guide-template-syntax-0521 branch May 25, 2016 22:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants