Add subroutes #24

sbatson5 · 2016-05-15T16:06:46Z

WIP PR for adding sub-routes per:
emberjs/guides#1374

locks · 2016-05-21T21:14:37Z

app/routes/rentals/show.js

+    return this.store.queryRecord('rental', { title: params.title })
+  },
+
+  setupController(controller, model) {


me no like, is this in the prose @toddjordan ?

I thought it made the template more readable to access it as rental instead of model but I can revert it, nbd. Is it considered bad practice to use setupController in this way?

@sbatson5 it's slightly controversial, but I personally think it's unnecessary and unexpected, since you expect the model property to be set with what's returned from the model hook.

in other words, there's some valid use cases for setupController but they're fewer than most people think they are ;)

toddjordan · 2016-05-23T12:35:06Z

Looks good :-) I was thinking that you could use transitionTo from the index route to redirect users going to '/' to the new rentals route. http://emberjs.com/api/classes/Ember.Route.html#method_transitionTo

Also take a look at the last merge, we've added styling and changed the markup a bit.

toddjordan · 2016-05-23T12:37:43Z

app/templates/rentals/index.hbs

@@ -0,0 +1,6 @@
+<ul>


I'd expect this template to reflect the content that currently show in the homepage (using this template instead)

sbatson5 · 2016-05-29T18:01:27Z

app/templates/rentals/index.hbs

@@ -0,0 +1,5 @@
+<ul class="results">
+  {{#each model as |rentalUnit|}}
+    {{rental-listing rental=rentalUnit goToShowRoute="goToShowRoute"}}


Added an action to make the component clickable per recommendation here:
emberjs/guides#1443 (comment)

Wondering if this is out of scope for a tutorial on sub-routes. Not sure I want to encourage writing actions this way with closure actions and the route-action helper becoming more prominent. But I don't want to introduce a controller just for the sake of a closure action.

Anyone have thoughts on a faster way to do this that doesn't take much explaining for a beginner's tutorial? Maybe just wrap component in a div and add the action to that?

@sbatson5 You should just wrap rental listing in a {{link-to}} like this:

{{#link-to "rentals.show" rentalUnit.slug}} {{rental-listing rental=rentalUnit}} {{/link-to}}

and remove all the "goToShowRoute" stuff

👍 I was definitely overcomplicating it 😛

toddjordan · 2016-05-31T05:47:06Z

I created a PR to your branch with what I was talking about: https://github.com/sbatson5/super-rentals/pull/1

sbatson5 · 2016-06-03T12:23:27Z

app/templates/rentals/show.hbs

@@ -0,0 +1,22 @@
+<div class="jumbo">
+  <a href="#" class="button right up-11">Edit</a>


@toddjordan Do you want an edit subroute added to this tutorial?

good question. We've been recently discussing whether to add a form section to the tutorial, and edit would be the logical place to do it. What I think is having the one subroute is fine for now, and when we add forms to the tutorial is when we would add the edit subroute.

So go ahead and remove edit and we'll add it back when we introduce a forms section

sbatson5 · 2016-06-15T14:33:58Z

@toddjordan Thanks! Added a PR: toddjordan/ember-cli-tutorial-style#1

toddjordan · 2016-06-15T14:51:09Z

Cool. I'll merge and re-publish the addon

toddjordan · 2016-06-15T15:12:34Z

@sbatson5 I updated the super-rentals repo with the updated version of the style addon with your update. you can rebase it into this pr...

Also we'll need to squash commits before merge.

toddjordan · 2016-06-15T15:13:44Z

also, currently seems some of the tests are failing in this pr

sbatson5 · 2016-06-15T16:30:09Z

Rebased and squashed commits. But not sure why Travis is failing. Have any insight @toddjordan ?

Also, I saw your PR I merged into this branch had changes to the vendor/gmap.js. Is that intended? Or should I remove it from this PR?

toddjordan · 2016-07-04T02:56:20Z

I pulled down the latest from your branch and tests all pass (we're close!). I did notice this though:

Would prefer the text to sit between the picture and the map instead of below it.

toddjordan · 2016-07-05T14:10:15Z

I was able to correct the listing style issue by updating the ember-cli-tutorial-style addon to be more specific about an anchor style. It seems adding the anchor to the listing for the subroute was picking up an unwanted style.

toddjordan · 2016-07-05T14:24:42Z

I updated the addon, all you'll need to do to fix is Update the ember-cli-tutorial-style version to 0.0.4!

I think it'll be then be good to go then and we can focus on getting the tutorial guide page out 👍

sbatson5 · 2016-07-07T23:37:39Z

@toddjordan Thanks so much for catching that. I saw that styling last time but just thought that npm hadn't updated. Forgot to go back and check it. Will update.

sbatson5 · 2016-07-07T23:54:04Z

@toddjordan I updated it to 0.0.4, nombom'ed but am still seeing that odd styling.

toddjordan · 2016-07-08T04:14:25Z

Hmm I'll recheck tomorrow
On Thu, Jul 7, 2016 at 7:54 PM Scott Batson notifications@github.com
wrote:

@toddjordan https://github.com/toddjordan I updated it to 0.0.4,
nombom'ed but am still seeing that odd styling.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#24 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ADcR5w992oaIbw01L6xHy4BZhosSPCtmks5qTZGdgaJpZM4Ie3NH
.

sbatson5 · 2016-07-09T13:23:11Z

I'm kind of stumped on this. When I look in node_modules, I can see all the css updates. But if I inspect the stylesheets in Chrome and Firefox, I am not seeing the css changes you or I added to the addon even after nombom'ing and running in incognito. I feel there is something obvious I'm missing.

toddjordan · 2016-07-09T14:06:12Z

Sorry been on vacation and didn't get to it as planned. Should be able to
help out Sunday or Monday
On Sat, Jul 9, 2016 at 9:23 AM Scott Batson notifications@github.com
wrote:

I'm kind of stumped on this. When I look in node_modules, I can see all
the css updates. But if I inspect the stylesheets in Chrome and Firefox, I
am not seeing the css changes you or I added to the addon even after
nombom'ing and running in incognito. I feel there is something obvious I'm
missing.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#24 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ADcR5_3r-2kwZZDIfDFgAQEZ0nX_8Bbqks5qT6C_gaJpZM4Ie3NH
.

toddjordan · 2016-07-10T02:26:49Z

@sbatson5 How did you update? The vendor css only gets updated when you run the blueprint, which comes from ember install, so your best course to update might be to remove the addon and re-run ember install, so it picks up the newest version and runs the blueprint

sbatson5 · 2016-07-10T13:56:56Z

Ah, I was just doing npm install. I thought ember serve would update the vendor file. Good to know! It looks much better now.

toddjordan · 2016-07-11T04:09:06Z

Awesome. I think this is good to go. I'll hold off on merge until we merge the tutorial guides PR. I'll look that that this week.

add unit-tests for sub-routes re-use rental-listing component add slug to mirage config JSON

toddjordan · 2016-08-06T03:44:39Z

Created a new branch out of this one for remaining updates. Closing in favor of: #35

sbatson5 mentioned this pull request May 15, 2016

Ember Tutorial emberjs/guides#1374

Closed

14 tasks

locks reviewed May 21, 2016
View reviewed changes

toddjordan reviewed May 23, 2016
View reviewed changes

sbatson5 mentioned this pull request May 28, 2016

add tutorial for sub-routes emberjs/guides#1443

Closed

sbatson5 reviewed May 29, 2016
View reviewed changes

sbatson5 reviewed Jun 3, 2016
View reviewed changes

sbatson5 mentioned this pull request Jun 15, 2016

add inline style to a tags in listing component toddjordan/ember-cli-tutorial-style#1

Merged

sbatson5 changed the title ~~[WIP] Add subroutes~~ Add subroutes Jun 18, 2016

Scott Batson added 6 commits July 15, 2016 21:57

add get/id route to ember-cli-mirage

153f444

add subroutes

5f05b64

add unit-tests for sub-routes re-use rental-listing component add slug to mirage config JSON

update style add-on

717a2a4

add rental routes acceptance test

3b02e8f

redirect to rentals route from index

f31fd87

fix JSHint issues

84135ac

toddjordan closed this Aug 6, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add subroutes #24

Add subroutes #24

sbatson5 commented May 15, 2016 •

edited

locks May 21, 2016

sbatson5 May 22, 2016

locks May 22, 2016

locks May 22, 2016

toddjordan commented May 23, 2016 •

edited

toddjordan May 23, 2016 •

edited

sbatson5 May 29, 2016

toddjordan May 31, 2016 •

edited

sbatson5 May 31, 2016

toddjordan commented May 31, 2016

sbatson5 Jun 3, 2016

toddjordan Jun 3, 2016

toddjordan Jul 4, 2016

sbatson5 commented Jun 15, 2016

toddjordan commented Jun 15, 2016

toddjordan commented Jun 15, 2016

toddjordan commented Jun 15, 2016

sbatson5 commented Jun 15, 2016 •

edited

toddjordan commented Jul 4, 2016 •

edited

toddjordan commented Jul 5, 2016

toddjordan commented Jul 5, 2016

sbatson5 commented Jul 7, 2016

sbatson5 commented Jul 7, 2016

toddjordan commented Jul 8, 2016

sbatson5 commented Jul 9, 2016

toddjordan commented Jul 9, 2016

toddjordan commented Jul 10, 2016

sbatson5 commented Jul 10, 2016 •

edited

toddjordan commented Jul 11, 2016

toddjordan commented Aug 6, 2016

		@@ -0,0 +1,22 @@
		<div class="jumbo">
		<a href="#" class="button right up-11">Edit</a>

Add subroutes #24

Add subroutes #24

Conversation

sbatson5 commented May 15, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toddjordan commented May 23, 2016 • edited

toddjordan May 23, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toddjordan May 31, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toddjordan commented May 31, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbatson5 commented Jun 15, 2016

toddjordan commented Jun 15, 2016

toddjordan commented Jun 15, 2016

toddjordan commented Jun 15, 2016

sbatson5 commented Jun 15, 2016 • edited

toddjordan commented Jul 4, 2016 • edited

toddjordan commented Jul 5, 2016

toddjordan commented Jul 5, 2016

sbatson5 commented Jul 7, 2016

sbatson5 commented Jul 7, 2016

toddjordan commented Jul 8, 2016

sbatson5 commented Jul 9, 2016

toddjordan commented Jul 9, 2016

toddjordan commented Jul 10, 2016

sbatson5 commented Jul 10, 2016 • edited

toddjordan commented Jul 11, 2016

toddjordan commented Aug 6, 2016

sbatson5 commented May 15, 2016 •

edited

toddjordan commented May 23, 2016 •

edited

toddjordan May 23, 2016 •

edited

toddjordan May 31, 2016 •

edited

sbatson5 commented Jun 15, 2016 •

edited

toddjordan commented Jul 4, 2016 •

edited

sbatson5 commented Jul 10, 2016 •

edited