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.

Sign up for GitHub

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

Jump to bottom

Karinna Iniguez - Trek - Octos #38

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

Karinna Iniguez - Trek - Octos #38

wants to merge 10 commits into from

Conversation

karinnainiguez
Copy link

@karinnainiguez karinnainiguez commented May 29, 2018 •
edited
Loading

TREK

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
What does it mean for code to be asynchronous? The code that we write is only applicable on certain actions, which means we do not load all of the functionality and code when the page loads.
Describe a piece of your code that executes asynchronously. How did this affect the way you structured it? I have a button on my page that appears when the page loads. But when the button is clicked, it loads the list of trips. In order to do that, I had to add an event handler on the button when it's clicked.
What kind of errors might the API give you? How did you choose to handle them? Creating a reservation can return an error because the appropriate fields were not filled in. I chose to show that message to the user in red text and clear the form.
Do you have any recommendations on how we could improve this project for the next cohort? It's pretty great, but it feels more like a 3 day project than an entire week. Maybe shorten the time spent on this project.

@kariabancroft
Copy link

TREK

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good
Comprehension questions Yes
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 Good - nice grid practice
Under the Hood
Trip data is retrieved using from the API Yes
JavaScript is well-organized and easy to read Yes
HTML is semantic Yes
Overall Overall you did a good job exploring the major learning goals of this assignment. Your code style and usage of JS syntax looks good.

kariabancroft
kariabancroft reviewed May 31, 2018
View reviewed changes
index.js

$('#trip-reservation').append(
`<form>
<input type="hidden" name="id" value="${data.id}">
Copy link

Choose a reason for hiding this comment

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

Good

index.js
$('#trip-list').empty();
axios.get(URL)

.then((response) => {
Copy link

Choose a reason for hiding this comment

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

Desired style is to include this callback directly below the get function call - rather than adding this additional whitespace

index.js
);

// load form using response data
loadForm(trip);
Copy link

Choose a reason for hiding this comment

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

I like that you created the separate form function here. Might also be good to put the trip details HTML in a separate function as well.

index.js

const reserveTrip = (event) => {
event.preventDefault();
const tripData = $('form').serialize();
Copy link

Choose a reason for hiding this comment

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

Interesting use of serialize!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kariabancroft kariabancroft kariabancroft left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@karinnainiguez @kariabancroft