docs(toh-6/ts): minor edits and enhancements #1686

chalin · 2016-06-17T22:34:55Z

Changes to prose:

Complete TODO item of displaying heroes.component errors.
Mainly copyedits.
Add of block statements so that prose can be used on Dart side.
Show excerpt and briefly explain of changes (previously missing):
- app/hero-detail.component.html
- app/heroes.component.ts error handling
Add missing file to changed/added files listing and makeTabs
- `toh-6/ts/app/in-memory-data.service.ts,
- toh-6/ts/sample.css

Code changes:

Mainly copyedits
Renamed heroes.component.ts delete to deleteHero to match
naming of other methods

Contributes to #1628.

chalin · 2016-06-17T22:37:51Z

@kwalrath @Foxandxss @johnpapa @wardbell : ready for review. As usual, because of the addition of blocks causes changes in indentation, it is best to review changes by ignoring whitespace, like this. Please see the original PR comment for a description of changes (besides copyedits).

cc @thelgevold

NOTE: this PR depends on angular#1686. Dart prose and example match TS except that: - No child-to-parent event emission occurs. - Support for Add Hero is added as an unconditional feature of the Heroes view. - http `_post` takes only a name - http `delete` takes only a hero id. - The Dart in-memory-data-service has been dropped in favor of an implementation based on the "standard" `http.testing.MockClient` class.

kwalrath · 2016-06-17T23:05:33Z

The copyedits look reasonable to me, but I'll leave the content review to the TS experts.

Foxandxss · 2016-06-18T13:36:30Z

public/docs/ts/latest/tutorial/toh-pt6.jade

+  .alert.is-helpful
+    :marked
+      This chapter is an introduction to the !{_Angular_http_library}.
+      Please don't be distracted by the details of this backend substitution. Just follow along with the example.


I personally don't like this two lines. Perhaps the tone.

I agree, but that was in the original TS text so I left it as is (but excluded it from the Dart edition). Remember that for this time, my goal was mainly to proof read, not a general edit.

Foxandxss · 2016-06-18T13:39:00Z

I left two comments on the prose, but the rest are good to me 👍

chalin · 2016-06-19T00:25:38Z

@kwalrath @Foxandxss : thank you both for reviewing this.
@naomiblack : it would be great if this could be merged before @johnpapa starts carving out the CLI editions.

chalin · 2016-06-19T00:39:16Z

I was reviewing my notes and noticed I had forgotten to remove an unnecessary change to a .css, relative to toh-5 (new #docregion that wasn't used). I just pushed a commit with that change.

Adam-Meisen · 2016-06-25T05:52:08Z

Is sample.css ever explicitly mentioned in the tutorial? I only realized it existed while trying to figure out why my delete buttons didn't look like they do in the examples. In fact, styling the delete buttons seems to be all that sample.css does.

Changes to prose: - Complete TODO item of displaying `heroes.component` errors. - Mainly copyedits. - Add of blocks statements so that prose can be used on Dart side. - Show excerpt and briefly explain of changes (previously missing): - `app/hero-detail.component.html` - `app/heroes.component.ts` error handling - Add missing file to changed/added files listing and makeTabs - `toh-6/ts/app/in-memory-data.service.ts, - `toh-6/ts/sample.css` Code changes: - Mainly copyedits - Renamed `heroes.component.ts` `delete` to `deleteHero` to match naming of other methods

chalin · 2016-06-26T18:46:37Z

@kwalrath : rebased.

Hi @Adam-Meisen : while it is true that sample.css is not mentioned in the prose, using a sample.css file is a convention followed in other chapters in the Dev Guide and the Cookbook.

It may make sense, as you have proposed, to merge the sample.css styles into one of the component's styles, but before doing so it may make sense to determine if there was a reason for doing it the way it is now.

NOTE: this PR depends on angular#1686. Dart prose and example match TS except that: - No child-to-parent event emission occurs. - Support for Add Hero is added as an unconditional feature of the Heroes view. - http `_post` takes only a name - http `delete` takes only a hero id. - The Dart in-memory-data-service has been dropped in favor of an implementation based on the "standard" `http.testing.MockClient` class.

Adam-Meisen · 2016-06-26T19:25:34Z

But then why does the file just appear at the end of part 6 without being
mentioned? Why does a stylesheet linked to in index.html contain contain
rules specific to a single component?
On Jun 26, 2016 1:46 PM, "Patrice Chalin" [email protected] wrote:

@kwalrath https://github.com/kwalrath : rebased.

Hi @Adam-Meisen https://github.com/Adam-Meisen : while it is true that
sample.css is not mentioned in the prose, using a sample.css file is a
convention followed in other chapters in the Dev Guide and the Cookbook.

It may make sense, as you have proposed, to merge the sample.css styles
into one of the component's styles, but before doing so it may make sense
to determine if there was a reason for doing it the way it is now.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1686 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AA-2UEbc8v1U4TSHpWeeTgnjfqwgKrYWks5qPskRgaJpZM4I4yIc
.

chalin · 2016-06-26T19:38:44Z

Adam: btw, I agree with your observations, including the idea of eliminating the sample.css file as you have proposed already. I was just suggesting that we (docs team, including myself) first take a broader look to understand why the current approach exists both for this chapter and the other chapters. (As to the lack of mention of sample.css, that could be quickly fixed by adding an appropriate sentence to the prose.)

Adam-Meisen · 2016-06-26T21:52:50Z

Sorry if I came off as accusatory, I agree with you.

If it helps, sample.css first appeared in e4db340464f5955670c0ce4e61587875a7d334b3 (merged in PR #1066) and has not been touched by any commit since. It seems that either no one noticed it in the PR, or no one thought it was worth mentioning.

chalin · 2016-06-26T23:00:24Z

No worries (I could have been clearer from the start). Thanks for that info.

kwalrath · 2016-06-27T18:14:44Z

Any objections to me merging this change? @Foxandxss?

Foxandxss · 2016-06-27T18:22:48Z

Works for me.

NOTE: this PR depends on angular#1686. Dart prose and example match TS except that: - No child-to-parent event emission occurs. - Support for Add Hero is added as an unconditional feature of the Heroes view. - http `_post` takes only a name - http `delete` takes only a hero id. - The Dart in-memory-data-service has been dropped in favor of an implementation based on the "standard" `http.testing.MockClient` class.

* docs(toh-6/dart): first edition of prose and example code NOTE: this PR depends on #1686. Dart prose and example match TS except that: - No child-to-parent event emission occurs. - Support for Add Hero is added as an unconditional feature of the Heroes view. - http `_post` takes only a name - http `delete` takes only a hero id. - The Dart in-memory-data-service has been dropped in favor of an implementation based on the "standard" `http.testing.MockClient` class. * post-review changes

googlebot added the CLA: yes label Jun 17, 2016

chalin mentioned this pull request Jun 17, 2016

Proof read toh-1 to -6 #1628

Closed

6 tasks

chalin force-pushed the chalin-toh-6-ts-minor-edits-0614 branch from 533d021 to c813689 Compare June 17, 2016 22:53

chalin mentioned this pull request Jun 17, 2016

docs(toh-6/dart): first edition of prose and example code #1687

Merged

Foxandxss reviewed Jun 18, 2016
View reviewed changes

Foxandxss added the action: review label Jun 18, 2016

Adam-Meisen mentioned this pull request Jun 26, 2016

docs(toh-6) move styles from sample.css to heroes.component.css #1751

Closed

chalin added 2 commits June 26, 2016 11:36

remove unnecessary change relative to toh-5

78a7a10

chalin force-pushed the chalin-toh-6-ts-minor-edits-0614 branch from d4b135c to 78a7a10 Compare June 26, 2016 18:36

Foxandxss added the pr_state: LGTM label Jun 27, 2016

kwalrath merged commit 3fe0fb0 into angular:master Jun 27, 2016

chalin deleted the chalin-toh-6-ts-minor-edits-0614 branch June 27, 2016 21:16

chalin mentioned this pull request Jun 28, 2016

docs(util-fns): generalize getExampleName() #1666

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs(toh-6/ts): minor edits and enhancements #1686

docs(toh-6/ts): minor edits and enhancements #1686

chalin commented Jun 17, 2016 •

edited

Loading

chalin commented Jun 17, 2016 •

edited

Loading

kwalrath commented Jun 17, 2016

Foxandxss Jun 18, 2016

chalin Jun 19, 2016

Foxandxss commented Jun 18, 2016

chalin commented Jun 19, 2016

chalin commented Jun 19, 2016

Adam-Meisen commented Jun 25, 2016 •

edited

Loading

chalin commented Jun 26, 2016

Adam-Meisen commented Jun 26, 2016

chalin commented Jun 26, 2016

Adam-Meisen commented Jun 26, 2016

chalin commented Jun 26, 2016

kwalrath commented Jun 27, 2016

Foxandxss commented Jun 27, 2016

docs(toh-6/ts): minor edits and enhancements #1686

docs(toh-6/ts): minor edits and enhancements #1686

Conversation

chalin commented Jun 17, 2016 • edited Loading

chalin commented Jun 17, 2016 • edited Loading

kwalrath commented Jun 17, 2016

Foxandxss Jun 18, 2016

Choose a reason for hiding this comment

chalin Jun 19, 2016

Choose a reason for hiding this comment

Foxandxss commented Jun 18, 2016

chalin commented Jun 19, 2016

chalin commented Jun 19, 2016

Adam-Meisen commented Jun 25, 2016 • edited Loading

chalin commented Jun 26, 2016

Adam-Meisen commented Jun 26, 2016

chalin commented Jun 26, 2016

Adam-Meisen commented Jun 26, 2016

chalin commented Jun 26, 2016

kwalrath commented Jun 27, 2016

Foxandxss commented Jun 27, 2016

chalin commented Jun 17, 2016 •

edited

Loading

chalin commented Jun 17, 2016 •

edited

Loading

Adam-Meisen commented Jun 25, 2016 •

edited

Loading