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

Update rendering-values.md with example of how to return a component from a function #1966

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

Conversation

BryanCrotaz
Copy link

Add example of function to return a dynamic component

Add example of function to return a dynamic component
@netlify
Copy link

netlify bot commented Oct 15, 2023

Deploy Preview for ember-guides ready!

Name Link
🔨 Latest commit a63f021
🔍 Latest deploy log https://app.netlify.com/sites/ember-guides/deploys/652c1b07159348000984fc8c
😎 Deploy Preview https://deploy-preview-1966--ember-guides.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

correct return type of someComponent function
Copy link
Contributor

@NullVoxPopuli NullVoxPopuli left a comment

Choose a reason for hiding this comment

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

very nice!

Copy link
Contributor

@MinThaMie MinThaMie left a comment

Choose a reason for hiding this comment

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

Thanks for this example, I'm wondering if it needs a little bit of text too. What this does :)

@NullVoxPopuli
Copy link
Contributor

It's about the same as all the other examples on the page. Can we merge this change, and then iterate on the page's tone and verbiage overall separately?

@MinThaMie
Copy link
Contributor

All the examples have a sentence that explains it. I would love to have that for this one too :)
Screenshot 2023-10-16 at 16 15 24

@BryanCrotaz
Copy link
Author

"or via a property on this object" only showed the template, and not how to define the property. I've filled that gap. It was only showing half of the how-to.

@MinThaMie
Copy link
Contributor

Aaah, since your component is called myComponent and the example uses someComponent, could we change that? And maybe add the sentence: "The implementation inside the component could look like this:" and then your example?

@BryanCrotaz
Copy link
Author

BryanCrotaz commented Oct 16, 2023

No we shouldn't rename, because that would be REALLY confusing. The only reason you'd do this would be to dynamically change which component you're providing. Having the name of one of those choices match the property name would not make sense.

In most other places the template and component code are placed alongside each other without extraneous words.

@@ -71,6 +71,25 @@ or via a property on some object
```handlebars
<this.someComponent />
```
```ts
Copy link
Member

Choose a reason for hiding this comment

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

There shouldn't be any TypeScript in the guides (yet). There is a whole process that is underway to add a typescript version of the guides but until that is done then you should write all your backing JS as JS 👍

@@ -71,6 +71,25 @@ or via a property on some object
```handlebars
Copy link
Member

Choose a reason for hiding this comment

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

If we are trying to equate this file to the JS backing it then we should specify the filename for both. There is information in the styleguide on how to do this https://github.com/ember-learn/guides-source/blob/master/CONTRIBUTING.md#style-guide

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