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

Jocelyn Gonzalez -- Carets #39

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

Conversation

jocegonz
Copy link

@jocegonz jocegonz commented Dec 4, 2017

BackTREK

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
What role does the Model play in Backbone? It allows us to create new objects.
How did the presence of Models and Collections change the way you thought about your app? I had to break down the role of my models to discern whether they would be a part of a collection.
How do Backbone Events compare to DOM events? They both react to changes, but backbone events react to changes in data.
How did you approach filtering? What was your data flow for this feature? n/a
What do you think of Backbone in comparison to raw JavaScript & jQuery? It is more structured.
Do you have any recommendations on how we could improve this project for the next cohort? It is a lot of information to digest the week before interviews.

@kariabancroft
Copy link

BackTREK

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene For a project spanning a few days - you should have many more commits to track your progress
Comprehension questions Yes
Organization
Models and collections are defined in separate files Yes
Code that relies on the DOM is located in or called by $(document).ready Yes
Code follows the Backbone data flow (DOM event -> update model or collection -> Backbone event -> update DOM) Yes
Functionality
Display list of trips Good
Display trip details Good
Register for a trip Good
Add a trip Good
Sort trips Did not complete
General
Snappy visual feedback for user actions Yes - though you could DRY this up a bit. I see the same jQuery code repeated many times to update #status-messages so take a look to see how you might be able to turn this into a function that could be used a bit more generically.
API error handling You have some functions created for error handling that you aren't doing any with - i'd encourage you to put some DOM manipulation in these in the future to inform your user when something has gone wrong
Client-side validation
Overall Overall, you hit the majority of learning goals on this assignment. Nice job. There are some opportunities for improvement across duplicated functionality and error handling.

// $("#message").html("<p>Unable to complete reservation</p>")
// });
// });
$(document).on('submit', '#add-reservation-form', events.addReservation)

Choose a reason for hiding this comment

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

While this is a totally valid way to dynamically bring in this event, it is always the preference to use an element more specific than document if at all possible.

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