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

Ana Lisa Sutheland: Octos C9: Trek #24

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

Conversation

The-Beez-Kneez
Copy link

@The-Beez-Kneez The-Beez-Kneez commented May 28, 2018

TREK

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
What does it mean for code to be asynchronous? It means that it will take statements outside of the main program flow, and allow the code after the asynchronous call to be executed immediately without waiting.
Describe a piece of your code that executes asynchronously. How did this affect the way you structured it? I am not 100% sure, I am still unsure how to properly identify a piece of asynchronous code.
What kind of errors might the API give you? How did you choose to handle them? I used status messages to display errors in loading info from the API or (for form input) fields left blank. I also used positive messages to inform the user that things were loading or a small wait was needed, so they did not keep clicking.
Do you have any recommendations on how we could improve this project for the next cohort? I would like there to be a review refresher course on using HTML, in particular with a continually growing element like our trips list. My HTML/CSS Interface is still in progress.

@The-Beez-Kneez
Copy link
Author

Still working on HTML/CSS interface. But functionality is all there.

@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 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 no
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 Excellent job overall! I'm especially impressed with how well-organized your code is, and what a good job you've done with error handling. I've left a couple of comments inline for you to review, but in general this submission looks very good. Keep up the hard work!

<!-- FIXME: Need to render the form once the trip display has activated a trip display -->
<!-- form to reserve a spot -->
<section id="spot-save">
<h2>Reserve A Spot!</h2>

Choose a reason for hiding this comment

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

You might want to look into the CSS display: none for this.

content += '<ul';
for (const field in errors) {
for (const problem in errors[field]) {
content += `<li>${field}: ${problem}</li>`

Choose a reason for hiding this comment

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

Since errors[field] is an array, you probably want for (const problem of errors[field]). As is you get the indexes, not the strings.

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