Repository: angular
Issue: docs: comments/suggestions/typos for the Angular tutorials on MDN
Labels: feature,hotlist: devrel,P3,area: docs

Below are some comments/suggestions/typos for the [Angular tutorial series on MDN](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Client-side_JavaScript_frameworks#angular_tutorials):

##
Throughout the tutorial series:

- [ ] `to do` and `to-do` are used inconsistently :scream: :scream: :scream: :innocent:


##
In [Getting started with Angular](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Client-side_JavaScript_frameworks/Angular_getting_started):

- [ ] Suggestion:
  ```diff
  -This guide uses the [npm client](https://docs.npmjs.com/cli/install) command line interface
  +This guide uses the [npm client](https://www.npmjs.com/get-npm) command line interface
  ```
  I believe `https://www.npmjs.com/get-npm` is a better URL (which is also what we use in our `README.md`). The current URL (`https://docs.npmjs.com/cli/install`) is about the `npm install` command and not about installing the `npm` CLI.

- [ ] Suggestion:
  ```diff
  -which is installed with `Node.js` by default
  +which is installed with Node.js by default
  ```
  ...for consistency, since "Node.js" is used without backticks in several other places in the tutorial.

- [ ] The tutorial recommends creating a project in the `Desktop` directory. I think this is not a great idea, because not all OSed have a `Desktop` directory and for some that do have one (such as Windows), it is not a good practice to create large directories on the `Desktop` (as it can slow down the whole system).

- [ ] Nit (it is more common to use uppercase letters in key combinations):
  ```diff
  -press `Ctrl+c` while in the terminal
  +press `Ctrl+C` while in the terminal
  ```
  Also, does this work on macOS?

- [ ] WRT _"`app.component.ts`: Also known as the class"_:
  That sounds weird :grin: I've never heard anyone calling this "the class" :smiley:
  Cold this be a typo (for example, missing a word or something)? Maybe _"`AppComponent` class"_ or _"component class"_?

- [ ] WRT _"TypeScript offers [...] a more concise syntax than plain JavaScript"
  That's quite a controversial statement. TypeScript offers many benefits, but coinsisement over plain JavaScript is hardly one of them :grin:

- [ ] Suggestion:
  ```diff
  -  // the following metadata specifies the location of the other parts of the component
  -templateUrl: './item.component.html',
  +// the following metadata specifies the location of the other parts of the component
  +templateUrl: './item.component.html',
  ```

- [ ] Suggestion:
  ```diff
   })
  -
   export class ItemComponent {
  ```

- [ ] Suggestion:
  ```diff
   })
  -
   export class AppComponent {
  ```
  (3x)

- [ ] WRT _"write your HTML within backticks"_:
  That sounds arbitrary :grin: It is not necessary to use backticks (vs regular single/double quotes) and linters often bark at you if you unnecessarily use backticks. The template is a regular string literal and we should not recommend a particular way of creating it imo.

- [ ] Suggestion:
  ```diff
   export class AppComponent {
  -    title = 'To do application';
  +  title = 'To do application';
   }
  ```


##
In [Beginning our Angular todo list app](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Client-side_JavaScript_frameworks/Angular_todo_list_beginning):

- [ ] Suggestion:
  ```diff
   })
  -
   export class AppComponent {
  ```

- [ ] Suggestion:
  ```diff
  -Here it is, './app.component.html',
  +Here it is `'./app.component.html'`,
  ```

- [ ] WRT `export class AppComponent { /* ... */ get items() { ... } }`:
  I don't think it is a good idea to use a getter like this. It doesn't have any benefit over a regular method (afaict) and it covers up the fact that accessing the property results in a function invocation.
  Also, I don't think it is a good idea to use an array-returning function with `*ngFor` as it can result in many unnecessary invocations. I believe it is better to use a property (and update the value of that property every time the filter criteria change).

- [ ] Suggestion:
  ```diff
  -which calls the same`addItem()` method.
  +which calls the same `addItem()` method.
  ```

- [ ] WRT _"Pressing the **Enter** key also resets the value of `<input>` to an empty string. Alternatively, the user can click the **Add** button which calls the same `addItem()` method."_:
  When pressing the **Add** button, the input's value is nore reset. It sounds like a bad idea to have different behavior when pressing **Enter** vs clicking the **Add** button.
  One solution could be to pass the `newItem` `<input>` into the `addItem()` method and let it reset the value to the empty string.


##
In [Styling our Angular app](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Client-side_JavaScript_frameworks/Angular_styling):

- [ ] Adding styles for `body` in the `app.component.css` is certainly not a good idea (and does not have any effect in most cases - i.e. unless one uses [ViewEncapsulation.None](https://angular.io/api/core/ViewEncapsulation#None)).

- [ ] Suggestion:
  ```diff
  - ul li {
  -    list-style: none;
  +ul li {
  +  list-style: none;
   }
  ```

- [ ] I think it is a good idea to make it more explicit that styles in a component CSS file only affect elements inside that component's template (and nothing outside the component) - by default at least.


##
In [Creating an item component](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Client-side_JavaScript_frameworks/Angular_item_component):

- [ ] Suggestion:
  ```diff
  -functionality. the Angular event model is covered here.
  +functionality. The Angular event model is covered here.
  ```

- [ ] WRT _"The template variable, `#editedItem`, on the `<input>` means that Angular stores whatever a user types in this `<input>` in a variable called `editedItem`."_
  This is not very accurate. Angular stores a reference to the `<input>` element in `editedItem`. (From that, we can access whatever the user has typed in the `<input>` via the element's `value` property: `editedItem.value`)

- [ ] Suggestion:
  ```diff
  -on communication the `AppComponent` and the `ItemComponent`
  +on communication of/between the `AppComponent` and the `ItemComponent`
  ```

- [ ] Suggestion:
  ```diff
  -Configure the AppComponent first
  +Configure the `AppComponent` first
  ```

- [ ] WRT _"this.allItems.splice(this.allItems.indexOf(item), 1);"_:
  This is not a safe way to remove an item, because if the item is not in the list, then the last item will be removed.
  A better way is `this.allItems = this.allItems.filter(otherItem => otherItem !== item);`.

- [ ] Suggestion:
  ```diff
  -remove one item at at the `indexOf` the relevant item
  +remove one item at the `indexOf` the relevant item
  ```

- [ ] WRT _"which is bound to the `remove` property in the `ItemComponent`"_:
  This is a little confusing imo. It gives the impression that you can somehow bind methods to arbitrary child-component properties, which is not true. `ItemComponent#remove` is not a regular property; it is an `@Output`.
  Maybe change that clause to _"which is bound to the `remove` output in the `ItemComponent`"_

- [ ] WRT _"The following CSS adds basic styles, flexbox for the buttons, and custom checkboxes."_:
  This is a little confusing as well, since the styles below have very little to do with Flexbox for buttons. Maybe we meant _"colors for buttons"_?


##
In [Filtering our to-do items](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Client-side_JavaScript_frameworks/Angular_filtering):

- [ ] Nit:
  ```diff
  -filter == 'all'/'active'/'done'
  +filter === 'all'/'active'/'done'
  ```

In [MDN: Building Angular applications and further resources](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Client-side_JavaScript_frameworks/Angular_building):

- [ ] Suggestion:
  ```diff
  -**Objective:**	To learn how to build your Angular app.
  -**Objective:**	To learn how to build and deploy your Angular app.
  ```

- [ ] Suggestion:
  ```diff
  -ng  build --prod
  +ng build --prod
  ```

- [ ] I think it would be worth mentioning the various deploy schematics that allow you to deploy your app to various backends (sudh as GitHub pages, Firebase, Netlify, etc.) with a single command.