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

add couple of remarks regarding inline/block usage of -if- #1770

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ciur
Copy link

@ciur ciur commented Jan 23, 2022

Add couple of remarks regarding usage of block and inline version of if.

@locks
Copy link
Contributor

locks commented Feb 9, 2022

Thank you for taking the time to contribute!

I noticed a couple of things with your suggestions.
Generally, we try to focus content on the happy path (the working code examples) because people tend to miss the "won't work" wording and copy paste the code and get confused.
Another thing I noticed is that in the first example you could (and should) do <button disabled={{@disabled}} class="btn">Sign In</button> which is much more direct and does not require the repetition.

What do you think of improving the API documentation for {{if}} and putting a note in the Guides to check https://api.emberjs.com/ember/4.1/classes/Ember.Templates.helpers/methods/if?anchor=if for the full documentation?

@ciur
Copy link
Author

ciur commented Feb 9, 2022

What do you think of improving the API documentation for {{if}} and putting a note in the Guides to check https://api.emberjs.com/ember/4.1/classes/Ember.Templates.helpers/methods/if?anchor=if for the full documentation?

@locks do I understand correctly that you want me move added remarks to API documentation? or maybe you want me to delete remarks entirely and just to point user to the already exiting API docs?
I don't understand your suggestion, please rephrase.

@jenweber
Copy link
Contributor

@locks can you take another look at this, and ciur's comments above? Thanks!

@jenweber
Copy link
Contributor

jenweber commented Jun 6, 2022

Hi @ciur thanks for bringing this up! The learning team re-reviewed this PR and we have some suggested changes for you to consider.

One thing that jumped out to me is that with your code example additions, we now give some more real-life examples. We've heard before that people had trouble finding a full example of if-else if - else, so this will be a nice improvement!


Here's an example of a block `if`, wrapping some HTML elements:

```handlebars {data-filename="app/components/sign-in.hbs"}
Copy link
Contributor

Choose a reason for hiding this comment

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

The most common way to conditionally render an attribute would be like this:

<button disabled={{@disabled}} class="btn">Sign In</button>

Personally, I would use a different example.

@@ -218,6 +229,14 @@ It looks similar to a ternary operator.
{{if condition value1 value2}}
```

Similarly to block `if`, you need to pay attention where inline `if` is placed.
Inline `if` can be used only inside attribute values
Copy link
Contributor

Choose a reason for hiding this comment

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

Inline if can be used only inside attribute values of HTML elements.

That's not true.
Literally the example above shows valid usage that's unrelated to HTML attributes.
These are valid examples as well:

Conditional argument value:

<Foo @foo={{if this.bar "bar" "baz"}} />

Applying a modifier conditionally:

<button type="button" {{(if @onClick (modifier this.on "click" @onClick))}} />

@locks locks force-pushed the conditiona-block-remarks branch from 99ee765 to e6d17d4 Compare September 14, 2022 22:52
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.

5 participants