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

<pipes><Roxanne Agerone><Back Trek> #48

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

Conversation

RAgerone
Copy link

@RAgerone RAgerone commented Dec 7, 2017

BackTREK

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
What role does the Model play in Backbone? The model plays the role of the class. The class is each individual in the collection.
How did the presence of Models and Collections change the way you thought about your app? This is extremely interesting. The restful calls are much easier for me to work with than the custom api calls.
How do Backbone Events compare to DOM events? Backbone events are a little cleaner.
How did you approach filtering? What was your data flow for this feature? With filtering, I can fetch by continent. This was a difficult task because I wasn't sure whether to make a new file, or add on to the collection. I ended up adding a function to the collection that calls the api on a click event.
What do you think of Backbone in comparison to raw JavaScript & jQuery? I found the magic to be difficult to work with at first. I like to know what is happening in my code and it was disconcerting to have something happen when I'm not understanding the concepts. This took me a lot longer to work with than calling the api manually with javascript.
Do you have any recommendations on how we could improve this project for the next cohort? I hear that React is more user friendly. It also seems to be better documented.

@CheezItMan
Copy link

BackTREK

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good commit messages, more commits would be better
Comprehension questions Check, Backbone Events are triggered by changes in the data as opposed to user action.
Organization
Models and collections are defined in separate files Check
Code that relies on the DOM is located in or called by $(document).ready Check
Code follows the Backbone data flow (DOM event -> update model or collection -> Backbone event -> update DOM) Check
Functionality
Display list of trips Check
Display trip details Check
Register for a trip Yes, but the confirmation message tries to print an object instead of a string.
Add a trip Check
Sort trips Check, but you should add some styling which highlights the sorted column.
General
Snappy visual feedback for user actions A pretty good interface and uses modals to display information. I would avoid however using modals for validation failures. A modal is pretty good when you want the user to enter some information, but making them click an X after a typo or missed field is annoying.
API error handling Check
Client-side validation Check, very nicely done
Overall Congrats you hit all the learning goals for the project, nice work.

if (!attributes.age){
errors.age = ['cannot be blank'];
}else if(Number(attributes.age)){
this.set('age', Number(attributes.age));

Choose a reason for hiding this comment

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

Nice!

$('#continent-table').removeClass('show');
$('#continent-table').addClass('hidden');
tripList.fetch({
success: function(collection, response){}

Choose a reason for hiding this comment

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

You should also have an error event handler to let the user know when the API call fails.

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