docs(structural-directives): copyedits and some tweaks to code #3345
Conversation
chalin
force-pushed
the
chalin-struct-directives-copyedits-0304
branch
from
6a35f55 to
cb4726e
Compare
There was a problem hiding this comment.
I like the substantive changes. I added a very few more for you to add. Thanks!
As noted, we should not bother with makeExample -> makeExcerpt because we're converting this anyway. I'll accept it in this PR but not in future PRs because it wastes our time.
However, I can't accept the changes that move the anchors onto the end of l-main-section. I want anchors on their own lines as a#..., exactly as I had them.
Please revert those particular changes.
Then I'm happy to approve and merge.
| <input type="radio" name="heroes" [(ngModel)]="hero" [value]="h">{{h.name}} | ||
| </label> | ||
| </ng-container> | ||
| <label *ngFor="let h of heroes"> |
There was a problem hiding this comment.
Moving the ngFor to the label makes a lot of sense and we should do it UNLESS this is an example of <ng-container>. If it is, please restore. If it isn't, then your change is an improvement.
There was a problem hiding this comment.
It is not an example of <ng-container>. This code is not shown in the page, it is only there to support the ngSwitch code which is shown in the page.
| @@ -1,21 +0,0 @@ | |||
| // interesting but unused code | |||
There was a problem hiding this comment.
Please restore. This is harmless and I wanted to keep it around as an idea that I had. Sure, I'll probably forget all about it (I already did). But humor me.
There was a problem hiding this comment.
Done. (I thought that this file showed up in the plnkr and zip, but I just checked and it doesn't.)
| - [The <template> element](#template) | ||
| - [Write a structural directive](#unless) | ||
|
|
||
| Try the <live-example></live-example>. | ||
|
|
||
| a#definition | ||
| .l-main-section | ||
| .l-main-section#definition |
There was a problem hiding this comment.
Please don't "fix" these. I've created separate anchors on purpose, even where the header text would match the anchor name. I break fewer links this way and don't have to worry about what happens when I change a heading. I can also quickly find something when editing. So these kinds of changes actually make my life harder.
Most importantly, we're converting to pure markdown in v.2 and this makes it harder to figure out what's going on.
TAKE AWAY: Please revert them all and do not make further changes to structure/layout that have no visual effect
There was a problem hiding this comment.
All reverted.
do not make further changes to structure/layout that have no visual effect
Ok.
| +makeExample('structural-directives/ts/src/app/app.component.html', 'ngif')(format=".") | ||
| An asterisk (*) precedes the directive attribute name as in this example. | ||
|
|
||
| +makeExcerpt('src/app/app.component.html', 'ngif', '') |
There was a problem hiding this comment.
Yes, this is shorter and might be worth while if we were staying with Jade.. But we're converting all of this anyway and this kind of move is more of a distraction than a help right now.
I'll accept in THIS PR but please don't do this on future PRs.
|
|
||
| :marked | ||
| * The `*ngIf` directive moved to the `<template>` tag where it became a property binding,`[ngIf]`. | ||
| * The rest of the `<div>`, including its class attribute, moved inside the `<template>` tag. | ||
| * The `*ngIf` directive moved to the `<template>` element where it became a property binding,`[ngIf]`. |
There was a problem hiding this comment.
Note to self: all <template> will become <ng-template> in Angular v.4. I have to think about what this does to what we've been saying here.
| @@ -436,49 +456,50 @@ a#template | |||
| It is never displayed directly. | |||
| In fact, before rendering the view, Angular _replaces_ the `<template>` and its contents with a comment. | |||
|
|
|||
| If there is no structural directive, if you merely wrap some elements in a `<template>` and do nothing with it, | |||
| If there is no structural directive, and you merely wrap some elements in a `<template>` and do nothing with it, | |||
There was a problem hiding this comment.
Ugh. I think the following is better ... we don't need to say "and do nothing with it"
If there is no structural directive and you merely wrap some elements in a
<template>, those elements disappear.
| * that structural directives manipulate HTML layout. | ||
| * to use [`<ng-container>`](#ngcontainer) as a grouping element when there is no suitable host element. | ||
| * that the angular "de-sugars" [asterisk (\*) syntax](#asterisk) into a `<template>`. | ||
| * that the angular "de-sugars" [asterisk (*) syntax](#asterisk) into a `<template>`. |
There was a problem hiding this comment.
Please fix as:
that Angular "de-sugars" the ...
There was a problem hiding this comment.
Done.
Also, I dropped the hyphen in "desugars" (Kathy confirmed a few days ago that there shouldn't be any hyphen).
chalin
force-pushed
the
chalin-struct-directives-copyedits-0304
branch
from
cb4726e to
c5f37b8
Compare
|
All points addressed. And I've noted not to edit the |
- Fix variable name `li` --> `i` - a --> an `UnlessDirective` - Jade had both #ngswitch and #ngSwitch as link targets; fixed to use same capitalization as template syntax page. - Fixed broke fragment target (was missing #) - Other copy edits and post-review comments from @wardbell
chalin
force-pushed
the
chalin-struct-directives-copyedits-0304
branch
from
c5f37b8 to
fc256f6
Compare
|
@wardbell @kapunahelewong - the Structural Directives page currently has (essentially) this Table of Contents:
I think it would improve the flow if we covered all the structural directives first (since that is the purpose of this guide), before mentioning I'd like to suggest that we move the
I can do it in this PR if you agree. |
|
As noted in Slack, I'd like to get this PR in and then let Kapunahele and you work out the TOC question in a followup PR. |
This is a follow-up to angular#3345 (comment). No changes were done to the `<ng-container>` section prose. @wardbell: @kapunahelewong and I discussed this, and she agrees with the change -- other edits will come in another PR.
…3367) * docs(structural-directives): move ng-container section later in page See #3345 (comment). No changes were done to the `<ng-container>` section prose.
li-->iUnlessDirectivefixed to use same capitalization as template syntax page.
app/-->src/app/in prose summary