docs(toh-6): refactoring of 'add, edit, delete heroes' #2170
Conversation
| +makeExcerpt(_appModuleTsVsMainTs, 'in-mem-web-api', '') | ||
| :marked | ||
| The `forRoot` configuration method takes an `InMemoryDataService` class | ||
| that will prime the in-memory database as follows: |
There was a problem hiding this comment.
will prime -> primes
(no need for future tense here)
|
@kwalrath: a first pass at updated prose (see original comment for details). Once we settle on the prose we can ping the TS folks to review as well. |
|
@kwalrath: post-review edits submitted. |
| :marked | ||
| The `HeroDetailComponent` does most of the work. All we do is toggle an `*ngIf` flag that | ||
| swaps it into the DOM when we add a hero and removes it from the DOM when the user is done. | ||
| The delete HTTP method will be used to delete heroes and its invocation is like to `put` except for the method name: |
There was a problem hiding this comment.
This is hard to read and "like to put" isn't quite right. How about:
The hero service's delete method` uses the delete HTTP method to remove the hero from the server:
There was a problem hiding this comment.
Oh, that original sentence certainly wasn't right. Fixed
chalin
force-pushed
the
chalin-toh-6-hero-CUD-feature-silo-0822
branch
from
84a61a7 to
bafc01f
Compare
| .l-main-section | ||
| :marked | ||
| ## Add, Edit, Delete | ||
| ### Add, edit, delete heroes |
There was a problem hiding this comment.
Why is this ### instead of ##?
There was a problem hiding this comment.
Because this introduces the fact that we will be adding support for those three features.
Each feature is then a section at level 2.
There was a problem hiding this comment.
This heading used to be directly tied to the order of the sections to follow (post, put, delete, and then a save section that combined the first two). Now I don't even see "post(" in the Dart page unless I select lib/hero_service.dart at the bottom of the page.
I think this rewrite needs some more connecting material, plus a careful review to make sure it includes everything it needs.
(Which reminds me... I noticed that some of the earlier samples got longer, so we need to also make sure that this page doesn't include anything unnecessary. /*...*/ can be a good thing, reducing complexity and scariness.)
chalin
force-pushed
the
chalin-toh-6-hero-CUD-feature-silo-0822
branch
from
bafc01f to
e8c33a2
Compare
chalin
force-pushed
the
chalin-toh-6-hero-CUD-feature-silo-0822
branch
from
e8c33a2 to
8c47c86
Compare
|
@wardbell @Foxandxss @thelgevold: this is ready for TS-side review. Please see original PR comment for a description of the simplifications that have been made. I've squashed the post-Dart-review updates into the ts-only commit, so if you want, you can review only the TS-specific commit. |
|
I like what I see but I will honestly prefer if @wardbell has a say on this too. |
|
Elsewhere @thelgevold wrote: I think your edits look good, but let's wait for @wardbell's feedback. |
chalin
force-pushed
the
chalin-toh-6-hero-CUD-feature-silo-0822
branch
from
8c47c86 to
fc426b2
Compare
|
I will review. I thought I could just skim it but there's more going on than I can grok in a few minutes. Keep on me! |
Refactoring of "add, edit, delete heroes" section of toh-6 from one big bottom-up step into small independent feature slices, where the user achieves a "milesone" (i.e., can run the full app) after each feature section. The section rewrite is shorter and offers a better UX. Other simplifications: - Error handling is consistent: in the hero service we log to the console, everwhere else we just let errors bubble up. - Hero service methods renamed based on function (create, update) rather then lower-level implementation (post, put). - @output properties have been eliminated (since they weren't explained). E2E tests now pass on both the TS and Dart sides.
Refactoring of "add, edit, delete heroes" section of toh-6 from one big bottom-up step into small independent feature slices, where the user achieves a "milesone" (i.e., can run the full app) after each feature section. The section rewrite is shorter and offers a better UX. Other simplifications: - Error handling is consistent: in the hero service we log to the console, everwhere else we just let errors bubble up. - Hero service methods renamed based on function (create, update) rather then lower-level implementation (post, put). - @output properties have been eliminated (since they weren't explained). E2E tests now pass on both the TS and Dart sides. Post-Dart-review updates included.
wardbell
force-pushed
the
chalin-toh-6-hero-CUD-feature-silo-0822
branch
from
fc426b2 to
391806d
Compare
|
We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm. |
1 similar comment
|
We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm. |
|
Add my tweaks. Merging. |
Refactoring of "add, edit, delete heroes" section of toh-6 from one big bottom-up step into small independent feature slices, where the user achieves a "milesone" (i.e., can run the full app) after each feature section. The section rewrite is shorter and offers a better UX.
Other simplifications:
@Outputproperties have been eliminated (since they weren't explained).E2E tests now pass on both the TS and Dart sides.
Contributes to #1619.