-
Notifications
You must be signed in to change notification settings - Fork 877
docs(toh-5): TS/Dart review, and Dart resync #2115
Conversation
@wardbell @Foxandxss @brandonroberts @kwalrath: ready for review. As usual, Jade files are best viewed by ignoring whitespace. |
cc @kapunahelewong -- I included some of your copy edits, but mostly left my revision as is. I think that some / most of your concerns were addressed; others not (e.g. adding a complete set of new/changed files at the end of the chapter -- I was missing that too, but I can let you make the addition :). |
|
||
import 'hero_service.dart'; | ||
import 'heroes_component.dart'; | ||
|
||
// #enddocregion v2 | ||
@Component( |
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.
I wondered about having two @Component
annotations for a single class. Sure enough, when I try to pub serve
or pub build
this, I get a build error:
Build error:
Transform DirectiveProcessor on angular2_tour_of_heroes|lib/app_component_1.dart threw error: Only one Directive is allowed per class. Found unexpected "@Component(selector: 'my-app', template: '''
<h1>{{title}}</h1>
<a [routerLink]="['Heroes']">Heroes</a>
<router-outlet></router-outlet>''', directives: const [HeroesComponent], providers: const [HeroService])".
@Component(selector: 'my-app', template: '''
<h1>{{title}}</h1>
...
Does the serve/build work without errors for you?
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.
Fixing that now.
2f7822c
to
fdfb011
Compare
We'll add a third option, a `goBack` method that navigates backward one step in the browser's history stack | ||
We'll add a third option, a `goBack` method that navigates backward one step in the browser's history stack. | ||
|
||
+makeExcerpt('app/hero-detail.component.ts', 'goBack') |
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.
This is an excerpt but has no "..." to indicate that code is missing. (It's missing imports, the OnInit implementation, @Component
, ...)
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.
I guess I'd either use the "..." or find another way to make clear that you need to import dart:html to gain access to window
.
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.
Added:
We'll also need to import
dart:html
to gain access towindow
.import 'dart:html';
df68dd6
to
0ec8b82
Compare
LGTM! |
LGTM |
**NOTE: run `gulp add-example-boilerplate` after pulling in the commit.** This is preparatory work for angular#2035. As part of the the chapter review, the Dart .jade was enhanced to use Jade extends (angular#2018). By the same token it contributed to a post-RC5 resync (angular#2077). Other key changes: Dart and TS code: - Eliminated `styles.1.css` in favor of docregions in `styles.css`. - `docregion` tags renamed in a few places. - **No other code changes**. TS prose - Fixed: misnamed variable `routing` -> `appRoutes`. - All other changes are **minor copy edits**, or changes to support Dart via Jade extends. Diff of generated HTML for TS chapter was inspected to ensure only minor copy edits prevailed (i.e., that the support for Jade extends had no impact on the generated HTML).
- Some adjustments following actually doing the tutorial. In some cases code shown (e.g. this is what file foo should look like now) didn't match what the user would have. E.g., lingering @input on the hero property. - Fixed some lingering deprecated-router prose elements on TS side (e.g., still referring to a route by the old string names like `HeroDetail`). - Added extra step to `app.component.ts` creation rather than having a critical-call-out later on. - Reorder some prose for better harmony between TS and Dart prose (also improves the flow). - Moved the `styleUrls` call-out to the point of first use.
0ec8b82
to
663c5c6
Compare
663c5c6
to
9b079b7
Compare
NOTE: run
gulp add-example-boilerplate
after pulling in the commit.This PR has two major commits:
Lint reports no errors and e2e tests pass.
1. Commit: TS review and update/resync Dart
This is preparatory work for #2035. As part of the chapter review, the Dart .jade was enhanced to use Jade extends (#2108). These edits also contributed to a post-RC5 resync (#2077). Other key changes:
Dart and TS code:
styles.1.css
in favor of docregions instyles.css
.docregion
tags renamed in a few places.Dart code:
TS prose
HeroesComponent
was being added twice.Diff of generated HTML for TS chapter was inspected to ensure only minor copy edits prevailed (i.e., that the support for Jade extends had no impact on the generated HTML).
2. Commit: edits after doing tutorial
HeroDetail
).styleUrls
call-out to the point of first use.