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

[WIP] Revamped octane tutorial #1002

Closed

Conversation

vaidehijoshi
Copy link
Contributor

This is currently a work in progress PR for the new Octane tutorial. It's a bit messy right now—the code snippets still need formatting, the chapters need to be condensed and properly named—but it should give you an idea of how we're hoping to restructurre the tutorial to highlight Octane features and idioms.

Code changes to the super-rentals tutorial can be viewed on this branch.

Written in conjunction with @chancancode.

@chancancode
Copy link
Contributor

We will add back the redirects after we do a second pass to consolidate the chapters.

@chancancode
Copy link
Contributor

chancancode commented Aug 14, 2019

Some high-level goals:

  • Highlight Octane idioms and best practice
  • More component-centric
  • Re-order the topics for a gentler on-ramp, and provide better motivations for each topic
  • Remove accidental complexity (having the learn TDD and mirage API while learning Ember at the same time)
  • Remove anti-patterns (e.g. deploying with development environment so that mirage could work 😬)
  • Assume basic knowledge of modern JavaScript (e.g. class and modules), remove inline/ad-hoc introduction to those features in favor of linking to dedicated learning resources
  • Improve accessibility (use button and a tags properly, etc)

Copy link

@zvkemp zvkemp left a comment

Choose a reason for hiding this comment

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

I got up through section 8; will continue reviewing tomorrow.

guides/release/tutorial/7.md Show resolved Hide resolved
guides/release/tutorial/7.md Show resolved Hide resolved

Under the hood, the `<LinkTo>` component generates a regular `<a>` tag for us with the appropriate `href` for the specific route. This allows for perfect interoperability for all screen readers, as well as the ability for our users to bookmark the link or open it in a new tab.

However, when clicking on one of these special links, Ember will intercept the click, render the content for the new page, and update the URL &mdash; all performed locally without having to wait for the server, thus avoiding a full page refresh.
Copy link

Choose a reason for hiding this comment

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

Suggested change
However, when clicking on one of these special links, Ember will intercept the click, render the content for the new page, and update the URL &mdash; all performed locally without having to wait for the server, thus avoiding a full page refresh.
However, when we click one of these special links, Ember will intercept the click, render the content for the new page, and update the URL &mdash; all performed in the browser without having to wait for the server, thus avoiding a full page refresh.

Copy link

Choose a reason for hiding this comment

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

I think technically there should be no spaces on either side of em dashes, but I guess it's fine as long as it's used consistently

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll defer to @vaidehijoshi

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I kept spaces in them because it was much easier to read/type in markdown/the code editor. I've been consistently using the spaces in the tutorial prose, so it should be easy for me to do a find-and-replace for it at the end.

guides/release/tutorial/7.md Show resolved Hide resolved
guides/release/tutorial/8.md Show resolved Hide resolved
guides/release/tutorial/8.md Show resolved Hide resolved
guides/release/tutorial/8.md Show resolved Hide resolved
guides/release/tutorial/8.md Show resolved Hide resolved
guides/release/tutorial/8.md Show resolved Hide resolved
guides/release/tutorial/8.md Show resolved Hide resolved
vaidehijoshi and others added 2 commits August 14, 2019 19:19
This commit temporarily remove the old guides from the build in an attempt
to speed it up. This will break the tests. This must be reverted before
merging.
@jenweber
Copy link
Contributor

Yesssss I love this.

Let me know what help you need and when you want to get this merged in. The Octane Guides are WIP so we can merge in unfinished stuff at the point where it makes sense to break off work into smaller pieces.

Copy link

@zvkemp zvkemp left a comment

Choose a reason for hiding this comment

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

review for sections 9 - 14

guides/release/tutorial/9.md Show resolved Hide resolved
guides/release/tutorial/9.md Show resolved Hide resolved
guides/release/tutorial/9.md Show resolved Hide resolved
guides/release/tutorial/9.md Show resolved Hide resolved
guides/release/tutorial/9.md Show resolved Hide resolved
}
```

Here, in the *[component's constructor](todo://link to component's constructor)*, we *[initialized](TODO: link to initialized)* the *[instance variable](TODO: link to instance variable)* `this.isLarge` to the value `false`, since this is the default state that we want for our component.
Copy link

Choose a reason for hiding this comment

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

Suggested change
Here, in the *[component's constructor](todo://link to component's constructor)*, we *[initialized](TODO: link to initialized)* the *[instance variable](TODO: link to instance variable)* `this.isLarge` to the value `false`, since this is the default state that we want for our component.
Here, in the *[component's constructor](todo://link to component's constructor)*, we *[initialized](TODO: link to initialized)* the *[instance variable](TODO: link to instance variable)* `this.isLarge` with the value `false`, since this is the default state that we want for our component.

{{/if}}
```

The `{{#if ...}}...{{else}}...{{/if}}` *[conditional](TODO: link to conditional)* syntax allows us to render different content. We also have access to the component's instance variables from the template. Combining these two features, we can render either the small or the large version of the image accordingly.
Copy link

Choose a reason for hiding this comment

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

Suggested change
The `{{#if ...}}...{{else}}...{{/if}}` *[conditional](TODO: link to conditional)* syntax allows us to render different content. We also have access to the component's instance variables from the template. Combining these two features, we can render either the small or the large version of the image accordingly.
In the template, we have access to the component's instance variables. The `{{#if ...}}...{{else}}...{{/if}}` *[conditional](TODO: link to conditional)* syntax allows us to render different content depending on the result of the _predicate_ (in this case, the instance variable`this.isLarge`). Combining these two features, we can render either the small or the large version of the image accordingly.


The `{{#if ...}}...{{else}}...{{/if}}` *[conditional](TODO: link to conditional)* syntax allows us to render different content. We also have access to the component's instance variables from the template. Combining these two features, we can render either the small or the large version of the image accordingly.

We can verify this is working by temporarily changing the initial value in our JavaScript file. If we change `app/components/rental/image.js` to initialize `this.isLarge = true;` in the constructor, we should see the large version of the property image in the browser. Cool!
Copy link

Choose a reason for hiding this comment

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

Suggested change
We can verify this is working by temporarily changing the initial value in our JavaScript file. If we change `app/components/rental/image.js` to initialize `this.isLarge = true;` in the constructor, we should see the large version of the property image in the browser. Cool!
We can verify this works by temporarily changing the initial value in our JavaScript file. If we change `app/components/rental/image.js` to initialize `this.isLarge = true;` in the constructor, we should see the large version of the property image in the browser. Cool!


Once we've tested this out, we can change `this.isLarge` back to `false`.

Since this pattern of initializing instance variables from the constructor is pretty common, there happens to be a much more concise syntax for it:
Copy link

Choose a reason for hiding this comment

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

Suggested change
Since this pattern of initializing instance variables from the constructor is pretty common, there happens to be a much more concise syntax for it:
Since this pattern of initializing instance variables in the constructor is pretty common, there happens to be a much more concise syntax for it:

isLarge = false;
constructor() {
super(...arguments);
this.isLarge = false;
Copy link

@zvkemp zvkemp Aug 15, 2019

Choose a reason for hiding this comment

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

Should this line still be here? Can the constructor be elided entirely? May want to say something like "we can declare default values for instance variables outside the constructor"

edit: oops, didn't see the diff formatting


We did a few things here, so let's break it down.

First, we added the `@Tracked` *[decorator](TODO: link to decorator)* to the `isLarge` instance variable. This annotation tells Ember to monitor this variable for changes. Whenever this variable's value is updated, Ember will automatically re-render any templates that depends on its value.
Copy link

Choose a reason for hiding this comment

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

Suggested change
First, we added the `@Tracked` *[decorator](TODO: link to decorator)* to the `isLarge` instance variable. This annotation tells Ember to monitor this variable for changes. Whenever this variable's value is updated, Ember will automatically re-render any templates that depends on its value.
First, we added the `@Tracked` *[decorator](TODO: link to decorator)* to the `isLarge` instance variable. This annotation tells Ember to monitor this variable for updates. Whenever this variable's value changes, Ember will automatically re-render any templates that depend on its value.


First, we added the `@Tracked` *[decorator](TODO: link to decorator)* to the `isLarge` instance variable. This annotation tells Ember to monitor this variable for changes. Whenever this variable's value is updated, Ember will automatically re-render any templates that depends on its value.

In our case, whenever we assign a new value to `this.isLarge`, this annotation will cause Ember to re-evaluate the `{{#if this.isLarge}}` conditional in our template and will switch between the two blocks accordingly.
Copy link

Choose a reason for hiding this comment

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

Suggested change
In our case, whenever we assign a new value to `this.isLarge`, this annotation will cause Ember to re-evaluate the `{{#if this.isLarge}}` conditional in our template and will switch between the two blocks accordingly.
In our case, whenever we assign a new value to `this.isLarge`, the `@Tracked` annotation will cause Ember to re-evaluate the `{{#if this.isLarge}}` conditional in our template, and switch between the two blocks accordingly.

<div class="cta-note-body">
<div class="cta-note-heading">Zoey says...</div>
<div class="cta-note-message">
If you referenced a variable in the template but forgot to add the `@Tracked` decorator, you will get a helpful error in development mode when you change its value!
Copy link

Choose a reason for hiding this comment

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

Suggested change
If you referenced a variable in the template but forgot to add the `@Tracked` decorator, you will get a helpful error in development mode when you change its value!
If you reference a variable in the template but forget to add the `@Tracked` decorator, you will get a helpful error in development mode when you change its value!

</div>
</div>

Next, we added a `toogleSize` method to our class that simply toggles `this.isLarge` between `true` and `false`.
Copy link

Choose a reason for hiding this comment

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

Suggested change
Next, we added a `toogleSize` method to our class that simply toggles `this.isLarge` between `true` and `false`.
Next, we added a `toggleSize` method to our class that simply switches `this.isLarge` to the opposite of its current state (`false` becomes `true`, or `true` becomes `false`).

<div class="cta-note-body">
<div class="cta-note-heading">Zoey says...</div>
<div class="cta-note-message">
If you forget to do add the `@action` decorator, you will also get a helpful error when clicking on the button in development mode!
Copy link

Choose a reason for hiding this comment

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

Suggested change
If you forget to do add the `@action` decorator, you will also get a helpful error when clicking on the button in development mode!
If you forget to add the `@action` decorator, you will get a helpful error when clicking on the button in development mode!

@@ -0,0 +1,124 @@
At this point, a big part of our template is devoted to the `<img>` tag's `src` attribute, which is getting pretty long. One alternative is to move this computation into a JavaScript instead.

From within our JavaScript class, we have access to our component's arguments using the `this.args.*` API. Using that, we can create a new getter to compute the map's URL and reference that from the template instead.
Copy link

Choose a reason for hiding this comment

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

Suggested change
From within our JavaScript class, we have access to our component's arguments using the `this.args.*` API. Using that, we can create a new getter to compute the map's URL and reference that from the template instead.
From within our JavaScript class, we have access to our component's arguments using the `this.args.*` API. Using that, we can move the URL logic from the template into a new getter method.

<div class="cta-note-body">
<div class="cta-note-heading">Zoey says...</div>
<div class="cta-note-message">
`this.args` is an API provided by the `@glimmer/component` Glimmer component superclass. You may come across other components superclasses, such "classic" components in legacy codebases, that provides a different API for accessing component arguments from JavaScript code.
Copy link

Choose a reason for hiding this comment

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

Suggested change
`this.args` is an API provided by the `@glimmer/component` Glimmer component superclass. You may come across other components superclasses, such "classic" components in legacy codebases, that provides a different API for accessing component arguments from JavaScript code.
`this.args` is an API provided by the `@glimmer/component` Glimmer component superclass. You may come across other components superclasses, such "classic" components in legacy codebases, that provide different APIs for accessing component arguments from JavaScript code.


Note that we did not mark our getter as `@Tracked`. Unlike instance variables, getters cannot be "assigned" a new value directly, so it does not make sense for Ember to monitor them for changes.

That being said, the values _produced_ by getters can certainly change. In our case, the value produced by our `src` getter depends on the values of `lat`, `lng`, `width`, `height` and `zoom` from `this.args`. Whenever these *[dependencies](TODO: link to dependencies)* get updated, we would certainly expect `{{this.src}}` from our template to be updated accordingly.
Copy link

Choose a reason for hiding this comment

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

Suggested change
That being said, the values _produced_ by getters can certainly change. In our case, the value produced by our `src` getter depends on the values of `lat`, `lng`, `width`, `height` and `zoom` from `this.args`. Whenever these *[dependencies](TODO: link to dependencies)* get updated, we would certainly expect `{{this.src}}` from our template to be updated accordingly.
That being said, the values _produced_ by getters can certainly change. In our case, the value produced by our `src` getter depends on the values of `lat`, `lng`, `width`, `height` and `zoom` from `this.args`. Whenever these *[dependencies](TODO: link to dependencies)* get updated, we would expect `{{this.src}}` from our template to be updated accordingly.


That being said, the values _produced_ by getters can certainly change. In our case, the value produced by our `src` getter depends on the values of `lat`, `lng`, `width`, `height` and `zoom` from `this.args`. Whenever these *[dependencies](TODO: link to dependencies)* get updated, we would certainly expect `{{this.src}}` from our template to be updated accordingly.

Ember does this by automatically tracking any variables that were accessed while computing a getter's value. As long as the dependencies themselves are marked as `@Tracked`, Ember will know exactly when to invalidate and re-render any templates that may potentially contain any "stale" (outdated) getter values.
Copy link

Choose a reason for hiding this comment

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

Suggested change
Ember does this by automatically tracking any variables that were accessed while computing a getter's value. As long as the dependencies themselves are marked as `@Tracked`, Ember will know exactly when to invalidate and re-render any templates that may potentially contain any "stale" (outdated) getter values.
Ember does this by automatically tracking any variables that were accessed while computing a getter's value. As long as the dependencies themselves are marked as `@Tracked`, Ember will know exactly when to re-render any templates that contain any "stale" (outdated) getter values.


Note that the value of `this` here does _not_ refer to the component instance. We are not directly accessing or modifying the component's internal states (that would be extremely rude!).

Instead, they refer to a special *[test context](TODO: link to test context)* object, which we have access to inside the `render` helper. This provides a "bridge" for us to pass dynamic values, in the form of arguments, into our invocation of the component. This allows us to update these values as needed from the test function.
Copy link

Choose a reason for hiding this comment

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

Suggested change
Instead, they refer to a special *[test context](TODO: link to test context)* object, which we have access to inside the `render` helper. This provides a "bridge" for us to pass dynamic values, in the form of arguments, into our invocation of the component. This allows us to update these values as needed from the test function.
Instead, `this` refers to a special *[test context](TODO: link to test context)* object, which we have access to inside the `render` helper. This provides a "bridge" for us to pass dynamic values, in the form of arguments, into our invocation of the component. This allows us to update these values as needed from the test function.

@jenweber
Copy link
Contributor

We should merge this at the same time as #988

Let me know when you feel like you're at the point that we can merge this! Incremental, unfinished work is ok to have in here for now.

@vaidehijoshi
Copy link
Contributor Author

@jenweber Will do! I am aiming to try to get the tutorial prose to a good point this week so that the remaining work can be divided up into more discrete tasks that can be picked up by others. I'll let you know when @chancancode and I are ready.

@jenweber
Copy link
Contributor

jenweber commented Sep 4, 2019

Thanks everyone for the work here! I'm closing this PR since we are copying its changes over to https://github.com/ember-learn/super-rentals-tutorial, which is the new home for the tutorial writeup. See this issue for tracking progress.

@jenweber jenweber closed this Sep 4, 2019
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