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

A few things #2

Open
tylercollier-devmtn opened this issue Sep 28, 2017 · 7 comments
Open

A few things #2

tylercollier-devmtn opened this issue Sep 28, 2017 · 7 comments

Comments

@tylercollier-devmtn
Copy link
Contributor

Is there a live example? The live examples in the other minis and afternoon projects are very helpful. It should be mentioned at the top of the README.

In the nav header, why is there a Packages link? It goes to "#/packages", which is not a valid route.

What is the purpose of $scope.allPackages = response.data; (same question in bookedCtrl)? It's not used so it's confusing.

I find it kind of odd to order the code steps booked, packages, locations. Feels backwards in terms of flow. Sometimes code needs to be written backward compared to flow for the user, but this doesn't seem to be one of those cases so it's confusing to a coder to be told to do it that way. It's easier to code in the same flow I'd use the site as a user.

@devlemire
Copy link
Contributor

https://devmountain.github.io/angular-2-afternoon/#/

If you can figure out why it's not rendering correctly we could have a live example. It works on a local environment but not on gh-pages.

@devlemire
Copy link
Contributor

devlemire commented Sep 28, 2017

$scope.allPackages is used to explicitly store the value of the APIs response. It is all the packages. We then filter through all the packages to get the one we need specifically or a group of packages we need specifically.

I personally would of just filtered on response.data, but being explicit can help with reducing the amount of "mental hops".

@devlemire
Copy link
Contributor

If you feel like a re-write of instructions is absolutely necessary, you can request my time through Cory on the DevMountain admin channel.

@devlemire
Copy link
Contributor

As for the packages route, I'm just as confused as you are. The packages route requires an ID, so I don't know why it is in the navigation bar. This is a very old project, I'm not sure who the original owner of it is. I just standardized the instructions for it.

@devlemire
Copy link
Contributor

devlemire commented Sep 28, 2017

Thinking about the allPackages some more, it can definitely be argued why I didn't just use queries in the API requests. Both routes accomplish the same thing, at the time I was probably going for what is easier for the student.

@devlemire
Copy link
Contributor

The live example has been fixed thanks to a quick brainstorming session with @tylercollier-devmtn.

@tylercollier-devmtn
Copy link
Contributor Author

I personally would of just filtered on response.data, but being explicit can help with reducing the amount of "mental hops".

Maybe just assign it to a local allPackages variable instead of on $scope?

If you feel like a re-write of instructions is absolutely necessary, you can request my time through Cory on the DevMountain admin channel.

I apologize if my phrasing was blunt, and I would rather rephrase this to ask if it's the way it is on purpose, in terms of order. I know DevMountain has a lot of research on education and approach so maybe approaching it that way (in what seems backwards to me) is better for learning. Thoughts?

The packages route requires an ID, so I don't know why it is in the navigation bar.

Should we take it out?

Thinking about the allPackages some more, it can definitely be argued why I didn't just use queries in the API requests.

I have no problem with the fact that you fetched all packages and then filtered them in code. This is a fine approach as long as all packages is not a huge result set.

Thanks very much for getting the live example up!

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

No branches or pull requests

2 participants