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

sai's trek #18

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

sai's trek #18

wants to merge 6 commits into from

Conversation

ssamant
Copy link

@ssamant ssamant commented May 29, 2017

TREK

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
What does it mean for code to be asynchronous? it does not run in the order it appears in the code file. some of the code is waiting for an event (like a click)
Describe a piece of your code that executes asynchronously. How did this affect the way you structured it? the event handlers that show one trip are created in the document ready portion of the file; i chose not to put it in the var/function that shows all the trips b/c this created a new set of event handlers every time I called that function. Putting them in the document ready means they are available when a list of trips is shown
What kind of errors might the API give you? How did you choose to handle them? the API could give a failure error if it didn't have any trips or if you tried to reserve a trip with invalid info. I have a failure callback that says the API request failed; this could probably be more specific. Currently my trek site does not validate a user's info to reserve a trip; a better site would validate and return error messages for invalid info
What is an Underscore template? Describe one template you used for this project. An underscore template allows you to plug info generated from the javascript file (from jquery functions) into an html template -- combining JS and html and content similar to the ERD. I made a template for the list of trips and for the form for reserving a trip.
Do you have any recommendations on how we could improve this project for the next cohort? Fun project! More guidance on responsive sites; I used a table and it looked okay in the mobile preview in developers tools but i could probably also make a better mobile nav bar/menu

@CheezItMan
Copy link

TREK

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Probably could do more granular commits
Comprehension questions Check, thank for the feedback on responsive sites
Functionality
Click a button to list trips Check
Click a trip to see trip details Check
Fill out a form to reserve a spot The form is there, but no evidence shows on the screen that the trip reservation was submitted and no submit button.
Errors are reported to the user "TREK request failed." Check, could use a more descriptive error message like you noted in the comprehension questions.
Styling, Foundation grid layout Using a table not grid layout. It looks ok on a mobile screen, using a grid layout or a responsive menu it could be better.
Under the Hood
Trip data is retrieved using jQuery AJAX Check
JavaScript is well-organized and easy to read Some of your event handlers are a little big, they could be broken up a bit, but it's pretty good overall.
HTML is semantic nicely done
All dynamic content is rendered using Underscore templates Check
Extras Filters by continent
Overall Very nicely done, good work on the extras.

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