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

[OCTANE] Update component-basics #637

Merged
merged 2 commits into from
Mar 18, 2019

Conversation

Frozenfire92
Copy link
Contributor

@Frozenfire92 Frozenfire92 commented Mar 17, 2019

This PR updates the route /release/components/component-basics for #588

Summary

  • switch to using {{action this.sayHello}} instead of onclick={{this.sayHello}} as outlined in: Review and Octanify the template guides #598 (comment)
  • change code fences to ```handlebars and ```javascript
  • correct action import
  • add component usage filenames of application template
  • update example code with CustomButton to be simpler and more consistent with other sections of the guides (Icon component instead, consistent argument usage)
  • fix yield helper reference
  • add mention of Interacting with the DOM to the further guides footnote. Assuming this is the correct section name as mentioned in: Octane guides: Component section (tracking issue) #395 (comment)

Remaining work

@Frozenfire92
Copy link
Contributor Author

Question:

I noticed when generating components within an octane blueprint app (ember new my-app -b @ember/octane-app-blueprint)

they add Component to the end of the class name, like so:

export default class HelloButtonComponent extends Component {

is this going to be like the case when octane releases? if so we should likely update this example to have Component on the end of the class name as well

@mike-north
Copy link
Contributor

if so we should likely update this example to have Component on the end of the class name as well

I'd go beyond this and ask that code snippets in the guides match exactly what readers will see/experience as they follow along.

@jenweber
Copy link
Contributor

Looks good to go! I would like to merge this after #638, since I think it's MVP.

@jenweber jenweber merged commit 925c147 into ember-learn:octane Mar 18, 2019
lenoraporter pushed a commit to lenoraporter/guides-source that referenced this pull request Jul 19, 2020
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.

3 participants