docs: make structural directives guide generic #44895

dario-piotrowicz · 2022-01-29T21:27:07Z

tweak the current structural directives guide (currently mainly targeted
at the creation of custom structural directives) so that is more generic
and a point of reference for structural directives in general

this also includes the re-addition of the one-per-element section
removed in PR #40015

resolves #44786

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Bugfix
Feature
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Documentation content changes
angular.io application / infrastructure changes
Other... Please describe:

What is the current behavior?

Issue Number: #44786

Does this PR introduce a breaking change?

Yes
No

Other information

@AndrewKushnir 🙂👍
Before & After:
Before:

After:
The "Improving template type checking for custom directives" section should be moved to a different guide, I figured I could keep things cleared by moving it away from the page in a follow-up PR
This partially undoes changes made in PR docs: rewrite structural directives #40015

dario-piotrowicz · 2022-01-29T21:28:10Z

aio/content/examples/structural-directives/src/app/app.component.html

@@ -110,15 +110,20 @@ <h2 id="ngFor">NgFor</h2>

 <p class="code">&lt;div *ngFor="let hero of heroes; let i=index; let odd=odd; trackBy: trackById" [class.odd]="odd"&gt;</p>
 <!--#docregion inside-ngfor -->
-<div *ngFor="let hero of heroes; let i=index; let odd=odd; trackBy: trackById" [class.odd]="odd">
+<div


I split this and the one below because in the UI the lines were easily wrapped, this should marginally help

dario-piotrowicz · 2022-01-29T21:29:22Z

aio/content/guide/built-in-directives.md

@@ -240,15 +240,12 @@ then uses this template repeatedly to create a new set of elements and bindings
 in the list.
 For more information about shorthand, see the [Structural Directives](guide/structural-directives#shorthand) guide.

-{@a one-per-element}


this section isn't really explaining clearly the one-per-element topic, a section just for that has been added in the structural directives guide.

dario-piotrowicz · 2022-01-29T21:30:22Z

aio/content/guide/built-in-directives.md


-For more information about `NgFor` see the [NgForOf API reference](api/common/NgForOf).


this guide already talks about the NgFor in the previous sections, so putting here the link seems to me pretty unnecessary

dario-piotrowicz · 2022-01-29T21:30:46Z

aio/content/guide/structural-directives.md


 <div class="alert is-helpful">

 For the example application that this page describes, see the <live-example></live-example>.

 </div>

-For more information on Angular's built-in structural directives, such as `NgIf`, `NgForOf`, and `NgSwitch`, see [Built-in directives](guide/built-in-directives).
+## What they are


may not be the best section name ever.... 😓

You could eliminate the need for this header by moving lines 13-15 to above the example.
Because that's the obvious place to see a description of the subject you're about to discuss further in the topic, it doesn't need a heading to introduce it.

dario-piotrowicz · 2022-01-29T21:34:12Z

aio/content/guide/structural-directives.md

+{@a one-per-element}
+## One structural directive per element


This section was removed in PR #40015 (which by the way is the PR that made this guide non-generic)

I am not a big fan of moving changes back and forth but I believe the way this was refactored really just lacks clarity and makes things much more difficult to understand.

I put back the section with one or two minor tweaks as I thought it was already well written, if we want I can rewrite this with my own words too.

dario-piotrowicz · 2022-01-29T21:36:17Z

aio/content/guide/structural-directives.md

@@ -295,12 +309,14 @@ The value of the property can be either a general type-narrowing function based
 For example, consider the following structural directive that takes the result of a template expression as an input:

 <code-example language="ts" header="IfLoadedDirective">
-export type Loaded<T> = { type: 'loaded', data: T };


I updated this to use < and > since without them the type annotations are actually not displayed:

(PS: I also split the lines to help in readability)

dario-piotrowicz · 2022-01-29T21:39:37Z

aio/content/guide/structural-directives.md

-### Creating template fragments with `<ng-template>`
-
-Angular's `<ng-template>` element defines a template that doesn't render anything by default.
-With `<ng-template>`, you can render the content manually for full control over how the content displays.
-
-If there is no structural directive and you wrap some elements in an `<ng-template>`, those elements disappear.
-In the following example, Angular does not render the middle "Hip!" in the phrase "Hip! Hip! Hooray!" because of the surrounding `<ng-template>`.
-
-<code-example path="structural-directives/src/app/app.component.html" header="src/app/app.component.html (template-tag)" region="template-tag"></code-example>
-
-<div class="lightbox">
-  <img src='generated/images/guide/structural-directives/template-rendering.png' alt="template tag rendering">
-</div>
-


I don't think this should sit in the structural directives guide, I changed this (see line 70 above) to a quick alert redirecting to the ng-template api docs (in which I also added a shortened alert with the example shown here)

dario-piotrowicz · 2022-01-30T20:58:41Z

I just noticed that I am applying the fix for: #40739

in that issue it is suggested to move the example to a doc region, my fixing it here shouldn't prevent that from being done afterwards anyways (maybe as a follow-up PR, as I mentioned in the PR's description the section should also be moved elsewhere, maybe both things can be done in the same PR)

(PS: I volunteer myself for that by the way 😄)

AndrewKushnir

Awesome work @dario-piotrowicz 👍

I've left a few comments, please let me know what you think.

Once the feedback is addressed, we'd need a docs team review of the content as well.

Thank you.

aio/content/guide/structural-directives.md

aio/content/special-elements/core/ng-template.md

dario-piotrowicz · 2022-02-12T11:59:11Z

@AndrewKushnir thanks a lot for your awesome reviewing! 😍

(and sorry for making you put that many comments 🙏 😓)

I've addressed your comments, please have another look 🙂

aio/content/guide/structural-directives.md

atscott · 2022-02-14T19:36:08Z

Added @TeriGlover for editing review.

dario-piotrowicz · 2022-02-14T23:43:47Z

Thanks for the nice reviewing @atscott 🙂, all addressed 👍

dario-piotrowicz · 2022-03-29T22:33:50Z

Hi @TeriGlover, any change you could have a look at this PR? 🙇 🙏 🙇

AndrewKushnir · 2022-05-19T21:56:06Z

@bob-watson could you please help with the review for this PR from docs perspective (Andrew and myself verified that the context is correct from the technical perspective)? Thank you.

bob-watson

If the "what they are" heading really bugs you, I offered an alternative.
Other than that, it seems reasonable after a quick glance.

bob-watson · 2022-05-19T22:27:49Z

aio/content/guide/structural-directives.md


 <div class="alert is-helpful">

 For the example application that this page describes, see the <live-example></live-example>.

 </div>

-For more information on Angular's built-in structural directives, such as `NgIf`, `NgForOf`, and `NgSwitch`, see [Built-in directives](guide/built-in-directives).
+## What they are


You could eliminate the need for this header by moving lines 13-15 to above the example.
Because that's the obvious place to see a description of the subject you're about to discuss further in the topic, it doesn't need a heading to introduce it.

tweak the current structural directives guide (currently mainly targeted at the creation of custom structural directives) so that is more generic and a point of reference for structural directives in general this also includes the re-addition of the one-per-element section removed in PR angular#40015 resolves angular#44786

ngbot · 2022-05-20T01:05:02Z

I see that you just added the action: merge label, but the following checks are still failing:
status "pullapprove" is failing
3 pending code reviews

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken main, please try rebasing to main and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

AndrewKushnir · 2022-05-20T01:06:03Z

Merge-assistance: the approvals got reset after a fixup commit. I've reviewed and approved it, so this PR is ready for merge.

alxhub · 2022-05-23T20:44:50Z

This PR was merged into the repository by commit c8086b7.

tweak the current structural directives guide (currently mainly targeted at the creation of custom structural directives) so that is more generic and a point of reference for structural directives in general this also includes the re-addition of the one-per-element section removed in PR #40015 resolves #44786 PR Close #44895

amend the aio glossary using as the structural-directives guide link's title the outdated heading of that guide (changed in PR angular#44895)

) amend the aio glossary using as the structural-directives guide link's title the outdated heading of that guide (changed in PR #44895) PR Close #46228

angular-automatic-lock-bot · 2022-06-23T16:06:47Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

dario-piotrowicz commented Jan 29, 2022

View reviewed changes

dario-piotrowicz force-pushed the aio-general-structural-directives-guide branch from 1278b26 to a886017 Compare January 29, 2022 21:46

dario-piotrowicz marked this pull request as ready for review January 30, 2022 12:18

pullapprove bot requested a review from atscott January 30, 2022 12:18

jessicajaniuk added the comp: docs label Jan 31, 2022

ngbot bot added this to the Backlog milestone Jan 31, 2022

AndrewKushnir self-requested a review January 31, 2022 18:34

AndrewKushnir reviewed Feb 11, 2022

View reviewed changes

dario-piotrowicz requested a review from AndrewKushnir February 12, 2022 11:59

atscott requested changes Feb 14, 2022

View reviewed changes

atscott requested a review from TeriGlover February 14, 2022 19:35

dario-piotrowicz requested a review from atscott February 14, 2022 23:45

dario-piotrowicz force-pushed the aio-general-structural-directives-guide branch from 331b00c to 46800ff Compare February 26, 2022 11:41

dario-piotrowicz force-pushed the aio-general-structural-directives-guide branch from 46800ff to fcda1d0 Compare March 29, 2022 22:34

dario-piotrowicz force-pushed the aio-general-structural-directives-guide branch 2 times, most recently from d3deed3 to 37d041b Compare April 11, 2022 18:38

jessicajaniuk added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Apr 11, 2022

bob-watson reviewed May 19, 2022

View reviewed changes

dario-piotrowicz added 4 commits May 19, 2022 23:57

fixup! docs: make structural directives guide generic

4688148

fixup! docs: make structural directives guide generic

4f8db48

fixup! docs: make structural directives guide generic

dc1641e

dario-piotrowicz force-pushed the aio-general-structural-directives-guide branch from 37d041b to dc1641e Compare May 19, 2022 23:00

AndrewKushnir approved these changes May 20, 2022

View reviewed changes

AndrewKushnir added the action: rerun CI at HEAD label May 20, 2022

ngbot bot removed the action: rerun CI at HEAD label May 20, 2022

AndrewKushnir added action: rerun CI at HEAD and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels May 20, 2022

ngbot bot removed the action: rerun CI at HEAD label May 20, 2022

AndrewKushnir added the action: merge The PR is ready for merge by the caretaker label May 20, 2022

AndrewKushnir added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label May 20, 2022

alxhub closed this in c8086b7 May 23, 2022

dario-piotrowicz mentioned this pull request Jun 2, 2022

Aio structural directives small amends #46228

Closed

14 tasks

angular-automatic-lock-bot bot locked and limited conversation to collaborators Jun 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: make structural directives guide generic #44895

docs: make structural directives guide generic #44895

dario-piotrowicz commented Jan 29, 2022 •

edited

dario-piotrowicz Jan 29, 2022

dario-piotrowicz Jan 29, 2022

dario-piotrowicz Jan 29, 2022

dario-piotrowicz Jan 29, 2022

bob-watson May 19, 2022

dario-piotrowicz Jan 29, 2022

dario-piotrowicz Jan 29, 2022

dario-piotrowicz Jan 29, 2022

dario-piotrowicz commented Jan 30, 2022 •

edited

AndrewKushnir left a comment

dario-piotrowicz commented Feb 12, 2022

atscott commented Feb 14, 2022

dario-piotrowicz commented Feb 14, 2022

dario-piotrowicz commented Mar 29, 2022

AndrewKushnir commented May 19, 2022

bob-watson left a comment

bob-watson May 19, 2022

ngbot bot commented May 20, 2022

AndrewKushnir commented May 20, 2022

alxhub commented May 23, 2022

angular-automatic-lock-bot bot commented Jun 23, 2022


		For more information about `NgFor` see the [NgForOf API reference](api/common/NgForOf).

docs: make structural directives guide generic #44895

docs: make structural directives guide generic #44895

Conversation

dario-piotrowicz commented Jan 29, 2022 • edited

PR Checklist

PR Type

What is the current behavior?

Does this PR introduce a breaking change?

Other information

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dario-piotrowicz commented Jan 30, 2022 • edited

AndrewKushnir left a comment

Choose a reason for hiding this comment

dario-piotrowicz commented Feb 12, 2022

atscott commented Feb 14, 2022

dario-piotrowicz commented Feb 14, 2022

dario-piotrowicz commented Mar 29, 2022

AndrewKushnir commented May 19, 2022

bob-watson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ngbot bot commented May 20, 2022

AndrewKushnir commented May 20, 2022

alxhub commented May 23, 2022

angular-automatic-lock-bot bot commented Jun 23, 2022

dario-piotrowicz commented Jan 29, 2022 •

edited

dario-piotrowicz commented Jan 30, 2022 •

edited