-
Notifications
You must be signed in to change notification settings - Fork 47
Michelle - Edges - TREK #32
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
base: master
Are you sure you want to change the base?
Conversation
TREKWhat We're Looking For
Good work on this Michelle! I have a concern about how you handled finding the current trip id, and that may be because of your absences that week. I'm leaving a few comments on it. Otherwise, I think you did a great job with this project. Good work on it! |
| @@ -0,0 +1,152 @@ | |||
| // --- RE-USABLE ---// | |||
| const baseURL = 'https://trektravel.herokuapp.com/trips/'; | |||
| let id = 0; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're putting the id in this global scope in order to store what the current trip is... You should consider different approaches to get the value of id. Could you use a closure or read from the HTML? (details about this suggestion below)
| // --- RE-USABLE ---// | ||
| const baseURL = 'https://trektravel.herokuapp.com/trips/'; | ||
| let id = 0; | ||
| const post = '/reservations'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could probably have a better name
| // }); | ||
| }; | ||
|
|
||
| // did you happen to click on a trip? have i got a trip for you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good comment ^^^^
| }; | ||
|
|
||
| // did you happen to click on a trip? have i got a trip for you! | ||
| $(`.trip-list *`).click(loadTripDetails); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of relying on a selector like this, you'll either want to rely on a closure or event delegation
https://github.com/Ada-Developers-Academy/textbook-curriculum/blob/master/10-JavaScript/closures.md
TREK
Congratulations! You're submitting your assignment!
Comprehension Questions
Tripin the list by it's ID field. Would it be advantageous to keep the list in order by the id field? Explain why or why not.