docs: comments/suggestions/typos for the Angular tutorials on MDN

[gkalpak]

Below are some comments/suggestions/typos for the Angular tutorial series on MDN:

Throughout the tutorial series:

• to do and to-do are used inconsistently 😱 😱 😱 😇

In Getting started with Angular:

• Suggestion:

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

  -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):

  -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 😁 I've never heard anyone calling this "the class" 😃
  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 😁

• Suggestion:

  -  // 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:

   })
  -
   export class ItemComponent {

• Suggestion:

   })
  -
   export class AppComponent {

  (3x)

• WRT "write your HTML within backticks":
  That sounds arbitrary 😁 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:

   export class AppComponent {
  -    title = 'To do application';
  +  title = 'To do application';
   }

In Beginning our Angular todo list app:

• Suggestion:

   })
  -
   export class AppComponent {

• Suggestion:

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

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

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

• Suggestion:

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

• Suggestion:

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

  -on communication the `AppComponent` and the `ItemComponent`
  +on communication of/between the `AppComponent` and the `ItemComponent`

• Suggestion:

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

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

• Nit:

  -filter == 'all'/'active'/'done'
  +filter === 'all'/'active'/'done'

In MDN: Building Angular applications and further resources:

• Suggestion:

  -**Objective:**	To learn how to build your Angular app.
  -**Objective:**	To learn how to build and deploy your Angular app.

• Suggestion:

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

[destus90]

@gkalpak
As I know, Angular CLI with version 12 generates production build by default. So, the correct syntax is ng build. Anyway, --prod is deprecated in favour of --configuration=production.

angular/angular-cli#20128

[gkalpak]

Thx for the comment, @destus90. What you say is true, but for v12+. The tutorial uses v11.2.5, so we still need to use the --prod flag.
(We could upgrade the tutorial for v12 once that is released as stable.)

[MrJithil]

Do we have a pr for this? Or can we pick it up?

[gkalpak]

I don't think these can be fixed via PRs (since the tutorials leave on the MDN docs).
I'll let the docs team (cc @aikidave) confirm/decide on the need for changes and coordinate their implementation.

[MrJithil]

Okay

[aikithoughts]

Yes, I think those changes have to be made on the MDN site. I can look into when we can get these updates in. Contributions are also always welcome! :)

Thanks for reviewing the content, @MrJithil !

[JeanMeche]

I'm adding the devrel label for awareness if the team like to do some updates on this external page. cc @twerske / @MarkTechson