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

Selam Ainalem Amperes #28

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

Selam Ainalem Amperes #28

wants to merge 9 commits into from

Conversation

SelamawitA
Copy link

@SelamawitA SelamawitA commented May 29, 2018

TREK

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
What does it mean for code to be asynchronous? A program is not required to execute line by line. If there is a function that takes a lengthy amount of time like an API call, the program can continue to run until the result of the above function is needed.
Describe a piece of your code that executes asynchronously. How did this affect the way you structured it? The call to the API for all locations is a function that operates asynchronously. I added interactive messages to the user to prevent excessive clicks during the API calls and to let them know what was going on during the request (process/fail/success).
What kind of errors might the API give you? How did you choose to handle them? The API may have a 404 not found or an internal server error. For the 404 not found error I added a function to display the specific error message from the API to the user.
Do you have any recommendations on how we could improve this project for the next cohort? I feel like I missed out with using closures in this assignment. I'm not sure how important they are in the future with javascript but I kept running into cool design ideas and not being able to execute them how I'd like to because it involved a nested click event that would be significantly easier to execute with a closure. Maybe more practice with nested click event closures in the future if this is a common thing we may run into?

…ature (hover - bold text) over trips. Added On-click jquery method that is intended to be used for populating <ul> of trip details.
…s. New methods include: singleTripURL - path to API that requires trip id, loadTripdetails(), method that makes call to API with given trip ID passed in from click event, and .on() method that waits for a click event on the #all-trips list and passes in the list item to the loadTripdetails function.
…ption to toggle between viewing/not viewing a form. Added a form with a button to create a reservation. Buttons are currently not linked to API.
…ing - will refactor if theres time) form is displayed to user if they decide to purchase the currently viewed trip.
…ill be displayed a confirmation that a trip was reserved or an error message.
…yed error messages if they violate API input requirements.

$(`#create-trip`).submit(createTrip);
$(document).on("click","#submit-trip",function(){
$('#create-trip').toggle("slow")
Copy link

Choose a reason for hiding this comment

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

Don't forget your semicolons!


$(document).on("click","#submit-button",function(event){
$(`#submit-button`).toggle();
$(`#book-trip-form`).append(`<button id="button-success" class="button success">Trip Booked!</button>`);
Copy link

Choose a reason for hiding this comment

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

This always has a successful button, even if the reservation wasn't made correctly!

.catch((error) => {
reportStatus('fail')
if( error.message && error.response.data.errors){
let v = error.response.data.errors
Copy link

Choose a reason for hiding this comment

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

what does v stand for?

console.log(v)
reportError(error.message,error.response.data.errors)
}else
reportError(`encounted error: ${error.message}`)
Copy link

Choose a reason for hiding this comment

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

your formatting, indentation, and semicolons!


let reserveAtrip = function(tripID){
let singleURL =``
return singleURL+= `https://ada-backtrek-api.herokuapp.com/trips/${tripID}/reservations`
Copy link

Choose a reason for hiding this comment

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

you set the variable singleURL to an empty string, and then return it after concatenating a URL... isn't this the same as

let reserveAtrip = function(tripID){
  return `https://ada-backtrek-api.herokuapp.com/trips/${tripID}/reservations`
}

Also, camel case on this is a little off, I might call this function reserveTrip or reserveSingleTrip or something similar

@tildeee
Copy link

tildeee commented May 29, 2018

TREK

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene your commit messages are so good!
Comprehension questions x, it's okay to have not practiced closures in this one
Functionality
Click a button to list trips x
Click a trip to see trip details x
Fill out a form to reserve a spot x
Errors are reported to the user x
Styling x
Under the Hood
Trip data is retrieved using from the API x
JavaScript is well-organized and easy to read x
HTML is semantic x
Overall

Fffffaaaancy jQuery work!!!! Looks good!

Your code looks fine, except for a few comments I left and... you forgot semicolons at the end of lines all over the place! Particularly the lines where you appended a lot of HTML.

Good work though! Good work on the optional of creating a trip, too.

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