Skip to content

Conversation

@dev-elle-up
Copy link

@dev-elle-up dev-elle-up commented Jun 3, 2019

TREK

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
What does it mean for code to be asynchronous? Multiple things can happen at the same time, the same thing can happen at different times, and code execution does not have to resolve in order.
Describe a piece of your code that executes asynchronously. How did this affect the way you structured it? A piece of my code that executes asynchronously is the population of a trips details when a user clicks on a specific trip. Trips can be clicked in any order. An event handler is bound to a specific trip's tr element with its matching id html attribute when the GET request for all trips is made. This listener will be triggered at any time when that element (tr) is clicked. The only impact this had on the structure of my code is that I didn't need to put any click handlers in the $(document).ready() function.
What kind of errors might the API give you? How did you choose to handle them? The API gives an error if the user submits a form without a name or email address. I chose to handle these by setting the fields to 'required' in the html form. This prevents an API call to the server for an invalid request, reducing potential costs.
Suppose you needed to routinely find a specific Trip in 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. No. Keeping them in order would presumably be advantageous if you wanted to iterate through them. However, the id numbers are not guaranteed to be sequential so if you do this, the id number won't match the index or iterator value. A better strategy is to affix some way of looking up the id number of the item in the list. (I did this in my code.) Additionally, it's faster to look something up that is indexed or has a key than searching through an ordered list.

…hen clicked. The button click makes a GET request and then each trip is added to a tr and displayed in the DOM.
…save point before I try having the trip list populate a ul instead of a table.
…n of function to make reservation. Need to bug fix.
…Added required keyword to HTML form to prevent unnecessary API calls. Added some comments on on items that still need attention.
… details to show - currently undefined. Still working on this.
…calls if Show All Trips button is clicked again.
…w. The form starts as hidden and will not flash during initial page load.
Made the same change on both branches.
… small refactor to remove a variable that seemed not-so-useful.
@CheezItMan
Copy link

TREK

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good number of commits and good commit messages
Comprehension questions Check
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 Check
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, well done!
Errors are reported to the user Check, but you're not reporting validation errors. See my inline comment.
Site is clearly laid out and easy to navigate It looks nice
Under the Hood
Callback functions are not nested more than 2 levels deep Check
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 No, but it's not needed here.
HTML is semantic Check
CSS is DRY, uses CSS Grid, Flexbox, and/or Bootstrap Well done, you hit all the learning goals here. This is excellent work!
Overall

$("#trip-details").append(`<p><span class="bold">Summary:</span><br> ${trip.about}</p>`);
$("#trip-details").append(`<p><span class="bold">Price:</span> $${trip.cost.toFixed(2)}</p>`);

$('#reservation-form').unbind('submit');

Choose a reason for hiding this comment

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

Very good idea to use unbind to remove other event handlers from the form.


$(`#${trip.id}`).unbind()
$(`#${trip.id}`).bind('click', {thisTrip:trip}, function(event){
// Chris: I realized when I was just about done with this project that .bind() is deprecated. D: This should be changed to a .on() method instead. I will refactor if I get some more time, or perhaps we could talk through it.

Choose a reason for hiding this comment

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

Yup, this still works fine, the better way is to use .on like we did in class.

if (error.response.data && error.response.data.errors) {
reportStatus(`${customMessage} Details: ${error.message}, ${error.response.data.errors}`);
} else {
reportStatus(`${customMessage} Details: ${error.message}`);

Choose a reason for hiding this comment

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

You should respond by also reporting any validation errors. You can loop through the content like we did in Rails.

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