Skip to content
This repository has been archived by the owner on May 26, 2019. It is now read-only.

Update routes sections for Ember CLI #14

Merged
merged 1 commit into from
Mar 21, 2015

Conversation

stevekinney
Copy link
Contributor

Current status: proof-reading / open for discussion

  • Switch all route code samples to ES6 module syntax
  • Switch all controller code samples to ES6 module syntax
  • Switch all view code samples to ES6 module syntax
  • Migrate lines 227-229 to Ember Data syntax in asynchronous-routing.md
  • Update table in defining-your-routes.md
  • Rename mentions using globals outside of code samples (See Referring to objects #13)

@joostdevries joostdevries mentioned this pull request Feb 24, 2015
19 tasks
@trek
Copy link
Member

trek commented Mar 17, 2015

@stevekinney is this still being actively worked on? If not, I'd like to let someone else claim it.

@stevekinney
Copy link
Contributor Author

@trek Yes. It’s just taking a little longer than I initially anticipated because some parts needed to be reworked in order to have it make a coherent narrative.

<td><code>PostsController</code><br>↳<code>PostsNewController</code></td>
<td><code>PostsRoute</code><br>↳<code>PostsNewRoute</code></td>
<td><code>app/controllers/posts</code><br>↳<code>app/controllers/posts/new</code></td>
<td><code>app/routes/posts</code><br>↳<code>app/posts/new</code></td>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For this table (and other similar tables), I used there Ember CLI file path. It seemed like the best choice at the time, but I'm not super convinced. Do you think it makes more sense to use the controller:post style instead?

Choose a reason for hiding this comment

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

Should you use the pods syntax (app/posts/controller) instead? If we're going to default to pods in 2.0 and it's fully supported already, it would make sense to start moving that way now.

Copy link
Contributor

Choose a reason for hiding this comment

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

no @kylecoberly. in another Issue it was settled that we use the current structure.
There's a pods RFC incoming to finalize them, we can change then.

Choose a reason for hiding this comment

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

Fair enough!
On Mar 19, 2015 7:34 PM, "Ricardo Mendes" notifications@github.com wrote:

In source/routing/defining-your-routes.md
#14 (comment):

 <td><code>posts</code><br>↳<code>posts/index</code></td>
/posts/new posts.new - PostsController
PostsNewController - PostsRoute
PostsNewRoute - app/controllers/posts
app/controllers/posts/new - app/routes/posts
app/posts/new

no @kylecoberly https://github.com/kylecoberly. in another Issue it was
settled that we use the current structure.
There's a pods RFC incoming to finalize them, we can change then.


Reply to this email directly or view it on GitHub
https://github.com/emberjs/guides/pull/14/files#r26813361.

@trek
Copy link
Member

trek commented Mar 19, 2015

Can you rebase/squash?

@stevekinney stevekinney changed the title [WIP] Update routes sections for Ember CLI Update routes sections for Ember CLI Mar 19, 2015
Remove global controllers from code samples

Adjust code blocks & conventions for async routing

Use filenames in route defining tables

Omit custom generated controllers

I’d like to rework this part but right now it doesn’t fit into the rest.

Remove globals from index

Adjust code block notation; swap naming convention in router flow

Adjust code blocks; change routes

Not sure about the syntax for the legacy `LoadingRoute` section.

Adjust code blocks, naming conventions

Adjust code blocks; naming conventions for query params

Adjust code blocks for sending events

Fix code blocks in template rendering

Adjust code blocks and naming conventions

Adjust code blocks and naming conventions for specifying model

Fix erroneous character
@stevekinney stevekinney force-pushed the update-routes-sections-for-cli branch from eb04dd9 to 74ee238 Compare March 19, 2015 19:51
@stevekinney
Copy link
Contributor Author

@trek Done.

@locks
Copy link
Contributor

locks commented Mar 20, 2015

:shipit:

@@ -94,19 +94,19 @@ App.TardyRoute = Ember.Route.extend({
});
```

When transitioning into `TardyRoute`, the `model` hook will be called and
When transitioning into `route:tardy`, the `model` hook will be called and
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

why route:tardy? This is the factory name in the container, and users won't normally see this. They probably transitionTo or link-to the tardy route.

Choose a reason for hiding this comment

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

I think this is what Steve was asking about below. I agree that the factory name is a little in-the-weeds for the guides, but it might be useful to have something to differentiate between tardy the route and tardy the component. I think that's exactly the kind of distinction that beginners get stuck on. What if it were "When transitioning into tardy (app/routes/tardy)", or some kind of hover, rollover, or annotation to the same effect?

trek added a commit that referenced this pull request Mar 21, 2015
@trek trek merged commit 183e5c4 into emberjs:master Mar 21, 2015
@stevekinney
Copy link
Contributor Author

Done (#116). Let me know if you have any issues.

App.TopChartsAlbumsRoute = App.FilterRoute.extend();
App.TopChartsArtistsRoute = App.FilterRoute.extend();
App.TopChartsPlaylistsRoute = App.FilterRoute.extend();
// app/top-charts-songs/route.js
Copy link
Member

Choose a reason for hiding this comment

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

O_O.

I'm reauditing the routes section. There appear to be many find/replace errors and places where find/replace didn't properly find at all. Will republish 1.11

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ugh. Super, super sorry about that. I'll get them all fixed up today.

Copy link
Member

Choose a reason for hiding this comment

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

I already fixed it, no worries. It was a bit of a pain.I basically needed to hand audit/fix all the guides over Sunday, which kind of negates the help we got :-/

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

Successfully merging this pull request may close these issues.

None yet

5 participants