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

Trek-Cara-Octors #44

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

MississippiBrenn
Copy link

TREK

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
What does it mean for code to be asynchronous?
Describe a piece of your code that executes asynchronously. How did this affect the way you structured it? Any code that made more than one request at once, and one such code was making the request for a trip, and including information about submit. I hid some of the parts of the page until they were needed.
What kind of errors might the API give you? How did you choose to handle them? Something like, the thing could not be loaded. I handled the errors as status messages.
Do you have any recommendations on how we could improve this project for the next cohort? Nope

@droberts-sea
Copy link

TREK

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene I would like to see more descriptive commit messages - "wave 1" does not tell the reader anything about what changed or why. Also, the node_modules folder should not be included in git.
Comprehension questions yes - Regarding asynchronous code, anything that runs at some unspecified time in the future is asynchronous, including everything from AJAX success / failure callbacks to button click handlers.
Functionality
Click a button to list trips yes
Click a trip to see trip details yes
Fill out a form to reserve a spot yes
Errors are reported to the user yes
Styling yes
Under the Hood
Trip data is retrieved using from the API yes
JavaScript is well-organized and easy to read mostly - see inline
HTML is semantic yes
Overall Good job overall! This code is for the most part well-organized and easy to read, and it's clear the learning goals of this assignment were met. I especially appreciate the care you put into error handling. I've left a couple inline comments for you to review below, but in general I am quite happy with this submission. Keep up the hard work!

const clickTrip = (trip) => {
console.log(trip)

$('#view-trip').removeClass('hidden')

Choose a reason for hiding this comment

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

You probably want to empty the view-trip section here too. As is, if you click on more than one trip the details for all the trips display on the screen at once.

$('#tablebody').append(`<p>Name: ${response.data.name}</p>`);
$('#tablebody').append(`<p>Id: ${response.data.id}</p>`); $('#tablebody').append(`<p>Continent: ${response.data.continent}</p>`);
$('#tablebody').append(`<p>About: ${response.data.about}</p>`);
$('#tablebody').append(`<p>Category: </category>${response.data.category}</p>`);

Choose a reason for hiding this comment

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

It might make sense to break the code to render trip details out into a separate function - as is, this function is doing two things (making the request, and rendering data). See also http://callbackhell.com/

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.

2 participants