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

Ampers: Abinnet #36

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

Ampers: Abinnet #36

wants to merge 12 commits into from

Conversation

Abiaina
Copy link

@Abiaina Abiaina commented May 29, 2018

TREK

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
What does it mean for code to be asynchronous? to have the code be read and executed at different times. there are promises made and events listening for.
Describe a piece of your code that executes asynchronously. How did this affect the way you structured it? the list of trips code is loaded and styled before the get request is finished processing, I had to make a promise using the axios and .then to load the list of trips.
What kind of errors might the API give you? How did you choose to handle them? 500 internal server, not found, and I chose to render them back to the user for the most part but changed the wording to make it more user friendly. For the most part I only said that there was an error and prompted the user to try again without any specifics...except in the form.
Do you have any recommendations on how we could improve this project for the next cohort?
I spent a lot of time trying to understand foundation, I wish I had understood it better in past assignments. I am unsure of how to change the curriculum to do this but maybe it would be nice to get a short refresher ?

@CheezItMan
Copy link

TREK

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good commit messages, and reasonable number of commits
Comprehension questions Check, I'm sorry you had issues with Foundation.
Functionality
Click a button to list trips Check
Click a trip to see trip details Check
Fill out a form to reserve a spot Check
Errors are reported to the user Yes, but missing required fields like email or name are not reported
Styling Styled a bit, and good responsive page.
Under the Hood
Trip data is retrieved using from the API Check
JavaScript is well-organized and easy to read Pretty well organized, nice!
HTML is semantic Sooooo many freaking divs!!!
Overall Nice work, you hit all the learning goals. The only things of note is that you didn't report validation errors to the user and some odd styling. Every other row of trip changes background color on mouseover. Just seems odd that it's every other row. Nice work, I hope you're enjoying React now!


.catch((error) => {
console.log(error);
showBanner(`Unable to Reserve: ${error.message }`);

Choose a reason for hiding this comment

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

You should also be checking for validation errors like we did in class here.

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