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 - Shubha #23

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

Sockets - Shubha #23

wants to merge 14 commits into from

Conversation

shubha-rajan
Copy link

TREK

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
What does it mean for code to be asynchronous? When code is asynchronous, it waits for an event to be completed before executing the next action. Meanwhile, during that wait time, other code can run. This is useful for when you're doing things that can take a lot of time, like getting data from an API, because code that is dependent on the data can execute as soon as data has been received, while other code, like code to display a loading screen, can run immediately after the request is sent.
Describe a piece of your code that executes asynchronously. How did this affect the way you structured it? Axios requests execute asynchronously. Because of this, the code was structured so that a then and catch method were chained on to the request.
What kind of errors might the API give you? How did you choose to handle them? The API might throw an error if required values were missing in a post request. I chose to handle errors by displaying a message in the browser.
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 advantageous because regardless of where the element is on the page and its position in the list, you can use jquery to select for it by ID.

@droberts-sea
Copy link

TREK

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene yes
Comprehension questions yes
Functionality
Click a button to load and view a list of trips yes
Clicking the "load" button twice does not cause each trip to display twice yes
Click a trip to load and view trip details yes
Clicking a different trip loads different details yes
Open network tab, then fill out a form to reserve a spot yes
Submitting the form only sends one POST request yes
Errors are reported to the user yes
Site is clearly laid out and easy to navigate yes
Under the Hood
Callback functions are not nested more than 2 levels deep yes
Callback functions are given descriptive names yes
Code is generally well-organized and easy to read yes
All API calls have both success and error callbacks defined yes
Optional but recommended: closures are used to keep track of which trip is selected yes
HTML is semantic yes
CSS is DRY, uses CSS Grid, Flexbox, and/or Bootstrap yes
Overall Excellent job overall! This code is 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 setReservationButtonHandler = (tripID, continent) => {
$('#reserve-button').off();
$('#reserve-button').click((event) => {
event.preventDefault();

Choose a reason for hiding this comment

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

Instead of a click on the button, you should listen for a submit event on the form itself. This will fire both when the button is clicked and when the user presses <enter>.

Also, good catch turning off any previous event handler - this avoids a tricky bug with sending multiple POST requests.

const buildTripClickHandler = (trip) => {

const handler = () => {
changeSelected(trip.id);

Choose a reason for hiding this comment

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

Good use of a closure to keep track of the trip data.

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