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

Sockets - Karla #43

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

Sockets - Karla #43

wants to merge 3 commits into from

Conversation

kguadron
Copy link

@kguadron kguadron commented Jun 5, 2019

TREK

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
What does it mean for code to be asynchronous? It means sections of code can load without having to wait for other sections to load first, an event can be processing and the rest of the page can still be accessible
Describe a piece of your code that executes asynchronously. How did this affect the way you structured it? I created a button the user can click to load the all the treks, so when the page first loads it's not automatically loaded to the page but when that event is initiated, it doesn't disturb the rest of the page
What kind of errors might the API give you? How did you choose to handle them? they can be server errors, bad request errors, etc. I have a function that displays the error message on the screen and in the console
Suppose you needed to routinely find a specific Trip in the list by its ID field. Would it be advantageous to keep the list in order by the id field? Explain why or why not. it wouldn't be advantages because we can directly call on the trip with it's id number, the order of the ids does not affect the retrieval time.

@CheezItMan
Copy link

CheezItMan commented Jun 14, 2019

TREK

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Too few commits, although you have good commit messages
Comprehension questions Asynchronous code doesn't mean it can load without others loaded, it means it can run or execute at any particular time.
Functionality
Click a button to load and view a list of trips Check
Clicking the "load" button twice does not cause each trip to display twice It doesn't display twice, but it does do multiple get requests! see my inline code notes.
Click a trip to load and view trip details Check
Clicking a different trip loads different details Check
Open network tab, then fill out a form to reserve a spot Check
Submitting the form only sends one POST request Check
Errors are reported to the user Works well except that the form can't report network errors.
Site is clearly laid out and easy to navigate I like the background image, although it's a bit pixelated.
Under the Hood
Callback functions are not nested more than 2 levels deep You've got a bunch of stuff nested into loadTreks, edit sorry I see that you refactored with index2.js, the multiple files confused me a bit.
Callback functions are given descriptive names Check
Code is generally well-organized and easy to read Check
All API calls have both success and error callbacks defined Check
Optional but recommended: closures are used to keep track of which trip is selected Nice use of a closure!
HTML is semantic Check
CSS is DRY, uses CSS Grid, Flexbox, and/or Bootstrap Basic styling and grid in place
Overall You hit the learning goals here. See my inline notes about your bug when showing trip details and let me know if you have questions. Nice work!

return trekDetail;
};
const clickedTrek = showTrekDetails(trek);
$("li").click(clickedTrek);

Choose a reason for hiding this comment

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

So everytime I load the list of trips you add a new event handler to listen for events on the li elements. This means, if I click on See TREKS 3 times and click on one of the trips it will call clickedTrek 3 times.

This is better to do with event delegation or use $('li').off(); to clear the event handlers before adding a new one.

// $('#reservation-form').submit(makeReservation);
});

console.log('dee is here!');

Choose a reason for hiding this comment

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

Hi Dee!

@@ -0,0 +1,155 @@
const URL = 'https://trektravel.herokuapp.com/trips/';

Choose a reason for hiding this comment

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

Only submit the files that actually run please!

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