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

Stef -- Carets #34

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

Stef -- Carets #34

wants to merge 21 commits into from

Conversation

SesameSeeds
Copy link

BackTREK

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
What role does the Model play in Backbone? Models are used to represent data from your server and actions you perform on them will be translated to RESTful operations. Thanks https://cdnjs.com.
How did the presence of Models and Collections change the way you thought about your app? I had to think about how they worked in general to be perfectly honest.
How do Backbone Events compare to DOM events? Backbone events listen to changes with Backbone Models and Collections. In this example when a new trip is created the new trip needs to run an isValid method. DOM events are where elements or objects are changed directly.
How did you approach filtering? What was your data flow for this feature? I did not filter as we were told we did not have to do so.
What do you think of Backbone in comparison to raw JavaScript & jQuery? I think it's a lot. I also think I will enjoy it better with more time and practice.
Do you have any recommendations on how we could improve this project for the next cohort? Definitely do not have this right before interview week, please for the love of the next cohort. Also, more information/details and time working through things, because it took a lot to get to urlRoot just to display the details of a trip for example. Having more time and information probably would really have helped this project. I enjoy these type of projects, such as I did API muncher, but I would have liked more time/info to get an end product I could be proud of.

… the life of me get any messages to be cooperative so all my users will have to use the console.
@CheezItMan
Copy link

BackTREK

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good number of commit and good commit messages
Comprehension questions Check
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 Registers for a trip, but doesn't report success to the user. It also has trouble with validations.
Add a trip You are having trouble reading the name field of the add trip form. See my notes in your code.
Sort trips Check, nicely done!
General
Snappy visual feedback for user actions Not fancy styling, but it's laid out properly.
API error handling It reports API errors
Client-side validation Client-side validations are there, but it's not reporting the result to the user, outside of the console.
Overall Not bad, you got almost all the functionality done, things missing include some jQuery to select fields, and reporting things to the user. I left some notes for you in your code.

});
} else {
console.log('Trip is invalid, sad trombone.');
console.log(trip.validationError);

Choose a reason for hiding this comment

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

It's good that you're checking validation errors, but this needs to be reported to the user as well.

$('#status-messages ul').empty();
console.log(response.responseJSON.errors);

for(let key in response.responseJSON.errors) {

Choose a reason for hiding this comment

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

Good, good to use for letting the user know the validation error issues.


<form id="reservation-trip-form" method="post">
<label for="name">Full Name</label>
<input type="text" name="name"></input>

Choose a reason for hiding this comment

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

This isn't being selected by app.js Because it's having trouble with $('input[name="name"]') Instead an alternative way of being selected would be appropriate, an id etc.

event.preventDefault();
const tripData = {};
fields.forEach( (field) => {
const val = $(`input[name=${field}]`).val();

Choose a reason for hiding this comment

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

This isn't working for the name field.

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