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 - Kee - BackTREK #47

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

Conversation

anemonekey
Copy link

BackTREK

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
What role does the Model play in Backbone? Models are handy places to store logic and data structure, like they do in Rails.
How did the presence of Models and Collections change the way you thought about your app? It helped set a foundation for how I wanted my data to look and interact with one another.
How do Backbone Events compare to DOM events? Backbone events are triggered by changes in data while DOM events depend on user actions.
How did you approach filtering? What was your data flow for this feature? Did not do this..
What do you think of Backbone in comparison to raw JavaScript & jQuery? Backbone helped DRY out code and made things more readable.
Do you have any recommendations on how we could improve this project for the next cohort? I think a lighter project before Interview Week would have been better. This had a lot of information that I didn't absorb because I was dazed with the foreboding of interviews;;

@CheezItMan
Copy link

BackTREK

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene You need more commits and keep the commit messages describing the functionality provided.
Comprehension questions Check, although Backbone model also 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
Code follows the Backbone data flow (DOM event -> update model or collection -> Backbone event -> update DOM)
Functionality
Display list of trips Check
Display trip details Check
Register for a trip MISSING
Add a trip INCOMPLETE you don't have the function you reference to read the details from the form.
Sort trips You sort the list, but you don't render the results. See my notes in your code.
General
Snappy visual feedback for user actions The styling here is really great. I really like it. It looks really nice and you obviously put a lot of time into it.
API error handling MISSING
Client-side validation MISSING
Overall You have wave 1 complete, and Wave 2 partially complete, but you have a lot of work to do. You definitely spent a lot of time on styling, but should have put more into the functionality. I know it was a crazy week, but you definitely need practice using Backbone Models and collections. Try putting more time into this area on our Stock app this week.

<div class="row content-wrapper">
<section class="col-sm-12 col-md-6 all-trips">
<div class="row search-wrapper">
<form class="col-sm-12 form-inline" action="">

Choose a reason for hiding this comment

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

You don't seem to have any JavaScript catching this form's submit action, so it causes a page reload.


console.log(`About to fetch data from ${ tripList.url }`);

tripList.on('update', render);

Choose a reason for hiding this comment

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

You also need an:

tripList.on('sort', render);

Otherwise it sorts, but doesn't display the results on the screen!

$('#add-trip-form').on('submit', (event) => {
event.preventDefault();

const tripData = readForm();

Choose a reason for hiding this comment

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

You don't HAVE a readForm() function.

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