Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DOC lts] Angle Bracket invocation #17554

Merged
merged 3 commits into from
Feb 5, 2019
Merged

Conversation

cah-danmonroe
Copy link

This is the initial angle bracket invocation conversion for the docs. It should be merged back through version 3.4

@cah-danmonroe cah-danmonroe changed the title [DOC] Angle Bracket invocation [DOC lts] Angle Bracket invocation Feb 2, 2019
Copy link
Contributor

@locks locks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some bits and bobs

packages/@ember/-internals/glimmer/lib/component.ts Outdated Show resolved Hide resolved
@@ -106,6 +110,12 @@
When yielding the component via the `hash` helper, the component is invoked directly.
See the following snippet:

```
<PersonForm as |form|>
{{form.nameInput placeholder="Username"}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{{form.nameInput placeholder="Username"}}
<form.nameInput @placeholder={{"Username"}} />

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{{form.nameInput placeholder="Username"}}
<form.nameInput @placeholder="Username" />

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wycats which of these is preferable?

Copy link
Author

@cah-danmonroe cah-danmonroe Feb 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used <form.nameInput @placeholder={{"Username"}} /> for now

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally speaking there is no need for curlies around a static string. Its not "wrong", just a bit odd...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the curlies

@@ -34,6 +37,10 @@ import { INVOKE, UPDATE } from '../utils/references';
Additionally, the `mut` helper can be combined with the `action` helper to
mutate a value. For example:

```handlebars
<MyChild @childClickCount={{this.totalClicks}} @click-count-change={{action (mut totalClicks))}} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This example needs to be revised (both syntax).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure what you mean here.

@@ -22,10 +22,14 @@ const normalizeTextValue = (value: any): string => {
Example:

```handlebars
{{some-component name=(concat firstName " " lastName)}}
<SomeComponent @name={{concat firstName " " lastName}} />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You actually don't need concat in this example at all (this was one of the selling points of angle bracket invocation). This works perfectly:

<SomeComponent @name="{{firstName}} {{lastName}}" />

Maybe we don't need dual examples in this case?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left both examples but explained that it's not needed for angle bracket invocation.

@@ -117,6 +117,12 @@ class ConditionalHelperReference extends CachedReference {

You can use the `if` helper inside another helper as a nested helper:

```handlebars
<SomeComponent height={{if isBig "100" "10"}} />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<SomeComponent height={{if isBig "100" "10"}} />
<SomeComponent @height={{if isBig "100" "10"}} />

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -150,6 +156,12 @@ export function inlineIf(_vm: VM, { positional }: Arguments) {

You can use the `unless` helper inside another helper as a subexpression.

```handlebars
<SomeComponent height={{unless isBig "10" "100"}} />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<SomeComponent height={{unless isBig "10" "100"}} />
<SomeComponent @height={{unless isBig "10" "100"}} />

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good to me. Going to go ahead and land (to avoid annoying rebase issues), but if @emberjs/learning-team-managers have any tweaks they'd like to see we can get them in a follow up PR...

@rwjblue rwjblue merged commit 64ee6a6 into emberjs:master Feb 5, 2019
@locks
Copy link
Contributor

locks commented Feb 5, 2019

Sounds good, thanks for pushing it forward!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants