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 - Sairagul - BackTrek #45

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

Conversation

sairagula
Copy link

BackTREK

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
What role does the Model play in Backbone? Model holds data and business logic, like the models in Rails
How did the presence of Models and Collections change the way you thought about your app? Collection is helpful with dealing a group of related models. It reminded me a controller in Rails. Model takes all business logic, and controller handles loading and saving new models to the server and creating additional helper methods if needed
How do Backbone Events compare to DOM events? Backbone events occur when data changes, DOM events get triggered by user action
How did you approach filtering? What was your data flow for this feature? I didn't do filtering yet
What do you think of Backbone in comparison to raw JavaScript & jQuery? Backbone was a lot more structured, which made my code cleaner
Do you have any recommendations on how we could improve this project for the next cohort? For me it would have been better if we had this big project after the interview week, so that I could focus on it better.

@CheezItMan
Copy link

BackTREK

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene You need more commits! Be a committer! Also for commit messages make them about functionality added, not waves completed.
Comprehension questions Check, and Backbone models and collections handle API communication.
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 Check
Add a trip Check
Sort trips Check
General
Snappy visual feedback for user actions The layout is nicely done and responds quickly. You also provide the user feedback with success and error messages.
API error handling Well done!
Client-side validation Check, but note that the validation error show behind the modal. This can make it hard to see. If you're using a modal for adding trips, have the errors show there.
Overall Nicely done, you hit all the learning goal. Nice work


const Reservation = Backbone.Model.extend({

url: function() {

Choose a reason for hiding this comment

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

This would be better as a urlRoot function.

generatedHTML.on('click',(event) => {
$('#trips').hide();
$('#trip').show();
trip.fetch({

Choose a reason for hiding this comment

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

Consider adding an error event handler to deal with times that the API doesn't work and let the user know.

tripList.fetch({
success(model, response) {
render(model);
}

Choose a reason for hiding this comment

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

Consider adding an error event handler to deal with times that the API doesn't work and let the user know.

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